Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary: Ref T603. Currently, we hard-code defense against setting policies to "Public" in several places, and special case only the CAN_VIEW policy. In fact, other policies (like Default View) should also be able to be set to public. Instead of hard-coding this, move it to the capability definitions.
Test Plan: Set default view policy in Maniphest to "Public", created a task, verified default policy.
Reviewers: btrahan, asherkin
Reviewed By: asherkin
CC: asherkin, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7276
Summary:
Ref T603. Allows the Differential view policy to be configured with a default.
I've omitted "edit" because I want to wait and see how comment/comment-action policies work out. I could imagine locking "edit" down to only the owner at some point, and providing a wider "interact" capability, or something like that, which would cover accept/reject/commandeer. Users in this group could still edit indirectly by commandeering first.
Test Plan: Created new revisions from the CLI and conduit.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7269
Summary:
Ref T603. I want to let applications define new capabilities (like "can manage global rules" in Herald) and get full support for them, including reasonable error strings in the UI.
Currently, this is difficult for a couple of reasons. Partly this is just a code organization issue, which is easy to fix. The bigger thing is that we have a bunch of strings which depend on both the policy and capability, like: "You must be an administrator to view this object." "Administrator" is the policy, and "view" is the capability.
That means every new capability has to add a string for each policy, and every new policy (should we introduce any) needs to add a string for each capability. And we can't do any piecemeal "You must be a {$role} to {$action} this object" becuase it's impossible to translate.
Instead, make all the strings depend on //only// the policy, //only// the capability, or //only// the object type. This makes the dialogs read a little more strangely, but I think it's still pretty easy to understand, and it makes adding new stuff way way easier.
Also provide more context, and more useful exception messages.
Test Plan:
- See screenshots.
- Also triggered a policy exception and verified it was dramatically more useful than it used to be.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7260
Summary: Ref T1279. Prerequisite for adding icons or other type information to tokenizers, since we don't currently have enough information to prefill them when rendering things from the server side. By passing handles in, the tokenizer can extract type information.
Test Plan:
- Searched by user in Audit.
- Sent Conpherence from profile page.
- Tried to send an empty conpherence.
- Searched Countdown by user.
- Edited CCs in Differential.
- Edited reviewers in Differential.
- Edited a commit's projects.
- Searched lint by owner.
- Searched feed by owner/project.
- Searched files by owner.
- Searched Herald by owner.
- Searched Legalpad by owner.
- Searched Macro by owner.
- Filtered Maniphest reports by project.
- Edited CCs in Maniphest.
- Searched Owners by owner.
- Edited an Owners package.
- Searched Paste by owner.
- Searched activity logs by owner.
- Searched for mocks by owner.
- Edited a mock's CCs.
- Searched Ponder by owner.
- Searched projects by owner.
- Edited a Releeph project's pushers.
- Searched Releeph by requestor.
- Edited "Uses Symbols" for an Arcanist project.
- Edited all tokenizers in main search.
- Searched Slowvote by user.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7248
Summary: Ref T603. We currently bomb out here, but should just continue forward. I'm fairly certain we don't even use this for anything anymore (it has been replaced by "depends on") but need to check that.
Test Plan: Created a new revision with `arc diff`.
Reviewers: ljalonen, btrahan, #blessed_reviewers, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7255
Summary:
Ref T1279. This is a logical change.
- "Reject" (nee "Request Changes") is now sticky. The review won't transition to "Accepted" until the reviewer clears their objection. In practice, I think it always worked like this anyway (without technical enforcement, users just followed this rule naturally, since disobeying this rule is kind of a dick move) so I don't expect this to change much. I think this rule is easier to understand than the old rule now, given the multi-reviewer status and blocking reviewers.
- "Blocking Reviewer" and "Reject" now prevent a revision from transitioning to "Accepted". When reviewers accept, resign, or are removed, we do a check to see if the reivsion has: at least one user reviewer who has accepted; zero rejects; and zero blocks. If all conditions are satisfied, we transition it to "accepted".
Practically, the primary net effect of this is just to make blocking reviews actually block.
This is pretty messy, but there's not much we can do about it until after T2222, since we have two completely separate editor pathways which are both responsible for adjusting status. Eventually, these can merge into a single sane editor which implements reasonable rules in reaonable ways. But that day is not today.
Test Plan: With three users and a project, made a bunch of accepts, rejects, resigns and reviewer removals. I think I probably covered most of the pathways? There are a lot of interactions here.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, wisutsak.jaisue.7
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7245
Summary: Ref T1279. These reviewers don't actually create a logical block yet (that is, revisions still transition to "accepted" even in their presence), but this handles everything except that.
Test Plan: Added Herald rules and updated revisions; see screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7244
Summary:
Ref T1279. With the new per-reviewer status, you can always accept or reject a revision.
This is primarily cosmetic/UI changes. In particular, you've always been able to reject a rejected revision, the UI just didn't show you an option.
Test Plan: Accepted accepted revisions; rejected rejected revisions. See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7243
Summary: Ref T1279. If you accept a revision, also accept on behalf of all the projects you have authority to accept for.
Test Plan:
- Accepted a revision which I was a reviewer on, saw my own status and an authority project's status change to "Accepted".
- Accepted a revision which I was not a reviewer on, saw my own status be added (as "Accepted") and the project's status update.
Also, see screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, wisutsak.jaisue.7
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7242
Summary:
Ref T1279. We currently determine reviewers at display time, but this is bad for several reasons:
- It puts queries very close to the display layer.
- We have to query for each revision if we want to figure out authority for several.
- We need to figure it out in several places, so we'll end up with copies of this logic.
- The logic isn't trivial (exceptions for the viewer, exceptions to that rule for install configuration).
- We already do this "figure it out when we need it" stuff in Diffusion for audits and it's really bad: we have half-working copies of the logic spread all over the place.
Instead, put it in the Query. Callers query for it and get the data attached to the reviewer objects.
Test Plan:
- Looked at some revisions, verified the correct lines were highlighted.
- Looked at a revision I created and verified that projects I was a member of were not highlighted.
- With self-accept enabled, these //are// highlighted.
- Looked at a revision I did not create and verified that projects I was a member of were highlighted.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7241
Summary: Ref T1279. No logical changes, but cosmetically highlight stuff you have authority for, like we do in Diffusion.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7237
Summary:
Ref T1279. Although I think this is a bad idea in general (we once supported it, removed it, and seemed better off for it) users expect it to exist and want it to be available. Give them enough rope to shoot themselves in the foot.
I will probably write some lengthy treatise on how you shouldn't use this rule later.
Implementation is straightforward because Differential previously supported this rule.
This rule can also be used to add project reviewers.
Test Plan: Made some "add reviewers" rules, created revisions, saw reviewers trigger.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7235
Summary: Ref T1279. Show separate sections for "Reviewers" and "Project Reviewers" (Differential) and for "Auditors" and "Package/Project Auditors" (Diffusion/Audit).
Test Plan:
- Looked at a commit. Saw separation.
- Looked at a revision. Saw separation.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7233
Summary:
Ref T1279. Two changes to the search/query for Differential:
- "Reviewers" now accepts users and projects.
- "Responsible Users" now includes revisions where a project you are a member of is a reviewer.
Test Plan:
- Searched for project reviewers.
- Verified that the dashboard now shows reviews which I'm only part of via project membership.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7231
Summary:
Ref T1279. No actual logical changes, but:
- You can now add projects as reviewers from the revision view typeahead ("Add Reviewers" action).
- You can now add projects as reviewers from the revision detail typeahead.
- You can now add projects as reviewers from the CLI (`#yoloswag`).
- Generated commit messages now list project reviewers (`Reviewers: #yoloswag`).
I'll separate projects from users in the "Reviewers" tables in the next revision.
Test Plan:
- Added projects as reviewers using the web UI and CLI.
- Used `arc amend --show --revision Dnnn` to generate commit messages.
- Viewed revision with project reviewers in web UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7230
Summary: Ref T1279. Updates status to 'accepted' or 'commented' when the user takes those actions.
Test Plan:
- Commented on a revision, got a comment icon.
- Accepted a revision, got an accept icon.
- Commented again, icon stayed as "accept".
- Faked the "old diff" states.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7229
Summary:
Ref T1279. No logical changes, just updates the reviewer display style.
We currently keep track of only "requested changes".
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7228
Summary:
Ref T1279. @champo did a lot of this work already; we've been doing double writes for a long time.
Add "double reads" (reading the edge table as both the "relationship" table and as the "reviewer status" table), and migrate all the data.
I'm not bothering to try to recover old reviewer status (e.g., we could infer from transactions who accepted old revisions) because it wold be very complicated and doesn't seem too valuable.
Test Plan:
- Without doing the migration, used Differential. Verified that reads and writes worked. Most of the data was there anyway since we've been double-writing.
- Performed the migration. Verified that everything was still unchanged.
- Dropped the edge table, verified all reviweer data vanished.
- Migrated again, verified the reviewer stuff was restored.
- Did various cc/reviewer/subscriber queries, got consistent results.
Reviewers: btrahan
Reviewed By: btrahan
CC: champo, aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7227
Summary: Right now emails don't include bookmark info (wasn't added in D2897). Lets include it so it's consistent with the web UI.
Test Plan: Inspected code, made sure it matched web UI code. Verified that web UI with these changes was consistent with rendering before refactoring.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7215
Summary:
Does what it says on the label. We already had 'Any changed file content', now we have 'Any added file content' and 'Any removed file content'.
- There is a bit of copied/pasted code here: I'm open to suggestions on how to refactor it so it's less redundant.
- The wording seems a little awkward, and as @epriestley mentioned in T3829, moved code will be detected less than ideally.
Test Plan: Created Herald Rules, verified via dry run that they were triggered in appropriate situations.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3829
Differential Revision: https://secure.phabricator.com/D7214
Test Plan: Confirm the API returns a single flat result with a unified git diff.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran, charles
Differential Revision: https://secure.phabricator.com/D7199
Test Plan:
Enable inline patches:
```
bin/config set metamta.differential.patch-format 'unified'
bin/config set metamta.differential.inline-patches 100000000
```
Create a new diff and confirm it renders correctly via email.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7198
Summary: I removed the only callsite in D7179, but forgot to remove this code.
Test Plan: Grepped for callsites.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7194
Summary:
- "revision" is misspelled.
- Remove an unused variable.
Test Plan: Used API console to call method.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7184
Summary: See D7162. This was like 99% my fault. Just provide a header; the new ones look pretty reasonable.
Test Plan: Viewed Diffusion change view, no exception.
Reviewers: vrana, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7183
Summary: Ref T603. Clean these up and move them to a single place.
Test Plan:
- Downloaded a raw diff.
- Enabled "attach diffs", created a revision, got an email with a diff.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7179
Summary: Ref T603. Swaps out most `PhabricatorFile` loads for `PhabricatorFileQuery`.
Test Plan:
- Viewed Differential changesets.
- Used `file.info`.
- Used `file.download`.
- Viewed a file.
- Deleted a file.
- Used `/Fnnnn` to access a file.
- Uploaded an image, verified a thumbnail generated.
- Created and edited a macro.
- Added a meme.
- Did old-school attach-a-file-to-a-task.
- Viewed a paste.
- Viewed a mock.
- Embedded a mock.
- Profiled a page.
- Parsed a commit with image files linked to a revision with image files.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7178
Summary:
Three changes here.
- Add `setActionList()`, and use that to set the action list.
- Add `setPropertyList()`, and use that to set the property list.
These will let us add some apropriate CSS so we can fix the border issue, and get rid of a bunch of goofy `.x + .y` selectors.
- Replace `addContent()` with `appendChild()`.
This is just a consistency thing; `AphrontView` already provides `appendChild()`, and `addContent()` did the same thing.
Test Plan:
- Viewed "All Config".
- Viewed a countdown.
- Viewed a revision (add comment, change list, table of contents, comment, local commits, open revisions affecting these files, update history).
- Viewed Diffusion (browse, change, history, repository, lint).
- Viewed Drydock (resource, lease).
- Viewed Files.
- Viewed Herald.
- Viewed Legalpad.
- Viewed macro (edit, edit audio, view).
- Viewed Maniphest.
- Viewed Applications.
- Viewed Paste.
- Viewed People.
- Viewed Phulux.
- Viewed Pholio.
- Viewed Phame (blog, post).
- Viewed Phortune (account, product).
- Viewed Ponder (questions, answers, comments).
- Viewed Releeph.
- Viewed Projects.
- Viewed Slowvote.
NOTE: Images in Files aren't on a black background anymore -- I assume that's on purpose?
NOTE: Some jankiness in Phortune, I'll clean that up when I get back to it. Not related to this diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7174
Summary: Missed this case in my sandbox
Test Plan: Reload a test diff
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7168
Summary: This adds the 'PHUIObjectBox' to nearly every place that should get it. I need to comb through Diffusion a little more. I've left Differential mostly alone, but may decide to do it anyways this weekend. I'm sure I missed something else, but these are easy enough to update.
Test Plan: tested each new layout.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7162
Summary:
Ref T603. This could probably use a little more polish, but improve the quality of policy error messages.
- Provide as much detail as possible.
- Fix all the strings for i18n.
- Explain special rules to the user.
- Allow indirect policy filters to raise policy exceptions instead of 404s.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7151
Summary:
Ref T603. Adds clarifying text which expands on policies and explains exceptions and rules. The goal is to provide an easy way for users to learn about special policy rules, like "task owners can always see a task".
This presentation might be a little aggressive. That's probably OK as we introduce policies, but something a little more tempered might be better down the road.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7150
Summary:
Ref T603. Make Differential behaviors for logged-out and underprivleged users more similar to other apps.
I'm going to drop this "anonymous access" thing at some point, but `reviews.fb.net` actually looks like it's running semi-modern code, so leave it alive until we have a more compelling replacement in the upstream.
Test Plan: As a logged out user, browsed Differential and clicked things and such.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7148
Summary:
Ref T603. When a diff is attached to a revision, try to guess the repository if possible. In cases where we succeed, this automatically gives us intuitive policy behavior (i.e., you can see a revision if you can see the repository the change is against).
I pulled this into a funky little "Lookup" class for two reasons:
- It's used in two places;
- I anticipate that we might need to add some sort of `explainWhy()` method if users find the heuristics confusing.
Test Plan: Created and updated revisions, saw them pick up the correct repository association. Ran Herald dry run against associable and nonassociable revisions, saw correct values populate.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7147
Summary: This isn't too useful most of the time since we don't automatically populate this data yet, but works fine.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7144
Summary: Ref T603. I think T2222 is fraught with peril so I'm not going to try to sequence it ahead of T603 for Differential. Provide access to policy controls in Differential's edit view.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7142
Summary: Adds some padding to the right
Test Plan: Looked at a diff
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7143
Summary: Ref T603. Makes the majority of reads policy aware (and pretty much all the important ones).
Test Plan:
- Created a comment with `differential.createcomment`.
- Created a new revision with `arc diff` in order to exercise `differential.creatediff`.
- Created an inline comment with `differential.createinline`.
- Added a comment to a revision.
- Edited an inline comment.
- Edited a revision.
- Wrote "Depends on ..." in a summary, saved, verified link was created.
- Browsed a file in Diffusion.
- Got past the code I changed in the Releeph request thing.
- Edited a Releeph request.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7136
Summary: Ref T603. Moves policy information from a custom field to the header for revisions.
Test Plan: Looked at a revision.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7135
Summary:
Ref T603. Read policies out of policy columns.
When a revision is associated with a repository (which is currently never), require view access on the repository to see the revision (or, require the viewer to be the owner). This is a blanket "do the right thing" rule which should make Differential's default policies align with user expectations.
Future diffs will populate the `repositoryPHID` when a revision is created.
Test Plan: Tooled around Differential. None of this stuff does anything yet, so nothing very exciting happened.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7134
Summary: Ref T603. Paves the way for policy controls.
Test Plan: Ran storage upgrade, bumbled around in Differential.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7133
Summary: Ref T603. This swaps almost all queries against the repository table over to be policy aware.
Test Plan:
- Made an audit comment on a commit.
- Ran `save_lint.php`.
- Looked up a commit with `diffusion.getcommits`.
- Looked up lint messages with `diffusion.getlintmessages`.
- Clicked an external/submodule in Diffusion.
- Viewed main lint and repository lint in Diffusion.
- Completed and validated Owners paths in Owners.
- Executed dry runs via Herald.
- Queried for package owners with `owners.query`.
- Viewed Owners package.
- Edited Owners package.
- Viewed Owners package list.
- Executed `repository.query`.
- Viewed "Repository" tool repository list.
- Edited Arcanist project.
- Hit "Delete" on repository (this just tells you to use the CLI).
- Created a repository.
- Edited a repository.
- Ran `bin/repository list`.
- Ran `bin/search index rGTESTff45d13dffcfb3ea85b03aac8cc36251cacdf01c`
- Pushed and parsed a commit.
- Skipped all the Drydock stuff, as it it's hard to test and isn't normally reachable.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7132
Summary:
Ref T603. Make almost every task read policy-aware. Notable exceptions are:
- Edge editor -- this stuff is prescreened and should be moved to ApplicationTransactions eventually anyway.
- Search/attach stuff -- this stuff needs some general work. The actual list should be fine since you can't pull handles. There may be a very indirect hole here where you could attach an object you can't see (but do know the ID of) to an object you can see. Pretty fluff.
- The "Tasks" field in Differential will let you reference objects you can't see. Possibly this is desirable, in the case of commandeering revisions. Mostly, it was inconvenient to get a viewer (I think).
Test Plan:
- Called `maniphest.info`.
- Called `maniphest.update`.
- Batch edited tasks.
- Dragged and dropped tasks to change subpriority.
- Subscribed and unsubscribed from a task.
- Edited a task.
- Created a task.
- Created a task with a parent.
- Created a task with a template.
- Previewed a task update.
- Commented on a task.
- Added a dependency.
- Searched for "T33" in object search dialog.
- Created a branch "T33", ran `arc diff`, verified link.
- Pushed a commit with "Fixes T33", verified close.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7119
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.
Test Plan: reload a few forms in maniphest, projects, differential
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7120
Summary:
`getArcanistProjectName()` has some logic which gets messy with the `self::ATTACHABLE` mechanism. This makes `differential.getdiff` and similar Conduit methods throw an exception when querying a diff which doesn't have a project. See <http://pastebin.com/Czzrd0Jz>.
Instead, unconditionally attach a project (possibly `null`) when loading diffs if they need projects.
Test Plan: Ran `differential.getdiff` against a `arc diff --raw` diff with no project, got a result instead of an exception.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, sttwister
Differential Revision: https://secure.phabricator.com/D7101
Summary: Adds status icons and colors to Maniphest and Differential. Also minor tweaks to them in hovercards. Probably some other stuff too.
Test Plan: Test many diff and task states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7098
