1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-24 15:52:41 +01:00
Commit graph

16429 commits

Author SHA1 Message Date
epriestley
a542024b63 Update Phortune subscriptions for modern infrastructure
Summary:
Depends on D20720. Ref T13366.

  - Use modern policies and policy interfaces.
  - Use new merchant authority cache.
  - Add (some) transactions.
  - Move MFA from pre-upgrade-gate to post-one-shot-check.
  - Simplify the autopay workflow.
  - Use the "reloading arrows" icon for subscriptions more consistently.

Test Plan: As a merchant-authority and account-authority, viewed, edited, and changed autopay for subscriptions.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20721
2019-08-22 21:07:17 -07:00
epriestley
26ec924732 When a page throws an exception and response construction throws another exception, throw an aggregate exception
Summary:
Depends on D20719. Currently, if a page throws an exception (like a policy exception) and rendering that exception into a response (like a policy dialog) throws another exception (for example, while constructing breadcrumbs), we only show the orginal exception.

This is usually the more useful exception, but sometimes we actually care about the other exception.

Instead of guessing which one is more likely to be useful, throw them both as an "AggregateException" and let the high-level handler flatten it for display.

Test Plan: {F6749312}

Differential Revision: https://secure.phabricator.com/D20720
2019-08-22 21:04:36 -07:00
epriestley
201634848e Make Phortune payment methods transaction-oriented and always support "Add Payment Method"
Summary:
Depends on D20718. Ref T13366. Ref T13367.

  - Phortune payment methods currently do not use transactions; update them.
  - Give them a proper view page with a transaction log.
  - Add an "Add Payment Method" button which always works.
  - Show which subscriptions a payment method is associated with.
  - Get rid of the "Active" status indicator since we now treat "disabled" as "removed", to align with user expectation/intent.
  - Swap out of some of the super weird div-form-button UI into the new "big, clickable" UI for choice dialogs among a small number of options on a single dimension.

Test Plan:
  - As a mechant-authority and account-authority, created payment methods from carts, subscriptions, and accounts. Edited and viewed payment methods.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13367, T13366

Differential Revision: https://secure.phabricator.com/D20719
2019-08-22 21:04:04 -07:00
epriestley
c4e0ac4d27 Update PhortunePaymentMethod for modern policy interfaces
Summary:
Depends on D20717. Ref T13366. Make PhortunePaymentMethod use an extended policy interface for consistency with modern approaches. Since Accounts have hard-coded policy behavior (and can't have object policies like "Subscribers") this should have no actual impact on program behavior.

This leaves one weird piece in the policy dialog UIs, see T13381.

Test Plan: Viewed and edited payment methods as a merchant and account member. Merchants can only view, not edit.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20718
2019-08-22 21:03:16 -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
a3213ab20b In Phortune, use actual merchant authority (not authority grants) to control account visibility
Summary:
Depends on D20715. Ref T13366. See that task for discussion.

Replace the unreliable "grantAuthority()"-based check with an actual "can the viewer edit any merchant this account has a relationship with?" check.

This makes these objects easier to use from a policy perspective and makes it so that the `Query` alone can fully enforce permissions properly with no setup, so general infrastructure (like handles and transactions) works properly with Phortune objects.

Test Plan: Viewed merchants and accounts as users with no authority, direct authority on the account, and indirect authority via a merchant relationship.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20716
2019-08-22 21:01:55 -07:00
epriestley
277bce5638 In Phortune, write relationships between payment accounts and merchants they interact with
Summary:
Depends on D20713. Ref T13366. When a payment account establishes a relationship with a merchant by creating a cart or subscription, create an edge to give the merchant access to view the payment account.

Also, migrate all existing subscriptions and carts to write these edges.

This aims at straightening out Phortune permissions, which are currently a bit wonky on a couple of dimensions. See T13366 for detailed discussion.

Test Plan:
  - Created and edited carts/subscriptions, saw edges write.
  - Ran migrations, saw edges write.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20715
2019-08-22 21:01:04 -07:00
epriestley
e3ba53078e Add scaffolding for ad-hoc email addresses associated with Phortune accounts
Summary: Depends on D20697. Ref T8389. Add support for adding "billing@enterprise.com" and similar to Phortune accounts.

Test Plan: Added and edited email addresses for a payment account.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T8389

Differential Revision: https://secure.phabricator.com/D20713
2019-08-22 20:57:35 -07:00
epriestley
719a7d82c5 Refactor the Phortune account detail page into a series of smaller, more focused sections
Summary:
Ref T13366. Some of the information architecture is a little muddy here, notably an item called "Billing / History" which contains payment methods.

Split things up a bit to prepare for adding support for "Email Addresses".

Test Plan: {F6676988}

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20697
2019-08-22 20:56:43 -07:00
epriestley
ecbc82da33 Expose "commits.add|set|remove" on "maniphest.edit" API calls
Summary: See PHI1396. Ideally this would be some kind of general-purpose tie-in to object relationships, but see D18456 for precedent.

Test Plan: Used `maniphest.edit` to edit associated commits for a task.

Differential Revision: https://secure.phabricator.com/D20731
2019-08-22 13:34:33 -07:00
epriestley
353155a203 Add "modifiedStart" and "modifiedEnd" constraints to "differential.revision.search"
Summary:
Fixes T13386. See PHI1391. These constraints largely exist already, but are not yet exposed to Conduit.

Also, tweak some keys to support the underlying query.

Test Plan: Ran `differential.revision.search` queries with the new constraints.

Maniphest Tasks: T13386

Differential Revision: https://secure.phabricator.com/D20730
2019-08-22 13:34:18 -07:00
epriestley
109d7dcaf1 Convert "Empower" from state-based MFA to one-shot MFA
Summary: Ref T13382. Currently, the "Make Administrator" action in the web UI does state-based MFA. Convert it to one-shot MFA.

Test Plan: Empowered and unempowered a user from the web UI, got one-shot MFA'd. Empowered a user from the CLI, no MFA issues.

Maniphest Tasks: T13382

Differential Revision: https://secure.phabricator.com/D20729
2019-08-22 08:34:46 -07:00
epriestley
f1b054a20f Correct the interaction between overheating and offset-based paging
Summary:
Ref T13386. If you issue `differential.query` with a large offset (like 3000), it can overheat regardless of policy filtering and fail with a nonsensical error message.

This is because the overheating limit is based only on the query limit, not on the offset.

For example, querying for "limit = 100" will never examine more than 1,100 rows, so a query with "limit = 100, offset = 3000" will always fail (provided there are at least that many revisions).

Not all numbers work like you might expect them to becuase there's also a 1024-row fetch window, but basically small limits plus big offsets always fail.

Test Plan: Artificially reduced the internal window size from 1024 to 5, then ran `differential.query` with `offset=50` and `limit=3`. Before: overheated with weird error message. After: clean result.

Maniphest Tasks: T13386

Differential Revision: https://secure.phabricator.com/D20728
2019-08-21 20:27:35 -07:00
epriestley
5741514aeb When a client submits an overlong "sourcePath", truncate it and continue
Summary:
Ref T13385. Currently, if you run `arc diff` in a CWD with more than 255 characters, the workflow fatals against the length of the `sourcePath` database column.

In the long term, removing this property is likely desirable.

For now, truncate long values and continue. This only meaningfully impacts relatively obscure interactive SVN workflows negatively, and even there, "some arc commands are glitchy in very long working directories in SVN" is still better than "arc diff fatals".

Test Plan:
  - Modified `arc` to submit very long source paths.
  - Ran `arc diff`.
    - Before: Fatal when inserting >255 characters into `sourcePath`.
    - After: Path truncated at 255 bytes.

Maniphest Tasks: T13385

Differential Revision: https://secure.phabricator.com/D20727
2019-08-21 19:28:18 -07:00
epriestley
c439931373 Respect "disabled" custom field status granted by "subtype" configuration in form validation
Summary:
Fixes T13384. Currently, the subtype "disabled" configuration is not respected when selecting fields for `ROLE_EDIT`.

The only meaningful caller for `ROLE_EDIT` is transaction validation, but transaction validation should respect fields being disabled by subtype configuration.

Test Plan:
  - Added a "required" Maniphest custom field "F", then "disabled" it in a subtype "S".
  - Created a task of subtype "S".
    - Before: Form submission fails with error "F is required", even though the field is not actually visible on the form and can not be set.
    - After: Form submits cleanly and creates the task.

Maniphest Tasks: T13384

Differential Revision: https://secure.phabricator.com/D20726
2019-08-21 18:03:20 -07:00
epriestley
64b399d9be Remove "bin/accountadmin" and "scripts/user/add_user.php"
Summary:
Fixes T13382. Depends on D20724. These ancient scripts are no longer necessary since we've had a smooth web-based onboarding process for a long time.

I retained `bin/user empower` and `bin/user enable` for recovering from situations where you accidentally delete or disable all administrators. This is normally difficult, but some users are industrious.

Test Plan: Grepped for `accountadmin` and `add_user.php`, found no more hits.

Maniphest Tasks: T13382

Differential Revision: https://secure.phabricator.com/D20725
2019-08-20 17:58:20 -07:00
epriestley
fc34554892 Replace "bin/people profileimage" with "bin/user enable|empower"
Summary:
Ref T13382.

  - Remove "bin/people profileimage" which previously generated profile image caches but now feels obsolete.
  - Replace it with "bin/user", with "enable" and "empower" flows. This command is now focused on regaining access to an install after you lock your keys inside.
  - Document the various ways to unlock objects and accounts from the CLI.

Test Plan:
  - Ran `bin/user enable` and `bin/user empower` with various flags.
  - Grepped for `people profileimage` and found no references.
  - Grepped for `bin/people` and found no references.
  - Read documentation.

Maniphest Tasks: T13382

Differential Revision: https://secure.phabricator.com/D20724
2019-08-20 17:51:14 -07:00
epriestley
721a86401f Implement "drydock.resource.search"
Summary: Fixes T13383. Provide a basic "drydock.resource.search". Also allow "drydock.lease.search" to be queried by resource PHID.

Test Plan: Called "drydock.resource.search" and "drydock.lease.search" with various constraints.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13383

Differential Revision: https://secure.phabricator.com/D20723
2019-08-20 13:07:07 -07:00
epriestley
803eb29c71 Fix flag typo in "Managing Caches" documentation
Summary: See PHI1392. This flag is `--all`, not `--all-caches`.

Test Plan: Ran `bin/cache purge --all`.

Differential Revision: https://secure.phabricator.com/D20722
2019-08-20 12:36:22 -07:00
epriestley
d890c03ac3 Namespace all column references in ProjectQuery to fix ambiguity with Ferret constraints
Summary:
Fixes T13378. If we join Ferret tables and page, we can end up with an ambiguous `id` column here.

Explicitly refer to "project.x" in all cases that we're interacting with the project table.

Test Plan:
  - Changed page size to 3.
  - Issued a Projects query for "~e", matching more than 3 results.
  - Clicked "Next Page".
    - Before: ambiguous id column fatal.
    - After: next page.

Maniphest Tasks: T13378

Differential Revision: https://secure.phabricator.com/D20714
2019-08-15 12:21:54 -07:00
epriestley
82cf97ad65 When many commits are discovered at once, import them at lower priority
Summary:
Ref T13369. See that task for discussion.

When the discovery daemon finds more than 64 commits to import, demote the worker queue priority of the resulting tasks.

Test Plan:
  - Pushed one commit, ran `bin/repository discover --verbose --trace ...`, saw commit import with "at normal priority" message and priority 2500 ("PRIORITY_COMMIT").
  - Pushed 3 commits, set threshold to 3, ran `bin/repository discover ...`, saw commist import with "at lower priority" message and priority 4000 ("PRIORITY_IMPORT").

Maniphest Tasks: T13369

Differential Revision: https://secure.phabricator.com/D20712
2019-08-12 12:59:00 -07:00
epriestley
006cb659cb Make the success message from "bin/config" more clear
Summary:
Ref T13373. When you "bin/config set x ..." a value, the success message ("Set x ...") is somewhat ambiguous and can be interpreted as "First, you need to set x..." rather than "Success, wrote x...".

Make the messaging more explicit. Also make this string more translatable.

Test Plan: Ran `bin/config set ...` with various combinations of flags, saw more clear messaging.

Maniphest Tasks: T13373

Differential Revision: https://secure.phabricator.com/D20711
2019-08-12 12:50:03 -07:00
epriestley
c092492a53 Fix missing display cell in daemon summary table
Summary: Fixes T13374. The "Temporary Failures" row is missing a cell definiton from the addition of "Average Queue Time".

Test Plan: Viewed "/daemon/" with some temporary failures and and odd number of rows above the "Temporary Failures" row. Saw cell properly zebra-striped.

Maniphest Tasks: T13374

Differential Revision: https://secure.phabricator.com/D20710
2019-08-12 12:46:48 -07:00
epriestley
0a3c26998f When the feed query on project profile pages overheats, contain the damage
Summary:
Ref T13349. This is almost the same change as D20678, but for project profiles instead of user profiles.

The general reproduction case is "view a project where you can't see more than 50 of the 500 most recent feed stories".

Test Plan:
  - Forced all queries to overheat.
  - Viewed a project profile page.
  - Before: overheating fatal near top level.
  - After: damage contained to feed panel.

Maniphest Tasks: T13349

Differential Revision: https://secure.phabricator.com/D20704
2019-08-08 21:01:50 -07:00
epriestley
9bd74dfa6c Autofocus the "App Code" input on the TOTP prompt during MFA gates after login
Summary: See downstream <https://phabricator.wikimedia.org/T229757>. The "autofocus" attribute mostly just works, so add it to this input.

Test Plan: As a user with TOTP enabled, established a new session. Saw browser automatically focus the "App Code" input on the TOTP prompt screen.

Differential Revision: https://secure.phabricator.com/D20703
2019-08-08 12:54:22 -07:00
epriestley
46d9065bf1 Drop test for awardable badges on "Badges" tab of user profiles to avoid overheating
Summary:
Fixes T13370. We currently show an "Award Badge" button conditionally, based on whether the viewer can award any badges or not.

The query to test this may overheat and this pattern isn't consistent with other UI anyway. Stop doing this test.

Test Plan:
  - Created 12 badges.
  - As a user who could not edit any of the badges, viewed the "Badges" section of a user profile.

Maniphest Tasks: T13370

Differential Revision: https://secure.phabricator.com/D20702
2019-08-08 10:18:54 -07:00
epriestley
937edcdc58 Fix a warning in BoardLayoutEngine when no objects are being updated
Summary: Fixes T13368. Some workflows (like "Move tasks to...") execute board layout without objects to update. In these cases, we can hit a warning because `objectPHIDs` is not initialized to `array()`.

Test Plan: Went through the "Move tasks to..." workflow on a workboard, no longer saw a warning when trying to iterate over an empty `objectPHIDs` list.

Maniphest Tasks: T13368

Differential Revision: https://secure.phabricator.com/D20701
2019-08-08 10:17:59 -07:00
epriestley
0561043a1f In "Move task to..." workflow, separate visible and hidden columns in the dropdown
Summary:
Ref T13368. Currently, both visible and hidden columns are shown in the "Move tasks to..." dropdown on workflows from workboards.

When the dropdown contains hidden columns, move them to a separate section to make it clear that they're not likely targets.

Test Plan:
  - Used "Move tasks to project..." targeting a board with no hidden columns. Saw a single ungrouped dropdown.
  - Used "Move tasks to project..." targeting a board with hidden columns. Saw a dropdown grouped into "Visible" and "Hidden" columns.

Maniphest Tasks: T13368

Differential Revision: https://secure.phabricator.com/D20700
2019-08-07 09:23:53 -07:00
epriestley
6deac35659 Don't show proxy (subproject/milestone) columns as options in "Move tasks..." workflows from workboards
Summary:
Ref T13368. Proxy columns should not be selectable from this workflow. If you want to move tasks to milestone/subproject X, do "Move tasks to project..." and pick X as the project.

(This could be made to work some day.)

Test Plan: Went through a "Move tasks to project..." workflow targeting a project with subprojects. No longer saw subproject columns presented as dropdown options.

Maniphest Tasks: T13368

Differential Revision: https://secure.phabricator.com/D20699
2019-08-07 09:23:29 -07:00
epriestley
31254c5124 Correct column options presented in "Move tasks to project..." on workboards
Summary: Ref T13368. The column options presented to the user are currently incorrect because the wrong set of columns are drawn from.

Test Plan: On a workboard, used "Move tasks to project..." to target another board, saw that board's columns.

Maniphest Tasks: T13368

Differential Revision: https://secure.phabricator.com/D20698
2019-08-07 09:22:57 -07:00
epriestley
87f878ec8a Stop trying to CC merchants on invoices/receipts
Summary:
Fixes T13341. Currently, cart emails (invoices/receipts) are sent to members of the associated merchant account. This was just a simple way to keep an eye on things when this was first written.

The system works fine, and recent changes (almost certainly D20525) stopped these emails from working (presumably because of the slightly weird merchant permissions model).

This could be sorted out in more detail, but it looks like the path forward is to introduce a side channel for email anyway (via T8389), and that's a better way to implement this behavior since it means the normal recipients won't see a bunch of random staff/merchant email addresses on their receipts.

Test Plan: Grepped for `merchant` in this editor.

Maniphest Tasks: T13341

Differential Revision: https://secure.phabricator.com/D20696
2019-08-02 10:51:17 -07:00
epriestley
6c41508906 Fix an issue where lines with more than one pattern match highlighted improperly in Diffusion
Summary:
Ref T13339. If a search pattern matches more than once on a line, we currently render the line incorreclty, duplicating some of the text.

`substr()` is being called as though the third parameter was `end_offset`, but it's actually `length`. Correct the parameter.

Test Plan:
Before:

{F6676625}

After:

{F6676623}

Maniphest Tasks: T13339

Differential Revision: https://secure.phabricator.com/D20695
2019-08-02 09:44:59 -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
3069ef4166 Prevent object titles in the "Object Attacher" dialog from triggering Quicksand "Close Dialog on Navigation" behavior
Summary:
Fixes T13363. Currently, these are genuine links which we intercept events for.

Make them pseudolinks instead. Possible alternative approaches are:

  - Keep them as genuine links, but mark them as non-navigation links for Quicksand. (But: yuck, weird special case.)
  - Keep them as genuine links, and have the dialog handler `JX.Stratcom.pass()` to see if anything handles the event. (But: the "pass()" pattern generally feels bad.)

"Tableaus" or whatever comes out of T10469 some day will probably break everything anyway?

Test Plan:
  - Opened the "Edit Related Tasks... > Edit Subtasks" dialog.
  - Clicked task title links (not the "open in new window" icon, and not the "Select" button).
  - Before: Dialog (sometimes) closed abruptly.
  - After: Task is consistently selected as part of the attachment set.

Maniphest Tasks: T13363

Differential Revision: https://secure.phabricator.com/D20693
2019-08-01 12:25:28 -07:00
epriestley
f5c380bfc9 Add very basic support for generating PDF documents
Summary: Ref T13358. This is very minimal, but technically works. The eventual goal is to generate PDF invoices to make my life easier when I have to interact with Enterprise Vendor Procurement.

Test Plan: {F6672439}

Maniphest Tasks: T13358

Differential Revision: https://secure.phabricator.com/D20692
2019-08-01 10:50:24 -07:00
epriestley
b81c8380fb Document support for "limit" in tokenizer-based Custom Fields
Summary:
Fixes T13356. This option is supported and works fine, it just isn't documented.

Add documentation and fix the config option to actually link to it to make life a little easier.

Test Plan: Read documentation.

Maniphest Tasks: T13356

Differential Revision: https://secure.phabricator.com/D20691
2019-07-31 13:13:24 -07:00
epriestley
8e263a2f64 Support "date" custom fields in "*.edit" endpoints
Summary: Fixes T13355. This didn't appear to be a ton of extra work, we just didn't get it for free in the original implementation in D14635.

Test Plan:
  - Saw "date" custom fields appear in Conduit API documentation for "maniphest.edit".
  - Set custom "date" field to null and non-null values via the API.

{F6666582}

Maniphest Tasks: T13355

Differential Revision: https://secure.phabricator.com/D20690
2019-07-31 13:10:14 -07:00
epriestley
76cd181bf3 Don't try to emit project board update events if there are no projects to update
Summary: Ref T4900. We may execute a bad query here if the task has no projects at all.

Test Plan: Edited a task with no new or old projects. Instead of an exception, things worked.

Maniphest Tasks: T4900

Differential Revision: https://secure.phabricator.com/D20689
2019-07-31 12:48:41 -07:00
epriestley
47d497aa60 When users visit a Phame post URI with an old blog ID, canonicalize the URI instead of 404'ing
Summary:
Fixes T13353. If you:

  - Visit a blog post and save the URI.
  - Move the blog post to a different blog.
  - Revisit the old URI.

...we currently 404. We know what you're trying to do and should just redirect you to the new URI instead. We already do this if you visit a URI with a noncanonical slug.

Test Plan:
  - Created post A.
  - Copied the live URI.
  - Moved it to a different blog.
  - Visited the saved URI from the earlier step.
  - Before: 404.
  - After: Redirect to the canonical URI.

Maniphest Tasks: T13353

Differential Revision: https://secure.phabricator.com/D20688
2019-07-31 11:44:25 -07:00
epriestley
2ec39afcd1 Deprecate ancient "slowvote.info" API method
Summary: Depends on D20686. Fixes T13350. Now that "slowvote.poll.search" exists, deprecate this old method.

Test Plan: Reviewed method description in Condiut API console in the web UI.

Maniphest Tasks: T13350

Differential Revision: https://secure.phabricator.com/D20687
2019-07-31 11:28:08 -07:00
epriestley
f92480fb77 Fix two minor display issues with the Conduit "*.search" API documentation
Summary:
Depends on D20685. Ref T13350. Currently:

  - When a SearchEngine parameter is marked as hidden from Conduit, we may still render a table of possible values. Instead, only render the table if the parameter is actually usable.
  - The table header is hard-coded to say `'statuses'`, which is just a silly mistake. (Most commonly, this table does have `statuses` constants.)

Test Plan: Viewed the Conduit API documentation for the new "slowvote.poll.search" API method, saw more sensible display behavior.

Maniphest Tasks: T13350

Differential Revision: https://secure.phabricator.com/D20686
2019-07-31 11:27:05 -07:00
epriestley
0b0ab1bd7c Add a "slowvote.poll.search" API method
Summary: Ref T13350. Add a modern "*.search" API method for Slowvote so "slowvote.info" can be deprecated with a reasonable replacement.

Test Plan: Used Conduit test console to call method, saw reasonable results.

Maniphest Tasks: T13350

Differential Revision: https://secure.phabricator.com/D20685
2019-07-31 11:26:41 -07:00
epriestley
d81d0c3ea0 Fix an issue where editing cards on a workboard with implicit column ordering could reorder cards improperly
Summary:
Depends on D20680. Ref T4900. The "BoardLayoutEngine" operates on PHIDs without knowledge of the underlying objects, but this means it has to be sensitive to PHID input order when falling back to a default layout order.

We use "default layout order" on workboards which are sorted by "Natual" order but which have one or more cards which no user has ever reordered. For example, if you add 10 tasks to a project, then create a board, there's no existing order for those tasks in the "Backlog" column. The layout engine uses the input order to place them in the column, with the expectation that input order is ID/creation order, so new cards will end up on top.

I think this code never really made an explicit effort to guarantee that the LayoutEngine received objects in ID order, and it just sort of happened to by coincidence and good fortune. Some recent change has disrupted this, so the edit operation can end up with the PHIDs arranged in arbitrary order.

Explicitly put them in ID order so we always get an implicit default layout order to fall back to. Also, update to `msortv()`.

Test Plan:
  - Tagged several tasks with project X, a project without a board yet.
  - Created the project X workboard.
  - (Did not drag any tasks around on the project X board!)
  - Viewed the board in "Natural" order.

This creates a view of the board where tasks are ordered by implicit/virtual/input order. The expectation, and "view" behavior of this board, is that this order is "newest on top".

  - Edited one of the cards on the board, changing the title (don't reorder it!)
  - Before: page state synchronized with cards in arbitrary/random/different order.
  - After: page state synchronized with cards in the same order as before ("newest on top").

Reviewers: amckinley

Maniphest Tasks: T4900

Differential Revision: https://secure.phabricator.com/D20681
2019-07-30 13:17:30 -07:00
epriestley
7d41535010 When a task card is edited, emit update events for old boards and parent boards
Summary:
Ref T4900. When a card is edited, we currently emit an update notification for all the projects the task is tagged with. This isn't quite the right set:

  - We want to emit notifications for projects the task //was previously// tagged with, so it can be removed from boards it should no longer be part of.
  - We want to emit notifications for ancestors of projects the task is or was tagged with, so parent project boards can be updated.
  - However, we don't need to emit notifications for projects that don't actually have workboards.

Adjust the notification set to align better to these rules.

Test Plan:
  - Removal of Parent Project: Edited a task on board "A > B", removing the "B" project tag. Saw board A update in another window.
  - Normal Update: Edited a task title on board X, saw board X update in another window.
  - Used `bin/aphlict debug` to inspect the notification set, saw generally sensible-seeming data going over the wire.

Reviewers: amckinley

Maniphest Tasks: T4900

Differential Revision: https://secure.phabricator.com/D20680
2019-07-30 13:16:33 -07:00
epriestley
7e09da3313 Fix policy behavior of "slowvote.info" API method
Summary: Ref T13350. This ancient API method is missing modern policy checks.

Test Plan:
  - Set visibility of vote X to "Only: epriestley".
  - Called "slowvote.info" as another user.
  - Before: retrieved poll title and author.
  - After: policy error.
  - Called "slowvote.info" on a visible poll, got information before and after.

Maniphest Tasks: T13350

Differential Revision: https://secure.phabricator.com/D20684
2019-07-30 11:55:55 -07:00
epriestley
f6621a5fdc Tailor "Restart All Builds" for the complex realities of modern build restart rules
Summary:
Fixes T13348. Currently, the Harbormaster UI shows "Restart All Builds", but it really means "Restart Restartable Builds", which is often fewer than "All" builds (because of autobuilds, permissions, and/or configuration).

Remove the misleading term "All" and make the workflow preview exactly which builds will and will not be affected, and why.

Test Plan:
{F6636313}

{F6636314}

{F6636315}

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13348

Differential Revision: https://secure.phabricator.com/D20679
2019-07-24 09:25:46 -07:00
epriestley
99c864f5e6 Provide a basic detail view for user activity logs
Summary:
Depends on D20673. Ref T13343. Since we're now putting log IDs in email, make the UI a little better for working with log IDs.

Some day, this page might have actions like "report this as suspicious" or whatever, but I'm not planning to do any of that for now.

Test Plan: {F6608631}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13343

Differential Revision: https://secure.phabricator.com/D20674
2019-07-24 07:14:07 -07:00
epriestley
60db658d52 Record account recovery email links in the user activity log and make the mail message reference the log
Summary:
Depends on D20672. Ref T13343. When a user requests an account access link via email:

  - log it in the activity log; and
  - reference the log in the mail.

This makes it easier to ban users misusing the feature, provided they're coming from a single remote address, and takes a few steps down the pathway toward a button in the mail that users can click to report the action, suspend account recovery for their account, etc.

Test Plan:
  - Requested an email recovery link.
  - Saw request appear in the user activity log.
  - Saw a reference to the log entry in the mail footer.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13343

Differential Revision: https://secure.phabricator.com/D20673
2019-07-24 07:13:34 -07:00
epriestley
57799bc82b Give user log types a tokenizer and datasource instead of a page of checkboxes
Summary: Depends on D20671. Ref T13343. Now that log types are modular, provide a datasource/tokenizer for selecting them since we already have a lot (even after I purged a few in D20670) and I'm planning to add at least one more ("Request password reset").

Test Plan: {F6608534}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13343

Differential Revision: https://secure.phabricator.com/D20672
2019-07-24 07:11:42 -07:00
epriestley
32dd13d434 Modularize user activity log message types
Summary:
Depends on D20670. Ref T13343. The user activity message log types are currently hard-coded, so only upstream code can really use the log construct.

Under the theory that we're going to keep this log around going forward (just focus it a little bit), modularize things so the log is extensible.

Test Plan:
Grepped for `UserLog::`, viewed activity logs in People and Settings.

(If I missed something here -- say, misspelled a constant -- the effect should just be that older logs don't get a human-readable label, so stakes are very low.)

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13343

Differential Revision: https://secure.phabricator.com/D20671
2019-07-24 07:10:18 -07:00