Summary:
Ref T6367. Removes `multiplexMail()`!
We can't pass a single body into a function which splits it anymore: we need to split recipients first, then build bodies for each recipient list. This lets us build separate bodies for each recipient's individual translation/access levels.
The new logic does this:
- First, split recipients into groups called "targets".
- Each target corresponds to one actual mail we're going to build.
- Each target has a viewer (whose translation / access levels will be used to generate the mail).
- Each target has a to/cc list (the users who we'll ultimately send the mail to).
- For each target, build a custom mail body based on the viewer's access levels and settings (language prefs not actually implemented).
- Then, deliver the mail.
Test Plan:
- Read new config help.
Then did a bunch of testing, primarily with `bin/mail list-outbound` and `bin/mail show-outbound` (to review generated mail), `bin/phd debug taskmaster` (to run daemons freely) and `bin/worker execute --id <id>` (to repeatedly test a specific piece of code after identifying an issue).
With `one-mail-per-recipient` on (default):
- Sent mail to multiple users.
- Verified mail showed up in `mail list-outbound`.
- Examined mail with `mail show-outbound`.
- Added a project that a subscriber could not see.
- Verified it was not present in `X-Phabricator-Projects`.
- Verified it was rendered as "Restricted Project" for the non-permissioned viewer.
- Added a subscriber, then changed the object policy so they could not see it and sent mail.
- Verified I received mail but the other user did not.
- Enabled public replies and verified mail generated with public addresses.
- Disabld public replies and verified mail generated with private addresses.
With `one-mail-per-recipient` off:
- Verified that one mail is sent to all recipients.
- Verified users who can not see the object are still filtered.
- Verified that partially-visible projects are completely visible in the mail (this violates policies, as documented, as the best available compromise).
- Enabled public replies and verified the mail generated with "Reply To".
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: carlsverre, epriestley
Maniphest Tasks: T6367
Differential Revision: https://secure.phabricator.com/D13131
Summary:
Ref T6367. Do all mail, feed, notification and search stuff from the daemons, in all editors.
There are four relatively-stateful editors (Audit, Differential, Phriction, PhortuneCart) which needed special care to move state into the daemons properly.
Beyond that, I moved mailTo/mailCC/feedRelated/feedNotify to be computed before we enter the worker:
- This is simpler, since a lot of editors rely on being able to call `$object->getReviewers()` or similar to compute them.
- This is more correct, since we want to freeze the lists at this moment in time.
Finally, I renamed `loadEdges` to `willPublish` and made it a slightly more general hook.
---
This is a bit fragile and I'm not //thrilled// about it.
It would probably be cleaner to have separate Editor and Publisher classes (something like @fabe's D11329 did). However, I think that's quite a lot of work, and I'd like to see stronger motivation for it (either in this actually being more fragile than I think, or there being other things we get out of it). Overall, I'm comfortable with this change, just definitely not a big fan of the "save" + "load" pattern since I think it's really fragile, nonobvious, hard to debug/predict, etc.
Test Plan:
Directly updated editors:
- Created a new Phriction page, saw "Document Content".
- Edited a Phriction page, saw "Document Diff".
- Edited a revision, got normal looking mail.
- Faked in `changedPriorToCommitURI` and verified it survived the state boundary.
- Sent Audit mail.
- Sent invoice mail.
Indirect editors - for these, I just made a change and made sure the mail generated:
- Updated a paste.
- Updated an event.
- Updated a thread.
- Updated a task.
- Updated a mock.
- Updated a question.
- Updated a project.
- Updated a file.
- Updated an initiative.
- Updated a Legalpad document.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, fabe
Maniphest Tasks: T6367
Differential Revision: https://secure.phabricator.com/D13115
Summary:
Ref T6367. This is similar to D11329, but not quite as ambitious.
Allow Editors to implement `supportsWorkers()` and move their publishing work into a daemon. So far, only Paste supports this.
Most of the complexity here is saving and restoring state across the barrier between the web process and the worker process, but I think this is ~90% of it and then we'll pick up a couple of random things in applications.
I'm primarily trying to keep this as gradual as possible.
Test Plan:
- Published transactions with and without daemon support.
- Looked at mail, feed.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: fabe, epriestley
Maniphest Tasks: T6367
Differential Revision: https://secure.phabricator.com/D13107
Summary:
Ref T7703. See that task and inline for a bunch of discussion.
Briefly, when we run implicit policy rules ("to see a revision, you must also be able to see its repository") at query time, they don't apply to other viewers we might check later.
We do this very rarely, but when we do we're often doing it for a bunch of different viewers (for example, in Herald) so I don't want to just reload the object a million times.
Test Plan:
- Added and executed unit tests.
- Wrote a "flag everything" Herald rule, as in the original report in T7703, and no longer got "Unknown Object" flags on revisions.
- Rigged up a lot of cases in the web UI and couldn't find any inconsistencies, although this case is normally very hard to hit.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7703
Differential Revision: https://secure.phabricator.com/D13104
Summary:
Fixes T8387. This completes conversion of lists into users.
These settings allow administrators to reduce the amount of mail delivered to lists, ahead of T5791.
Test Plan: {F468206}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13138
Summary: Ref T8387. Mostly completeness, but you might want to choose html vs text mail.
Test Plan: {F468203}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13137
Summary:
Ref T8387. Ref T6367. This allows selection of a language, which will be respected in email delievered to the list users.
For example, you could have a German list that gets mail in German or something. I don't know that the feature is really useful, it's mostly just for completeness.
I also supported it for bots, mostly so their pronouns can be configured.
Test Plan: {F468186}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6367, T8387
Differential Revision: https://secure.phabricator.com/D13136
Summary: Ref T8377. These were picked up by tests.
Test Plan: `arc unit --everything`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13130
Summary: Ref T8387. This is now completely obsoleted by mailing list users.
Test Plan: Grepped for `mailinglist` and related symbols.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13129
Summary:
Ref T8387. This migrates lists in the database to users, and replaces all subscriptions.
This won't update Herald rules or saved search queries, but they're presumably rare and infeasibly complex to migrate.
Test Plan: This migration is relatively re-runnable, so I ran it a bunch of times with different setups using `bin/storage adjust --apply`. It successfully migrated lists into users and replaced them in all the places they were subscribed.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13128
Summary: Ref T8387. Mostly just modernizes this panel to work for viewers vs users. Auto-verify these edits since they aren't otherwise verifiable.
Test Plan:
- Added, changed, removed addresses for a list.
- Used panel normally for my own account.
- Verified bots don't get a panel.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13125
Summary:
Ref T8387. Adds new mailing list users.
This doesn't migrate anything yet. I also need to update the "Email Addresses" panel to let administrators change the list address.
Test Plan:
- Created and edited a mailing list user.
- Viewed profile.
- Viewed People list.
- Searched for lists / nonlists.
- Grepped for all uses of `getIsDisabled()` / `getIsSystemAgent()` and added relevant corresponding behaviors.
- Hit the web/api/ssh session blocks.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, tycho.tatitscheff, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13123
Summary:
Ref T8387. This mostly merges D10565 + D10480. I'm going to touch this to add mailing list stuff shortly so I wanted to clean those up.
This isn't super pretty but is fully flexible and consistent with other modern query UIs.
This should be more-or-less backward compatible.
Test Plan: Fiddled with the new options.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, cburroughs, epriestley
Maniphest Tasks: T8044, T8387
Differential Revision: https://secure.phabricator.com/D13122
Summary:
Ref T8387. This describes changes I haven't made yet, but plan to make.
Also removes the long-deprecated actAsUser capability so I can remove the caveat about it from the documentation.
Test Plan: `grep`, reading
Reviewers: btrahan, eadler
Reviewed By: btrahan, eadler
Subscribers: eadler, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13120
Summary: This got removed by accident in D12933.
Test Plan: Editing names works now.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13144
Summary: When destroying an object, also remove all associated flags and tokens.
Test Plan: Awarded a Maniphest task with a token and flagged it as well. Destroyed it with `./bin/remove destroy` and saw flag and token removed from the database.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13112
Summary:
This simply adds colors, icons to ProjectQueryConduitAPIMethod and
ProjectCreateConduitAPIMethod.
Additionally, adds 'tags' to ProjectCreateConduitAPIMethod
Change-Id: Ic6332fe174a59ecfd60cea281ccb0ed938136141
Test Plan:
create a new project with project.create, specify icon, color and tags
call project.query with colors and icons args, check for results
containing matching projects.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: legoktm, aklapper, Korvin, epriestley, valhallasw, qgil
Differential Revision: https://secure.phabricator.com/D13098
Summary: Closes T8371, Query for recurring event parents should accept a "not a child of anything" clause
Test Plan: querying for a list of events with recurring events should still work the same.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T8371
Differential Revision: https://secure.phabricator.com/D13140
Summary: This class is no longer used since D10792.
Test Plan: `grep`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13116
Summary: Ref T8371, Cancelled recurring events should propogate to real child events
Test Plan: Create recurring event, create and exception to a ghost event, cancel recurring event, real ghost event should be treated as cancelled while the recurring event remains cancelled.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T8371
Differential Revision: https://secure.phabricator.com/D13121
Summary: Ref T8394, List view should only allow 100 ghost events per recurring event
Test Plan: Open list view with many recurring events. Shouldn't hang.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8394
Differential Revision: https://secure.phabricator.com/D13132
Summary: This class is no longer used.
Test Plan: `grep`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13117
Summary: This class is no longer used after D13032.
Test Plan: `grep`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13118
Summary: This class is no longer used after D12850.
Test Plan: `grep`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13119
Summary: Fixes T8382, Edit link of an exception to a ghost event should not create new event
Test Plan: Create recurring event, open ghost, edit, save, edit again. Should edit event created after first edit, not new event.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T8382
Differential Revision: https://secure.phabricator.com/D13108
Summary: Now that `ManiphestTask` implements `PhabricatorApplicationTransactionInterface`, the transaction removal happens automatically.
Test Plan: Used `./bin/remove destroy` to delete a `ManiphestTask` and saw related transactions removed as well.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13110
Summary: Ref T8357, DRAFT, recurring events need optional end dates
Test Plan: Edit recurring event, set end date, save, recurring ghosts should not generate after end date
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8357
Differential Revision: https://secure.phabricator.com/D13088
Summary: got some errors in my sandbox when I updated. this fixes them and I think is the right naming convention.
Test Plan: /settings/panel/emailpreferences/ started working again! /spaces/ also loaded unit tests passed without errors
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13103
Summary:
Fixes T8372. We currently store the entire resource path in the cachekey; shorten it to a maximum of 64 characters.
The key size is 128 and T8372 had an example of a 112-byte key which could run over 128 with a slightly-long cache namespace.
Test Plan:
- Not 100% sure this is the issue, but it seems likely.
- Dumped the cachekey before and after the change.
- Saw values like `celerity:/res/1432859262T/phabricator/e2460e8f/core.pkg.css-6eGjUCJORU3j` now.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8372
Differential Revision: https://secure.phabricator.com/D13095
Summary: Fixes T8375. This column is `repositoryID`, not `repository`.
Test Plan: Examined schema.
Reviewers: joshuaspence, btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8375
Differential Revision: https://secure.phabricator.com/D13097
Summary: Fixes T7603. This application is no longer used.
Test Plan: Went to `/repository/` and got a 404.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley
Maniphest Tasks: T7603
Differential Revision: https://secure.phabricator.com/D12734
Summary: Ref T7604. Change two migrations to query arcanist project information using `queryfx` directly to avoid the need for the `LiskDAO` fields to exist.
Test Plan:
Ran the following commands to verify that things weren't majorly broken:
- `./bin/storage upgrade --apply phabricator:20150503.repositorysymbols.2.php`
- `./bin/storage upgrade --no-quickstart --namespace test`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D13011
Summary: Fixes T8369, Both ghost instances and exceptions to generated ghosts should be editable
Test Plan: Create recurring event, open /E{id}/1, edit, save. New exception should not be "recurring", and should be editable, as the regular ghost events.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8369
Differential Revision: https://secure.phabricator.com/D13089
Summary: Ref T7604. Remove these "arcanist project" fields from the `LiskDAO` classes, but leave the data intact (there are no more read/writes to this data).
Test Plan:
- Grepped for calls to these methods.
- Ran `./bin/storage adjust` to check for schema issues.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D13012
Summary: I think the language here is inverted? On a commit which is reverted, the text states `$author added a reverted commit: $commit`. In reality, `$commit` is the //reverting// commit, not the //reverted// commit.
Test Plan: Think about English.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13067
Summary: Closes T8358, Edit recurring ghost events, not recurring event.
Test Plan: Create recurring event, original event should not be editable, but ghost events should be editable.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8358
Differential Revision: https://secure.phabricator.com/D13075
Summary: Fixes T8354. Adds a class to each `th` and `td` for specificity.
Test Plan: Write a large table as an inline comment, see it wraps property on smaller screens.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8354
Differential Revision: https://secure.phabricator.com/D13076
Summary: If a host artifact exists in a Harbormaster build, where the Drydock lease no longer exists, then an error will be raised because of the attempt to access an undefined index. This changes the code to use `idx()` so that it correctly returns null instead.
Test Plan: Tested in production.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: joshuaspence, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13074
Summary:
Ref T7984. With this, an install can add an ExternalSymbolsSource to src/extensions, which will include whatever
source they have.
Test Plan: search for php and python builtins.
Reviewers: joshuaspence, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T7984
Differential Revision: https://secure.phabricator.com/D13036
Summary: Using `##` can cause some formatting issues, see D13071.
Test Plan: See D13071.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13072
Summary: Fixes T8347, Generated ghost events should start generating as early as the range start minus the duration of the event.
Test Plan: Create a year long weekly event, a year later, each day should have about 52 events.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8347
Differential Revision: https://secure.phabricator.com/D13069
Summary: Closes T8356, Ghost events should provide a link to original event
Test Plan: Open an instance at /calendar/event/{id}/{index}. It should have a link to /calendar/event/{id}. Original recurring event should not link to itself.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T8356
Differential Revision: https://secure.phabricator.com/D13068
