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

267 commits

Author SHA1 Message Date
Mihir Kedia
6a3c1ba05c Fix bin/lipsum
Summary:
- Set a default edit policy to match view policy for projects
- It seems like initializeNew* is a better init function than new Phabricator*,
  so ported two classes to use it and spot-checked the others (this also fixed
  a fatal.)
- Other minor bugs

Test Plan: Ran lipsum for a while without any fatals

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, jhurwitz, Korvin

Differential Revision: https://secure.phabricator.com/D13559
2015-07-07 05:33:03 -07:00
Joshua Spence
f2435fd1d0 Return $this from setter methods
Summary: Return `$this` from setter methods for consistency. I started writing a [[https://secure.phabricator.com/differential/diff/32506/ | linter rule]] to detect this, but I don't think it is trivial to do this properly.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13422
2015-07-06 22:53:43 +10:00
Joshua Spence
acb1eb81cc Move some PhabricatorSearchField subclasses
Summary: Move some `PhabricatorSearchField` subclasses to be adjacent to the application to which they belong. This seems generally better to me than lumping them all together in the `src/applications/search/field/` directory. I was also wondering if it makes sense to rename these subclasses as `PhabricatorXSearchField` rather than `PhabricatorSearchXField` (as per T5655), but wasn't really sure if these objects are meant to be search-fields, or just fields belonging to the #search application.

Test Plan: N/A.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13374
2015-07-06 22:52:05 +10:00
epriestley
b55f9b6120 Merge branch 'master' into redesign-2015 2015-06-22 12:26:41 -07:00
epriestley
d1983560a6 Show when objects have a non-default policy
Summary:
Fixes T6787. I'm kind of cheating a little bit here by not unifying default selection with `initializeNew(...)` methods, but I figure we can let this settle for a bit and then go do that later. It's pretty minor.

Since we're not doing templates I kind of want to swap the `'template'` key to `'type'` so maybe I'll do that too at some point.

@chad, freel free to change these, I was just trying to make them pretty obvious. I //do// think it's good for them to stand out, but my approach is probably a bit inconsistent/heavy-handed in the new design.

Test Plan:
{F525024}

{F525025}

{F525026}

{F525027}

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: johnny-bit, joshuaspence, chad, epriestley

Maniphest Tasks: T6787

Differential Revision: https://secure.phabricator.com/D13387
2015-06-22 11:46:59 -07:00
Chad Little
801607381d [Redesign] PhabricatorApplicationSearchResultView
Summary: Ref T8099. This adds a new class which all search engines return for layout. I thought about this a number of ways, and I think this is the cleanest path. Each Engine can return whatever UI bits they needs, and AppSearch or Dashboard picks and lays the bits out as needed. In the AppSearch case, interfaces like Notifications, Calendar, Legalpad all need more custom layouts. I think this also leaves a resonable path forward for NUX as well. Also, not sure I implemented the class correctly, but assume thats easy to fix?

Test Plan: Review and do a search in each application changed. Grep for all call sites.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D13332
2015-06-19 11:46:20 +01:00
epriestley
53ef057b1b Merge branch 'master' into redesign-2015 2015-06-15 08:06:23 -07:00
epriestley
52f8756c3c Add a "template" parameter to application default policies
Summary:
Ref T5681. Ref T6860. This doesn't do anything interesting on its own, just makes the next diff smaller.

In the next diff, policies become aware of the types of objects they're acting on. We need to specify which object type all the "Default View/Edit" settings are for so they get the right rules.

For example, a rule like "Allow task author" is OK for "View Policy" on a task, and also OK for "Default View Policy" on ManiphestApplication. But it's not OK for "Can Create Tasks" on ManiphestApplication.

So annotate all the "template"/"default" policies with their types. The next diff will use these to let you select appropriate rules for the given object type.

Test Plan:
  - Used `grep` to find these.
  - This change has no effect.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5681, T6860

Differential Revision: https://secure.phabricator.com/D13251
2015-06-11 13:25:30 -07:00
epriestley
0bc8382dfd Support Spaces in ApplicationEmail
Summary:
Ref T8498. Allow ApplicationEmail addresses to be put into spaces:

  - You can only see and send to addresses in Spaces you have access to.
  - Objects are created into the same space their address is associated with.

Test Plan:
  - Used `bin/mail receive-test` to send mail to various `xyz-bugs@...` addresses.
  - Saw objects created in the proper space.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8498

Differential Revision: https://secure.phabricator.com/D13247
2015-06-11 10:23:56 -07:00
epriestley
739bdeccab Improve some Spaces behaviors
Summary:
Ref T8449. Try out some more subtle behaviors:

  - Make the "Space" control part of the policy control, so the UI shows "Visible To: [Space][Policy]". I think this helps make the role of spaces more clear. It also makes them easier to implement.
  - Don't show the default space in headers: instead, show nothing.
  - If the user has access to only one space, pretend spaces don't exist (no edit controls, no header stuff).

This might be confusing, but I think most of the time it will all align fairly well with user expectation.

Test Plan:
  - Viewed a list of pastes (saw Space with non-default space, no space with default space, no space with user in only one space).
  - Viewed a paste (saw Space with non-default space, saw no space with default space, saw no space with user in only one space).
  - Edited spaces on objects (control as privileged user, no control as locked user).
  - Created a new paste in a space (got space select as privileged user, no select as locked user).

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8449

Differential Revision: https://secure.phabricator.com/D13229
2015-06-10 15:52:18 -07:00
epriestley
57b898af9a Merge branch 'master' into redesign-2015 2015-06-10 07:44:58 -07:00
epriestley
bf87976d25 Support ordering in SearchField
Summary:
Ref T8441. Ref T7715. Automatically generate a modern "Order" control in ApplicationSearch for engines which fully support SearchField.

Notably, this allows the standard "Order" control to automatically support custom field orders. We do this in Maniphest today, but in an ad-hoc way.

Test Plan: Performed order-by queries in Almanac (Services), Pholio, Files, People, Projects, and Paste.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13193
2015-06-08 12:21:48 -07:00
epriestley
3cdaf52ce9 Make Subscribers automatically provide working SearchFields
Summary:
Ref T8441. Ref T7715. For modern Query classes, automatically make subscriber queries and SearchField integrations work.

In particular, we can just drive this query with EdgeLogic and don't need to do anything specific on these Query classes beyond making sure they're implemented in a way that picks up all of the EdgeLogic clauses.

Test Plan:
  - Searched for subscribers in Pholio, Files, Paste, and Projects.
  - Searched for all other fields in Projects to check that Query changes are OK.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13191
2015-06-08 12:20:53 -07:00
Joshua Spence
259b40fc28 Fix some method visibilities
Summary: See D13185, the `testMethodVisibility()` test was not being executed.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13186
2015-06-08 09:52:14 +10:00
epriestley
2492fef029 Move Pholio to SearchFields
Summary:
Ref T8441. Ref T7715.

  - Update PholioSearchEngine.
  - Automatically add project fields.
  - Update Paste to support project search.
  - Simplify common Query class construction.

Test Plan:
  - Searched for pastes.
  - Searched for mocks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13174
2015-06-07 07:31:28 -07:00
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
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
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
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
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
Chad Little
79c6f1c60a [Redesign] Update Paste UI
Summary: Ref T8099, Adds a bit more color/contract to Paste.

Test Plan: Review a list of Pastes, embedded Pastes, and the main Paste UI.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D13083
2015-05-31 12:13:28 -07:00
Joshua Spence
36e2d02d6e phtize all the things
Summary: `pht`ize a whole bunch of strings in rP.

Test Plan: Intense eyeballing.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12797
2015-05-22 21:16:39 +10:00
epriestley
f99c7beb90 Fully remove all the public-create-mail settings
Summary: Fixes T5703. These have been unused in production for a while and the new stuff seems good.

Test Plan: Mostly `grep`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5703

Differential Revision: https://secure.phabricator.com/D12949
2015-05-20 14:21:33 -07:00
Bob Trahan
81a475d5a6 Transactions - make implementing TYPE_XXXX_POLICY transactions optional
Summary: Ref T6403. This was actually simple stuff.

Test Plan: changed the edit policy of a paste. changed the edit and join policy of a phame blog.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6403

Differential Revision: https://secure.phabricator.com/D12933
2015-05-19 12:58:18 -07:00
Bob Trahan
01a8ba5a97 Transactions - make TYPE_COMMENT implementation optional
Summary: Ref T6403. Conpherence keeps track of comments for message counts so we needed some special attention there. Otherwise, straight-forward.

Test Plan: left a comment on a diff with inline comments. sent messages in conpherence successfully. verified unread count incremented correctly for sent messages for users.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6403

Differential Revision: https://secure.phabricator.com/D12932
2015-05-19 12:33:55 -07:00
Bob Trahan
18e0ee0791 Transactions - move TYPE_SUBSCRIBERS to require optional implementation
Summary: Ref T6403. This one was pretty easy since no one does anything custom with subscribers.

Test Plan: subscribed / unscribed to a random commit ("audit"). joined / left, watched / unwatched a project

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6403

Differential Revision: https://secure.phabricator.com/D12930
2015-05-19 11:48:02 -07:00
Bob Trahan
16c8d44c37 Transactions - make customization of TYPE_EDGE optional
Summary: Ref T6403. This does TYPE_EDGE since I just had to deal with T8252. Look like this fixes a few editors (maybe) that would have had fatals with mentions like slowvote and ponder.

Test Plan: made a phame post mentioning a task and it worked! joined / left a project, watched / unwatched a project and that worked! blind faith for other sites.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6403

Differential Revision: https://secure.phabricator.com/D12929
2015-05-19 11:26:53 -07:00
epriestley
b0862a8176 Filter pastes at the query level if we're unable to load their content
Summary:
Fixes T8230. I think I broke this in D12354 since I missed the fact that this could filter results, because it's constructed in a sort of weird way. Try to make the logic a little more clear, while retaining the relevant properties:

  - Pastes with unloadable content should be discarded;
  - the input order should be retained in the result set.

Test Plan: Faked content misses and saw things work instead of fatal.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8230

Differential Revision: https://secure.phabricator.com/D12909
2015-05-18 13:57:20 -07:00
Bob Trahan
3ef0721ada Reduce PhabricatorUser::getOmnipotentUser calls by adding a getViewer method to PhbaricatorDestructionEngine
Summary:
Fixes T6956. Before this change, we called PhabricatorUser::getOmnipotentUser in the various delete methods to query the data. Now, we use $engine->getViewer(), since its always a good thing to have less calls to PhabricatorUser::getOmnipotentUser thrown around the codebase.

I used the "codemod" tool to audit the existing calls to PhabricatorDestructorEngine (all of them) so ostensibly this gets all the spots. If I missed something though, its still going to work, so this change is very low risk.

Test Plan: ./bin/remove destroy P1; visit P1 and get a 404

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6956

Differential Revision: https://secure.phabricator.com/D12866
2015-05-15 14:07:17 -07:00
Joshua Spence
61b178f44e Use PhutilInvalidStateException
Summary: Use `PhutilInvalidStateException`. Depends on D12803.

Test Plan: Unit tests pass.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12829
2015-05-14 07:53:52 +10:00
Joshua Spence
acb45968d8 Use __CLASS__ instead of hard-coding class names
Summary: Use `__CLASS__` instead of hard-coding class names. Depends on D12605.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12806
2015-05-14 07:21:13 +10:00
Joshua Spence
4ea9d76f66 Add some missing type hints
Summary: Add some typehints for Remarkup rules.

Test Plan: Browsed around some Remarkup text.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12694
2015-05-05 07:33:00 +10:00
epriestley
f5580c7a08 Make buildWhereClause() a method of AphrontCursorPagedPolicyAwareQuery
Summary:
Ref T4100. Ref T5595.

To support a unified "Projects:" query across all applications, a future diff is going to add a set of "Edge Logic" capabilities to `PolicyAwareQuery` which write the required SELECT, JOIN, WHERE, HAVING and GROUP clauses for you.

With the addition of "Edge Logic", we'll have three systems which may need to build components of query claues: ordering/paging, customfields/applicationsearch, and edge logic.

For most clauses, queries don't currently call into the parent explicitly to get default components. I want to move more query construction logic up the class tree so it can be shared.

For most methods, this isn't a problem, but many subclasses define a `buildWhereClause()`. Make all such definitions protected and consistent.

This causes no behavioral changes.

Test Plan: Ran `arc unit --everything`, which does a pretty through job of verifying this statically.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, hach-que, epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12453
2015-04-20 10:06:09 -07:00
Chad Little
ca5909cac6 Add Mailtags to Paste
Summary: Fixes T7786. Adds very basic mailtag support.

Test Plan: Tested changing language, title to a Paste, didn't get notifications on my test account.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7786

Differential Revision: https://secure.phabricator.com/D12404
2015-04-13 15:30:59 -07:00
epriestley
156b156e77 Give Conduit params/return/errors protected visibility
Summary:
Ref T7803. Ref T5873. I want to drive Conduit through more shared infrastructure, but can't currently add parameters automatically.

Put a `getX()` around the `defineX()` methods so the parent can provide default behaviors.

Also like 60% of methods don't define any special error types; don't require them to implement this method. I want to move away from this in general.

Test Plan:
  - Ran `arc unit --everything`.
  - Called `conduit.query`.
  - Browsed Conduit UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T5873, T7803

Differential Revision: https://secure.phabricator.com/D12380
2015-04-13 11:58:35 -07:00
epriestley
e6174ed45c Fix an issue where pastes could be reordered as a side effect of cache fills
Summary: Ref T7803. Pastes which needed a cache fill would incorrectly be dropped to the bottom of the list. Stop doing that.

Test Plan: Loaded a list of pastes with some that needed cache fills, saw them appear in the correct order.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12354
2015-04-13 11:58:16 -07:00
Sébastien Santoro
bf68ab2562 Fixed typo in PhabricatorPasteTransaction
Summary: update -> updated

Test Plan: read again the sentence

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12350
2015-04-11 08:39:39 -07:00
Joshua Spence
ea376685ae Fix some odd looking arrays
Summary: These arrays looks a little odd, most likely due to the autofix applied by `ArcanistXHPASTLinter::LINT_ARRAY_SEPARATOR`. See D12296 in which I attempt to improve the autocorrection from this linter rule.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12281
2015-04-05 22:29:39 +10:00
epriestley
eb81fd1562 Expose all application mail receivers
Summary:
Fixes T7199. This still isn't a shining example of perfect code, but the raw amount of copy/paste is much lower than it used to be.

  - Reduce code duplication between existing receivers.
  - Expose receiving objects in help menus where appropriate.
  - Connect some "TODO" receivers.

Test Plan:
  - Sent mail to every supported object type.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12249
2015-04-01 11:52:02 -07:00
epriestley
c32fee0e48 Fully modularize mail commands
Summary: Ref T7199. Everyone can have a mail command! You can have a mail command! You can have a mail command! Mail commands for everyone!

Test Plan: Used `bin/mail receive-test` to issue commands against files and pastes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12238
2015-04-01 08:40:00 -07:00
epriestley
161f936871 Lift common code for transaction-based reply handlers into parent class
Summary:
Ref T7199. Essentially all of the reply handlers now apply transactions to something which implements PhabricatorApplicationTransactionInterface.

We can share code between them by lifting this stuff into a superclass.

First, convert paste. Also rename `PasteMockMailReceiver` to `PasteMailReceiver` (this got mis-copied from Pholio at some point, I think).

Test Plan: Used `bin/mail receive-test` to send comments + `!unsubscribe` to pastes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12236
2015-04-01 08:39:21 -07:00
epriestley
0d99c84bd7 Modernize email command parsing
Summary:
Ref T7199. This prepares for an exciting new world of more powerful "!action" commands. In particular:

  - We parse multiple commands per mail.
  - We parse command arguments (these are currently not used).
  - We parse commands at the beginning or end of mail.

Additionally:

  - Do a quick modernization pass on all handlers.
  - Break legacy compatibility with really hacky Facebook stuff (see T1992). They've theoretically been on notice for a year and a half, and their setup relies on calling very old reply handler APIs directly.
  - Some of these handlers had some copy/paste fluff.
  - The Releeph handler is unreachable, but fix it //in theory//.

Test Plan:
- Sent mail to a file; used "!unsubscribe".
- Sent mail to a legalpad document; used "!unsubscribe".
- Sent mail to a task; used various "!close", "!claim", "!assign", etc.
- Sent mail to a paste.
- Sent mail to a revision; used various "!reject", "!claim", etc.
- Tried to send mail to a pull request but it's not actually reachable.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12230
2015-03-31 16:48:27 -07:00
epriestley
030e05aa4c Remove reply handler instructions from email
Summary:
Ref T7199. Although this is useful for discovery, it's un-useful enough that we already have an option to disable it, and most applications do not provide any meaningful instructions.

Throwing it away makes it easier to move forward and lets us get rid of a config option.

This is becoming a more advanced/power-user feature anyway, and the new syntax will be significantly more complex and hard to explain with a one-liner. I'm currently thinking that I'll maybe make the "help" menu a dropdown and give it some options like:

  +---+
  | O |
  +---+---------------------+
  | Maniphest Documentation |
  | Maniphest Email Actions |
  +-------------------------+

Then you click the "Email Actions" thing and get a runtime-derived list of available options. Not sure if I'll actually build that, but I think we can fairly throw the in-mail instructions away even if we don't go in that specific direction.

Test Plan: Grepped for `replyHandlerInstructions`, got no hits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12229
2015-03-31 16:48:17 -07:00
epriestley
d403700e1f Convert all tokenizers to take token/scalar inputs
Summary: Ref T7689. Ref T4100. This advances the goals of removing `loadViewerHandles()` (only 67 callsites remain!) and letting tokenizers some day take token functions like `viewer()` and `members(differential)`.

Test Plan:
- Sent a new message; used "To".
  - I simplified the cancel URI construction slightly because it's moot in all normal cases.
- Edited a thread; used "Add Participants".
- Searched rooms; used "Participants".
- Searched countdowns; used "Authors".
- Created a diff; used "Repository".
- Edited a revision; edited "Projects"; edited "Reveiwers"; edited "Subscribers".
- Searched for revisions; edited "responsible users"; "authors"; "reviwers"; "subscribers"; "repositories".
- Added revision comments; edited "Add Reveiwers"; "Add Subscribers".
- Commented on a commit; edited "Add Auditors"; "Add subscribers".
- Edited a commit; edited "Projects".
- Edited a repository; edited "Projects".
- Searched feed, used "include Users"; "include Proejcts".
- Searched files, used "authors".
- Edited initiative; edited "Projects".
- Searched backers; used "Backers".
- Searched initiatives; used "Owners".
- Edited build plans; edited "Run Command".
- Searched Herald; used "Authors".
- Added signature exemption in Legalpad.
- Searhced legalpad; used "creators"; used "contributors".
- Searched signatures; used "documents"; used "signers".
- Created meme.
- Searched macros; used "Authors".
- Used "Projects" in Maniphest reports.
- Used Maniphest comment actions.
- Edited Maniphest tasks; edited "Assigned To"; edited "CC"; edited "projects".
- Used "parent" in Maniphest task creation workflow.
- Searched for projects; used "assigned to"; "in any projec"; "in all projects"; "not in projects"; "in users' projects"; "authors"; "subscribers".
- Edited Maniphest bug filing domains, used "Default Author".
- Searched for OAuth applications, used "Creators".
- Edited Owners pacakge; edited "Primary Owner"; edited "Owners".
- Searched for Owners packages; used "Owner".
  - OMG this UI is OLD
- Edited a paste; edited "Projects".
- Searched for paste; used "Authors".
- Searched user activity log; used "Actors"; used "Users".
- Edited a mock; edited "Projects"; edited "CC".
- Searched for mocks; used "Authors".
- Edited Phortune account; edited "Members".
- Edited Phortune merchant account; edited "Members".
- Searched Phrequent; used "Users".
- Edited Ponder question; sued "projects".
- Searched Ponder; used "Authors"; used "Answered By".
- Added project members.
- Searched for projects; used "Members".
- Edited a Releeph product; edited "Pushers".
- Searched pull requests; searched "Requestors".
- Edited an arcanist project; used "Uses Symbols From".
- Searhced push logs; used "Repositories"; used "Pushers".
- Searched repositories; used "In nay project".
- Used global search; used Authors/owners/Subscribers/In Any Project.
- Edited a slowvote; used "Projects".
- Searched slovotes; used "Authors".
- Created a custom "Users" field; edited and searched for it.
- Made a whole lot of typos in this list. ^^^^^^

Did not test:

- Lint is nontrivial to test locally, I'll test it in production.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100, T7689

Differential Revision: https://secure.phabricator.com/D12224
2015-03-31 14:10:55 -07:00
epriestley
64dddc76c5 Remove Controller->getHandle() and Controller->loadHandles()
Summary: Ref T7689. Modernize all callsites of these methods.

Test Plan:
- Poked at dashboards.
  - Pretty sure this code is technically unreachable right now.
- Viewed commit; viewed "Audit Status".
- Viewed a fund; viewed "Payable to"; viewed "Owner".
- Viewed herald rules; viewed "Author"; viewed "Applies To".
- Viewed a Legalpad document; viewed "Contributors".
- Viewed Phame post list; viewed blog; viewed post (viewed "Blog", viewed "Blogger").
- Viewed a macro; viewed "Audio".
- Viewed a Phriction page; viewed "Last Author".
- Viewed a Ponder question; viewed "Author".
- Viewed a Ponder answer; viewed header.
  - Behavior changed very slightly here; whatevs.
- Viewed a Countdown; viewed "Author".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12210
2015-03-31 05:48:20 -07:00
epriestley
a8271ecd40 Remove most callsites to Controller->renderHandlesForPHIDs()
Summary: Ref T7689. This moves most of the easy/testable callsites off `Controller->renderHandlesForPHIDs()`.

Test Plan:
- Viewed a file; viewed author; viewed "attached" tab.
- Viewed a mock; viewed attached tasks.
- Viewed a credential; viewed "Used By".
- Viewed a paste; viewed author; viewed forks; viewed forked from.
- Viewed a dashboard; viewed panel list.
- Viewed a dashboard panel; viewed "Appears On".
- Viewed a Phortune account; viewed "Members"; viewed payment methods.
- Viewed a Phortune merchant account; viewed "Members".
- Viewed Phortune account switcher; viewed "Accounts".
  - I just removed "Members:" here since it felt kind of out-of-place anyway.
- Viewed a Phragment fragment, viewed "Latest Version", viewed "Snapshots".
- Viewed a Phargment snapshot, viewed "Fragment".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: hach-que, epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12207
2015-03-31 05:48:19 -07:00
Chad Little
59a35af395 Minor touchup to Paste Embed UI
Summary: Consolidate colors / spacing.

Test Plan:
Test embedding a paste, a list of pastes, and a PasteView for new colors, space.

{F321622}

{F321623}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11896
2015-02-26 11:09:10 -08:00