1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-04 20:52:43 +01:00
Commit graph

6542 commits

Author SHA1 Message Date
Chad Little
4cc40933c4 Color consolidation
Summary: Another pass at consolidating colors

Test Plan: Test various pages and UI elements

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6905
2013-09-07 09:13:55 -07:00
Chad Little
d06788c1e5 Move Workboards to PHUI
Summary: This is just renaming to PHUI (I like shorter text :)

Test Plan: reload workboard examples page, seems to not fatal and looks very appealing

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6904
2013-09-06 14:06:12 -07:00
epriestley
b4d9a8d547 Add a "before" parameter to feed.query
Summary: See IRC. We already have "after", add the corresponding "before". This makes polling for updates much easier.

Test Plan: Ran queries with "before" and "after".

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6903
2013-09-06 10:45:08 -07:00
Chad Little
78cbfc3a02 Fix extra divs in PHUIBoxView
Summary: This moves the outer classes to the outmost div

Test Plan: Reviewed a few PHUIBox pages

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6902
2013-09-06 10:42:44 -07:00
Guy Warner
03783c4f63 Alert user that projects/ is protected in Phriction
Summary: Fixes T3447 - adds check if slug is projects/ alerts user

Test Plan: Create new doc in Phriction. Type projects/whatever and get error

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3447

Differential Revision: https://secure.phabricator.com/D6901
2013-09-06 10:42:02 -07:00
epriestley
95dc69a30e Fix "Show All Changes" button in Diffusion
Summary: See IRC. We currently render a "show all changes" button for commits which have more than 100 but fewer than 1000 changes, but it doesn't actually do anything. Make it do what it's supposed to.

Test Plan: Set the limit to 2; clicked the button.

Reviewers: chad, staticshock, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6900
2013-09-06 10:05:24 -07:00
epriestley
a40861e5c6 Publish Doorkeeper object stories to JIRA
Summary:
Ref T3687. Publish stories into JIRA.

These need some voicing fixes, which maybe involves straightening out the feed code. For example, they're voiced in-context ("updated this revision") when they should be voiced out-of-context ("updated D123").

Generally, this is similar to the Asana stuff but a lot simpler since we don't need to do any state management.

Test Plan: {F57366}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6892
2013-09-05 16:51:20 -07:00
epriestley
470bb4931b Fix a warning in the JIRA field
Summary: Ref T3687. The `value` property may be `null`.

Test Plan: Loaded a revision with the JIRA field enabled but no issues attached, no longer saw a warning about a bad argument to `foreach()`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6890
2013-09-05 16:51:13 -07:00
epriestley
2d240142c8 Fix 20130820.file-mailkey-populate.php for databases in strict mode
Summary:
Fixes T3803. Turns out my advice was sort of terrible. :/

In strict mode, the `INSERT INTO x (y, z)` raises an error unless `(y, z, ...)` includes //all// columns without default values.

Test Plan: Ran `storage upgrade` on a strict-mode install. Verified no inserts were performed.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3803

Differential Revision: https://secure.phabricator.com/D6894
2013-09-05 15:58:28 -07:00
Bob Trahan
228496cdbe File - add transactions and editor
Summary: this ends up being a little weird since you can't actually edit files. Also, since we create files all sorts of ways, sometimes without even having a user, we don't  bother logging transactions for those events. Fixes T3651. Turns out this work is important for T3612, which is a priority of mine to help get Pholio out the door.

Test Plan: left a comment on a file. it worked! use bin/mail to verify mail content looked correct.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran, wez

Maniphest Tasks: T3651, T3612

Differential Revision: https://secure.phabricator.com/D6789
2013-09-05 13:11:02 -07:00
Chad Little
4b061a766a More Diviner style updates
Summary: This adds a number of new styles for Diviner documentation. Not sure I've covered all the bases or wrote this in the most efficient manner, but passing it along now for early review before tightening everything up.

Test Plan: Review various class pages.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6888
2013-09-05 12:29:07 -07:00
epriestley
a3d4f4c457 Fix an issue with darkconsole.always-on and logged-out users
Summary:
Fixes T3796. When this got split out into tabs, the data endpoints were accidentally locked down. Open them up again if the setting is on.

Also, when you open/close the console we try to save the preference. Just no-op if you're logged out. Previously, you'd see the requests in DarkConsole since they failed.

Test Plan: Enabled `darkconsole.always-on` and toggled the console on and off as a logged-out user. Disabled the preference and verified it was no longer accessible.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3796

Differential Revision: https://secure.phabricator.com/D6886
2013-09-05 11:16:32 -07:00
Guy Warner
fc82e81fe2 Add isExplicitUpload to true on macro uploads
Summary: Fixes T3798. Macros now show up as manually uploaded

Test Plan: Upload a macro, go to file, new macro is visiable

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3798

Differential Revision: https://secure.phabricator.com/D6887
2013-09-05 10:22:07 -07:00
epriestley
0ae7335316 Add missing (int) casts to Herald rule editing
Summary: Fixes T3792. These raise errors if the database is in strict mode and you try to create an "any" rule.

Test Plan: Created a rule with "any".

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3792

Differential Revision: https://secure.phabricator.com/D6883
2013-09-04 12:07:26 -07:00
Chad Little
589f1c7696 Diviner CSS tweaks
Summary: Update Diviner table layouts, make it purty.

Test Plan: test many diviner pages

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6880
2013-09-04 11:50:57 -07:00
epriestley
4252c93c8a Remove block of commented-out code
Summary: I left a clamp in the patient.

Test Plan: derp-a-derpderp

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6882
2013-09-04 11:02:12 -07:00
Andrew Gallagher
8bba3f280d Only associate branch with task if maniphest is enabled
Summary:
Previously, maniphest tasks would get upated by diffs on branches
with tasky names, even if maniphest was disabled.

Test Plan:
Tested createing a diff in sandbox with maniphest disabled, on a
git branch named using the format "t###".  Without this change,
if there happened to be a task in the maniphest DB which matched,
it was updated an email was sent to users.

Reviewers: epriestley

Reviewed By: epriestley

CC: wez, slawekbiel, whhone, Korvin, aran

Differential Revision: https://secure.phabricator.com/D6881
2013-09-03 18:22:01 -07:00
epriestley
853544b54a Add "JIRA Issues" field to Differential
Summary:
Ref T3687. This adds a field which allows you to link Differential Revisions to JIRA issues.

This is just about as basic as it can get, but gets the job done. The field enables itself if you have a JIRA auth provide. You enter JIRA issues in a comma-delimited format and it generates appropriate edges.

Nothing is pushed to the issues yet.

The only real rough part here is that if you commandeer a revision which is linked to issues you can't see, editing it is difficult via the CLI. This seems pretty much like a non-issue, but at some point we can let the field throw some kind of "RecoverableInvalidFieldException" which just warns the user. The "no reviewers, continue anyway?" prompt could then use that too.

Test Plan:
  - Edited via web UI, tried valid/invalid edits, checked that edges showed up in the database, added/removed issues, clicked issue links.
  - Edited via CLI, tried valid/invalid edits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6879
2013-09-03 17:27:51 -07:00
epriestley
825fb9c85a Add JIRA doorkeeper and remarkup support
Summary:
Ref T3687. Adds a Doorkeeper bridge for JIRA issues, plus remarkup support. In particular:

  - The Asana and JIRA remarkup rules shared most of their implementation, so I refactored what I could into a base class.
  - Actual bridge implementation is straightforward and similar to Asana, although probably not similar enough to really justify refactoring.

Test Plan:
  - When logged in as a JIRA-connected user, pasted a JIRA issue link and saw it enriched at rendering time.
  - Logged in and out with JIRA.
  - Tested an Asana link, too (seems I haven't broken anything).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6878
2013-09-03 17:27:38 -07:00
epriestley
e5b4ce5525 Reduce the amount of OAuth1/OAuth2 code duplication for rendering login buttons
Summary: Ref T3687. These buttons don't work quite the same way, but are similar enough that the code seems worth consolidating.

Test Plan: Viewed and clicked both OAuth1 (Twitter, JIRA) and OAuth2 (Facebook) login buttons. Got logins.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6874
2013-09-03 10:30:53 -07:00
epriestley
25eb401e18 Handle user aborts during auth workflows in Phabricator
Summary: Depends on D6872. Ref T3687. Give the user a nice dialog instead of a bare exception.

Test Plan: Cancelled out of Twitter and JIRA workflows. We should probably do this for the OAuth2 workflows too, but they're a bit of a pain to de-auth and I am lazy.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6873
2013-09-03 10:30:39 -07:00
Chad Little
6e63adaf54 Standard colors on config-tables
Summary: Moving standard colors in some new places, going well so far.

Test Plan: reload page

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6876
2013-09-03 10:18:43 -07:00
Chad Little
4bb920f396 Tighten up ObjectItemList icons
Summary: Pulls the icon flush right in the objectlistview.

Test Plan: Review my stale diffs, and UIExamples page.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6875
2013-09-03 08:24:41 -07:00
Chad Little
7d8367a552 Color tweaks for action list, headers
Summary: Starting to roll out the standard colors and spacing to action list, headers, and property views. Also softened the grey borders a hex.

Test Plan: Review Maniphest and Differential on desktop and mobile. Felt the flow of standardization waft over me.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6869
2013-09-03 07:00:06 -07:00
Gareth Evans
fcba0c74d9 Replace all "attach first..." exceptions with assertAttached()
Summary:
Ref T3599
Go through everything, grep a bit, replace some bits.

Test Plan: Navigate around a bit

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3599

Differential Revision: https://secure.phabricator.com/D6871
2013-09-03 06:02:14 -07:00
epriestley
4e12a375f3 Add JIRA as an authentication provider
Summary:
Ref T3687. Depends on D6867. This allows login/registration through JIRA.

The notable difference between this and other providers is that we need to do configuration in two stages, since we need to generate and save a public/private keypair before we can give the user configuration instructions, which takes several seconds and can't change once we've told them to do it.

To this effect, the edit form renders two separate stages, a "setup" stage and a "configure" stage. In the setup stage the user identifies the install and provides the URL. They hit save, we generate a keypair, and take them to the configure stage. In the configure stage, they're walked through setting up all the keys. This ends up feeling a touch rough, but overall pretty reasonable, and we haven't lost much generality.

Test Plan: {F57059}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6868
2013-09-03 05:53:21 -07:00
epriestley
25e43e872b Add Twitter as an authentication provider
Summary: Ref T3687. Depends on D6864. Implements the `OAuth1` provider in Phabricator (which is mostly similar to the OAuth2 provider, but doesn't share quite enough code to actually extend a common base class, I think) and Twitter as a concrete subclass.

Test Plan:
Created a Twitter provider. Registered, logged in, linked, refreshed account link.

{F57054}

{F57056}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3687

Differential Revision: https://secure.phabricator.com/D6865
2013-09-03 05:53:08 -07:00
Chad Little
63d2b11cab More grey text updates
Summary: More grey tweaks, breaking these up so I can test and tweak each as needed.

Test Plan: Review pages.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6866
2013-09-02 13:57:48 -07:00
Bob Trahan
83e0380046 Maniphest - fix fatal in custom query for users with no projects
Summary: See https://github.com/facebook/phabricator/issues/380 for report.

Test Plan: Maniphest -> Custom Query -> Put user with no project in "Any User Projects" field -> Search : observe no fatal

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6862
2013-09-02 13:24:41 -07:00
Aviv Eyal
cf13885736 User preference for time format
Summary: Also, don't try to load prefs for non-users.

Test Plan: toggle, save, look at something with a time. arc unit.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6796
2013-09-02 12:55:11 -07:00
Chad Little
35b209c464 Fix spacing, color in Conpherence Pontificate button
Summary: Spacing got broken during forms update.

Test Plan: View Conpherence

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6863
2013-09-02 12:51:21 -07:00
epriestley
b7f6956ec9 Allow Diviner groups to be configured in .book files
Summary:
Ref T988. Currently, every class/function needs to be annotated with `@group`, but 99% of this data can be inferred from file structure, at least in this project. Allow group specifications like:

  "paste" : {
    "name" : "Paste",
    "include" : "(^src/applications/paste/)"
  }

..to automatically put everything defined there in the "paste" group. A list of regexps is also supported. Depends on D6855.

Test Plan: Regenerated documentation with `bin/diviner generate --book src/docs/book/phabricator.book --clean`, observed all Paste stuff go in the paste group.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D6856
2013-09-02 11:33:02 -07:00
Chad Little
fef3e074d8 Move #888 and #999 to $lightgreytext
Summary: Moves lighter grey text to a common color.

Test Plan: sandbox, review diff

Reviewers: epriestley, btrahan, mutech.aka

Reviewed By: mutech.aka

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6859
2013-09-02 08:12:18 -07:00
Chad Little
dd995984eb UIColor Blues and common color integration
Summary: This adds standard 'blues' and start integration of standard colors for text, backgrounds, and borders.

Test Plan: sb

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6857
2013-09-02 08:10:47 -07:00
Chad Little
921bc32928 Move #666 to $greytext in UIColor
Summary: Split some of these up for safe regexes.

Test Plan: reload celerity

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6858
2013-09-02 08:08:54 -07:00
epriestley
eb3690f8c6 Add "Authored" as a default filter to Differential
Summary: Fixes T3786. Not 100% sold on this (I don't want to restore all of the original filters, since users can and should just build the weird ones if they use them), but this is almost certainly the most useful of the defaults which ApplicationSearch removed.

Test Plan: Viewed `/differential/`, executed the query.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3786

Differential Revision: https://secure.phabricator.com/D6860
2013-09-01 19:15:52 -07:00
Chad Little
fb7c7de519 Add Base Grey set of colors
Summary: This adds a set of standard grey colors for use in shading objects and importance. I'll follow up and start implementing in another diff.

Test Plan: Color UI Examples, Adobe Kuler

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6853
2013-08-30 14:40:46 -07:00
epriestley
480a17f6e0 Restore white background to Paste
Summary: I think we lost this recently (maybe with the line highlighting change), but it's more readable if we put these on a white background.

Test Plan: Looked at a Paste (like `/P123`), saw white background.

Reviewers: chad, btrahan, asherkin

Reviewed By: asherkin

CC: aran

Differential Revision: https://secure.phabricator.com/D6852
2013-08-30 09:29:31 -07:00
epriestley
1b6f71dec1 In Diviner, parse "abstract" and "final" in PHP classes, interfaces and methods
Summary: Ref T988. Adds support for the "abstract" and "final" keywords in the atomizer.

Test Plan: Looked at abstract/final stuff.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D6850
2013-08-30 09:19:18 -07:00
epriestley
bf50e0f870 Compose and display method information in Diviner
Summary:
Ref T988. As mentioned elsewhere, one broad goal of this iteration is to reduce the amount of boilerplate documentation we need to write. For example:

  - I want to set `@group` by default in most cases.
  - I want to inherit things like `@param` and `@return` by default.
  - I want to inherit method documentation by default.
  - I want to inherit `@task` information.

This implements most of the method inheritance stuff.

This //looks// super gross, but I believe we now compose all of the information of interest at display time and can work on rendering it sensibly in the near future.

Test Plan: {F56790}

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran, sascha-egerer

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D6849
2013-08-30 09:17:27 -07:00
epriestley
cf0bf34255 Allow MetaMTA adapters to indicate that a mail is permanently undeliverable
Summary: Currently, adapters can only fail mail temporarily. Allow them to indicate a permanent failure by throwing a special exception.

Test Plan: Added and ran unit tests.

Reviewers: wez, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6847
2013-08-30 08:21:50 -07:00
Wez Furlong
11f1268e99 don't throw BadMethodCallException on diffs that add images
Summary:
I don't know if there is something more sinister going on
under the covers, but we have a couple of diffs that trigger:

Unhandled Exception ("BadMethodCallException")
Call to a member function getMetadata() on a non-object

when the diff page is handling its async render calls.  One diff
in particular has multiple image adds and thus has a stack of of these
error dialogs to close.

This isn't a new regression, we just haven't gotten around to debugging
it until now (reported on 6/12)

One revision that triggers it has two diffs.  If I show Base -> Diff 1
I don't hit the error.  When I select Base -> Diff 2, or Diff 1 -> Diff
2, the error triggers.

I don't understand what this means, but this diff avoids the null object
reference that causes the exception.

Test Plan:
Load the offending diff, don't hit the error. The diff loads
the images that were added

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6851
2013-08-30 08:15:10 -07:00
epriestley
34356c7154 Fix outdated link to MetaMTA web UI in MetaMTA tasks
Summary: Missed this when moving most MetaMTA responsibilities to the CLI. Show the correct command to get data rather than linking to a 404.

Test Plan: {F56733}

Reviewers: wez, btrahan, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6846
2013-08-29 17:37:17 -07:00
epriestley
b932c606cb Implement "Add to CC" rule for Audits
Summary:
D6660 accidentally allowed you to build Herald rules for commits that take action "Add to CC", but provided no implementation.

Someone at Facebook then wrote such a rule.

Fix forward since there's no real reason not to allow this.

Test Plan: Used `./scripts/repository/reparse.php --herald rXnnnn` to trigger rules. Observed rule trigger and subsequent subscription.

Reviewers: wez, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6845
2013-08-29 17:03:40 -07:00
epriestley
b16d5b9ba4 Fix one more rule with remarkup fullscreen mode
Fixes T3782. After D6844, the menu bar would still float on top of this.

Auditors: chad
2013-08-29 16:03:13 -07:00
epriestley
2b6271d02c Remove "Chaos" mode and fix fullscreen in Conpherence
Summary:
Fixes T3782. Two changes:

  - Remove the "Chaos" mode, which wasn't as funny as I'd hoped and has had a good run.
  - Fix "Order" (now "Fullscreen") mode in Conpherence. Best fix I could come up with is dropping the "position: fixed" on all parents while in the mode.

Test Plan: Used Fullscreen mode in Conpherence in Chrome.

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3782

Differential Revision: https://secure.phabricator.com/D6844
2013-08-29 15:59:15 -07:00
epriestley
63dbccb8a1 Fix default order for Differential queries
Summary:
Fixes T3781. The UI defaults to "Created" but the query defaults to "Modified". Make the two consistent.

In particular, an issue this fixes is that previously a `/differential/?authors=duck` page would show "Order: Created" but actually order by "Modified".

Test Plan: Visited `/differential/?authors=duck` and verified the revisions were ordered by creation date.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3781

Differential Revision: https://secure.phabricator.com/D6843
2013-08-29 15:07:30 -07:00
epriestley
eb32b5c812 Show host information on setup issue screen
Summary: Ref T3780. Facebook has some environmental / itermittent stuff which would be easier to debug with host information on the setup issue screen.

Test Plan:
Checked both in-chrome and out-of-chrome versions of this screen, both looked reasonable.

{F56694}

Reviewers: wez, btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T3780

Differential Revision: https://secure.phabricator.com/D6842
2013-08-29 14:22:05 -07:00
epriestley
f1c75a6382 Allow construction of ApplicationSearch queries with GET
Summary:
Ref T3775 (discussion here). Ref T2625.

T3775 presents two problems:

  # Existing tools which linked to `/differential/active/epriestley/` (that is, put a username in the URL) can't generate search links now.
  # Humans can't edit the URL anymore, either.

I think (1) is an actual issue, and this fixes it. I think (2) is pretty fluff, and this doesn't really try to fix it, although it probably improves it.

The fix for (1) is:

  - Provide a helper to read a parameter containing either a list of user PHIDs or a list of usernames, so `/?users[]=PHID-USER-xyz` (from a tokenizer) and `/?users=alincoln,htaft` (from an external program) are equivalent inputs.
  - Rename all the form parameters to be more digestable (`authorPHIDs` -> `authors`). Almost all of them were in this form already anyway. This just gives us `?users=alincoln` instead of `userPHIDs=alincoln`.
  - Inside ApplicationSearch, if a request has no query associated with it but does have query parameters, build a query from the request instead of issuing the user's default query. Basically, this means that `/differential/` runs the default query, while `/differential/?users=x` runs a custom query.

Test Plan: {F56612}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625, T3775

Differential Revision: https://secure.phabricator.com/D6840
2013-08-29 11:52:29 -07:00
epriestley
5a11f08ba4 Use bar colors to show revision status in revision lists
Summary:
Ref T3772. The original version of D5451 had a very colorful version of this which felt a bit arbitrary, and we moved away from it after discussion, particularly [[ https://secure.phabricator.com/D5451#comment-8 | here (chad) ]] and [[ https://secure.phabricator.com/D5451#comment-14 | here (me) ]] and [[ https://secure.phabricator.com/D5451#comment-19 | here (chad again) ]].

The core of my objection was that status and priority to the viewer aren't the same: a "needs revision" revision that you authored is high priority (you need to revise it), but a "needs revision" revision that someone else authored is low priority (you're waiting on them to revise it). If we color by status, revisions in both high priority and low priority states will be colored red. We can instead color by viewer priority (blocking others = red, needs attention = orange, waiting on others = blue; or something), but that would be redundant (we already group by it, so you'd get big chunks of stuff with the same color and color would have no utility), confusing (in ungrouped views, the colors would not be self-explanatory) and weirdly inconsistent (different users would see objects having different colors).

I still think all this holds, but I also thought that "viewer priority" was enormously more important than "state", since I use the former frequently and the latter very rarely. From T3772, it sounds like some users use "state" a lot more than I do (i.e., they want to find "accepted" revisions within a "viewer priority" group like "Action Required"). This is a possible approach to that.

I think another issue was the heavy use of the color in the original; this restores a more conservative version of it which doesn't have as much weight. In particular:

  - Revisions in the "Needs Review" state retain the default color, rather than orange.
  - Revisions in the "Closed" state have the disabled effect.

Test Plan: See screenshot.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T3772

Differential Revision: https://secure.phabricator.com/D6839
2013-08-29 09:16:51 -07:00