Page MenuHomePhabricator

Plans: Improve Phacility UI for managing instance managers and cards
Open, LowPublic

Description

The most common problems users run into in the Phacility instance UI are:

  1. Creating mailing lists instead of actual users.
  2. Confusion around "Administrator" vs "Instance Manager".
  3. Difficulty managing cards and autopay.

Creating Users: We show a big colorful warning on the page with instructions about how to do this right, but the default path where you just blindly click buttons does the wrong thing.

Screen Shot 2018-06-21 at 5.31.50 AM.png (908×1 px, 123 KB)

To some degree, this behavior is "intentionally bad" to minimize how much hacky special casing we have to put into the upstream, but I think we need more hacky special casing than we currently have. Instead of hiding "Create Normal User", we should leave it visible and just redirect you to the admin.phacility.com interface when you click it, showing instructions if you don't have permission.

The actual technical changes are:

  • Provide some kind of (minimally hacky?) way to hook this workflow to redirect instead of acting. A minimal version of this is probably just to intercept it in SiteConfig, but that feels unreasonably gross.
  • Remove PhabricatorPeopleCreateGuidanceContext and the "Guidance Engine" hook since we'll no longer need the banner.
  • Remove the "Can Create (non-bot) Users" permission? If we do this, we need another method to disable other mechanisms for creating user accounts (via invite, LDAP, etc).

Instance Managers: Users have reasonable confusion about "Administrators" vs "Instance Managers", and the distinction between "can create image macros" and "can instruct us to permanently destroy all your data". It's also not inherently obvious that you need a high level of access to add/invite users, because it affects billing, although I think this is probably the right rule.

Some small steps we could take are:

  • On the instance "Members" page, call out instance managers.
  • Provide an easier way to promote/demote users from that page.
  • Rewrite the documentation to point at this pathway instead of the Billing pathway.

One advantage of this approach is that we could then send users a link directly to the dialog if they still have trouble, i.e. "click this link" instead of "read this documentation".

Cards and Autopay: This is somewhat tricky in general because this stuff lives in Phortune, and Phortune is necessarily very general (supports multiple accounts, etc).

Some small changes are:

  • Split "Billing / History" into "Payment Methods" and "Payment History". The only hint that "Payment Methods" is in this item is that the icon is a credit card.
  • Provide a "New Card" action directly from the "Payment Methods" section.
  • Provide direct "Remove Card" actions from the instance "Billing" page.
  • Provide direct "Make Autopay" actions from the instance "Billing" page.
  • Maybe move "Disable Instance" back to "Overview" to simplify this interface.
  • Maybe provide a direct list of instance managers on the instance "Billing" page?

Additionally, we could reasonably:

  • Remove the "Upload Files" action, which is obsoleted by "Support", since all uploads are just drag-and-drop into support issue descriptions/comments now.
  • Clarify some of the language that uses "administrative" or similar to mean "Instance Manager".

I'd also like to consider making this UI look more like the full-width modern Phortune UI again. Although the user-facing pages are generally in reasonable shape, some of the administrative interfaces like "Import Accounts" are in pretty rough shape.


See PHI740. It's not obvious: (a) that the member list for support pacts is the same as the instance manager list; or (b) that you have to click the "Support" link from the instance detail page to get them to synchronize. Better would be to synchronize on-page with an explanation of the rule.


See many user interactions with MFA. Currently, users can configure TOTP on both central accounts and instance accounts.

Approximately 99% of users who configure central MFA appear to do so by accident and then require us to strip it. At least until T9515 (and perhaps far beyond that) it would be helpful to disable this ability, or at least put like five roadblocks on it.

There is currently no way to selectively disable MFA types, but this is a reasonable ability to add, particularly since we'd presumably like to disable SMS by default (see T920) even once we build it.


Currently, there's no link to the printable version of an invoice until an order completes. You should be able to print it either before completion (as an invoice) or afterwards (as a receipt).


The recent policy changes mean that applying an INSTANCE_STATUS transaction requires CAN_EDIT permission, explicitly. Staff do not have CAN_EDIT, so we currently can not suspend instances through the web UI. The non-hacky way to fix this is to modularize instance transactions, then add a policy change similar to the change to the "Disable User" transaction. This also impacts "Silence" / "Unsilence".


See PHI942. We can give self-hosted installs a much better prediction for migration downtime by using data from the cluster than data from secure in the changelog. See also T11760.

See PHI954. The "support pact members" cache is currently updated only when you actually click "Support", but this is often silly and unintuitive.


See T13215, which might be nice to take a look at since it's probably not terribly difficult.


If you try to invite a "Mailing List" user to an instance by email, you get a slightly misleading error message ("import this user rather than inviting them").


See PHI1982. You can add SSH keys to your central (admin.phacility.com) account, but it's unlikely any user ever actually wants to do this (they almost certainly want to add keys to their instance account, instead). This is similar to the TOTP issue above.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision

Event Timeline

epriestley created this task.

See also PHI689 and PHI629 for an adjacent issue with autopay.

We should probably try to handle this at the Phortune layer for the most part. In particular:

  • When you remove a card, the dialog should show all the subscriptions that card is configured as autopay for, and warn you that they will no longer autopay.
  • When you add a card, we should possibly make it easy to also set the card as an autopay source for any active subscriptions.
  • Whatever instance-level autopay status stuff we currently render should be more consistent (e.g., always render, not only have a failure state?) and consider a disabled card to be an unconfigured state.
epriestley added a revision: Restricted Differential Revision.Jul 13 2018, 6:14 PM
epriestley added a commit: Restricted Diffusion Commit.Jul 13 2018, 8:30 PM

See also PHI786 for an example of a general issue: voiding invoices is currently inconvenient: there's no actual "void" state for invoices so you have to go hunt them down manually and DELETE them. There should be a formal "Void" state they can be kicked into.

The MerchantsPhacilityOrders UI is also unsearchable. It should have filtering support, and the staff instance view should have a link to it directly to make voiding/reviewing orders for a given instance more realistic.

I'm descheduling this out of T13151 since none of this is too burning and it's grown kind of big. Probably makes more sense after T13076 as we move toward private clusters, since these UIs will need updates anyway.

epriestley renamed this task from Improve Phacility UI for managing instance managers and cards to Plans: Improve Phacility UI for managing instance managers and cards.Aug 10 2018, 6:05 PM
epriestley moved this task from Backlog to Near Future on the Phacility board.
epriestley added a project: Plans.

Somewhat related:

  • The "put the invoicee's contact details on the invoice" request in T7607 surfaced again somewhat recently.
  • T8389 has also cropped up once or twice and is generally adjacent to a lot of the rest of this.

...we currently can not suspend instances through the web UI. The non-hacky way to fix this...

Here's the hacky way:

<?php

require_once 'scripts/init/init-script.php';

if ($argc != 2) {
  die("usage: suspend <instance>\n");
}

$viewer = PhabricatorUser::getOmnipotentUser();
$next_status = PhacilityCore::INSTANCESTATUS_SUSPENDED;

$content_source = PhabricatorContentSource::newForSource(
  PhabricatorConsoleContentSource::SOURCECONST);

$instances_phid = id(new InstancesApplication())
  ->getPHID();

$instance = id(new InstancesInstanceQuery())
  ->setViewer($viewer)
  ->withNames(array($argv[1]))
  ->executeOne();
if (!$instance) {
  throw new Exception(pht('No such instance!'));
}

$xactions = array();
$xactions[] = id(new InstancesInstanceTransaction())
  ->setTransactionType(InstancesInstanceTransaction::TYPE_STATUS)
  ->setNewValue($next_status);

$editor = id(new InstancesInstanceEditor())
  ->setActor($viewer)
  ->setActingAsPHID($instances_phid)
  ->setContentSource($content_source)
  ->setContinueOnNoEffect(true)
  ->setContinueOnMissingFields(true);

$editor->applyTransactions($instance, $xactions);
  • PHI979 describes a case where we'd like to automatically increase the worker task limit based on instance size. admin should also get an increased limit.
  • Now that MFA can be disabled completely, I want to disable MFA on admin (pretty much everyone adding it is adding it by accident) by deprecating the provider.
  • T13178 is adjacent to a lot of this.
epriestley added a revision: Restricted Differential Revision.Jan 28 2019, 8:05 PM
epriestley added a revision: Restricted Differential Revision.Jan 29 2019, 2:15 AM
epriestley added a revision: Restricted Differential Revision.Jan 29 2019, 10:32 PM
epriestley added a revision: Restricted Differential Revision.Jan 29 2019, 11:44 PM
epriestley added a commit: Restricted Diffusion Commit.Jan 30 2019, 2:23 PM
epriestley added a commit: Restricted Diffusion Commit.
epriestley added a commit: Restricted Diffusion Commit.Jan 30 2019, 2:34 PM
epriestley added a revision: Restricted Differential Revision.Jan 30 2019, 7:40 PM
epriestley added a commit: Restricted Diffusion Commit.Feb 2 2019, 5:27 PM
epriestley added a commit: Restricted Diffusion Commit.

See PHI954. The "support pact members" cache is currently updated only when you actually click "Support", but this is often silly and unintuitive.

@amckinley This is referring to the thing we hit IRL, although this description is admittedly pretty vague/brief.