Summary: Ref T2852. Currently, we publish commits with no audit requests and reviews with no CCs or reviewers into Asana. This creates undesired notifications, so drop events which would publish an object that doesn't exist yet and has no followers or respible users.
Test Plan: Used `bin/feed republish` to publish a story about an object with no related users, saw the publish abort with the new message. Added a CC, published again, got a publish.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6727
Summary:
We currently check if daemons are running using the filesystem and process list. These checks reach the wrong result for a lot of users because their webservers can't read the filesystem or process list. They also reach the wrong result for daemons running on other machines.
Instead, query the active daemon list to see if daemons are running. This should be significantly more reliable.
(We didn't do this before because the running daemon list mechanism didn't exist when the check was written, and at the time it was more complex than doing a simple filesystem/process list thing.)
Test Plan: Viewed `/repositories/` with and without daemons running, saw appropriate warning or lack of warning.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6722
Summary: This moved to CLI.
Test Plan: Read.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6724
Summary: Fixes T3703. Clear question notifications when viewing a question.
Test Plan: Gave a question a token, logged in as author, saw notification, viewed question page, notification was marked read.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3703
Differential Revision: https://secure.phabricator.com/D6723
Summary:
Fixes T3698. Sometimes views need to render differently depending on whether they contain content or not. The existing approach for this is `isEmptyContent()`, which doesn't work well and is sort of hacky (it implies double-rendering content, which is not always free or side-effect free).
Instead, provide a test for an element without children. This test is powerful enough to catch the easy cases of `null`, etc., and just do the expected thing, but will not catch a View which is reduced upon rendering. Since this is rare and we have no actual need for it today, just accept that as a limitation.
Test Plan:
Viewed Timeline and Feed UI examples. Viewed Feed (feed), Pholio (timelineview), and Differential (old transactionview).
{F53915}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3698
Differential Revision: https://secure.phabricator.com/D6718
Summary: This fixed a bug with macros search finding macros flagged by any user. We should only look at flags by the current user.
Test Plan: Verify that no macros flagged by another user show up in macros search.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6717
Summary: Cleaning up my mess, (No Filtering) should be the default selected option in macros search form.
Test Plan: Go to /macro/query/advanced/ and verify that (No Filtering) is the default selected option.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3692
Differential Revision: https://secure.phabricator.com/D6715
Summary: I think we accidentally forgot to include this action in D6660.
Test Plan: verified it showed up in the UI to have the action be an audit
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6712
Summary: we get participation data ordered, then query conpherences by phid... be sure to resort the conpherences based on participation data. I missed this in testing 'cuz my test data is so trashy, but it is glaringly obvious in production. :/
Test Plan: replied to a very old conpherence and noted it was first in the notification panel
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3641
Differential Revision: https://secure.phabricator.com/D6711
Summary: Fixes T3690. Uses standard colors, smaller borders.
Test Plan: Review Menu with and without a notification
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, aran
Maniphest Tasks: T3690
Differential Revision: https://secure.phabricator.com/D6710
Summary: Reuse the existing flags functionality for searching macros. Currently implemented as a simple select element (for color).
Test Plan: Flagged some macros and tried searching by them.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6709
Summary: Fixes T3641. Probably needs some @chad love though on colors and what have you. Technique was to jam this into the existing notifications stuff as much as possible. I think its "okay" but if we were to add more stuff here (like a 3rd application) this could get a quality pass to consolidate even more code.
Test Plan: played with it in Chrome and Safari - looks reasonable
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3641
Differential Revision: https://secure.phabricator.com/D6708
Summary:
This is mostly for personal reasons / lols, but they have a perfectly functional OAuth2 API and it takes like 15 minutes to add a provider now and I was in this code anyway...
@chad, we could use JIRA, Twitter and Twitch.tv auth icons if you have a chance.
Test Plan: {F53564}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6706
Summary: Ref T2852. Asana adds the actor as a follower when they create a task, so subtasks currently have up to two followers (the actor and the reviewer) when they should have only one (the reviewer). Simply removing the actor is an effective remedy for this because unfollowing tasks occurs with sneaky ninja stealth in Asana and doesn't generate notifications or even transaction activity.
Test Plan: Synchronized a revision without this patch, saw two followers on the subtask. Synchronized a revision after this patch, saw the "removeFollowers" fire and only one follower on the subtask, with no record of the removal in notifications or the transaction log.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6700
Summary: Fixes a query in √D6260.
Test Plan: View a Releeph RQ and verify that the "churn" field renders and has the right numbers in it.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6702
Summary: Use standard colors.
Test Plan: create status
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6701
Summary: Ref T2769. This isn't a real adapter and its methods are increasingly hacky messes. Make "dry run" a first-class concept on the HeraldEngine instead and remove the adapter.
Test Plan: Ran Herald via test console and via CLI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6693
Summary:
Ref T2769. This will house the transaction list and replace the "edit log" stuff.
The UI is a little bit rough and can probably share more code with the transaction history, but seems mostly-reasonable.
Test Plan: {F53253}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6690
Summary: Ref T2769. The `HeraldRule` class has some query logic; move it into `HeraldRuleQuery`. Also some minor cleanup.
Test Plan: Ran test console, created a new revision, used `reparse.php --herald`. Verified rules triggered correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6689
Summary:
Ref T2769. Move all of this stuff into Adapters and get rid of the hard-coded classes.
I cheated in two places.
Test Plan: Edited and activated Herald rules.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6688
Summary: Ref T2769. Moves all traces of HeraldConditionConfig into Adapters.
Test Plan: Edited rules and used Test Console to exercise both affected code paths. Tried to save invalid rules to hit error pat.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6679
Summary: Ref T2769. Get rid of the last use of `HeraldContentTypeConfig` by moving repetition options into Adapters.
Test Plan: Viewed / edited Herald rules.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6664
Summary: Ref T2769. Use Adapters to build all the strings for transcripts, then get rid of the old maps.
Test Plan: Viewed revision and commit transcripts.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6663
Summary: Ref T2769. This cleans up almost every use of the HeraldContentTypeConfig class.
Test Plan: Viewed and edited Herald rules.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6662
Summary: Ref T2769. Shift the bulk of value and action config into Adapters.
Test Plan: Viewed and edited Herald list and rules.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, vrana
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6660
Summary: Ref T2769. Herald has a giant hard-coded list of fields. Primarily make these dynamic and adapter-based.
Test Plan: Viewed and edited Herald rules.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6657
Summary:
Ref T2769. Get content types out of hard-coded config and into dynamic adapters.
This removes the "MERGE" and "OWNERS" content types, which were vestigal. These needs are likely better addressed through subscriptions/transactions, and are obsolete, and haven't existed for 2+ years and no one has asked for them to be restored.
Test Plan: Mostly a bunch of grep. Viewed rule list, rule edit. Edited a revision.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6656
Summary: Ref T2769. I'm planning to keep this pretty simple, but we have this ad-hoc edit log for rules already and some other mess that we can clean up.
Test Plan: No effect yet; see future changes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6654
Summary: Ref T2769. Ref T2625. Herald is currently a giant mishmash of hard-codes and weird special cases. Move toward modernization and normality.
Test Plan: {F52716}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625, T2769
Differential Revision: https://secure.phabricator.com/D6652
Summary:
Ref T603. Ref T2769. Herald currently interacts with policies in a bad way; specifically, I can create a rule which emails me for everything, and thus learn about objects I can't otherwise see.
This shouldn't be possible, so I'm going to reduce personal rules to have only the viewer's scope.
For global rules, I think I'm always going to let any user edit them, but make who the rule acts as part of the configuration. There will be an option to make a rule omnipotent, but only admins (or some other special subset of users) will be able to select it.
Transactions/subscriptions will provide a check against users editing global rules in ways that are bad.
Test Plan: Next diffs.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2769
Differential Revision: https://secure.phabricator.com/D6649
Summary:
Ref T3684. The URI itself is reflected in a few places. It is generally not dangerous because we only let you add random stuff to the end of it for one or two controllers (e.g., the file download controller lets you add "/whatever.jpg"), but:
- Remove it entirely in the main request, since it serves no purpose.
- Remove query parameters in Ajax requests. These are available in DarkConsole proper.
Also mask a few things in the "Request" tab; I've never used these fields when debugging or during support, and they leak quasi-sensitive information that could get screenshotted or over-the-shoulder'd.
I didn't mitgate `__metablock__` because I think the threat is so close to 0 that it's not worthwhile.
Test Plan: Used Darkconsole, examined Requests tab.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3684
Differential Revision: https://secure.phabricator.com/D6699
Summary: Ref T3684 for discussion. This could be cleaned up a bit (it would be nice to draw entropy once per request, for instance, and maybe respect CSRF_TOKEN_LENGTH more closely) but should effectively mitigate BREACH.
Test Plan: Submitted forms; submitted forms after mucking with CSRF and observed CSRF error. Verified that source now has "B@..." tokens.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3684
Differential Revision: https://secure.phabricator.com/D6686
Summary:
Ref T2852. Two issues:
- Embeds (`T12`, `{T12}`) have some handle issues because handles run afoul of visibility checks under some configs. Make handles unconditionally visible.
- Asana links don't render correctly into text mode. Give them a valid text mode rendering so they don't flip out.
Test Plan: Made comments with `T12` and `http://app.asana.com/...` and published them to Asana.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6696
Summary: Ref T2852. After some discussion, Asana doesn't want "close" stories either.
Test Plan: Used `bin/feed republish` to publish close and non-close stories from Differential and Diffusion. Verified comments were synchronized in the expected cases.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6697
Summary: Tightens up the CSS to display more items (4 wide on 15") and fixes some mobile CSS issues with appseach. Fixes T3614
Test Plan: Tested Pholio, Macros, mobile layouts
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3614
Differential Revision: https://secure.phabricator.com/D6694
Summary: We already show transaction and maniphest comments.
Test Plan: Review my feed, see diff comment.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6687
Summary: This adds hovercards to most stories and removes the profile photo from one line stories. I don't know about my implementation, which has difficulties with application transactions (because it shows status). Which leads me to a bigger question, which is can we render all people through a common function like AphrontTagView so we can easily class and/or hovercard it anywhere.
Test Plan: Reviewed my feed, various stories.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6684
