1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00
Commit graph

10249 commits

Author SHA1 Message Date
epriestley
b5dfd34e4a Support PhabricatorSpacesInterface in ApplicationSearch UI
Summary: Ref T8441. Does what it says, provided other conditions (like using the new SearchField stuff) are fulfilled.

Test Plan:
{F473836}

{F473837}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8441

Differential Revision: https://secure.phabricator.com/D13171
2015-06-05 11:21:45 -07:00
epriestley
65b891b095 Move some value massaging into SearchFields
Summary: Ref T8441. Ref T7715. Let SearchFields do some value massaging before buiding queries so we don't have to work as hard in each SearchEngine. Particularly, they can handle evaluateTokens() from Tokenizers.

Test Plan: Paste automatically gained access to `viewer()`, etc.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13170
2015-06-05 11:21:22 -07:00
epriestley
08966ba311 Begin modularizing fields in SearchEngines
Summary:
Ref T8441. Ref T7715. Add a layer of indirection to fields in search engines. This will allow us to:

  - Simplify SearchEngine code, which has collected a lot of duplication around expressing what is effectively field types.
  - Automatically add fields like "Spaces" and "Projects" (primary driver for T8441).
  - Reorder or hide fields (not sure if we really want to do this, but it seems plausible, and this will let us play around with it, at least).
  - Drive Conduit Query methods via SearchEngines, so the same code specifies both the search UI and the `application.query` endpoint (primary driver in T7715).

Test Plan:
  - Searched for stuff in Paste, everything behaved exaclty like it used to (except that I removed the "no language" checkbox, which seemed like fluff from a bygone era).
  - Searched for stuff in other applications, saw no changes.
  - Hit date field errors.
  - Used query strings to specify values.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13169
2015-06-05 11:21:08 -07:00
epriestley
68486c4541 Rename PhabricatorSearchField to PhabricatorSearchFieldDocumentType
Summary: Ref T8441. I want to use `PhabricatorSearchField` for a better, more useful object.

Test Plan: `grep`, `arc lint`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8441

Differential Revision: https://secure.phabricator.com/D13168
2015-06-05 11:01:25 -07:00
epriestley
fc3d11809f Give Nuance form sources a web UI
Summary:
Ref T8434. Hard-codes form sources as a complaint form.

This form is close to perfect and I'm not actually sure we need to let users customize it at all.

Test Plan: {F473587}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8434

Differential Revision: https://secure.phabricator.com/D13166
2015-06-05 11:01:06 -07:00
epriestley
f529ade9f7 Add Nuance Queue list/edit/detail views
Summary: Ref T8434. Throw these together.

Test Plan: Created a new Queue.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8434

Differential Revision: https://secure.phabricator.com/D13163
2015-06-05 10:49:35 -07:00
epriestley
2c065f3f3d Add a list view for Nuance sources
Summary: Ref T8434. Now you can list them.

Test Plan: Listed sources.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8434

Differential Revision: https://secure.phabricator.com/D13162
2015-06-05 10:44:18 -07:00
epriestley
837e6b5ca7 Slightly modernize NuanceSource
Summary:
Ref T8434. Minor cleanup/modernization. I made type selection modal (like Herald, Auth, etc) so we can render the form on the next screen based on the type.

{F472519}

Test Plan: Created a new source, edited an existing source.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8434

Differential Revision: https://secure.phabricator.com/D13161
2015-06-05 10:44:02 -07:00
epriestley
1f65a50f04 Slightly modernize NuanceQueue
Summary: Ref T8434. Touch things up a bit. This also cleans up the missing ApplicationTransactionInterface for T6367.

Test Plan: Didn't create a new queue. Didn't edit an existing queue.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: jeremyb, epriestley

Maniphest Tasks: T8434

Differential Revision: https://secure.phabricator.com/D13160
2015-06-05 10:43:37 -07:00
epriestley
ef90007a21 Support Spaces transactions
Summary:
Ref T8424. This adds crude integration with Paste's edit/view workflows: you can change the space a Paste appears in, see transactions, and get a policy callout.

Lots of rough edges and non-obviousness but it pretty much works.

Test Plan:
  - Created and updated Pastes.
  - Moved them between spaces, saw policy effects.
  - Read transactions.
  - Looked at feed.
  - Faked query to return no spaces, saw control and other stuff vanish.
  - Faked query to return no spaces, created pastes.
  - Tried to submit bad values and got errors.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13159
2015-06-05 10:42:49 -07:00
Erik Fercak
5deaeec668 Use a class constant instead of a hardcoded string
Test Plan: Submitted a form - saw nothing out of ordinary.

Reviewers: #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D13167
2015-06-05 07:27:35 -07:00
Joshua Spence
006a877996 Allow ghost atoms to be rendered
Summary: Ref T4558. Allow ghost atoms to be rendered in #diviner. This functionality didn't exist previously, but was hinted at by the TODO comments.

Test Plan: Generated #diviner documentation for rARC and then removed a class (before re-generating the documentation). Navigated to the documentation for the removed class and saw "This atom no longer exists".

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Maniphest Tasks: T4558

Differential Revision: https://secure.phabricator.com/D13114
2015-06-05 18:07:13 +10:00
epriestley
c1c897b961 Provide core policy support for Spaces
Summary:
Ref T8424. No UI or interesting behavior yet, but integrates Spaces checks:

  - `PolicyFilter` now checks Spaces.
  - `PolicyAwareQuery` now automatically adds Spaces constraints.

There's one interesting design decision here: **spaces are stronger than automatic capabilities**. That means that you can't see a task in a space you don't have permission to access, //even if you are the owner//.

I //think// this is desirable. Particularly, we need to do this in order to exclude objects at the query level, which potentially makes policy filtering for spaces hugely more efficient. I also like Spaces being very strong, conceptually.

It's possible that we might want to change this; this would reduce our access to optimizations but might be a little friendlier or make more sense to users later on.

For now, at least, I'm pursuing the more aggressive line. If we stick with this, we probably need to make some additional UI affordances (e.g., show when an owner can't see a task).

This also means that you get a hard 404 instead of a policy exception when you try to access something in a space you can't see. I'd slightly prefer to show you a policy exception instead, but think this is generally a reasonable tradeoff to get the high-performance filtering at the Query layer.

Test Plan:
  - Added and executed unit tests.
  - Put objects in spaces and viewed them with multiple users.
  - Made the default space visible/invisible, viewed objects.
  - Checked the services panel and saw `spacePHID` constraints.
  - Verified that this adds only one query to each page.

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: chad, epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13156
2015-06-04 17:46:32 -07:00
epriestley
e595478f1d Make policy violation dialog more flexible
Summary:
Ref T8424. When users are rejected because they can't see the space an object is in, this isn't really a capability rejection. Don't require a capability when rejecting objects.

This mostly simplifies upcoming changes.

Test Plan:
  - Viewed a capability exception dialog, it looked the same as always.
  - (After additional changes, viewed a space exception dialog.)

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13155
2015-06-04 17:45:51 -07:00
epriestley
763b63a0fb Add spacePHID infrastructure and implement in Paste
Summary:
Ref T8424. I'm using Paste as a testbed application because Spaces make some degree of sense for it but it's also flat/simple.

This doesn't do anything interesting or useful and mostly just making the next (more interesting) diff smaller.

Test Plan:
  - Ran `bin/storage upgrade -f`.
  - Browsed pastes.
  - Created a paste.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13154
2015-06-04 17:45:24 -07:00
epriestley
52a29be70d Introduce a request cache mechanism
Summary:
Ref T8424. This adds a standard KeyValueCache to serve as a request cache.

In particular, I need to cache Spaces (they are frequently accessed, sometimes by multiple viewers) but not have them survive longer than the scope of one request.

This request cache is explicitly destroyed by each web request and each daemon request.

In the very long term, building this kind of construct supports reusing PHP interpreters to run web requests (see some discussion in T2312).

Test Plan:
  - Added and executed unit tests.
  - Ran every daemon.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13153
2015-06-04 17:27:31 -07:00
epriestley
a15444aa79 Remove PhabricatorStartup::getGlobal/setGlobal mechanism
Summary:
Ref T8424. Fixes T7114. This was envisioned as a per-request cache for reusing interpreters, but isn't a good fit for that in modern Phabricator.

In particular, it isn't loaded by the daemons, but they have equal need for per-request caching.

Since I finally need such a cache for Spaces, throw the old stuff away before I built a more modern cache.

Also resolves T7114 by dropping filtering on $_SERVER. I'm pretty sure this is the simplest fix, see D12977 for a bit more discussion.

Test Plan: Called `didFatal()` from somewhere in normal code and verified it was able to use the access log.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7114, T8424

Differential Revision: https://secure.phabricator.com/D13152
2015-06-04 17:26:52 -07:00
epriestley
e5b923743a Make CSRF salt per-user instead of per-request
Summary:
Fixes T8326. This removes calls to PhabricatorStartup from places that daemons may access.

This salt doesn't need to be global; it's embedded in the token we return. It's fine if we use a different salt every time. In practice, we always use the same viewer, so this change causes little or no behavioral change.

Ref T8424. For Spaces, I need a per-request cache for all spaces, because they have unusual access patterns and require repeated access, in some cases by multiple viewers.

We don't currently have a per-request in-process cache that we, e.g., clear in the daemons.

We do have a weak/theoretical/forward-looking attempt at this in `PhabricatorStartup::getGlobal()` but I'm going to throw that away (it's kind of junky, partly because of T8326) and replace it with a more formal mechanism.

Test Plan:
  - Submitted some forms.
  - Grepped for `csrf.salt`.
  - Viewed page source, saw nice CSRF tokens with salt.
  - All the salts are still the same on every page I checked, but it doesn't matter if this isn't true everywhere.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8326, T8424

Differential Revision: https://secure.phabricator.com/D13151
2015-06-04 17:26:23 -07:00
Joshua Spence
b9d004e9c4 Integrate Diviner with global search
Summary: Fixes T7458. Integrates #diviner into #applicationsearch by indexing `DivinerLiveBook` and `DivinerLiveSymbol` search documents. Depends on D13157.

Test Plan: Ran `./bin/search index --all --type BOOK` and `./bin/search index --all --type ATOM` and then searched for various symbols via global search.

Reviewers: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7458

Differential Revision: https://secure.phabricator.com/D13090
2015-06-05 07:27:48 +10:00
Joshua Spence
0d50c40973 Fix a few Diviner method calls
Summary: I missed these in D13157.

Test Plan: Browsed around #diviner.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13158
2015-06-05 07:27:13 +10:00
Joshua Spence
32f669d566 Invert include/exclude logic on DivinerAtomQuery
Summary: Fixes T8401. Change `withIncludeGhosts()` to `withExcludeGhosts()` and `withIncludeUndocumentable()` to `withExcludeDocumentable()`. In particular, this allows querying for atoms by PHID to work as expected.

Test Plan: I got confused with double negatives so I might have gotten some of these wrong... I poked around Diviner and re-generated documentation to verify that this is working as expected.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8401

Differential Revision: https://secure.phabricator.com/D13157
2015-06-05 07:23:19 +10:00
epriestley
0fc0af6443 Let Maniphest send mail again.
Ha ha ha! Fixes T8422.

Auditors: btrahan
2015-06-04 09:49:10 -07:00
epriestley
98aae51c3d Fix an issue with mentions in transactions
Ref T6367. Fixes T8415.

Maniphest filters transactions too early. This happens automatically later. Remove the code.

Transactions should be filtered per-user. If a transaction is hidden for some users, we shouldn't mail them. Move the filtering logic to be per-user.

Stack:

```
[Thu Jun 04 05:51:05.371061 2015] [:error] [pid 25111] [client 127.0.0.1:56497] [2015-06-04 05:51:05] EXCEPTION: (Exception) Transaction ("PHID-XACT-TASK-x4jlylat47s6ttr", of type "core:edge") requires a handle ("PHID-DREV-rs7jaoxbcb3av6biq4b5") that it did not load. at [<phabricator>/src/applications/transactions/storage/PhabricatorApplicationTransaction.php:277]
[Thu Jun 04 05:51:05.372546 2015] [:error] [pid 25111] [client 127.0.0.1:56497] arcanist(head=master, ref.master=4e83efb31d3e), instances(head=master, ref.master=db56d5d6ad91), ledger(head=master, ref.master=5694485699a4), libcore(), phabricator(head=xaction1, ref.master=04a22a8fa443, ref.xaction1=04a22a8fa443, custom=17), phutil(head=master, ref.master=c2cd90ee7aec), services(head=master, ref.master=2d76591c4f87)
[Thu Jun 04 05:51:05.372559 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #0 <#2> PhabricatorApplicationTransaction::getHandle(string) called at [<phabricator>/src/applications/transactions/storage/PhabricatorApplicationTransaction.php:463]
[Thu Jun 04 05:51:05.372564 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #1 <#2> PhabricatorApplicationTransaction::shouldHide() called at [<phabricator>/src/applications/maniphest/storage/ManiphestTransaction.php:163]
[Thu Jun 04 05:51:05.372568 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #2 <#2> ManiphestTransaction::shouldHide() called at [<phutil>/src/utils/utils.php:428]
[Thu Jun 04 05:51:05.372572 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #3 <#2> mfilter(array, string, boolean) called at [<phabricator>/src/applications/maniphest/editor/ManiphestTransactionEditor.php:380]
[Thu Jun 04 05:51:05.372576 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #4 <#2> ManiphestTransactionEditor::shouldSendMail(ManiphestTask, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:957]
[Thu Jun 04 05:51:05.372580 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #5 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(ManiphestTask, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2858]
[Thu Jun 04 05:51:05.372585 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #6 <#2> PhabricatorApplicationTransactionEditor::applyInverseEdgeTransactions(DifferentialRevision, DifferentialTransaction, integer) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:569]
[Thu Jun 04 05:51:05.372589 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #7 <#2> PhabricatorApplicationTransactionEditor::applyBuiltinExternalTransaction(DifferentialRevision, DifferentialTransaction) called at [<phabricator>/src/applications/differential/editor/DifferentialTransactionEditor.php:615]
[Thu Jun 04 05:51:05.372594 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #8 <#2> DifferentialTransactionEditor::applyBuiltinExternalTransaction(DifferentialRevision, DifferentialTransaction) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:489]
[Thu Jun 04 05:51:05.372611 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #9 <#2> PhabricatorApplicationTransactionEditor::applyExternalEffects(DifferentialRevision, DifferentialTransaction) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:827]
[Thu Jun 04 05:51:05.372616 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #10 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(DifferentialRevision, array) called at [<phabricator>/src/applications/differential/controller/DifferentialCommentSaveController.php:124]
[Thu Jun 04 05:51:05.372621 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #11 <#2> DifferentialCommentSaveController::processRequest() called at [<phabricator>/src/aphront/AphrontController.php:33]
[Thu Jun 04 05:51:05.372625 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #12 <#2> AphrontController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:226]
[Thu Jun 04 05:51:05.372629 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #13 phlog(Exception) called at [<phabricator>/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php:226]
[Thu Jun 04 05:51:05.372633 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #14 AphrontDefaultApplicationConfiguration::handleException(Exception) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:230]
[Thu Jun 04 05:51:05.372637 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #15 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:140]
[Thu Jun 04 05:51:05.372641 2015] [:error] [pid 25111] [client 127.0.0.1:56497]   #16 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:19]
```

Auditors: btrahan
2015-06-04 06:05:24 -07:00
epriestley
04a22a8fa4 Fix slop with previous patch, perhaps
Auditors: btrahan
2015-06-03 20:11:33 -07:00
epriestley
ef43a98440 Reload commits before publishing mail about them
This fixes this, which only crops up some of the time:

```
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000] [2015-06-04 03:03:10] EXCEPTION: (PhutilProxyException) Error while executing Task ID 902271. {>} (PhabricatorDataNotAttachedException) Attempting to access attached data on PhabricatorRepositoryCommit (via getCommitData()), but the data is not actually attached. Before accessing attachable data on an object, you must load and attach it.
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000] Data is normally attached by calling the corresponding needX() method on the Query class when the object is loaded. You can also call the corresponding attachX() method explicitly. at [<phabricator>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:166]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000] arcanist(head=master, ref.master=8c589f1f759f), phabricator(head=master, ref.master=4a45620b0458), phutil(head=master, ref.master=afc05a9a7f00)
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #0 <#2> PhabricatorLiskDAO::assertAttached(string) called at [<phabricator>/src/applications/repository/storage/PhabricatorRepositoryCommit.php:129]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #1 <#2> PhabricatorRepositoryCommit::getCommitData() called at [<phabricator>/src/applications/audit/editor/PhabricatorAuditEditor.php:669]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #2 <#2> PhabricatorAuditEditor::buildMailBody(PhabricatorRepositoryCommit, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2178]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #3 <#2> PhabricatorApplicationTransactionEditor::sendMailToTarget(PhabricatorRepositoryCommit, array, PhabricatorMailTarget) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2152]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #4 <#2> PhabricatorApplicationTransactionEditor::sendMail(PhabricatorRepositoryCommit, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:998]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #5 <#2> PhabricatorApplicationTransactionEditor::publishTransactions(PhabricatorRepositoryCommit, array) called at [<phabricator>/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php:21]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #6 <#2> PhabricatorApplicationTransactionPublishWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:91]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #7 <#2> PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:162]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #8 <#2> PhabricatorWorkerActiveTask::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:20]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #9 PhabricatorTaskmasterDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:185]
Daemon 39827 STDE [Thu, 04 Jun 2015 03:03:11 +0000]   #10 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:125]
```

Auditors: btrahan
2015-06-03 20:08:04 -07:00
epriestley
4a45620b04 Reload revisions before publishing mail about them
Fixes this, which only triggers on some kinds of mail:

```
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] [2015-06-04 02:56:38] EXCEPTION: (PhutilProxyException) Error while executing Task ID 902251. {>} (PhabricatorDataNotAttachedException) Attempting to access attached data on DifferentialRevision (via getActiveDiff()), but the data is not actually attached. Before accessing attachable data on an object, you must load and attach it.
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] Data is normally attached by calling the corresponding needX() method on the Query class when the object is loaded. You can also call the corresponding attachX() method explicitly. at [<phabricator>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:166]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000] arcanist(head=master, ref.master=8c589f1f759f), phabricator(head=master, ref.master=6dede2e2c513), phutil(head=master, ref.master=afc05a9a7f00)
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #0 <#2> PhabricatorLiskDAO::assertAttached(string) called at [<phabricator>/src/applications/differential/storage/DifferentialRevision.php:158]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #1 <#2> DifferentialRevision::getActiveDiff() called at [<phabricator>/src/applications/differential/customfield/DifferentialBranchField.php:72]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #2 <#2> DifferentialBranchField::updateTransactionMailBody(PhabricatorMetaMTAMailBody, DifferentialTransactionEditor, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2481]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #3 <#2> PhabricatorApplicationTransactionEditor::addCustomFieldsToMailBody(PhabricatorMetaMTAMailBody, DifferentialRevision, array) called at [<phabricator>/src/applications/differential/editor/DifferentialTransactionEditor.php:1208]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #4 <#2> DifferentialTransactionEditor::buildMailBody(DifferentialRevision, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2178]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #5 <#2> PhabricatorApplicationTransactionEditor::sendMailToTarget(DifferentialRevision, array, PhabricatorMailTarget) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2152]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #6 <#2> PhabricatorApplicationTransactionEditor::sendMail(DifferentialRevision, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:998]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #7 <#2> PhabricatorApplicationTransactionEditor::publishTransactions(DifferentialRevision, array) called at [<phabricator>/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php:21]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #8 <#2> PhabricatorApplicationTransactionPublishWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:91]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #9 <#2> PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:162]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #10 <#2> PhabricatorWorkerActiveTask::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:20]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #11 PhabricatorTaskmasterDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:185]
Daemon 39819 STDE [Thu, 04 Jun 2015 02:56:38 +0000]   #12 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:125]
```

Auditors: btrahan
2015-06-03 20:01:23 -07:00
epriestley
6dede2e2c5 Make PhameBlog implement PhabricatorApplicationTransactionInterface
Summary: Ref T6367.

Test Plan: Static correctness.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6367

Differential Revision: https://secure.phabricator.com/D13143
2015-06-03 18:59:42 -07:00
epriestley
069e60d2ff Send mail to targets in the user's translation
Summary: Ref T6367.

Test Plan:
  - Added and executed unit tests.
  - Sent mail to A (en_US) and B (en_A*).
  - Got one mail in English and one mail in ENGLISH.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6367

Differential Revision: https://secure.phabricator.com/D13142
2015-06-03 18:59:33 -07:00
epriestley
6db97bde12 Build separate mail for each recipient, honoring recipient access levels
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
2015-06-03 18:59:31 -07:00
epriestley
a9ceebbdb1 Move all ApplicationTransaction publishing to daemons
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
2015-06-03 18:59:29 -07:00
epriestley
1fc1114e29 Allow TransactionEditor to move publishing work to the daemons
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
2015-06-03 18:59:28 -07:00
epriestley
d6247ffca5 Add support for "Extended Policies"
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
2015-06-03 18:59:27 -07:00
epriestley
908b426ee2 Put PhabricatorMetaMTAMailingList back to keep bin/storage adjust happy for now
Auditors: btrahan
2015-06-03 18:51:40 -07:00
epriestley
69ee51dc53 Note extra mailing list settings in documentation
Summary: Ref T8387. Just mention this stuff since it's moderately useful.

Test Plan: Read.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8387

Differential Revision: https://secure.phabricator.com/D13139
2015-06-03 18:42:43 -07:00
epriestley
a4d874287f Allow administrators to configure "Email Settings" settings for lists
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
2015-06-03 18:42:41 -07:00
epriestley
d4a2034d2f Allow administrators to configure "Email Format" settings for lists
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
2015-06-03 18:42:40 -07:00
epriestley
14e318cb16 Allow administrators to edit the "Account" panel for mailing lists and bots
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
2015-06-03 18:42:39 -07:00
epriestley
02f0c099fa Fix some minor Spaces method visibility issues
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
2015-06-03 18:42:38 -07:00
epriestley
ba6cb62b49 Remove mailing lists application
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
2015-06-03 18:42:36 -07:00
epriestley
249ee9f104 Migrate mailing lists to mailing list users
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
2015-06-03 18:42:35 -07:00
epriestley
e0a3e6ba3b Allow administrators to manage addresses for mailing list users
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
2015-06-03 18:42:34 -07:00
epriestley
992c199577 Add "Mailing List" users
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
2015-06-03 18:42:33 -07:00
epriestley
13f0dac0ed Expand the power of user filtering
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
2015-06-03 18:42:32 -07:00
epriestley
8440b3efc0 Update account roles documentation and remove actAsUser
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
2015-06-03 18:42:09 -07:00
epriestley
8928aa2b67 Fix an issue with AlamancDeviceEditor
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
2015-06-03 18:41:21 -07:00
Joshua Spence
0b87e462cd Remove flags and tokens upon object destruction
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
2015-06-04 07:23:48 +10:00
lkassianik
55c73b20ca Attempt to optimize ghost generating code.
Summary: Ref T8394, Attempt to optimize ghost generating code.

Test Plan: Open Upcoming, cancelled only, list view query. Should not hang.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8394

Differential Revision: https://secure.phabricator.com/D13141
2015-06-03 13:01:26 -07:00
Mukunda Modell
dfac9d7f52 Add 'colors' and 'icons' fields to conduit api for Projects
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
2015-06-03 11:43:58 -07:00
lkassianik
d3a84687ba Query for recurring event parents should accept a "not a child of anything" clause
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
2015-06-03 09:27:39 -07:00
Joshua Spence
c26ea062d1 Remove unused PhrictionActionConstants class
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
2015-06-03 20:23:08 +10:00