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

267 commits

Author SHA1 Message Date
epriestley
082b7f95e6 Explicitly track inline comment reply threading
Summary:
Ref T1460. Track and store which comments are threaded replies to other comments, vs merely appearing on the same lines.

This doesn't actually write `hasReplies` yet, since that needs to happen when we un-draft comments on submission.

Test Plan: Made inline comments in Differential and Diffusion, including replies. Replies were marked as "Is Reply".

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1460

Differential Revision: https://secure.phabricator.com/D12017
2015-03-09 10:26:50 -07:00
epriestley
174dd220df Disable mentions in nonpublishing repositories
Summary:
Ref T6516. Although this behavior is somewhat-arguable as desirable, I think it's less surprising and more consistent to disable mentions when a repository is publishing.

In particular, if you import a repository developed on another Phabricator install, this stops all the `T123` in commit messages from creating mentions on your unrelated `T123` tasks.

We already disable autoclose, so `Closes T123` and `Ref T123` already have no effect, but a bare `T123` would generate a mention. Likewise, `@epriestley` would generate a mention.

If you import such a repository and then update it periodically, updates will activate autoclose and publishing (if you didn't disable them), but presumably this will hit a couple of tasks and you'll go change the settings if you forgot.

At some point, we may have some kind of use case for separating the "publish" setting into a "publish" setting and a "this is a local repository" setting. For example, if you work at Widget Corp, want to import Phabricator locally, //and// want to write Herald rules against it, you can't currently configure the repository to let you do all of this. But we haven't actually seen a use case for this yet.

Test Plan:
  - Pushed some commits with bare `T11`, saw mentions.
  - Disabled publishing for the repository, pushed some commits with
  - Imported a bunch of commits without seeing pipeline failures.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6516

Differential Revision: https://secure.phabricator.com/D11966
2015-03-04 10:36:38 -08:00
epriestley
8599145b5e Implement more consistent publishing rules for repositories
Summary:
Ref T7298. We are currently inconsistent about when we publish feed, email, notifications, audits and Herald rules.

Specifically, there are two settings which impact these things:

  - The "importing" flag, which is set when we're importing old commits.
  - The "herald-disabled" flag, which was expanded in scope some time ago and now actually means "disable publishing".

Various parts of the pipeline were checking only one of these flags. Instead, all of them should check both.

(For example, we should never email users about importing repositories, nor trigger audits on them.)

Test Plan: See next revision.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7298

Differential Revision: https://secure.phabricator.com/D11826
2015-02-19 10:38:05 -08:00
Chad Little
e2fcc3c187 Touch up Audit/Commit List UI
Summary: Fixes a few issues. The author of the commit is more prominent / not cut off. Auditors is in a more consistent location. More space is available for reasons. Commits by themselves look much less janky. Only downside is actual Audits are now 3 lines vs. 2, but the extra space is used well.

Test Plan:
Test list of audits and commits.

{F309237}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11817
2015-02-19 07:03:18 -08:00
Bob Trahan
d598edc5f3 MetaMTA - update documentation and make config a tad easier
Summary: Fixes T7088. Mainly this updates the documentation but I also snuck in tweaking how the domain reply handler is built. This does two main things -- makes the behavior consistent as some applications who didn't override this behavior would send out emails with reply tos AND makes it easier for us to deprecate the custom domain thing on a per application basis, which is just silly. On that note, the main documentation doesn't get into how this can be overridden, though I left in that mini blurb on the config setting itself. We could deprecate this harder and LOCK things if you want as well.

Test Plan: read docs, looked good. reasoned through re-factor

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7088

Differential Revision: https://secure.phabricator.com/D11725
2015-02-12 11:05:39 -08:00
Bob Trahan
5a9df1a225 Policy - filter app engines where the user can't see the application from panel editing
Summary: Fixes T7118. This does the basic "filter the list" thing, though it ends up being a little manual since I guess this hasn't come up before? There is also potential weird behavior if the user was using an app and lost access to it - they will have nothing selected on edit - but I think this is actually correct behavior in this circumstance.

Test Plan:
used a user who couldn't get access to the "quick create" apps and noted that the dropdown list on dashboard panel create was missing the expected engines

ran `arc unit --everything` to verify abstract method implemented everywhere

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7118

Differential Revision: https://secure.phabricator.com/D11687
2015-02-04 15:47:48 -08:00
Chad Little
8f1e0c0262 Revamp Profile with new IconNav
Summary: Revamps Profile to be like Projects, a mini portal and side nav with icons.

Test Plan: Viewed my own profile, as well as others. Test seeing my commits, tasks, diffs, and upcoming events. Checked mobile navigation.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11547
2015-02-02 12:13:48 -08:00
Chad Little
8b06804394 Remove getIconName from all applications
Summary: Not used anymore

Test Plan: grep for 'getIconName'

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11582
2015-01-30 12:11:21 -08:00
Chad Little
5d8bb61dde Add FontIcon bridge to AppIcons
Summary: Select a similar or better FontAwesome icon to represent each application

Test Plan: Visual inspection

Reviewers: epriestley, btrahan

Subscribers: hach-que, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11489
2015-01-24 23:43:01 -08:00
Joshua Spence
daadf95537 Fix visibility of PhutilArgumentWorkflow::didConstruct methods
Summary: Ref T6822.

Test Plan: `grep`. This method is only called from within `PhutilArgumentWorkflow::__construct`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11415
2015-01-16 07:42:07 +11:00
Joshua Spence
c2ac63e9ad Increase visibility of PhabricatorController::buildApplicationMenu methods
Summary: Ref T6822. This method needs to be `public` because it is called from `PhabricatorApplicationSearchController::buildApplicationMenu()`.

Test Plan: I wouldn't expect //increasing// method visibility to break anything.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11416
2015-01-16 07:41:26 +11:00
Joshua Spence
d6b882a804 Fix visiblity of LiskDAO::getConfiguration()
Summary: Ref T6822.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11370
2015-01-14 06:54:13 +11:00
Joshua Spence
e7f8e79742 Fix method visibility for PhabricatorController subclasses
Summary: Ref T6822.

Test Plan: Visual inspection. These methods are only called from within `PhabricatorController` subclasses.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11241
2015-01-07 07:34:59 +11:00
Joshua Spence
e448386d39 Fix method visibility for PhabricatorApplicationSearchEngine methods
Summary: Ref T6822.

Test Plan: Visual inspection. These methods are only called from within the `PhabricatorApplicationSearchEngine` class.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11242
2015-01-07 07:34:52 +11:00
Joshua Spence
7c2a7d0365 Modernize remaining edge types
Summary: Modernize remaining edges to subclass `PhabricatorEdgeType`. Largely based on D11045.

Test Plan: Browsed around and performed various actions include subscribing, unsubscribing and watching.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11116
2015-01-03 10:58:20 +11:00
Chad Little
61e26cd242 Remove 'Author:' byline text
Summary: The actual author here usually gets truncated by the extra text, which doesn't seem needed in most (all?) cases.

Test Plan: Look at list of recent commits.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11127
2015-01-02 11:39:47 -08:00
Fabian Stelzer
00495e3a0e remove unused FeedStory object in getTitleForFeed functions
Summary:
Removes an unused PhabricatorFeedStory Parameter from all getTitleForFeed() and getApplicationTransactionTitleForFeed() functions.
Ref D11088 Ref T6545

Test Plan: ran all unit tests and viewed some dashboard feeds

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6545

Differential Revision: https://secure.phabricator.com/D11146
2015-01-02 08:45:43 -08:00
Bob Trahan
2b99b4add8 Home - limit "status" queries to 100 and show 99+ if we hit that
Summary: Fixes T6595. This diff has two issues as is... 1) the differential data fetching is pretty cheesey, but it looks like we can't just issue three separate databases to get the right data? 2) the translations break, since I am turning this into a string (and not an int) so the whole pluralization bit fails. I think 1 is okay as is and 2 needs to be fixed though I am not sure how to best do that...

Test Plan: loaded home page and it looked nice...!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6595

Differential Revision: https://secure.phabricator.com/D10979
2014-12-12 12:02:25 -08:00
Bob Trahan
f6e635c8d2 Transactions - deploy buildTransactionTimeline to remaining applications
Summary:
Ref T4712. Specifically...

- Differential
 - needed getApplicationTransactionViewObject() implemented
- Audit
 - needed getApplicationTransactionViewObject() implemented
- Repository
 - one object needed PhabricatorApplicationTransactionInterface implemented
 - setShouldTerminate(true)
- Ponder
 - BONUS BUG FIX - leaving a comment on an answer had a bad redirect URI
 - both PonderQuestion and PonderAnswer needed PhabricatorApplicationTransactionInterface implemented
 - setShouldTerminate(true) on both "history" controllers
 - left a "TODO" on buildAnswers on the question view controller, which is non-standard and should be re-written eventually
- Phortune
 - BONUS BUG FIX - fix new user "createNewAccount" code to not fatal
 - PhortuneAccount, PhortuneMerchant, and PhortuneCart needed PhabricatorApplicationTransactionInterface implemented
 - setShouldTerminate(true) on Account view, merchant view, and cart view controller
- Fund
- Legalpad
- Nuance
  - NuanceSource needed PhabricatorApplicationTransactionInterface implemented
- Releeph (this product is kind of a mess...)
  - HACKQUEST - had to manually create an arcanist project to even be able to make a "product" and get started...!
  - BONUS BUG FIX - make sure to "setName" on product edit
  - ReleephProject (should be ReleepProduct...?), ReleephBranch, and ReleepRequest needed PhabricatorApplicationTransactionInterface implemented
- Harbormaster
  - HarbormasterBuildable, HarbormasterBuild, HarbormasterBuildPlan, and HarbormasterBuildStep all needed PhabricatorApplicationTransactionInterface implemented
  - setShouldTerminate(true) all over the place

Test Plan: foreach application, viewed the timeline(s) and made sure they still rendered

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T4712

Differential Revision: https://secure.phabricator.com/D10925
2014-12-03 15:35:47 -08:00
Chad Little
1ac84c1b59 Add addLinkSection to MailBody to properly format URIs
Summary: Fixes T6343. Grepped for all callsites and added addLinkSection where needed.

Test Plan: Tested Differential, Maniphest, Conpherence, Ponder and Macro. Inspect HTML mail for anchor tags. Inspect text mails for non-disruption.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: talshiri, Korvin, epriestley

Maniphest Tasks: T6343

Differential Revision: https://secure.phabricator.com/D10762
2014-10-30 15:24:10 -07:00
Bob Trahan
3bf770cb30 Repositories - don't send emails or publish feed stories about commits from importing repositories
Summary: ...also truncate authorName to 255 so that we don't get database errors. Ref T6350.

Test Plan: see T6350 - mostly doing it live - but I did sanity check and commit something and it worked!

Reviewers: epriestley, chad

Reviewed By: chad

Subscribers: Korvin, epriestley

Maniphest Tasks: T6350

Differential Revision: https://secure.phabricator.com/D10734
2014-10-21 10:05:45 -07:00
Bob Trahan
ec82ad2d16 Audit / Herald - make audits added via herald "required"
Summary: as opposed to "requested". Also re-jigger how the "reason" works so the herald editor can get more specific data rather than a generic message. Fixes T6345 along with companion diff D10726.

Test Plan: made a herald rule to add auditors to a commit and saw it work!

Reviewers: epriestley, chad

Reviewed By: chad

Subscribers: Korvin, epriestley

Maniphest Tasks: T6345

Differential Revision: https://secure.phabricator.com/D10730
2014-10-20 16:46:41 -07:00
Chad Little
eaf6ca3b64 Normalize AuditStatusConstant Colors
Summary: Ref T6345, This adds more consistent color choices to match how Phabricator generally works across Differential/Diffusion per user statuses.

Test Plan: Review a few Audits in my sandbox.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: Korvin, epriestley

Maniphest Tasks: T6345

Differential Revision: https://secure.phabricator.com/D10726
2014-10-20 15:47:10 -07:00
Bob Trahan
ba2963ecb3 Diffusion - fix commits not importing fully
Summary: Fixes T6336. Turns out that the function to update the import status updates that database and doesn't update the object. If the object doesn't get the pertinent update AND there's a herald rule that runs, then the object is later re-saved without ever getting the update flag.

Test Plan: logic in the ole sandbox and going to push it to prod and run re-parse on impacted commits

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley, chad

Maniphest Tasks: T6336

Differential Revision: https://secure.phabricator.com/D10723
2014-10-17 09:41:27 -07:00
epriestley
9352c76e81 Decouple some aspects of request routing and construction
Summary:
Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular:

  - Controllers no longer require `$request` to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call `setRequest()` before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects.
  - `$request` now offers `getURIData($key, ...)`. This is an alternate way of accessing `$data` which is currently only available on `willProcessRequest(array $data)`. Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request.
  - Introduce `handleRequest(AphrontRequest $request)` and deprecate (very softly) `processRequest()`. The majority of `processRequest()` calls begin `$request = $this->getRequest()`, which is avoided with the more practical signature.
  - Provide `getViewer()` on `$request`, and a convenience `getViewer()` on `$controller`. This fixes `$viewer = $request->getUser();` into `$viewer = $request->getViewer();`, and converts the `$request + $viewer` two-liner into a single `$this->getViewer()`.

Test Plan:
  - Browsed around in general.
  - Hit special controllers (redirect, 404).
  - Hit AuditList controller (uses new style).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5702

Differential Revision: https://secure.phabricator.com/D10698
2014-10-17 05:01:40 -07:00
Bob Trahan
d9dd098e40 Audit - fix erroneous mentions of tasks, etc via commit message
Summary: we don't want to mention these phids... when expanding transactions, build the unmnentionable map and make it so. slightly hairy due to how the editor framework works, but overall i think this is the right place to put these hooks.  Fixes T6331.

Test Plan: made a commit with a commit message that had fixes, refs, depends on, and auditors and saw no erroneous mentions

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, chad, epriestley

Maniphest Tasks: T6331

Differential Revision: https://secure.phabricator.com/D10721
2014-10-16 17:13:55 -07:00
Bob Trahan
ac83ff7060 Audit - add mail tags
Summary: Fixes T2497. I'm not sure where we are with subscribers and custom vs normal codepath, but the mailtags implementation makes no assumptions and can handle it either way.

Test Plan: made a commit and got some sensible mail tags

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T2497

Differential Revision: https://secure.phabricator.com/D10712
2014-10-16 08:41:15 -07:00
Bob Trahan
3dd92a78ac Move audit to application transactions
Summary:
Fixes T4896, T6293.

Do most of the work in the editor, but pull the raw patch in the daemon and set that on the editor. This is somewhat of a pre-optimization but it was easy enough to do and makes sense to me.

Test Plan:
made a commit and saw it get parsed.
made a commit with "Auditors: foo" field and saw audit made for foo
turned on inline patch and attach patch and saw the patches

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6293, T4896

Differential Revision: https://secure.phabricator.com/D10705
2014-10-15 13:20:12 -07:00
Joshua Spence
3cf9a5820f Minor formatting changes
Summary: Apply some autofix linter rules.

Test Plan: `arc lint` and `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D10585
2014-10-08 08:39:49 +11:00
epriestley
8fa8415c07 Automatically build all Lisk schemata
Summary:
Ref T1191. Now that the whole database is covered, we don't need to do as much work to build expected schemata. Doing them database-by-database was helpful in converting, but is just reudndant work now.

Instead of requiring every application to build its Lisk objects, just build all Lisk objects.

I removed `harbormaster.lisk_counter` because it is unused.

It would be nice to autogenerate edge schemata, too, but that's a little trickier.

Test Plan: Database setup issues are all green.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, hach-que

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10620
2014-10-02 09:51:20 -07:00
epriestley
943c62d1e9 Add missing expected keys and uniqueness
Summary:
Ref T1191.

  - Adds definitions for missing keys and keys with wrong uniqueness. Generally, I defined these before fixing the key query to actually pull all keys and support uniqueness.
  - Moves "key uniqueness" to note severity; this is fixable (probably?) and there are no remaining issues.
  - Moves "Missing Key" to note severity; missing keys are fixable and all remaining missing keys are really missing (either missing edge keys, or missing PHID keys):

{F210089}

  - Moves "Surplus Key" to note seveirty; surplus keys are fixable all remaining surplus keys are really surplus (duplicate key in Harbormaster, key on unused column in Worker):

{F210090}

Test Plan:
  - Vetted missing/surplus/unique messages.
  - 146 issues remaining.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10590
2014-10-01 07:53:50 -07:00
epriestley
0f73b15a70 Generate reasonable expected schemata for Audit and Auth
Summary: Ref T1191. This fills in some more features and gets audit and auth nearly generating reasonable expected schemata.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10500
2014-09-18 08:32:44 -07:00
Joshua Spence
0151c38b10 Apply some autofix linter rules
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10454
2014-09-10 06:55:05 +10:00
epriestley
f4f8e9bb96 Fix "Clsoed" typo
Summary: See D10112.

Test Plan: o.O

Reviewers: btrahan, chad, staticshock

Reviewed By: staticshock

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10446
2014-09-08 13:20:35 -07:00
cburroughs
fd45f64e47 Include all audit states in audit.query
Summary:
Additional audit states were made queryable for T5871.
Include them in Conduit's audit.query as well.  In doing so corrects
references from "status-foo" to "audit-status-foo".

Depends on D10271

Test Plan: with an api, issues queries and got sensible results

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10290
2014-08-21 06:14:23 -07:00
Bob Trahan
59b626d2c1 Audit - allow queries for "partial" and "accepted" audits
Summary: Fixes T5871. These queries get to use the actual column on the commit table since they are about the "aggregate" state of different audits.

Test Plan: issues queries and got sensible results.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5871

Differential Revision: https://secure.phabricator.com/D10271
2014-08-19 10:43:52 -07:00
Bob Trahan
f8af89a99e DiffusionCommitQuery - move phid to id mapping
Summary: Ref T5862. makes the exception work better

Test Plan: issued some queries from audit ui with and without repos - they worked

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5862

Differential Revision: https://secure.phabricator.com/D10268
2014-08-14 13:04:38 -07:00
Bob Trahan
644e950ea3 Audit - add ability to query by repositories
Summary: Fixes T5862. The Diffusion table uses `id` but all the other infrastructure uses `phid` so just do a quick load of the repositories to get the ids. Long term, we should re-key the table by phid I think.

Test Plan: made a query with a repository and got a proper result set

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5862

Differential Revision: https://secure.phabricator.com/D10245
2014-08-14 12:40:47 -07:00
epriestley
cebbca9e08 Add a "USERS" section to audit emails listing commit authors and committers
Summary: Fixes T5872. This won't show up in the initial email until T4896 is further along.

Test Plan:
```
RECIPIENTS
  discoball (Disco Ball)

BODY
epriestley added a comment.

ffkn

USERS
  epriestley (Author)

COMMIT
  http://local.aphront.com:8080/rPOEMS165b6c54f487
```

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5872

Differential Revision: https://secure.phabricator.com/D10266
2014-08-14 12:14:02 -07:00
epriestley
f6f9d78f3a Modularize mail tags
Summary:
Ref T5861. Currently, mail tags are hard-coded; move them into applications. Each Editor defines its own tags.

This has zero impact on the UI or behavior.

Test Plan:
  - Checked/unchecked some options, saved form.
  - Swapped back to `master` and saw exactly the same values.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5861

Differential Revision: https://secure.phabricator.com/D10238
2014-08-12 12:28:41 -07:00
epriestley
d38e89ef6b Fix several issues with application interactions while importing commits
Summary:
  - Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
  - Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
  - Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.

Test Plan:
  - Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
  - Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
  - Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5771, T4864, T5851

Differential Revision: https://secure.phabricator.com/D10221
2014-08-11 12:08:24 -07:00
epriestley
e8d272b0da Use standard infrastructure to attach commits to other objects
Summary:
Ref T4896. Now that we have a transaction editor, we can delete a giant block of hacks.

I believe this also resolves the commit/task attachment issues @joshuaspence and @mbishopim3 mentioned.

Test Plan: Attached and detached commits and tasks.

Reviewers: btrahan, joshuaspence, mbishopim3

Reviewed By: mbishopim3

Subscribers: mbishopim3, epriestley, joshuaspence

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10138
2014-08-04 12:03:58 -07:00
epriestley
725e2fa410 Write a "resign" audit relationship even if actor has no relationship
Summary: Ref T4896. I got this logic slightly wrong when porting it over: we always want to write this relationship, to allow members of a project with an audit request against a commit to resign and get it out of their queue.

Test Plan:
  - Resigned from a commit with an existing relationship.
  - Resigned from a commit with no existing relationship, saw one added.

Reviewers: btrahan, joshuaspence, mbishopim3

Reviewed By: mbishopim3

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10137
2014-08-04 12:03:48 -07:00
epriestley
b5750412c7 Apply normal Audit actions directly with Transaction editor
Summary: Ref T4896. This converts the last "CommentEditor" to a transaction editor and removes a large part of the old code.

Test Plan:
  - Added comments.
  - Accepted / added auditors.
  - Added inline comments.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10128
2014-08-02 14:45:39 -07:00
epriestley
25acf5d130 Apply Diffusion reply email directly with transaction editor
Summary: Ref T4896. Invoke the new editor directly instead of in a roundabout way when handling Audit email.

Test Plan: Used `bin/mail receive-test` to simulate mail, saw comment post with proper content source.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10127
2014-08-02 14:45:27 -07:00
epriestley
508260e4fe Apply diffusion.createcomment directly with transaction editor in Audit
Summary: Ref T4896. Use the new transaction-oriented `PhabricatorAuditEditor` directly instead of invoking it via the old editor.

Test Plan: Used Conduit to add a comment, use silent mode, and accept a commit.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10126
2014-08-02 14:45:09 -07:00
epriestley
a297450aa9 Apply "accept" and "resign" actions with transactions
Summary: Ref T4896. Applies these actions using new transaction stuff.

Test Plan:
  - Accepted and raised concern with my own commit, verifying the special project/package behavior.
  - Accepted and raised concern with another author's commit, verifying the authority-over-packages/projects behavior.
  - Accepted a commit I was not affiliated wiht, verifying the "join as an auditor" behavior.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10125
2014-08-02 14:44:57 -07:00
epriestley
78e164aea6 Use transactions to apply "resign" and "close" Audit actions
Summary: Ref T4896. Hook these up with new stuff.

Test Plan:
  - Closed an audit.
  - Resigned from an audit.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10124
2014-08-02 14:44:45 -07:00
epriestley
688f245a95 Use transactions to apply "add auditors" action in Audit
Summary:
Ref T4896. Move the write for "Add Auditors" inside the new Editor.

There are no longer any readers or writers for metadata, so remove the calls for it.

Test Plan: Added auditors from the web UI.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10123
2014-08-02 14:44:35 -07:00
epriestley
bb022d2376 Minor, restore Audit getMailThreading method
Summary: This also still has a callsite which I missed.

Auditors: btrahan
2014-08-02 01:26:45 -07:00
epriestley
950eeef4c0 Minor, restore Audit newReplyHandlerForCommit method
Summary: This still has a callsite which I missed.

Auditors: btrahan
2014-08-02 01:13:29 -07:00
epriestley
49bd5721c5 Use standard infrastructure for Feed in Audit
Summary: Ref T4896. Instead of using custom stuff, use standard stuff.

Test Plan: Viewed a bunch of feed stories and published some over the Asana bridge.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10114
2014-08-02 00:06:56 -07:00
epriestley
64736264a6 Use standard infrastructure for Audit email generation
Summary: Ref T4896. Replace custom stuff with standard stuff.

Test Plan:
  - Sent a bunch of email and it all looked sensible/correct.
  - Made sure to test inlines, specifically, as they're a bit tricky.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10112
2014-08-02 00:06:45 -07:00
epriestley
b787d3ef0d Use standard infrastructure for Audit search indexing
Summary: Ref T4896.

Test Plan: Made an unusual comment, then found it by searching.

Reviewers: btrahan, joshuaspence

Reviewed By: btrahan, joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10110
2014-08-02 00:06:35 -07:00
epriestley
5b969fb5b8 Provide a transaction editor to perform Audit row writes
Summary:
Ref T4896. Replaces more custom stuff with standard stuff. In particular:

  - No more fake proxy writes;
  - no more fake detection of `@mentions`.

For now, the old code still applies most of the effects and handles feed and email.

Test Plan:
  - Added comments.
  - Added comments with inline comments.
  - Added just inline comments.
  - Added comments with Conduit.
  - Previewed comments.
  - Added CCs explicitly and with `@mentions`.
  - Added auditors.
  - Accepted a commit.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10109
2014-08-02 00:06:25 -07:00
epriestley
89b942c183 Move Audit to proper Subscriptions
Summary:
Ref T4896. Currently, subscriptions to commits are stored as auditors with a special "CC" type.

Instead, use normal subscriptions storage, reads and writes.

Test Plan:
  - Ran migration and verified data still looked good.
  - Viewed commits in UI and saw "subscribers".
  - Saw "Automatically Subscribed", clicked Subscribe/Unsubscribe on a non-authored commit, saw subscriptions update.
  - Pushed a commit through Herald rules and saw them trigger subscriptions and auditors.
  - Used "Add CCs".
  - Added CCs with mentions.

Reviewers: btrahan, joshuaspence

Reviewed By: btrahan, joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10103
2014-08-02 00:06:13 -07:00
Bob Trahan
98c2ad38e5 Fix audit commenting where there are only inlines
Summary: been some changes here and this code was broked. turns out we re-assign $action like two lines later and never used the initial value, so we can simply delete the offending line. Fixes T5745.

Test Plan: submitted inline comment pre-patch and fatal. re-submitted post patch and great success!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5745

Differential Revision: https://secure.phabricator.com/D10078
2014-07-30 10:50:01 -07:00
epriestley
2082eda67b Convert Audit comment rendering to standard infrastructure
Summary: Ref T4896. Depends on D10055. This uses core rendering stuff for audit comments, and fixes all the wonkiness with inlines so we can actually land the migration.

Test Plan: Viewed, previewed and edited various types of comments in Diffusion.

Reviewers: chad, btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10056
2014-07-28 15:01:43 -07:00
epriestley
f965126dc4 Migrate audit comments to transactions
Summary:
Ref T4896. Depends on D10052. This is the major/scary migration, but not really so bad. It is substantially similar to D8210, but less complex because there are fewer actions here.

This moves `PhabricatorAuditComment` storage to `PhabricatorAuditTransaction`, then reads `PhabricatorAuditComment`s as a proxy around the new objects.

Test Plan:
  - Before migrating, browsed around. Nothing appeared broken.
  - Migrated cleanly.
  - Viewed old transactions (inlines, comments, accept/reject/etc, add auditors, add ccs, implicit CCs).
  - Added all of those comment types.
  - Edited a draft.
  - Deleted a draft.
  - Spot checked the database for sanity.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10055
2014-07-28 15:00:46 -07:00
epriestley
608e1d20b4 Write separate comments for every action in Audit
Summary:
Ref T4896. Depends on D10023. Prepares the code for the final migration.

The transaction table stores one row per distinct effect (e.g., add CCs) rather than one row per user action (e.g., "add CCs + comment"). We can double-read that table as long as the code doesn't expect transactions/comments to have multiple different effects, and doesn't try to write any such rows.

Everywhere that we were writing a big "X + Y" comment, write two separate "X" and "Y" comments instead. Like D10023, this disrupts the UI a little (you get more boxes), but that will be resolved once the rendering code swaps over. Otherwise, this retains the existing behavior.

Test Plan:
- Used `diffusion.createcomment` to add comments, raise concern, and accept.
- Previewed commenting, adding auditors/ccs, accepting, raising concern.
- Actually performed commenting, adding auditors/ccs, accepting, raising concern.
- Added a user with mentions.
- Added an explicit CC and a mention user.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10052
2014-07-28 15:00:18 -07:00
epriestley
bf39748011 Build separate comments for each inline comment in Audit
Summary:
Ref T4896. Moves us closer to migrating comments to transactions by building a transaction per inline.

This makes the UI a little wonky, and it will get slightly worse until we swap to the new UI and grouping/collapsing starts working. It's still usable, there's just a box per inline.

Test Plan:
  - Added a comment.
  - Added an inline comment.
  - Added a comment and an inline comment.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10023
2014-07-28 14:59:40 -07:00
epriestley
51b5bf1e67 Fix unmigrated load() call in Audit inlines
Summary: Fixes T5711. I missed this somehow in grepping. :/

Test Plan: Edited and deleted an inline draft.

Reviewers: chad, btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5711

Differential Revision: https://secure.phabricator.com/D10051
2014-07-25 06:23:44 -07:00
epriestley
3fca1b2d2d Fix some missing renames of Application classes
Summary: I think these got caught in the crossfire between Conduit and
Applications. Ref T5655.

Auditors: joshuaspence
2014-07-24 18:03:59 -07:00
epriestley
9700589279 Allow audit email to generate from multiple transactions
Summary: Ref T4896. Begins laying groundwork to split comments apart so they behave like transactions, ultimately enabling the migration.

Test Plan: Made several different types of comments, verified resulting email looks OK.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10022
2014-07-24 18:00:41 -07:00
epriestley
3d78c0eff7 Migrate Audit comment text into new storage
Summary:
Ref T4896. This is substantially similar to D8196.

Migrate the comment text out of the `audit_comment` table and into the `audit_transaction_comment` table. Do double reads on `PhabricatorAuditComment` so the APIs aren't disturbed. The old table is still updated.

Test Plan:
  - Before applying migration, cleared cache and browsed around. Things looked fine, except no comment text.
  - Applied migration.
  - Cleared cache, browsed around, saw all my old comments.
  - Added some new comments.
  - Spot checked migrated and new rows in database.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10020
2014-07-24 18:00:30 -07:00
epriestley
dc5c87f74c Hide Audit comment table reads behind an API
Summary: Ref T4896. Buries all direct access to the table so we can limit the surface area affected by the migration.

Test Plan:
  - Grepped for `PhabricatorAuditComment`.
  - Grepped for `audit_comment`.
  - Viewed a bunch of comments.
  - Added a comment.
  - Reindexed a commit.
  - Searched for unique term in new comment.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10019
2014-07-24 18:00:07 -07:00
epriestley
c01aa794c1 Migrate Audit inline comments to new storage
Summary:
Ref T4896. This is substantially identical to the process which Differential followed, and mostly copied from the original Differential migration and the Differential proxy object.

Basically, we move all the data over but the application can't tell, and the same APIs do reads and writes to the new table.

Test Plan:
 - Browsed UI before migrating, everything looked fine (but no inlines).
 - Ran migration.
 - Verified draft and published comments survived migration.
 - Added a draft.
 - Previewed draft.
 - Submitted draft.
 - Viewed standalone with drafts and published comments.
 - Sanity checked data in database, didn't see anything unusual.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10018
2014-07-24 17:59:54 -07:00
epriestley
416f3d9ede Add storage for new audit transactions and comments
Summary:
Ref T4896. This adds the new storage, without any code changes.

This storage is substantially identical to the Differential storage, except that `changesetID` has been replaced by `pathID`.

I've retained the properties intended to be used to implement T1460. They might not be quite right, but at least we'll be able to make any fixes consistently to both applications. For now, these fields are empty and ignored.

Test Plan: Ran `./bin/storage upgrade`. Nothing calls this code yet.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10017
2014-07-24 17:59:43 -07:00
epriestley
8605a1808d Hide direct accesses to Audit inline comment table behind API
Summary: Ref T4896. Move all direct accesses to the inline comment table behind a small amount of API to make it easier to migrate the table.

Test Plan:
  - Grepped for `PhabricatorAuditInlineComment`.
  - Grepped for `audit_inlinecomment`.
  - Created a draft comment.
  - Previewed a draft comment.
  - Reloaded page, still saw draft.
  - Viewed standalone, still saw draft.
  - Made comment, inline published.
  - Added a draft, saw both.
  - Edited inline comment.
  - Reindexed commit.
  - Searched for unique word in published comment, found commit.
  - Searched for unique word in draft comment, no results.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10016
2014-07-24 17:59:28 -07:00
Joshua Spence
023dee0d3b Rename Conduit classes
Summary: Ref T5655. Rename Conduit classes and provide a `getAPIMethodName` method to declare the API method.

Test Plan:
```
> echo '{}' | arc --conduit-uri='http://phabricator.joshuaspence.com' call-conduit user.whoami
Waiting for JSON parameters on stdin...
{"error":null,"errorMessage":null,"response":{"phid":"PHID-USER-lioqffnwn6y475mu5ndb","userName":"josh","realName":"Joshua Spence","image":"http:\/\/phabricator.joshuaspence.com\/res\/1404425321T\/phabricator\/3eb28cd9\/rsrc\/image\/avatar.png","uri":"http:\/\/phabricator.joshuaspence.com\/p\/josh\/","roles":["admin","verified","approved","activated"]}}
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin, hach-que

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9991
2014-07-25 10:54:15 +10:00
Joshua Spence
0c8f487b0f Implement the getName method in PhabricatorApplication subclasses
Summary: Provide an implementation for the `getName` method rather than automagically determining the application name.

Test Plan: Saw reasonable application names in the launcher.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10027
2014-07-23 23:52:50 +10:00
Joshua Spence
86c399b657 Rename PhabricatorApplication subclasses
Summary: Ref T5655. Some discussion in D9839. Generally speaking, `Phabricator{$name}Application` is clearer than `PhabricatorApplication{$name}`.

Test Plan:
# Pinned and uninstalled some applications.
# Applied patch and performed migrations.
# Verified that the pinned applications were still pinned and that the uninstalled applications were still uninstalled.
# Performed a sanity check on the database contents.

Reviewers: btrahan, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9982
2014-07-23 10:03:09 +10:00
epriestley
cab442fe8c Modernize "user, project or package" typeahead datasource
Summary: Ref T4420. Call this "auditor" since that's what it is.

Test Plan:
  - Edited auditors in auditor search.
  - Edited auditors in "add auditors" in Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4420

Differential Revision: https://secure.phabricator.com/D9888
2014-07-17 15:45:21 -07:00
epriestley
dcc6997793 Modernize "users" typeahead datasource
Summary: Ref T4420. Modernize users.

Test Plan:
- Edited "Commit Authors" on Audit search.
- Edited "Created By" on calendar search.
- Edited "invited" on calendar search.
- Edited "To" on "New conpherence message".
- Edited user on "Add user to conpherence thread".
- Edited "Authors" on countdown search.
- Edited "Author" on differential search.
- Edited "Responsible users" on differential search.
- Edited "Owner" on Diffusion lint search.
- Edited "include users" on Feed search.
- Edited "Authors" on file search.
- Edited "Authors" on Herald rule search.
- Edited a couple of user-selecting Herald fields on rules.
- Edited "user" on legalpad signature exemption.
- Edited "creator" on legalpad search.
- Edited "contributors" on legalpad search.
- Edited "signers" on legalpad signature search.
- Edited "Authors" on macro search.
- Edited "Reassign/claim" on task detail.
- Edited "assigned to" on task edit.
- Edited "assigned to", "users projects", "authors" on task search.
- Edited "creators" on oauthserver.
- Edited "authors" on paste search.
- Edited "actors" and "users" on activity log search.
- Edited "authors" on pholio search.
- Edited "users" on phrequent search.
- Edited "authors", "answered by" on Ponder search.
- Edited "add members" on project membership editor.
- Edited "members" on project search.
- Edited "pushers" on releeph product edit.
- Edited "requestors" on releeph request search.
- Edited "pushers" on diffusion push log.
- Edited "authors", "owners", "subscribers" on global search.
- Edited "authors" on slowvote search.
- Edited users in custom policy.
- Grepped for "common/authors", no hits.
- Grepped for "common/users", no (relevant) hits.
- Grepped for "common/accounts", no (relevant) hits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4420

Differential Revision: https://secure.phabricator.com/D9885
2014-07-17 15:44:18 -07:00
epriestley
16648c28bc Add GROUP BY to commit query
Summary:
Ref T4715. Some minor stuff I caught locally while poking around:

  - Since we don't `GROUP BY`, we can still get duplicate commits. These get silently de-duplicated by `loadAllFromArray()` because that returns an array keyed by `id`, but we fetch too much data and this can cause us to execute too many queries to fill pages. Instead, `GROUP BY` if we joined the audit table.
  - After adding `GROUP BY`, getting the audit IDs out of the query is no longer reliable. Instead, query audits by the commit PHIDs. This is approximately equiavlent.
  - Since we always `JOIN`, we currently never return commits that don't have any audits. If we don't know that all results will have an audit, just `LEFT JOIN`.
  - Add some `!== null` to catch the `withIDs(array())` issue that we hit with Khan Academy a little while ago.

Test Plan:
  - Verified that "All Commits" shows commits with no audits of any kind.
  - Verified that the raw data comes out of the query without duplicates.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5433, T4715

Differential Revision: https://secure.phabricator.com/D8879
2014-07-10 10:16:26 -07:00
Joshua Spence
8756d82cf6 Remove @group annotations
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.

Test Plan: Eye-ball it.

Reviewers: #blessed_reviewers, epriestley, chad

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9859
2014-07-10 08:12:48 +10:00
epriestley
b8bc0aa2b0 Allow users to select QueryPanel search engines from a list
Summary: Ref T4986. Instead of requiring users to know the name of an application search engine class, let them select from a list.

Test Plan:
Created a new panel.

{F165468}

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9500
2014-06-12 13:22:20 -07:00
Joshua Spence
d0128afa29 Applied various linter fixes.
Summary: Applied some more linter fixes that I previously missed because my global `arc` install was out-of-date.

Test Plan: Will run `arc unit` on another host.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9443
2014-06-09 16:04:12 -07:00
Joshua Spence
0a62f13464 Change double quotes to single quotes.
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.

Test Plan: Eyeballed it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9431
2014-06-09 11:36:50 -07:00
Chad Little
41ef6824be Make ObjectItem default as "Card"
Summary: This went smoother than expeced. Makes the rounded Card the default, also tweaked selected state a little.

Test Plan:
Test UIExamples, Maniphest, Home, Differential, Harbormaster, Audit. Everything seems normal

{F163971}

{F163973}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9408
2014-06-07 12:12:11 -07:00
epriestley
6df1a02413 (Redesign) Clean up older "Tile" code
Summary:
This does some backend cleanup of the tile stuff, and some general cleanup of other application things:

  - Users who haven't customized preferences get a small, specific set of pinned applications: Differential, Maniphest, Diffusion, Audit, Phriction, Projects (and, for administrators, Auth, Config and People).
  - Old tile size methods are replaced with `isPinnnedByDefault()`.
  - Shortened some short descriptions.
  - `shouldAppearInLaunchView()` replaced by less ambiguous `isLaunchable()`.
  - Added a marker for third-party / extension applications.

Test Plan: Faked away my preferences and viewed the home page, saw a smaller set of default pins.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9358
2014-06-03 15:47:27 -07:00
epriestley
81d95cf682 Make default view of "Applications" app a full-page launcher
Summary:
This probably needs some tweaks, but the idea is to make it easier to browse and access applications without necessarily needing them to be on the homepage.

Open to feedback.

Test Plan:
(This screenshot merges "Organization", "Communication" and "Core" into a single "Core" group. We can't actually do this yet because it wrecks the homepage.)

{F160052}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5176

Differential Revision: https://secure.phabricator.com/D9297
2014-05-29 12:17:54 -07:00
epriestley
28c198d902 Give audits a clickable commit title, even if they are importing or messageless
Summary:
Fixes T3854. Subversion allows commits with no message, and in other cases we might not have imported the message yet. In these cases, we may not render any text inside the link.

When we hit these cases, render appropriate replacement text.

Test Plan: {F156229}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T3854

Differential Revision: https://secure.phabricator.com/D9169
2014-05-17 15:25:31 -07:00
epriestley
15561a27c3 When a conduit method requires a string constant, call it "string-const" not "enum"
Summary: Ref T5058. The use of "enum" is confusing; we mean "choose one of these specific string constants". Make this more clear.

Test Plan: Viewed each call from the web UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5058

Differential Revision: https://secure.phabricator.com/D9127
2014-05-14 21:59:03 -07:00
epriestley
2592bfcbb3 Add an "authored commits" prebuilt filter to Audit
Summary: Add a prebuilt filter to show all of the viewer's commits across all repositories. I could go either way on this, but it seems maybe-useful (?), and we have similar prebuilt filters elsewhere.

Test Plan: scoped it out <.< >.>

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8881
2014-05-13 08:06:22 -07:00
Chad Little
b2f3001ec4 Replace Sprite-Icons with FontAwesome
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.

Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9052
2014-05-12 10:08:32 -07:00
epriestley
c72b753e54 Move audit rendering into SearchEngine
Summary:
Ref T4986. Updates audit.

Slightly tweaks on method visibility.

Just used a HandleQuery since we have to rebuild the whole view thing otherwise; this is an unusual case.

Test Plan:
  - Checked Audit.
  - Checked Feed.
  - Checked Slowvote.

{F151555}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9011
2014-05-08 20:04:18 -07:00
Bob Trahan
97f88f468b Audit - fix profile link
Summary: forgot to update this with new application search.

Test Plan: verified "View Commits" took me to my commits and the commits of another user from respective profile pages.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8951
2014-05-02 08:56:16 -07:00
Chad Little
f32662a729 Fix audit display on the homedashpageboxlist
Summary: we need set flush on the home display

Test Plan: checked home and audit home, both cards, proper spacing

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8939
2014-05-01 16:09:25 -07:00
Chad Little
da5f22172c Update Audit to use Cards in ObjectList
Summary: For general consistency with Differential / other application searches. May look at "Cards" as the default view for everything.

Test Plan: Reload my Audit page, easier to read and find status colors.

Reviewers: epriestley, btrahan

Reviewed By: btrahan

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8935
2014-05-01 14:37:06 -07:00
epriestley
9da255efaa Add a "problem commits" prebuilt filter to Audits
Summary: Ref T4715. We show this number on the homepage, provide an easy way to query matching commits.

Test Plan: Clicked "problem commits", saw them.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4715

Differential Revision: https://secure.phabricator.com/D8880
2014-04-28 08:26:08 -07:00
Bob Trahan
0db6aad80d Audit - polish list view a bit
Summary:
Grab an audit we have authority over if possible, relying on how that's sorted by actor first. This gets us the best description possible of what the audit is about in the list. Also sort out highlighting; right now it looks silly on some views when everything is highlighted.

An open question in the diff - when to highlight audits?

Options I see -

 - never
 - don't do it on "needs attention" but other views
 - calculate what percentage of shown audits user has authority over, if most ( > N% ) don't highlight, otherwise highlight
 - something else
 - some combo of the above

Test Plan: lists of audits looked better

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8876
2014-04-27 22:49:18 -07:00
Bob Trahan
2ecc04c159 Audit - move over to application search
Summary: ...also kills off "PhabricatorAuditCommitQuery" and "PhabricatorAuditQuery", by moving the work to "DiffusionCommitQuery". Generally cleans up some code around the joint on this too. Also provides policies for audit requests, which is basically the policy for the underlying commit. Fixes T4715. (For the TODO I added about files, I just grabbed T4713.)

Test Plan:
Audit: verified the three default views all showed the correct things, including highligthing. did some custom queries and got the correct results.
Diffusion: verified "blame view" still worked. verified paths were highlighted for packages i owned.
Home: verified audit boxes showed up with proper commits w/ audits
bin/audit: played around with it via --dry-run and got the right audits back

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: chad, epriestley, Korvin

Maniphest Tasks: T4715

Differential Revision: https://secure.phabricator.com/D8805
2014-04-27 09:43:05 -07:00
epriestley
3b5883d8c1 Fix an issue with embedding slowvotes
Summary:
In some applications, using `{V2}` syntax to embed a vote throws. The chain of causality looks like this:

  - We try to render a `phabricator_form()`.
  - This requires a CSRF token.
  - We look for a CSRF token on the user.
  - It's an omnipotent user with no token, so everything fails.

To resolve this, make sure we always pass the real user in.

Test Plan:
  - Lots of `grep`.
  - Made a Differential comment with `{V2}`.
  - Made a Diffusion comment with `{V2}`.
  - Made a Maniphest comment with `{V2}`.
  - Replied to a Conpherence thread with `{V2}`.
  - Created a Conpherence thread with `{V2}`.
  - Used Conduit to update a Conpherence thread with `{V2}`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, lkassianik

Differential Revision: https://secure.phabricator.com/D8849
2014-04-23 16:30:38 -07:00
epriestley
2f01214322 Restore reply instructions to ApplicationTransaction email
Summary: This "Reply to comment, etc., etc." section got lost along the way at some point. Restore it for transaction mail.

Test Plan: Received mail from Maniphest with reply instructions.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8700
2014-04-04 11:14:21 -07:00
Ben Alpert
deba531401 Send mail to audit comment author too
Summary: Previously, you would not receive a mail message for the first comment you make on an audit, but you would for subsequent comments because everyone who's made a comment would be CCed on the email. This mirrors DifferentialTransactionEditor's getMailTo which always adds `$object->getAuthorPHID()`.

Test Plan: With self mail turned on, made the first comment on a commit and received an email for it. With self mail turned off, commented on a different commit and saw in `bin/mail list-outbound` that the message was voided.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8650
2014-03-31 07:52:51 -07:00
Chad Little
cc626b0306 Add pager to all Audit Views
Summary: Fixes T1153, removes the "active" restriction from pagination

Test Plan: Bring up audit, test pager exists, and doesn't exist when needed

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: aik099, epriestley, Korvin

Maniphest Tasks: T1153

Differential Revision: https://secure.phabricator.com/D8643
2014-03-29 08:43:17 -07:00
epriestley
38cc38eaf6 Modernize documentation links
Summary:
  - Point them at the new Diviner.
  - Make them a little less cumbersome to write.

Test Plan: Found almost all of these links in the UI and clicked them.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8553
2014-03-17 15:01:31 -07:00
Michael Peters
5e47f2a862 Adding author information to AuditListView
Summary:
Adding the Author to the home page and Audit overview page,
so that at a glance you can see who authored the commits
that need to be audited

Test Plan: View home page and audit overview page and see that author is shown

Reviewers: #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8438
2014-03-07 08:40:35 -08:00
Joshua Spence
6270114767 Various linter fixes.
Summary:
- Removed trailing newlines.
- Added newline at EOF.
- Removed leading newlines.
- Trimmed trailing whitespace.
- Spelling fix.
- Added newline at EOF

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: hach-que, chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8344
2014-02-26 12:44:58 -08:00