Page MenuHomePhabricator

Ask 2nd confirmation when disabling instance on Phacility
Closed, DuplicatePublic

Assigned To
None
Authored By
swisspol
Jul 18 2015, 2:26 AM
Referenced Files
F641156: Screen Shot 2015-07-19 at 5.06.29 PM.png
Jul 20 2015, 12:08 AM
F640596: pasted_file
Jul 19 2015, 6:54 PM
F634183: Screen Shot 2015-07-17 at 7.46.29 PM.png
Jul 18 2015, 2:47 AM

Description

The fact an instance cannot be re-enabled by the user after disabling it is a big deal. This should really be shown in a separate confirmation dialog *after* confirming that you really intend to disable a specific instance (ref T8879).

PS: I don't know what the reasoning is to prevent people from doing this, but if it's related to preventing abusing the system by continuously stopping and restarting instances, maybe it could be allowed to re-enable an instance within 30mn or something.

Revisions and Commits

Event Timeline

swisspol raised the priority of this task from to Needs Triage.
swisspol updated the task description. (Show Details)
swisspol added a subscriber: swisspol.

Just to make sure, this is the dialog you confirmed your way through by accident?

Screen Shot 2015-07-17 at 7.46.29 PM.png (782×1 px, 117 KB)

I didn't confirm by "accident". What I did by accident was disabling the wrong instance which would be solved by T8879.

This dialog has too much text IMO and the very important fact you cannot re-enable an instance on your own is lost in the middle.

To add more context for when I say I didn't do by accident, it's because I didn't read the fine print, so ultimately it is my fault. BUT I didn't read it because I would expect only 1 important thing in there: the name of the instance to suspend. And since I had only 1 instance, it was pointless to read. It turns out there are 2 critical pieces of information in this dialog: the target instance AND the fact you cannot re-enable it on your own even though the UI hints otherwise.

This UI doesn't imply "Disable Instance" is one-way only at all:

pasted_file (181×212 px, 14 KB)

It's the regular list of actions UI used in Phab and you would expect this last one to turn into "Enable Instance" and that it would be allowed.

I can't really remember of such asymmetrical actions in other areas of the Phab UI, that's why it's so surprising IMO. For instance, when editing project policies, it you try to change the edit policy to one that would kick you out, you can't save it and there's a warning in red.

I don't know what the reasoning is to prevent people from doing this, but

It's to prevent them from disabling instances shortly before we generate an invoice, then re-enabling instances after an invoice is generated. Since we don't bill for disabled instances, this would skip a billing cycle (and, generally, let users have an instance for free indefinitely).

We could let users re-enable instances themselves, but if the instance had been disabled through a billing cycle we'd need to retroactively generate a prorated bill for the enabled fraction of the initial missed period. This is very complex to develop, test, and explain to users, and I believe our development efforts are probably better spent elsewhere. In at least some cases, this likely requires administrative intervention anyway (to apply billing adjustments to the old bill). From your point of view as an instance administrator, do you think this is valuable enough to spend 2-3 days building instead of other features?

We could also permit instances to be re-enabled if they had not been disabled through a billing cycle. This is a bit less complex, but not trivial, and would add more instructions to the dialog and sometimes degrade to the current state of affairs.

if it's related to preventing abusing the system by continuously stopping and restarting instances, maybe it could be allowed to re-enable an instance within 30mn or something.

From the timestamps on the email thread, it looks like this took 22 minutes to resolve, so it sounds like the issue would have taken 8 minutes longer to resolve if a 30-minute lockout was in place and you resolved it in this way. Is that somewhere in the ballpark, or do I have the timelines wrong?

From your point of view as an instance administrator, do you think this is valuable enough to spend 2-3 days building instead of other features?

Sounds like a loaded question 😉 The answer is no. Best to have a UX that prevents people from shooting themselves in the foot, than trying to build a complex system to compensate after the fact.

it sounds like the issue would have taken 8 minutes longer to resolve if a 30-minute lockout was in place and you resolved it in this way

I was suggesting a lock out the other way around: if you disable an instance, you can re-enable it within 30mn (or 15mn or whatever short amount of time). After that, you need to contact support. Now that I understand the reason for the current behavior it might not make sense.

However, here's another suggestion: don't allow people to disable instance outside of Phacility conservative work hours. This way it's guaranteed that in case of a mistake, one can reach support immediately.

However, here's another suggestion: don't allow people to disable instance outside of Phacility conservative work hours. This way it's guaranteed that in case of a mistake, one can reach support immediately.

Wouldn't it be simpler to just remove this capability entirely and prevent administrators from disabling instances on their own at all?

epriestley added a revision: Restricted Differential Revision.Jul 20 2015, 12:08 AM

That change adds a "safe mode" for specific accounts, which prevents them from using the console to disable instances and instructs them to contact support instead:

Screen Shot 2015-07-19 at 5.06.29 PM.png (181×556 px, 17 KB)

So is the idea that "safe mode" is going to be enabled for all accounts on Phacility?

No, it's enabled for your accounts only.