1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 00:32:42 +01:00
Commit graph

1528 commits

Author SHA1 Message Date
epriestley
646280972b Glue the new FormationView on top of the older Filetree view in Differential
Summary: Ref T13516. This glues "FormationView" to "ChangesetList". The actual tree is not functional in any meaningful way yet.

Test Plan: {F7373838}

Maniphest Tasks: T13516

Differential Revision: https://secure.phabricator.com/D21151
2020-04-22 08:29:04 -07:00
epriestley
fef2cdabfe Add a "FormationView" to support dynamic flank panels
Summary:
Ref T13516. Currently, the "File Tree" element is a semi-dynamic side panel that's implemented as a special mode of a side nav panel.

This implementation is fairly clunky, and arose from organic growth out of the side nav. As such, it has some weird behaviors, doesn't have builtin support for show/hide, and can't generalize easily.

Introduce a "FormationView" which supports loading a page up with piles of side panels in various modes.

Test Plan: No callers and no user-visible impact.

Maniphest Tasks: T13516

Differential Revision: https://secure.phabricator.com/D21150
2020-04-22 08:23:21 -07:00
epriestley
7a79131bf2 Replace old hard-coded URI-based "changes saved" jank with new overgeneralized cookie-based "changes saved" jank
Summary:
Ref T13515. Settings currently has some highly specialized code for rendering "Changes saved." messages. The "saved" state is communicated across a redirect-after-POST by adding `/saved/` to the end of the URI.

This isn't great. It needs a lot of moving pieces, including special accommodations in routing rules. It's user-visible. It has the wrong behavior if you reload the page or navigate directly to the "saved" URI.

Try this scheme, which is also pretty sketchy but seems like an upgrade on the balance:

  - Set a cookie on the redirect which identifies the form we just saved.
  - On page startup: if this cookie exists, save the value and clear it.
  - If the current page started with a cookie identifying the form on the page, treat the page as a "saved" page.

This supports passing a small amount of state across the redirect-after-POST flow, and when you reload the page it doesn't keep the message around. Applications don't need to coordinate it, either. Seems somewhat cleaner?

Test Plan: In Firefox, Safari, and Chrome: saved settings, saw a "Saved changes" banner without any URI junk. Reloaded page, saw banner vanish properly.

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21144
2020-04-19 09:04:31 -07:00
epriestley
8bdc713352 Make the "Keyboard Shortcuts" dialog in Differential less hideous
Summary:
Ref T13515. Adding "\" ("Open in External Editor") made this slighlty worse, but it was already pretty bad.

Long ago the keys had a special style on them, but this got changed and dropped somewhere around D16568 -- although at the time, I think they still had a grey background (see T11654).

Some later change removed this background.

Put the background back and separate the keystrokes into groups.

Test Plan: {F7370615}

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21141
2020-04-19 09:01:07 -07:00
epriestley
35a18146a2 Merge a small amount of remaining "libphutil/" code with Phabricator, break libphutil dependency
Summary: Ref T13395. Moves a small amount of remaining "libphutil/" code into "phabricator/" and stops us from loading "libphutil/".

Test Plan: Browsed around; there are likely remaining issues.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20981
2020-02-12 15:17:36 -08:00
epriestley
0e82bd024a Use the new "CurtainObjectRefList" UI element for subscribers
Summary:
Depends on D20966. Ref T13486. Curtains currently render subscribers in a plain text list, but the new ref list element is a good fit for this.

Also, improve the sorting and ordering behavior.

This makes the subscriber list take up a bit more space, but it should make it a lot easier to read at a glance.

Test Plan: Viewed object subscriber lists at varying limits and subscriber counts, saw sensible subscriber lists.

Maniphest Tasks: T13486

Differential Revision: https://secure.phabricator.com/D20967
2020-02-04 12:38:41 -08:00
epriestley
2a92fef879 Improve wrapping and overflow behavior for curtain panels containing long usernames
Summary:
Ref T13486. When a curtain element like "Author" in Maniphest has a very long username, the wrapping and overflow behavior is poor: the date is obscured.

Adjust curtain elements which contain lists of references to other objects to improve wrapping behavior (put the date on a separate line) and overflow behavior (so we get a "..." when a name overflows).

Test Plan: {F7179376}

Maniphest Tasks: T13486

Differential Revision: https://secure.phabricator.com/D20966
2020-02-04 12:31:18 -08:00
epriestley
6d4c6924d6 Update Herald rule creation workflow to use more modern UI elements
Summary: Ref T13480. Creating a rule in Herald currently uses the older radio-button flow. Update it to the "clickable menu" flow to simplify it a little bit.

Test Plan: Created new personal, object, and global rules. Hit the object rule error conditions.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20956
2020-02-04 07:37:54 -08:00
epriestley
26c2a1ba68 Move existing "Console" interfaces away from "setFixed(...)" on "TwoColumnView"
Summary: Depends on D20931. Ref T13362. Move all "Console"-style interfaces to use a consistent layout based on a new "LauncherView" which just centers the content.

Test Plan: Viewed all affected interfaces.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13362

Differential Revision: https://secure.phabricator.com/D20933
2020-02-04 06:52:23 -08:00
epriestley
de66a8ece1 Remove "stronger/weaker" policy color hints from object headers
Summary:
Fixes T13461. Some applications provide hints about policy strength in the header, but these hints are inconsistent and somewhat confusing. They don't make much sense for modern objects with Custom Forms, which don't have a single "default" policy.

Remove this feature since it seems to be confusing things more than illuminating them.

Test Plan:
  - Viewed various objects, no longer saw colored policy hints.
  - Grepped for all removed symbols.

Maniphest Tasks: T13461

Differential Revision: https://secure.phabricator.com/D20918
2019-11-18 22:05:26 -08:00
epriestley
b83b3224bb Add an "Advanced/Developer..." action item for viewing object handle details and hovercards
Summary:
Ref T13442. Ref T13157. There's a secret URI to look at an object's hovercard in a standalone view, but it's hard to remember and impossible to discover.

In developer mode, add an action to "View Hovercard". Also add "View Handle", which primarily shows the object PHID.

Test Plan: Viewed some objects, saw "Advanced/Developer...". Used "View Hovercard" to view hovercards and "View Handle" to view handles.

Maniphest Tasks: T13442, T13157

Differential Revision: https://secure.phabricator.com/D20887
2019-11-08 16:47:05 -08:00
epriestley
338b4cb2e7 Prevent workboard cards from being grabbed by the "Txxx" object name text
Summary:
Fixes T13452. We currently give users mixed signals about the interaction mode of this text: the cursor says "text" but the behavior is "grab".

Make the behavior "text" to align with the cursor. An alternate variation of this change is to remove the cursor, but this is preferable if it doesn't cause problems, since copying the task ID is at least somewhat useful.

Test Plan: In Safari, Firefox, and Chrome: selected and copied object names from workboard cards; and dragged workboard cards by other parts of their UI.

Maniphest Tasks: T13452

Differential Revision: https://secure.phabricator.com/D20898
2019-11-08 08:29:53 -08:00
epriestley
e46e383bf2 Clean up "Revisions/Commits" table in Maniphest slightly
Summary: Ref T13440. Give the table more obvious visual structure and get rid of the largely useless header columns.

Test Plan: Viewed table, saw a slightly cleaner result.

Maniphest Tasks: T13440

Differential Revision: https://secure.phabricator.com/D20885
2019-10-31 12:29:53 -07:00
epriestley
c48f300eb1 Add support for rendering section dividers in tables; use section dividers for changes on tasks
Summary: Depends on D20883. Ref T13440. In most cases, all changes belong to the same repository, which makes the "Repository" column redundant and visually noisy. Show repository information in a section header.

Test Plan: {F6989932}

Maniphest Tasks: T13440

Differential Revision: https://secure.phabricator.com/D20884
2019-10-31 12:13:25 -07:00
epriestley
5dafabd5b4 Fix deprecated argument order for "implode()"
Summary: Fixes T13428. In modern PHP, "implode()" should take the glue parameter first.

Test Plan:
Used the linter introduced in D20857 to identify affected callsites.

```
$ git grep -i implode | cut -d: -f1 | sort | uniq | xargs arc lint --output summary --never-apply-patches | grep -i glue
```

Maniphest Tasks: T13428

Differential Revision: https://secure.phabricator.com/D20858
2019-10-17 09:11:27 -07:00
epriestley
278092974f Don't offer personal saved queries in global "Search Scope" settings dropdown
Summary: Fixes T13405. We currently offer non-global custom saved queries here, but this doesn't make sense as a global default setting.

Test Plan: Saved a global search query, edited global search settings, no longer saw the non-global query as an option.

Maniphest Tasks: T13405

Differential Revision: https://secure.phabricator.com/D20793
2019-09-09 12:21:25 -07:00
epriestley
63c7302af1 Fix global search scope fatal on 404 page (or other pages with no Application)
Summary: Ref T13405. Some pages don't have a contextual application.

Test Plan: Viewed 404 page, no more fatal.

Maniphest Tasks: T13405

Differential Revision: https://secure.phabricator.com/D20792
2019-09-09 12:18:26 -07:00
epriestley
caccbb69d2 When users try to log out with no providers configured, warn them of the consequences
Summary: Fixes T13406. On the logout screen, test for no configured providers and warn users they may be getting into more trouble than they expect.

Test Plan:
  - Logged out of a normal install and a fresh (unconfigured) install.

{F6847659}

Maniphest Tasks: T13406

Differential Revision: https://secure.phabricator.com/D20789
2019-09-08 12:27:29 -07:00
epriestley
7e2bec9280 Add a global setting for controlling the default main menu search scope
Summary: Fixes T13405. The default behavior of the global search bar isn't currently configurable, but can be made configurable fairly easily.

Test Plan: Changed setting as an administrator, saw setting reflected as a user with no previous preference. As a user with an existing preference, saw preference retained.

Maniphest Tasks: T13405

Differential Revision: https://secure.phabricator.com/D20787
2019-09-06 08:39:28 -07:00
epriestley
0cc7e8eeb8 Update Phortune payment account interfaces to handle merchant vs customer views
Summary: Depends on D20716. Ref T13366. This implements the new policy behavior cleanly in all top-level Phortune payment account interfaces.

Test Plan: As a merchant with an account relationship (not an account member) and an account member, browsed all account interfaces and attempted to perform edits. As a merchant, saw a reduced-strength view.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20717
2019-08-22 21:02:41 -07:00
epriestley
1fe6311167 Modernize user and repository "delete" workflows and improve documentation
Summary:
Fixes T8830. Fixes T13364.

  - The inability to destroy objects from the web UI is intentional. Make this clear in the messaging, which is somewhat out of date and partly reflects an earlier era when things could be destroyed.
  - `bin/remove destroy` can't rewind time. Document expectations around the "put the cat back in the bag" use case.

Test Plan: Read documentation, clicked through both workflows.

Maniphest Tasks: T13364, T8830

Differential Revision: https://secure.phabricator.com/D20694
2019-08-02 09:30:50 -07:00
epriestley
aacc62463d Prevent editing and deleting comments in locked conversations
Summary:
Ref T13289. This tightens up a couple of corner cases around locked threads.

Locking is primarily motivated by two use cases: stopping nonproductive conversations on open source installs (similar to GitHub's feature); and freezing object state for audit/record-keeping purposes.

Currently, you can edit or remove comments on a locked thread, but neither use case is well-served by allowing this. Require "CAN_INTERACT" to edit or remove a comment.

Administrators can still remove comments from a locked thread to serve "lock a flamewar, then clean it up", since "Remove Comment" on a comment you don't own is fairly unambiguously an administrative action.

Test Plan:
  - On a locked task, tried to edit and remove my comments as a non-administrator. Saw appropriate disabled UI state and error dialogs (actions were disallowed).
  - On a locked task, tried to remove another user's comments as an administrator. This works.
  - On a normal task, edited comments normally.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13289

Differential Revision: https://secure.phabricator.com/D20551
2019-05-23 19:04:55 -07:00
epriestley
0fab41ff3c Show "hold reasons" on commit page, not on "Edit" page
Summary:
Depends on D20465. Ref T13277. Currently, when a commit is unpublished, we put a single line about it on the "Edit Commit" page. This is pretty much impossible to find.

Move it to the main page. This treatment is more big/bold than I'd probably like to end up, but we should probably overshoot on the explanatory text until users get used to this behavior.

Also, allow searching for only published / unpublished commits.

Test Plan: {F6395705}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13277

Differential Revision: https://secure.phabricator.com/D20466
2019-04-25 09:22:49 -07:00
epriestley
b8551bb5f9 Reduce drag-and-drop jank on dashboards
Summary:
Depends on D20414. Ref T13272. Several minor things here:

  - Currently, you can drag panels underneath the invisible "there are no items in this column" div and the "Create Panel / Add Existing Panel" buttons. This is silly; stop it.
  - Currently, when viewing a tab panel on a dashboard, you can drag the panels inside it. This is extremely silly. Make "movable" off by default and pass it through the async flow only when we actually need it.
  - Make the whole "Add Tab..." virtual tab clickable to open the dropdown. This removes the rare exception/todo combo I added earlier. {key F}
  - Add or remove some icons or something.

Test Plan: Moved panels around on dashboards. Tried to drag panels inside tab panels. Added tab. Things were less obviously broken.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

Differential Revision: https://secure.phabricator.com/D20415
2019-04-17 12:20:44 -07:00
epriestley
f13709b13b Update search indexes for Dashboards and Panels to Ferret, plus various minor fixes
Summary:
Depends on D20410. Ref T13272. Dashboards/Panels currently use older "ngram" indexing, which is a less-powerful precursor to Ferret. Throw away the ngram index and provide a Ferret index instead. Also:

  - Remove the NUX state, which links to the wrong place now and doesn't seem terribly important.
  - Add project tags to the search result list.
  - Make the "No Tags" tag a little less conspicious.

Test Plan:
  - Indexed dashboards and panels.
  - Searched for dashboards and panels via SearchEngine using Ferret "query" field.
  - Searched for panels via "Add Existing Panel" datasource typeahead.
  - Searched for dashboards via "Add Menu Item > Dashboard" on a ProfileMenu via typeahead.
  - Viewed dashboard NUX state (no special state, but no more bad link to "/create/").
  - Viewed dashboard list, saw project tags.
  - Viewed dashboards with no project tags ("No Tags" is now displayed but less visible).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13272

Differential Revision: https://secure.phabricator.com/D20411
2019-04-14 10:28:19 -07:00
epriestley
9ad9ac9be6 On Dashboard tab panels in edit mode, make the "Tab Name" and the "Dropdown Edit Caret" into different links
Summary:
Ref T13272. In edit mode, tab panels now have a dropdown menu. However, this sort of overrlaps with the actual action of clicking the tab to select it.

Separate these into different click targets so that "select tab X" and "open dropdown menu for X" are different operations.

This is more work than it appears because:

  - We have an "action icon" already, used when you put a dashboard on a portal/home to create an "Edit" link. It makes sense to attach dropdowns to this, but it has some hard-coded stuff.
  - In applications with a "Create <thing>" in the crumbs (like Maniphest), we may use a dropdown menu if there are multiple create forms available. However, this menu renders in a weird way by reading all the properties out of an actual "View" object and building something else.
  - The "list of tabs" stuff shares code with different "list of tabs" navigation used by Diffusion and Instances.

..but I think I fixed everything and didn't break anything.

Test Plan:
  - Clicked "select tab" and "open dropdown menu" as separate actions.
  - Viewed Diffusion, Maniphest with multiple create forms, Instances.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

Differential Revision: https://secure.phabricator.com/D20396
2019-04-12 06:08:32 -07:00
epriestley
a35fda2019 Rebuild Dashboards on EditEngine: v1 Major Jank Edition
Summary:
Depends on D20383. Ref T13272. Fixes T12363. See PHI997. This gets the edit flows for tab panels functional again. They aren't //nice//, and a lot of the workflows are fairly janky: for example, most of them end up with you on the tab panel's page, which isn't useful if you started on a dashboard page.

However, these flows were extremely janky before anyway (see T12363) and I suspect this is a net improvement even though it's a bit of a mess. I anticipate cleaning this up bit-by-bit in future diffs.

Test Plan: {F6366372}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272, T12363

Differential Revision: https://secure.phabricator.com/D20384
2019-04-10 08:59:32 -07:00
epriestley
12b9224387 Make the "Install Dashboard" flow smoother
Summary:
Depends on D20362. Ref T13272. Currently, Dashboards have an "Install Dashboard" flow which is pretty janky and only allows you to install things to the home page.

Instead, allow users to install things to any valid target (home, favorites, portals, projects). This also provides URIs like `dashboard/install/1/home/personal/` which allow you to link users to an "install a dashboard" page; this may or may not get used.

Test Plan: Installed dashboards on home, favorites, projects, and portals.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

Differential Revision: https://secure.phabricator.com/D20364
2019-04-09 13:34:09 -07:00
epriestley
248d79f36d Fix "Actions" button on Phame standalone/live pages (bonus: JX.sprintf())
Summary:
See <https://discourse.phabricator-community.org/t/non-functional-actions-menu-on-live-phame-views/2593>. Several layers here:

The "Actions" button is broken because a menu behavior is failing, since we aren't rendering the menu.

When a behavior fails to initialize, catch and log the exception and continue. Previously, we stopped initializing behaviors if any failed, but behaviors are usually independent and continuing with an explicit exception seems reasonable.

Give "JX.log()" some "sprintf()" semantics to make logging the behavior failure easier. We can probably afford these extra 200 bytes now in 2019.

This fixes the button and gives us explicit errors in the log. So far, so good.

Then, when a page won't render chrome, don't try to render the main menu. This fixes the actual errors (we no longer try to initialize menu behaviors for nodes which don't exist).

Completely hide the "Actions" and "Comment" flows if the viewer isn't logged in. Although this isn't completely consistent with other applications, I think it's more appropriate for Phame. In applications like Maniphest, we show a full set of controls (but disable them) so that users who are not currently logged in have a clear path to interact with the content, under the assumption that this is a relatively common workflow. This is probably less common for Phame, where we expect most anonymous viewers not to log in or interact.

Finally, parametrize a one-off border color and add a border under the crumbs at the top of the page.

Test Plan:
  - Viewed a "Live" Phame blog post page, clicked "Actions", got a dropdown.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20378
2019-04-04 06:10:14 -07:00
epriestley
18732a0d2f Make Portals reachable without knowing the URI
Summary:
Depends on D20360. Ref T13275. This makes the "Dashboards" application start on a Drydock-like console page where you pick portals, dashboards, or panels.

Probably the "Dashboards" application should either be renamed to "IntelliknowledgePro" or Portals should be split off into a separate application eventually, but let's see how things go like this for now, since restructuring probably breaks some URIs at least a little bit so I'd like more confidence that we're headed in the right direction before we do it.

Test Plan:
  - Visited Dashboards via typeahead, got options for Dashboards/Portals/Panels.
  - Visited Portals pages, got simplified crumbs.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13275

Differential Revision: https://secure.phabricator.com/D20361
2019-04-02 15:23:36 -07:00
epriestley
971a272bf6 Automatically build mobile menus from navigation, and clean up external ProfileMenu API
Summary:
Depends on D20355. Ref T13275. Ref T13247. Currently, "Hamburger" menus are not automatically built from navigation menus. However, this is (I'm almost completely sure?) a reasonable and appropriate default behavior, and saves us some code around profile menus.

With this rule in place, we can remove `setApplicationMenu()` and `getApplicationMenu()` from `StandardPageView`, since they have no callers.

This also updates a lot of profile menu callsites to a new API which is added in the next change.

Test Plan: See the next two changes.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13275, T13247

Differential Revision: https://secure.phabricator.com/D20356
2019-04-02 15:17:44 -07:00
epriestley
47bf382435 Allow profile menu items to be locked to the top or bottom of the menu
Summary:
Depends on D20353. Ref T13275. This is just some small quality-of-life fixes:

  - When you add items to menus, they currently go below the "Edit Menu/Manage Menu" links by default. This isn't a very good place for them. Instead, lock "edit" items to the bottom of the menu.
  - Lock profile pictures to the top of the menu. This just simplifies things a little.
  - Show more iconography hints on the "edit menu items" UI.
  - Add a "drag stuff to do things" hint if some stuff can be dragged.

Test Plan:
  - Added new items to a Portal, they didn't go to the very bottom. Instead, they went above the "Edit/Manage" links; a sensible place for them.
  - Viewed the "edit menu items" screen, saw more hints and visual richness.
  - Viewed/edited Home, Projects, Portals, Favorites

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13275

Differential Revision: https://secure.phabricator.com/D20355
2019-04-02 15:08:20 -07:00
epriestley
c53ed72e4c Provide a clearer UI for "view all results" in partial result panels
Summary:
In some cases, we show a limited number of one type of object somewhere else, like "Recent Such-And-Such" or "Herald Rules Which Use This" or whatever.

We don't do a very good job of communicating that these are partial lists, or how to see all the results. Usually there's a button in the upper right, which is fine, but this could be better.

Add an explicit "more stuff" button that shows up where a pager would appear and makes it clear that (a) the list is partial; and (b) you can click the button to see everything.

Test Plan: {F6302793}

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D20315
2019-03-25 14:35:08 -07:00
epriestley
00543f0620 Remove the ability to drag tasks up and down on (non-Workboard) priority list views
Summary:
Ref T13074. Today, in normal task list views in Maniphest (not workboards), you can (sometimes) reorder tasks if the view is priority-sorted.

I suspect no one ever does this, few users know it's supported, and that it was basically rendered obsolete the day we shipped workboards.

This also means that we need to maintain a global "subpriority" for tasks, which distinguishes between different tasks at the same priority level (e.g., "High") and maintains a consistent ordering on workboards.

As we move toward making workboards more flexible (e.g., group by author / owner / custom fields), I'd like to try moving away from "subpriority" and possibly removing it entirely, in favor of "natural order", which basically means "we kind of remember where you put the card and it works a bit like a sticky note".

Currently, the "natural order" and "subpriority" systems are sort of similar but also sort of in conflict, and the "subpriority" system can't really be extended while the "natural order / column position" system can.

The only real reason to have a global "subpriority" is to support the list-view drag-and-drop.

It's possible I'm wrong about this and a bunch of users love this feature, but we can re-evaluate if we get feedback in this vein.

(This just removes UI, the actual subpriority system is still intact and still used on workboards.)

Test Plan: Viewed task lists, was no longer able to drag stuff. Grepped for affected symbols. Dragged stuff in remaining grippable lists, like "Edit Forms" in EditEngine config.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13074

Differential Revision: https://secure.phabricator.com/D20263
2019-03-12 12:47:36 -07:00
epriestley
aa470d2154 Show user availability dots (red = away, orange = busy) in typeaheads, tokenizer tokens, and autocompletes
Summary:
Ref T13249. See PHI810. We currently show availability dots in some interfaces (timeline, mentions) but not others (typeheads/tokenizers).

They're potentially quite useful in tokenizers, e.g. when assigning tasks to someone or requesting reviews. Show them in more places.

(The actual rendering here isn't terribly clean, and it would be great to try to unify all these various behaviors some day.)

Test Plan:
{F6212044}

{F6212045}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13249

Differential Revision: https://secure.phabricator.com/D20173
2019-02-19 10:57:20 -08:00
epriestley
dbcf41dbea Fix a couple more "URI->alter()" callsites in paging code
Summary: `grep` had a hard time finding these.

Test Plan: Will just hotfix this since I'm still reasonably in the deploy window, this currently fatals: <https://secure.phabricator.com/search/query/_dgatshiRBSy/#R>

Reviewers: amckinley

Differential Revision: https://secure.phabricator.com/D20186
2019-02-16 07:28:35 -08:00
epriestley
2ca316d652 When users confirm Duo MFA in the mobile app, live-update the UI
Summary: Ref T13249. Poll for Duo updates in the background so we can automatically update the UI when the user clicks the mobile phone app button.

Test Plan: Hit a Duo gate, clicked "Approve" in the mobile app, saw the UI update immediately.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13249

Differential Revision: https://secure.phabricator.com/D20169
2019-02-15 14:38:15 -08:00
epriestley
b09cf166a8 Clean up a couple more URI alter() calls
Summary:
See <https://discourse.phabricator-community.org/t/create-new-phriction-document-fails-with-unhandled-exception-invalidargumentexception/2406>.

These weren't obviously nullable from a cursory `grep`, but are sometimes nullable in practice.

Test Plan: Created, then saved a new Phriction document.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20184
2019-02-15 14:07:17 -08:00
epriestley
c5772f51de Fix Content-Security-Policy headers on "Email Login" page
Summary:
In D20100, I changed this page from returning a `newPage()` with a dialog as its content to returning a more modern `newDialog()`.

However, the magic to add stuff to the CSP header is actually only on the `newPage()` pathway today, so this accidentally dropped the extra "Content-Security-Policy" rule for Google.

Lift the magic up one level so both Dialog and Page responses hit it.

Test Plan:
  - Configured Recaptcha.
  - Between D20100 and this patch: got a CSP error on the Email Login page.
  - After this patch: clicked all the pictures of cars / store fronts.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20163
2019-02-14 12:53:33 -08:00
epriestley
be21dd3b52 Fix some "URI->alter(X, null)" callsites
Summary:
Ref T13250. This internally calls `replaceQueryParam(X, null)` now, which fatals if the second parameter is `null`. I hit these legitimately, but I'll look for more callsites and follow up by either allowing this, removing `alter()`, fixing the callsites, or some combination.

(I'm not much of a fan of `alter()`.)

Test Plan: Browsing a paginated list no longer complains about URI construction.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13250

Differential Revision: https://secure.phabricator.com/D20162
2019-02-14 11:59:07 -08:00
epriestley
5892c78986 Replace all "setQueryParam()" calls with "remove/replaceQueryParam()"
Summary: Ref T13250. See D20149. Mostly: clarify semantics. Partly: remove magic "null" behavior.

Test Plan: Poked around, but mostly just inspection since these are pretty much one-for-one.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim

Maniphest Tasks: T13250

Differential Revision: https://secure.phabricator.com/D20154
2019-02-14 11:56:39 -08:00
epriestley
88d5233b77 Fix specifications of some "Visual Only" elements
Summary: See PHI823. These got "visual-only" but should acutally get "aural => false" to pick up "aria-hidden".

Test Plan: Viewed page source, saw both "visual-only" and "aria-hidden".

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20157
2019-02-13 12:26:28 -08:00
epriestley
187356fea5 Let the top-level exception handler dump a stack trace if we reach debug mode before things go sideways
Summary:
Depends on D20140. Ref T13250. Currently, the top-level exception handler doesn't dump stacks because we might not be in debug mode, and we might double-extra-super fatal if we call `PhabricatorEnv:...` to try to figure out if we're in debug mode or not.

We can get around this by setting a flag on the Sink once we're able to confirm that we're in debug mode. Then it's okay for the top-level error handler to show traces.

There's still some small possibility that showing a trace could make us double-super-fatal since we have to call a little more code, but AphrontStackTraceView is pretty conservative about what it does and 99% of the time this is a huge improvement.

Test Plan: {F6205122}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13250

Differential Revision: https://secure.phabricator.com/D20142
2019-02-11 15:36:19 -08:00
epriestley
a20f108034 When an edit overrides an object lock, note it in the transaction record
Summary:
Ref T13244. See PHI1059. When you lock a task, users who can edit the task can currently override the lock by using "Edit Task" if they confirm that they want to do this.

Mark these edits with an emblem, similar to the "MFA" and "Silent" emblems, so it's clear that they may have bent the rules.

Also, make the "MFA" and "Silent" emblems more easily visible.

Test Plan:
Edited a locked task, overrode the lock, got marked for it.

{F6195005}

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: aeiser

Maniphest Tasks: T13244

Differential Revision: https://secure.phabricator.com/D20131
2019-02-09 06:10:07 -08:00
epriestley
7469075a83 Allow users to be approved from the profile "Manage" page, alongside other similar actions
Summary:
Depends on D20122. Fixes T8029. Adds an "Approve User" action to the "Manage" page.

Users are normally approved from the "Approval Queue", but if you click into a user's profile to check them out in more detail it kind of dead ends you right now. I've occasionally hit this myself, and think this workflow is generally reasonable enough to support upstream.

Test Plan: {F6193742}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T8029

Differential Revision: https://secure.phabricator.com/D20123
2019-02-07 15:04:23 -08:00
epriestley
949afb02fd On login forms, autofocus the "username" field
Summary: Depends on D20120. Fixes T8907. I thought this needed some Javascript nonsense but Safari, Firefox and Chrome all support an `autofocus` attribute.

Test Plan: Loaded login page with password auth enabled in Safari, Firefox, and Chrome; saw username field automatically gain focus.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T8907

Differential Revision: https://secure.phabricator.com/D20122
2019-02-07 15:03:43 -08:00
Austin McKinley
f2236eb061 Autofocus form control for adding TOTP codes
Summary: Ref D20122. This is something I wanted in a bunch of places. Looks like at some point the most-annoying one (autofocus for entering TOTOP codes) already got fixed at some point.

Test Plan: Loaded the form, got autofocus as expected.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D20128
2019-02-07 11:56:49 -08:00
epriestley
c9ff6ce390 Add CSRF to SMS challenges, and pave the way for more MFA types (including Duo)
Summary:
Depends on D20026. Ref T13222. Ref T13231. The primary change here is that we'll no longer send you an SMS if you hit an MFA gate without CSRF tokens.

Then there's a lot of support for genralizing into Duo (and other push factors, potentially), I'll annotate things inline.

Test Plan: Implemented Duo, elsewhere.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13231, T13222

Differential Revision: https://secure.phabricator.com/D20028
2019-01-24 15:10:57 -08:00
epriestley
1729e7b467 Improve UI for "wait" and "answered" MFA challenges
Summary:
Depends on D19906. Ref T13222. This isn't going to win any design awards, but make the "wait" and "answered" elements a little more clear.

Ideally, the icon parts could be animated Google Authenticator-style timers (but I think we'd need to draw them in a `<canvas />` unless there's some clever trick that I don't know) or maybe we could just have the background be like a "water level" that empties out. Not sure I'm going to actually write the JS for either of those, but the UI at least looks a little more intentional.

Test Plan:
{F6070914}

{F6070915}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D19908
2018-12-28 00:18:53 -08:00
epriestley
543f2b6bf1 Allow any transaction group to be signed with a one-shot "Sign With MFA" action
Summary:
Depends on D19896. Ref T13222. See PHI873. Add a core "Sign With MFA" transaction type which prompts you for MFA and marks your transactions as MFA'd.

This is a one-shot gate and does not keep you in MFA.

Test Plan:
  - Used "Sign with MFA", got prompted for MFA, answered MFA, saw transactions apply with MFA metadata and markers.
  - Tried to sign alone, got appropriate errors.
  - Tried to sign no-op changes, got appropriate errors.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222

Differential Revision: https://secure.phabricator.com/D19897
2018-12-28 00:09:30 -08:00