1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-05 13:16:14 +01:00
Commit graph

17418 commits

Author SHA1 Message Date
epriestley
d2e1c4163a When a project has a custom icon, use that icon to label the project policy in the policy dropown
Summary:
Fixes T8808. Currently, all project use the default ("Briefcase") project icon when they appear in a policy dropdown.

Since project policies are separated out into a "Members of Projects" section of the dropdown anyway, there is no reason not to use the actual project icon, which is often more clear.

Test Plan: {F6849927}

Maniphest Tasks: T8808

Differential Revision: https://secure.phabricator.com/D20799
2019-09-09 13:38:12 -07:00
epriestley
1d1a60fdda Improve rendering of Herald rules in "Another Herald rule..." field
Summary:
Fixes T9136.

  - Fix a bug where the name is rendered improperly.
  - Put disabled rules at the bottom.
  - Always show the rule monogram so you can distingiush between rules with the same name.

Test Plan: {F6849915}

Maniphest Tasks: T9136

Differential Revision: https://secure.phabricator.com/D20798
2019-09-09 13:29:49 -07:00
epriestley
7593a265d5 When Herald changes object subscribers, always hide the feed story
Summary:
Fixes T8952. These feed stories are not interesting and tend to be generated as collateral damage when a non-story update is made to an old task and someone has a "subscribe me" Herald rule.

Also clean up some of the Herald field/condition indexing behavior slightly.

Test Plan: Wrote a "Subscribe X" herald rule, made a trivial update to a task. Before: low-value feed story; after: no feed story.

Maniphest Tasks: T8952

Differential Revision: https://secure.phabricator.com/D20797
2019-09-09 13:17:36 -07:00
epriestley
4547714463 Add a "Remove flag" action to Herald
Summary: Fixes T13409. This is a companion to the existing "Mark with flag" rule.

Test Plan: Used a "remove flag" rule on an object with no flag (not removed), the right type of flag (removed), and a different type of flag (not removed).

Maniphest Tasks: T13409

Differential Revision: https://secure.phabricator.com/D20796
2019-09-09 13:15:52 -07:00
epriestley
d965d9a669 Index Herald fields, not just actions, when identifying objects related to a particular Herald rule
Summary:
Fixes T13408. Currently, when a package (or other object) appears in a field (rather than an action), it is not indexed.

Instead: index fields too, not just actions.

Test Plan:
  - Wrote a rule like "[ Affected packages include ] ...".
  - Updated the search index.
  - Saw rule appear on "Affected By Herald Rules" on the package detail page.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13408

Differential Revision: https://secure.phabricator.com/D20795
2019-09-09 12:50:43 -07:00
epriestley
aaaea57591 Fix fatal during redirection safety check for searching for Phabricator base-uri with no trailing slash
Summary: Fixes T13412. If you search for "https://phabricator.example.com" with no trailing slash, we currently redirect you to "", which is fouled by a safety check in the redirection flow.

Test Plan:
  - Searched for "https://local.phacility.com"; before: fatal in redirection; after: clean redirect.
  - Searched for other self-URIs, got normal redirects.

Maniphest Tasks: T13412

Differential Revision: https://secure.phabricator.com/D20794
2019-09-09 12:45:24 -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
535c8e6bdc Remove the "ONLY_FULL_GROUP_BY" SQL mode setup warning and change the setup key for "STRICT_ALL_TABLES"
Summary:
Ref T13404. Except for one known issue in Multimeter, Phabricator appears to function properly in this mode. It is broadly desirable that we run in this mode; it's good on its own, and enabled by default in at least some recent MySQL.

Additionally, "ONLY_FULL_GROUP_BY" and "STRICT_ALL_TABLES" shared a setup key, so ignoring one would ignore both. Change the key so that existing ignores on "ONLY_FULL_GROUP_BY" do not mask "STRICT_ALL_TABLES" warnings.

Test Plan: Grepped for `ONLY_FULL_GROUP_BY`.

Maniphest Tasks: T13404

Differential Revision: https://secure.phabricator.com/D20791
2019-09-09 12:17:51 -07:00
epriestley
f16365ed07 Weaken the guidance recommending that installs enable "STRICT_ALL_TABLES"
Summary: Ref T13404. Enabling "STRICT_ALL_TABLES" is good, but if you don't want to bother it doesn't matter too much. All upstream development has been on "STRICT_ALL_TABLES" for a long time.

Test Plan: {F6847839}

Maniphest Tasks: T13404

Differential Revision: https://secure.phabricator.com/D20790
2019-09-09 12:17:05 -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
Aviv Eyal
318e8ebdac Allow bin/config to create config file
Summary:
See D20779, https://discourse.phabricator-community.org/t/3089. `bin/config set` complains about
missing config file as if it's un-writable.

Test Plan: run `bin/config set` with missing, writable, unwritable conf.json and parent dir.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D20788
2019-09-08 00:16:19 +00: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
adc2002d28 Make it easier to parse "X-Forwarded-For" with one or more load balancers
Summary:
Fixes T13392. If you have 17 load balancers in sequence, Phabricator will receive requests with at least 17 "X-Forwarded-For" components in the header.

We want to select the 17th-from-last element, since prior elements are not trustworthy.

This currently isn't very easy/obvious, and you have to add a kind of sketchy piece of custom code to `preamble.php` to do any "X-Forwarded-For" parsing. Make handling this correctly easier.

Test Plan:
  - Ran unit tests.
  - Configured my local `preamble.php` to call `preamble_trust_x_forwarded_for_header(4)`, then made `/debug/` dump the header and the final value of `REMOTE_ADDR`.

```
$ curl http://local.phacility.com/debug/
<pre>

HTTP_X_FORWARDED_FOR =
   FINAL REMOTE_ADDR = 127.0.0.1
</pre>
```

```
$ curl -H 'X-Forwarded-For: 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>

HTTP_X_FORWARDED_FOR = 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6
   FINAL REMOTE_ADDR = 3.3.3.3
</pre>
```

```
$ curl -H 'X-Forwarded-For: 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>

HTTP_X_FORWARDED_FOR = 5.5.5.5, 6.6.6.6
   FINAL REMOTE_ADDR = 5.5.5.5
</pre>
```

Maniphest Tasks: T13392

Differential Revision: https://secure.phabricator.com/D20785
2019-09-05 04:30:13 -07:00
epriestley
764db4869c Make "bin/storage destroy" target individual hosts in database cluster mode
Summary:
Ref T13336. Currently, "bin/storage destroy" destroys every master. This is wonderfully destructive, but if replication fails it's useful to be able to destroy only a replica.

Operate on a single host, and require "--host" to target the operation in cluster mode, so `bin/storage destroy --host dbreplica001` is a useful operation.

Test Plan: Ran `bin/storage destroy` with various flags locally. Will destroy `secure002` and refresh replication.

Maniphest Tasks: T13336

Differential Revision: https://secure.phabricator.com/D20784
2019-09-04 10:11:08 -07:00
epriestley
f7290bbbf2 Update a straggling "getAuthorities()" call in Fund
Summary: Ref T13366. The "authorities" mechanism was replaced, but I missed this callsite. Update it to use the request cache mechanism.

Test Plan: As a user without permission to view some initiatives, viewed a list of initiatives.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20783
2019-09-04 07:15:20 -07:00
epriestley
22b075df97 Fix "ONLY_FULL_GROUP_BY" issue in SystemAction queries
Summary: Ref T13404. This query is invalid under "sql_mode=ONLY_FULL_GROUP_BY". Rewrite it to avoid interacting with `actorIdentity` at all; this is a little more robust in the presence of weird data and not really more complicated.

Test Plan:
  - Enabled "ONLY_FULL_GROUP_BY".
  - Hit system actions (e.g., login).
    - Before: error.
    - After: clean login.
  - Tried to login with a bad password many times in a row, got properly limited by the system action rate limiter.

Maniphest Tasks: T13404

Differential Revision: https://secure.phabricator.com/D20782
2019-09-03 16:50:33 -07:00
epriestley
e0d6994adb Use the "@" operator to silence connection retry messages if initializing the stack with database config optional
Summary:
Depends on D20780. Ref T13403. During initial setup, it's routine to run "bin/config" with a bad database config. We start the stack in "config optional" mode to anticipate this.

However, even in this mode, we may emit warnings if the connection fails in certain ways. These warnings aren't useful; suppress them with "@".

(Possibly this message should move from "phlog()" to "--trace" at some point, but it has a certain amount of context/history around it.)

Test Plan:
  - Configured MySQL to fail with a retryable error, e.g. good host but bad port.
  - Ran `bin/config set ...`.
  - Before: saw retry warnings on stderr.
  - After: no retry warnings on stderr.
  - (Turned off suppression code artificially and verified warnings still appear under normal startup.)

Maniphest Tasks: T13403

Differential Revision: https://secure.phabricator.com/D20781
2019-09-03 12:54:17 -07:00
epriestley
f8eec38c94 When "mysqli->real_connect()" fails without setting an error code, recover more gracefully
Summary: Depends on D20779. Ref T13403. Bad parameters may cause this call to fail without setting an error code; if it does, catch the issue and go down the normal connection error pathway.

Test Plan:
  - With "mysql.port" set to "quack", ran `bin/storage probe`.
  - Before: wild mess of warnings as the code continued below and failed when trying to interact with the connection.
  - After: clean connection failure with a useful error message.

Maniphest Tasks: T13403

Differential Revision: https://secure.phabricator.com/D20780
2019-09-03 12:51:20 -07:00
epriestley
d9badba147 Give "bin/config" a friendlier error message if "local.json" is not writable
Summary: Ref T13403. We currently emit a useful error message, but it's not tailored and has a stack trace. Since this is a relatively routine error and on the first-time-setup path, tailor it so it's a bit nicer.

Test Plan:
  - Ran `bin/config set ...` with an unwritable "local.json".
  - Ran `bin/config set ...` normally.

Maniphest Tasks: T13403

Differential Revision: https://secure.phabricator.com/D20779
2019-09-03 12:47:06 -07:00
epriestley
8ff3a133c4 Generalize repository proxy retry logic to writes
Summary:
Ref T13286. The current (very safe / conservative) rules for retrying git reads generalize to git writes, so we can use the same ruleset in both cases.

Normally, writes converge rapidly to only having good nodes at the head of the list, so this has less impact than the similar change to reads, but it generally improves consistency and allows us to assert that writes which can be served will be served.

Test Plan:
  - In a cluster with an up node and a down node, pushed changes.
  - Saw a push to the down node fail, retry, and succeed.
  - Did some pulls, saw appropriate retries and success.
  - Note that once one write goes through, the node which received the write always ends up at the head of the writable list, so nodes need to be explicitly thawed to reproduce the failure/retry behavior.

Maniphest Tasks: T13286

Differential Revision: https://secure.phabricator.com/D20778
2019-09-03 12:34:10 -07:00
epriestley
ff3d1769b4 Instead of retrying safe reads 3 times, retry each eligible service once
Summary: Ref T13286. When retrying a read request, keep retrying as long as we have canididate services. Since we consume a service with each attempt, there's no real reason to abort early, and trying every service allows reads to always succeed even if (for example) 8 nodes of a 16-node cluster are dead because of a severed network link between datacenters.

Test Plan: Ran `git pull` in a clustered repository with an up node and a down node; saw retry count dynamically adjust to available node count.

Maniphest Tasks: T13286

Differential Revision: https://secure.phabricator.com/D20777
2019-09-03 10:43:33 -07:00
epriestley
95fb237ab3 On Git cluster read failure, retry safe requests
Summary:
Depends on D20775. Ref T13286. When a Git read request fails against a cluster and there are other nodes we could safely try, try more nodes.

We DO NOT retry the request if:

  - the client read anything;
  - the client wrote anything;
  - or we've already retried several times.

Although //some// requests where bytes went over the wire in either direction may be safe to retry, they're rare in practice under Git, and we'd need to puzzle out what state we can safely emit.

Since most types of failure result in an outright connection failure and this catches all of them, it's likely to almost always be sufficient in practice.

Test Plan:
  - Started a cluster with one up node and one down node, pulled it.
  - Half the time, hit the up node and got a clean pull.
  - Half the time, hit the down node and got a connection failure followed by a retry and a clean pull.
  - Forced `$err = 1` so even successful attempts would retry.
  - On hitting the up node, got a "failure" and a decline to retry (bytes already written).
  - On hitting the down node, got a failure and a real retry.
  - (Note that, in both cases, "git pull" exits "0" after the valid wire transaction takes place, even though the remote exited non-zero. If the server gave Git everything it asked for, it doesn't seem to care if the server then exited with an error code.)

Maniphest Tasks: T13286

Differential Revision: https://secure.phabricator.com/D20776
2019-09-03 10:08:43 -07:00
epriestley
b6420e0f0a Allow repository service lookups to return an ordered list of service refs
Summary:
Ref T13286. To support request retries, allow the service lookup method to return an ordered list of structured service references.

Existing callsites continue to immediately discard all but the first reference and pull a URI out of it.

Test Plan: Ran `git pull` in a clustered repository with an "up" node and a "down" node, saw 50% serivce failures and 50% clean pulls.

Maniphest Tasks: T13286

Differential Revision: https://secure.phabricator.com/D20775
2019-09-03 10:05:40 -07:00
epriestley
9316cbf7fd Move web application classes into "phabricator/"
Summary: Ref T13395. Companion change to D20773.

Test Plan: See D20773.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20774
2019-09-02 07:58:59 -07:00
epriestley
b2b17485b9 Clean up two straggling UI issues in Phortune
Ref T13401. The checkout UI didn't get fully updated to the new View objects,
and account handles are still manually building a URI that goes to the wrong
place.
2019-08-31 09:36:23 -07:00
epriestley
533a5535b6 Remove the "grant authority" mechanism from users
Summary:
Ref T13393. See some previous discussion in T13366.

Caching is hard and all approaches here have downsides, but the request cache likely has fewer practical downsides for this kind of policy check than other approaches. In particular, the grant approach (at least, as previously used in Phortune) has a major downside that "Query" classes can no longer fully enforce policies.

Since Phortune no longer depends on grants and they've now been removed from instances, drop the mechanism completely.

Test Plan: Grepped for callsites, found none.

Maniphest Tasks: T13393

Differential Revision: https://secure.phabricator.com/D20754
2019-08-30 09:26:08 -07:00
epriestley
3c26e38487 Provide a simple read-only maintenance mode for repositories
Summary:
Ref T13393. While doing a shard migration in the Phacility cluster, we'd like to stop writes to the migrating repository. It's safe to continue serving reads.

Add a simple maintenance mode for making repositories completely read-only during maintenance.

Test Plan: Put a repository into read-only mode, tried to write via HTTP + SSH. Viewed web UI. Took it back out of maintenance mode.

Maniphest Tasks: T13393

Differential Revision: https://secure.phabricator.com/D20748
2019-08-29 15:23:10 -07:00
epriestley
c6642213d5 Straighten out replication/cache behavior in "bin/storage dump"
Summary:
Fixes T13336.

  - Prevent `--no-indexes` from being combined with `--for-replica`, since combining these options can only lead to heartbreak.
  - In `--for-replica` mode, dump caches too. See discussion in T13336. It is probably "safe" to not dump these today, but fragile and not correct.
  - Mark the "MarkupCache" table as having "Cache" persistence, not "Data" persistence (no need to back it up, since it can be fully regenerated from other datasources).

Test Plan: Ran `bin/storage dump` with various combinations of flags.

Maniphest Tasks: T13336

Differential Revision: https://secure.phabricator.com/D20743
2019-08-28 08:25:40 -07:00
epriestley
0943561dcb Fix incorrect construction of subtype map when validating "subtype" transactions against non-subtypable objects
Summary:
Fixes T13389. Currently, we try to "newSubtypeMap()" unconditionally, even if the underlying object does not support subtypes.

  - Only try to build a subtype map if subtype transactions are actually being applied.
  - When subtype transactions are applied to a non-subtypable object, fail more explicitly.

Test Plan: Clicked "Make Editable" in a fresh Calendar transaction form, got an editable form instead of a fatal from "newSubtypeMap()". (Calendar events are not currently subtypable.)

Maniphest Tasks: T13389

Differential Revision: https://secure.phabricator.com/D20741
2019-08-28 06:57:04 -07:00
epriestley
7198bd7db7 When "utf8mb4" is available, use it as the default client charset when invoking standalone "mysql" commands
Summary:
Fixes T13390. We have some old code which doesn't dynamically select between "utf8mb4" and "utf8". This can lead to dumping utf8mb4 data over a utf8 connection in `bin/storage dump`, which possibly corrupts some emoji/whales.

Instead, prefer "utf8mb4" if it's available.

Test Plan: Ran `bin/storage dump` and `bin/storage shell`, saw sub-commands select utf8mb4 as the client charset.

Maniphest Tasks: T13390

Differential Revision: https://secure.phabricator.com/D20742
2019-08-27 16:36:48 -07:00
epriestley
97a4a59cf2 Give the Phortune external portal an order view
Summary:
Depends on D20739. Ref T13366. Slightly modularize/update components of order views, and make orders viewable from either an account context (existing view) or an external context (new view).

The new view is generally simpler so this mostly just reorganizes existing code.

Test Plan: Viewed orders as an account owner and an external user.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20740
2019-08-26 07:49:17 -07:00
epriestley
a0a3879712 In Phortune, send order email to account external addresses
Summary: Depends on D20738. Ref T13366. Fixes T8389. Now that the infrastructure is in place, actually send email to external addresses.

Test Plan: Used `bin/phortune invoice` to generate invoices and saw associated external accounts receive mail in `bin/mail list-outbound`.

Maniphest Tasks: T13366, T8389

Differential Revision: https://secure.phabricator.com/D20739
2019-08-26 07:48:27 -07:00
epriestley
4e13551e85 Add credential rotation and statuses (disabled, unsubscribed) to Phortune external email
Summary: Depends on D20737. Ref T13367. Allow external addresses to have their access key rotated. Account managers can disable them, and anyone with the link can permanently unsubscribe them.

Test Plan: Enabled/disabled addresses; permanently unsubscribed addresses.

Maniphest Tasks: T13367

Differential Revision: https://secure.phabricator.com/D20738
2019-08-26 07:47:44 -07:00
epriestley
8f6a1ab015 Roughly support external/email user views of Phortune recipts and invoices
Summary: Ref T13366. This gives each account email address an "external portal" section so they can access invoices and receipts without an account.

Test Plan: Viewed portal as user with authority and in an incognito window.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20737
2019-08-26 07:39:08 -07:00
epriestley
a39a37fc0e Update the Phortune cart/invoice workflow for policy changes
Summary:
Depends on D20734. Ref T13366. This makes the cart/order flow work under the new policy scheme with no "grantAuthority()" calls.

It prepares for a "Void Invoice" action, although the action doesn't actually do anything yet.

Test Plan: With and without merchant authority, viewed and paid invoices and went through the other invoice interaction workflows.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20735
2019-08-23 07:09:00 -07:00
epriestley
b3f8045b87 Make minor flavor updates
Summary: Refresh the 404 text since it hasn't been updated in a while, and swap the "Save Query" button back to grey since I never got used to blue.

Test Plan: Hit 404 page, saved a query.

Differential Revision: https://secure.phabricator.com/D20734
2019-08-23 07:08:09 -07:00
epriestley
9bcd683c08 Update Phortune Merchant UI to bring it in line with Account UI
Summary:
Depends on D20732. Ref T13366. This generally makes the "Merchant" UI look and work like the "Payment Account" UI.

This is mostly simpler since the permissions have largely been sorted out already and there's less going on here and less weirdness around view/edit policies.

Test Plan: Browsed all Merchant functions as a merchant member and non-member.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20733
2019-08-22 21:12:33 -07:00
epriestley
c93ac91dc6 Update Charge and Cart policies in Phortune, and make URIs more consistent
Summary:
Ref T13366. Depends on D20721. Continue applying UI and policy updates to the last two Phortune objects.

Charges aren't mutable and Carts are already transactional, so this is less involved than prior changes.

Test Plan: Viewed various charge/order interfaces as merchants and account members.

Maniphest Tasks: T13366

Differential Revision: https://secure.phabricator.com/D20732
2019-08-22 21:09:35 -07:00
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