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

9589 commits

Author SHA1 Message Date
epriestley
2ecdbbec10 Don't revert Conpherence page titles when updating notifications
Summary: Fixes T7709. `JX.Title` deals with adding `(1)`, etc., counts, so send updates through it.

Test Plan: Clicked between some threads in Conpherence, no title flickering.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7709

Differential Revision: https://secure.phabricator.com/D12260
2015-04-02 13:41:48 -07:00
epriestley
a804f0ab93 Make file policies for emailed files more consistent
Summary:
Fixes T7712. Currently, files sent via email get default policies, like they were dragged and dropped onto the home page.

User expectation is better aligned with giving files more restrictive policies, like they were draggged and dropped directly onto an object.

Make files sent via email have restricted default visibility. Once we identify the sender, set them as the file author. Later, the file will become visible to other users via attachment to a task, revision, etc.

Test Plan: Sent some files via email; verified they got restrictive policies, correct authorship, and appropriate object attachment.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7712

Differential Revision: https://secure.phabricator.com/D12255
2015-04-02 13:41:39 -07:00
epriestley
e74c386d56 Fix an issue where RepositoryQuery may fail for viewers who can't use Diffusion
Summary:
See IRC. We don't always reach `loadPage()`, particularly if `canViewerUseQueryApplication()` fails.

Perform initialization steps in `willExecute()` instead.

Trace from IRC:

```
2015/04/02 11:48:32 [error] 26979#0: *48012 FastCGI sent in stderr: "PHP message: [2015-04-02 11:48:32] EXCEPTION: (Exception) You must execute() the query before accessing the identifier map. at [<phabricator>/src/applications/repository/query/PhabricatorRepositoryQuery.php:134]
PHP message:   #0 PhabricatorRepositoryQuery::getIdentifierMap() called at [<phabricator>/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php:26]
PHP message:   #1 DiffusionRepositoryRemarkupRule::loadObjects(array) called at [<phabricator>/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php:309]
PHP message:   #2 PhabricatorObjectRemarkupRule::didMarkupText() called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:292]
PHP message:   #3 PhutilRemarkupEngine::postprocessText(array) called at [<phabricator>/src/infrastructure/markup/PhabricatorMarkupEngine.php:138]
PHP message:   #4 PhabricatorMarkupEngine::process() called at [<phabricator>/src/infrastructure/markup/PhabricatorMarkupEngine.php:71]
PHP message:   #5 PhabricatorMarkupEngine::renderOneObject(PhabricatorMarkupOneOff, string, PhabricatorUser) called at [<phabricator>/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php:47]
PHP message:   #6 PhabricatorStandardCustomFieldRemarkup::renderPropertyViewValue(array) called at [<phabricator>/src/infrastructure/customfield/field/PhabricatorCustomField.php:1191]
PHP message:   #7 PhabricatorCustomField::renderPropertyViewValue(array) called at [<phabricator>/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php:176]
PHP message:   #8 PhabricatorCustomFieldList::appendFieldsToPropertyList(PhabricatorProject, PhabricatorUser, PHUIPropertyListView) called at [<phabricator>/src/applications/project/controller/PhabricatorProjectProfileController.php:220]
PHP message:   #9 PhabricatorProjectProfileController::buildPropertyListView(PhabricatorProject, PhabricatorActionListView) called at [<phabricator>/src/applications/project/controller/PhabricatorProjectPr
```

Test Plan:
  - Used `rX` in remarkup.
  - Used `rX` in search.
  - Browed Diffusion, etc.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12264
2015-04-02 13:41:30 -07:00
epriestley
638cdb8d57 Don't show comment draft state to non-object-owners
Summary: Fixes T7720. We currently leak the "draft" state of checkboxes; never treat checkboxes as drafts if you can't mark them.

Test Plan: Checked a box, reloaded page in other browser. Previously, the draft state partially propagated. Now, it no longer does.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7720

Differential Revision: https://secure.phabricator.com/D12262
2015-04-02 07:27:53 -07:00
Elan Kugelmass
1ae1cdd7ba Herald queries correctly label Object Rules
Summary: Fixes T7730. Herald queries used to incorrectly label object rules as global rules. An object rule is now labeled as such.

Test Plan: Made a few rules and looked at the herald query page.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7730

Differential Revision: https://secure.phabricator.com/D12259
2015-04-02 05:00:51 -07:00
Chad Little
8e5153e866 Make busy indicator durable column aware
Summary: Fixes T7699, moves busy indicator when durable column is present

Test Plan: visit uiexamples busy page, toggle column on and off

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7699

Differential Revision: https://secure.phabricator.com/D12257
2015-04-01 20:10:32 -07:00
epriestley
e9886c4353 Fix an issue where we would try to release an unheld lock
Summary: Fixes T7484. If the lock failed, we'd still try to unlock it, which is incorrect.

Test Plan: Ran two `bin/repository update X` in different windows, got proper LockException instead of indirect symptomatic "not locked by this process" exception.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7484

Differential Revision: https://secure.phabricator.com/D12253
2015-04-01 17:37:46 -07:00
Michael Krasnow
898ccd7552 Add translation for the string "Submit" for the Very Wow translation
Summary: Added Submit translation for Very Wow Locale

Test Plan: Look at the code and hope it works

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12254
2015-04-01 16:41:53 -07:00
Chad Little
de737e8039 Update Participants UI in Conpherence Full
Summary: This shrinks the UI to fit more people in the participants list, useful for rooms. Also update the remove icon.

Test Plan:
Review a lot of people in a room, so so many.

{F354233}

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12213
2015-04-01 15:49:25 -07:00
Bob Trahan
fa673dde35 Conpherence - finish basic application search
Summary:
Fixes T7584. Adds the ability to specify rooms, messages, or both. Adds policy icon to rooms result view and envelope icon to messages result view. Fixes a missing group by clause in thread query. Enforces having participant phid if the query isn't looking at rooms and doesn't have other particpant phids.

This last bit has a small UI quirk if the user searches for "messages" or "both" with no participant phids as we don't give them the feedback that they were included in the query. We could just slap the viewer in the particpants list in this case but it seemed like a buggier feeling experience to have the viewer appear up there? (Especially so in messages case, where we are basically being smart about policy filtering to come.)

Test Plan: clicked around and got sensible results

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7584

Differential Revision: https://secure.phabricator.com/D12232
2015-04-01 15:48:47 -07:00
Chad Little
410380be3f Update Conpherence full message column UI
Summary:
Minor things

 - Use radiused avatars like timeline
 - Fix edge case with conpherence edited and date markers

Test Plan: Review a number of odd states in Conpherence full.

Reviewers: epriestley, btrahan

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12251
2015-04-01 13:15:53 -07:00
epriestley
983c6f9d4b Fix a tokenizer issue in a rare edge case
Summary:
Ref T7689. We may not propgate the viewer down to the controls in some rendering pathways. An example is "Create Task..." on workboard columns.

Instead, move the propagation a little deeper.

Test Plan: "Create Task..." on a workboard column.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12250
2015-04-01 12:41:23 -07: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
52eab87608 Implement the "!priority" and "!status" mail commands
Summary: Ref T7199. Adds "!priority" and "!status".

Test Plan:
  - Used `!priority` and `!status` to adjust tasks.
  - Changed config and provided keywords.

{F355976}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12248
2015-04-01 11:51:53 -07:00
epriestley
be1fbba698 Implement a "!subscribe" mail command
Summary: Ref T7199. Implements "!subscribe".

Test Plan: Used `bin/mail receive-test` to apply the command to objects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12247
2015-04-01 11:51:52 -07:00
epriestley
cb6349b88c Implement a "!projects" mail command
Summary: Ref T7199. Implements `!projects` for all objects which implement `PhabricatorProjectInterface`.

Test Plan: Added projects to a task via email.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12246
2015-04-01 11:51:51 -07:00
epriestley
25f8a37f85 Provide better general high-level documentation on mail commands
Summary: Ref T7199. This makes the page look less janky and provides more context about how mail commands work and how to use them.

Test Plan: {F355959}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12245
2015-04-01 11:51:49 -07:00
epriestley
c169199e64 Allow applications to have multiple "help" menu items
Summary:
Ref T7199. Convert the single help menu item into a dropdown and allow applications to list multiple items there.

When an application has mail command objects, link them in the menu.

Test Plan:
{F355925}

{F355926}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12244
2015-04-01 11:51:48 -07:00
epriestley
6f95b325c6 Generate mail command documentation from active commands
Summary: Ref T7199. This needs some polish and isn't reachable from the UI, but technically has all of the information.

Test Plan:
{F355899}

{F355900}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12241
2015-04-01 11:51:47 -07:00
epriestley
6f59b2ab87 Move Maniphest to modular mail commands
Summary:
Ref T7199. This fully modularizes mail command handling in Maniphest.

I had to add a couple of minor not-totally-solid-feeling tricks to deal with the "create" case, but they feel not-too-bad, and a million times better than what came before.

Test Plan: Used all commands with `receive-test`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12240
2015-04-01 08:40:42 -07:00
epriestley
d4bfaa2feb Move Differential to modular mail commands
Summary: Ref T7199. Convert Differential to modern modular commands.

Test Plan: Used `bin/mail receive-test` to send command and comment mail to Differential.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12239
2015-04-01 08:40:12 -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
7c5f71b691 Subclass most ReplyHandlers from TransactionReplyHandler
Summary: Ref T7199. Half of these aren't even reachable, but make some progress toward reducing the amount of nonsense and garbage in mail handling.

Test Plan: Tested all reachable handlers with `bin/mail receive-test`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12237
2015-04-01 08:39:50 -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
afd86a0420 Improve rules for embedding files received via email
Summary:
Ref T7199. Ref T7712. This improves the file rules for email:

  - Embed visible images as thumbnails.
  - Put all other file types in a nice list.

This "fixes" an issue caused by the opposite of the problem described in T7712 -- files being dropped if the default ruleset is too restrictive. T7712 is the real solution here, but use a half-measure for now.

Test Plan:
  - Sent mail with two non-images and two images.
  - Got a nice list of non-images and embeds of images.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7712, T7199

Differential Revision: https://secure.phabricator.com/D12235
2015-04-01 08:39:03 -07:00
Bob Trahan
6f028e16e8 Conpherence - fix fatal if you don't have conpherences
Summary: Fixes T7721. Looks like this got renamed at some point but skipped. Also, change the true / false idx look ups to correct integers here since there's a warning in the error logs in this case without it.

Test Plan: made  new user. loaded up conpherence and it worked. tried all the links too for good measure and they worked

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7717, T7721

Differential Revision: https://secure.phabricator.com/D12243
2015-04-01 07:59:55 -07:00
epriestley
77b23035c3 Allow Maniphest to process multiple commands via email
Summary:
Ref T7199. Two notable changes:

  - Process multiple commands.
  - Process commands when creating //or// updating a task.

And generally clean things up a bit.

Test Plan:
  - Used `receive-test` to execute all commands for new tasks.
  - Used `receive-test` to execute all commands for existing tasks.
  - Used a combination of commands to produce varied effects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12234
2015-03-31 17:25:19 -07:00
epriestley
34db543d27 Remove all application-specific reply handler class overrides
Summary:
Ref T7199. In the vein of D12231, these options were a bad idea.

  - They once served a very narrow, Facebook-specific need (see T1992), except even Facebook only used the Differential setting AFAIK.
  - Outside of that special case, they are unused and essentially unusable (generally speaking, they do not meaningfully implement anything modular or replaceable).
  - I have no knowledge of any install ever changing these settings, and can imagine no reason why they would.

Moving forward:

  - If they really need to, they can fork locally and chagne one line.
  - I expect "!actions" to make mail at least somewhat more modular soon, anyway.
  - Any derived handlers would break after T7199 and need to be rewritten anyway, so this is just taking advantage of a BC break to do cleanup.

Test Plan:
  - Grepped for removed configuration.
  - Sent some mail from applications, verified the reply handlers set proper reply addresses.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12233
2015-03-31 17:22:01 -07:00
epriestley
bad645f1ec Remove all application-specific reply handler domains
Summary:
Ref T7199. These were a bad idea which got copy-pasted a bunch.

  - There is zero reason to ever set these to different things.
  - Unsurprisingly, I don't know of any install which has them set to different things.

Unless I've completely forgotten about it, this option was not motivated by some obscure business need, it was just a bad decision which didn't catch anyone's attention at the time.

We partially remedied the mistake at some point by introducing `metamta.reply-handler-domain`, which works as a default for all applications, but never cleaned this mess up.

Test Plan: Sent some mail from applications, verified it picked up appropraite reply handler domains.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12231
2015-03-31 16:48:40 -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
7cf726c7f7 Fix an issue with FileMailReceiver not working well
Summary: Ref T7199. Guess no one has ever tried to reply to file mail.

Test Plan: Used `bin/mail receive-test` to send mail to files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

Differential Revision: https://secure.phabricator.com/D12228
2015-03-31 16:47:58 -07:00
Bob Trahan
e0acb75be9 Conpherence - fix possible fatal
Summary: if users have no conpherences I think this is possible? just init it with the rest of the variables we init when we have no conpherence and we should be good to go. fixes T7671.

Test Plan: logicypoo

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7671

Differential Revision: https://secure.phabricator.com/D12227
2015-03-31 14:13:41 -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
7711ea9855 Move handle fetching into tokenizer Datasources
Summary:
Ref T7689. This serves two goals:

  - I want to remove Controller->loadViewerHandles(). A nontrivial number of these callsites are loading handles to pass to tokenizers. Since tokenizers need to take strings eventually anyway, we can do less work by letting them take PHIDs now.
  - A few changes out, I want tokenizers to accept parameterized tokens (like `viewer()`, `members(differential)`, etc.), so the `setValues()` signature needs to change eventually anyway.

I made this work and converted a handful of callsites as an example; upcoming changes will convert more.

Test Plan:
- Viewed Almanac binding editor; used "Interface".
- Edited Almanac services; used "Projects".
- Edited Almanac devices; used "Projects".
- Searched for commits; used "Auditors"; "Commit Authors", "Repositories".
- Searched for calendar events; used "Created By"; used "Invited".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12218
2015-03-31 14:10:32 -07:00
epriestley
ba10e8ed41 Reduce cleverness of tablet breakpoint adjustment
Summary:
Fixes T7713. 2 much math X.X

Specifically, we could end up with some nonsense widths here (smaller than the phone breakpoint) with an initially-hidden column.

Test Plan: Hit tablet breakpoint with column open and closed.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7713

Differential Revision: https://secure.phabricator.com/D12225
2015-03-31 13:44:49 -07:00
Bob Trahan
3cb5cac018 Conpherence - fix bad query from D12222
Summary: D12222 did an evil thing. This correctly scopes this to just the threads you are participating in and not all threads which then get filtered from policy later, super slowly...

Test Plan: still 'works' and probably wont kill prod

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12223
2015-03-31 13:02:45 -07:00
Bob Trahan
f75248de6f Conpherence - refine menu interactions with respect to "Show More"
Summary:
Ref T7566. Prior to this diff, we had a broken mess in the "Messages" section. Now, "Messages" behave like rooms in that whatever is loaded at page load time is at the top of the list.

Additionally, refine "show more" behavior such that it simply shows the next X, but if there exists X + 1 then we have another "show more" that kicks you to application search. Theoretically, there are still corner cases where users are in a ton of rooms or a ton of messages respectively, but this feels pretty good.

Consolidates title rendering code so we always render the list of participants and no more "No Title".

Also remove the policy icons for messages consistently, helping to differentiate them from rooms at a glance.

Test Plan: clicked around in conpherence main - looked good. tried "show more" and it worked! played around in durable column and things seemed reasonable there too.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7566

Differential Revision: https://secure.phabricator.com/D12222
2015-03-31 12:45:32 -07:00
Chad Little
fb58932eb8 Less janky searching on mobile devices
Summary: Removes odd borders and padding, just a white search interface now.

Test Plan:
review at smaller breakpoints in sandbox, submit some items for search.

{F355209}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12221
2015-03-31 11:27:24 -07:00
Chad Little
d81d8caa93 Hide Search text better
Summary: Previously, this text was transparent, moved it to a new span and set visibility on it.

Test Plan: review search in FF, Chrome. Text no longer displays.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7710

Differential Revision: https://secure.phabricator.com/D12219
2015-03-31 09:59:09 -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
0f52fc771d Remove implode_selected_handle_links()
Summary: Ref T7689. Use the newer, less-janky stuff for rendering handles.

Test Plan:
- Viewed a revision hovercard; viewed "Author", viewed "Reviewers", viewed "Tasks".
- Viewed a task hovercard; viewed "Assigned To"; viewed "Projects"; viewed other edge fields.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12209
2015-03-31 05:48:20 -07:00
epriestley
e1eafd784e Remove Controller->renderHandlesForPHIDs()
Summary: Ref T7689. Remove all remaining callsites for this method.

Test Plan:
- Viewed a custom policy; viewed handles in the policy rules.
- Viewed a Releeph product; viewed "Pushers".
- Viewed a project; viewed "Watchers"; viewed "Members"; viewed "Looks Like".
- Viewed repository edit; viewed "Credential"; viewed "Storage Service"; viewed "Projects".
- Viewed repository detail; viewed "Projects".
- Viewed commit; viewed (faked) "Reverts"; viewed (faked) "Reverted By".
  - These are kind of a pain to generate so I faked 'em.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12208
2015-03-31 05:48:19 -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
epriestley
dec03cf076 Prepare a replacement for Controller->renderHandlesForPHIDs()
Summary:
Ref T7689. This gives HandleLists `renderList()` and `renderHandle()` methods, which return views that can perform just-in-time data fetching and generally look and feel like other rendering code, instead of being odd pseudo-functional methods on `Controller`.

Also converts callsites on the Maniphest detail page to use these methods.

Next changes will wipe out more of the callsites.

Test Plan:
  - Viewed Maniphest detail page with many relevant handles.
  - Created a new subtask.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12205
2015-03-31 05:48:19 -07:00
epriestley
580590fcc9 Remove Controller->getLoadedHandles()
Summary: Ref T7689. Removes this part of the `Controller->loadHandles()` + `Controller->getLoadedHandles()` mechanism.

Test Plan:
  - Viewed Herald transcripts.
  - Viewed Maniphest tasks with attached revisions and commits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12204
2015-03-31 05:48:19 -07:00
epriestley
1752be630c Improve handle semantics with HandlePool / HandleList
Summary:
Ref T7689, which discusses some of the motivation here. Briefly, these methods are awkward:

  - Controller->loadHandles()
  - Controller->loadViewerHandles()
  - Controller->renderHandlesForPHIDs()

This moves us toward better semantics, less awkwardness, and a more reasonable attack on T7688 which won't double-fetch a bunch of data.

Test Plan:
  - Added unit tests.
  - Converted one controller to the new stuff.
    - Viewed countdown lists, saw handles render.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12202
2015-03-31 05:48:19 -07:00
Bob Trahan
3f738e1935 Conpherence - fix see more link
Summary: Fixes T7705.

Test Plan: link works

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7705

Differential Revision: https://secure.phabricator.com/D12217
2015-03-30 16:53:54 -07:00
Bob Trahan
b200c3bd78 Conpherence - actually remove route from feedback in D12215
Summary: pebkac issue of some sort and I didn't actually commit removing the defunct /conpherence/room/ uri route

Test Plan: made a new room and it worked

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12216
2015-03-30 16:29:08 -07:00
Bob Trahan
89fe35cb0e Conpherence - add application search in a few more places.
Summary: Ref T7584. In Conpherence main view, this adds a "search" link right in the "Rooms" header. This piece addresses an outstanding item on T7584. This diff also adds a search button in the durable column that takes you to the application search. This kind of a big product bet that rooms are going to be dominating things and its most useful to find another room quickly from this view. That said, I think the application search should get massaged slightly to allow searching threads and this won't be much of a trade off at all.

Test Plan: verified new search links took me to correct place and displayed reasonably.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7584

Differential Revision: https://secure.phabricator.com/D12215
2015-03-30 16:25:24 -07:00