Summary: I considered at the time just making all tables taller. This removes the special casing and adds the space universally. On first glance all smaller tables look great, but Diffusion seems a little bloated. After a short time period though that went away for me. I do think Diffusion overall needs a UI refresh.
Test Plan: Tested numerous tables in Phortune, Diffusion, etc. Spacing feels more readable.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12328
Summary: Fixes T7778. This was likely caused by removing an `array_filter()` somewhere in the course of T7731, but I'd rather have the code be more correct.
Test Plan:
Sent mail on a task with no owner.
- Before patch: unknown recipient.
- After patch: expected recipients.
Reviewers: btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T7778
Differential Revision: https://secure.phabricator.com/D12320
Summary: Add some more translations to make the interface very wow (Thanks to Robert Calaceto)
Test Plan: Squinted my eyes and stared at the UI until stuff made sense.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, chad, epriestley
Differential Revision: https://secure.phabricator.com/D12312
Summary: Ref T5501. This expands cache information a little more.
Test Plan: {F362975}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5501
Differential Revision: https://secure.phabricator.com/D12316
Summary:
Ref T5501. This code was headed down a bad road; dump an indirection layer between rendering and data gatehring.
In particular, this will make it much easier to lift these issues into setup warnings eventually.
Test Plan: Viewed cache status page.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5501
Differential Revision: https://secure.phabricator.com/D12315
Summary:
Ref T5501. This is just getting version detection and availability right, probably.
Eventually, this will get lifted up a bit and "$remedy" will turn into setup issues (or maybe one setup issue saying "your cache setup is messed up, click here to understand why").
Test Plan:
{F362935}
I intend to shove these up to production one-by-one since production is APC and local is Opcache + APCu.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5501
Differential Revision: https://secure.phabricator.com/D12314
Summary: This is the new search UI as seen in Pholio. Unrounds it (mostly, and makes the eyeglass more button like.
Test Plan:
Review in all header colors, check photoshop for pixel layout consistency. Review in FF, Chrome, Safari, IE. Make sure I didn't mess up mobile.
{F359366}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12277
Summary: This reverts commit 55c00ebfa1. T7111, T7112 and T7113 have all been resolved now.
Test Plan: Maybe wait a few more weeks... I just wanted to ensure that this wasn't forgotten.
Reviewers: btrahan, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11910
Summary: I think D11212 missed the feed variants. add 'em.
Test Plan: hope and pray and push to prod and see
Reviewers: joshuaspence, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12309
Summary:
Fixes T7255.
Note however that some datasources - notably user or project - don't implement the class thing in a clean way since multiple classes apply. For now, we just show these datasources to the user.
Also, I guess this could be done more efficiently by querying for all the applications at once via an application query? LMK if you want me to make that change.
Test Plan: loaded /typeahead/class/ and played with it a bit with no issues
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7255
Differential Revision: https://secure.phabricator.com/D12307
Summary: Fixes T7486. Implement HTTP response messages such as `200 OK` and `404 Not Found`. The status codes were taken from http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html.
Test Plan: Navigated to `/foo` and saw the response showing `404 Not Found` in the Network tab of Chrome.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7486
Differential Revision: https://secure.phabricator.com/D12299
Summary: Ref T6875. #chatlog is deprecated and will eventually be merged into #conpherence.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: cburroughs, epriestley
Maniphest Tasks: T6875
Differential Revision: https://secure.phabricator.com/D12097
Summary: Fixes T7502.
Test Plan: Went to `/diviner/` and saw a link to the documentation at `/help/documentation/PhabricatorDivinerApplication/`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7502
Differential Revision: https://secure.phabricator.com/D12094
Summary: Fixes T7693.
Test Plan: made a conpherence with my main test account and a throwaway test account. deleted the test account. removed myself from the thread. added myself back to the thread.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7693
Differential Revision: https://secure.phabricator.com/D12306
Summary:
Fixes T7731. When a user writes a "Send me an email" rule, always try send them an email, even if their notification settings would normally downgrade it to a notification.
In particular, this is stronger than these downgrades:
- Downgrades due to "self actions";
- downgrades due to "mail tags".
Test Plan:
- Wrote various Herald rules with "Send me an email" rules.
- Used `bin/mail list-outbound` / `show-outbound` to vet generated mail.
- Mail reacted properly to a variety of conditions (disabled accounts, settings, "send me an email" rule, forced delivery).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7731
Differential Revision: https://secure.phabricator.com/D12300
Summary:
Ref T7731. Looking forward to T5791, I eventually anticipate writing an interface which looks like a webmail UI where users can review mail they've been sent and understand why they recieved (or did not receive) the mail. Roughly like `bin/mail list-outbound` / `bin/mail show-outbound` work today, but policy-aware (so you can only see messages where delivery was attempted to you).
We currently record a list of "reasons" why a mail is undeliverable, but this list is string-based (so it can not be translated once we start persisting it) and has only negative reasons (so it can not be used to fully understand reasons for delivery or nondelivery).
Make it code-based (so it can be translated) and allow both positive and negative reasons to be listed (so positive reasons can be understood).
Test Plan: Used `bin/mail show-outbound` to review mail delivery reasons, including the positive reason we currently have (forced delivery of authentication mail).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7731
Differential Revision: https://secure.phabricator.com/D12297
Summary:
Ref T7731. For no particular reason, we currently put `ruleID` and `rulePHID` on `HeraldEffect` objects.
Pretty much all callers need the `HeraldRule` objects instead, and some go to great lengths to get them.
Just attach the `Rule` objects.
Test Plan: Will test thoroughly after next-ish changeset.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7731
Differential Revision: https://secure.phabricator.com/D12269
Summary:
Ref T7731. Every adapter subclass currently implements this effect in an essentially identical way.
Some day far from now the effects will be modular and this mess will vanish completely, but reduce its sprawl for now.
Test Plan: I'll test this thoroughly at the end of the change sequence since writing rules is a pain.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7731
Differential Revision: https://secure.phabricator.com/D12268
Summary:
Ref T7731.
- This does nothing.
- I don't know what this was supposed to do.
- It didn't do anything when it was introduced in rP084c79d85a in 2011, either.
iiam
Test Plan:
- `grep`
- ???
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7731
Differential Revision: https://secure.phabricator.com/D12267
Summary: Fixes T7765. The existing sort was based on when people were added to the conpherence and basically feels random in time. Instead, sort current logged in user to the top and make the rest of the list alphabetical.
Test Plan:
viewed a conpherenece and noted my logged in user at top and the rest was sorted alphabetically
viewed a room that i was not a participant in and participant list rendered correctly
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7765
Differential Revision: https://secure.phabricator.com/D12302
Summary: Fixes T7764. These settings have low utility, are no longer used by default, have become less useful on modern Windows which has a better selection of available fonts, and will eventually be subsumed (at least, for the most part) by T4103.
Test Plan:
- Grepped for strings.
- Viewed settings.
- Changed font to "24px impact".
- Viewed diffs with default and custom font.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: chad, epriestley
Maniphest Tasks: T7764
Differential Revision: https://secure.phabricator.com/D12301
Summary: In its current form, this file is not being linted. This doesn't seem to be intentional.
Test Plan: Introduced a linter error and ran `arc lint`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12282
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
Summary: Fixes T7582. Basically if its a room we should be able to change title + policy and if its a thread just the title. T7582 had ideas to do a dropdown but "view in column" doesn't make sense from conpherence afaik - what would the page you'd end up with the column be? (maybe home?) Anyway, that is iteration we can add laters
Test Plan: edited room metadata successfully from main and column view. edtied thread title from main and column view.
Reviewers: epriestley, chad
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7582
Differential Revision: https://secure.phabricator.com/D12252
Summary:
Fixes T7484. There's a bunch of spooky mystery here but the current behavior can probably cause problems in at least some situations.
Also moves a couple callsigns to monograms (see T4245).
Test Plan:
- Faked a short lock length to hit the exception.
- Updated normally.
- Grepped for other use sites, none seemed suspicious or likely to overflow the lock length.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7484
Differential Revision: https://secure.phabricator.com/D12263
Summary:
Ref T7689. Fixes T7688. When there are a bunch of tokenizers with different values of the same object type (e.g. 5 tokenizers with users a, b, c, d, e) we currently issue 5 separate queries to load their handles.
Improve this behavior in the common case so we can usually batch these loads.
Test Plan:
- In Maniphest, set the various search fields to different values.
- Used DarkConsole to examine query plan.
- Saw fewer queries after patch than before.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7688, T7689
Differential Revision: https://secure.phabricator.com/D12261
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
