microsoft / vscode-cosmosdb

Azure Databases extension for VS Code
https://marketplace.visualstudio.com/items?itemName=ms-azuretools.vscode-cosmosdb
MIT License
155 stars 69 forks source link

vCore: add an extended confirmation dialog for "risky" operations #2355

Closed tnaum-ms closed 3 weeks ago

tnaum-ms commented 1 month ago

Currently it's used in vCore only, but it also adds a new "Setting":

image

A confirmation prompt when option is DISABLED:

image

A confirmation prompt when option is ENABLED:

image


BTW: I just noticed that Azure Resources has a similar setting in place, however, they'd ask the user to input the name of the resource being deleted.

image

sevoku commented 1 month ago

Couple of thoughts:

  1. what is the default setting for azureResourceGroups.deleteConfirmation? Seems to be Enter Name? I'm wondering if it'd make more sense to reuse that to have a centralised setting, why would users want to have different preference for databases?
  2. Data Explorer is always asking for the name: image
  3. If we stick with the existing setting, we could still do the number puzzle for Click Button preference depending on the severity of the operation.
  4. Enter Name is not applicable for bulk operations such as bulk deleting documents, so for these operations we could always show the number puzzle.
  5. In your example the "default" button seems to be the correct number: image So that hitting "Enter" would be enough to confirm with the keyboard. We should make sure the "required" number is not the first/default keyboard selection.
tnaum-ms commented 1 month ago

Thank you @sevoku ! I wasn't aware of azureResourceGroups.deleteConfirmation existence. This is the best way to go. Also, Dmitrii made good remarks over chat about testing and e2e scripts. I'll update this code to use the deleteConfirmation that's already implemented and tested.

sevoku commented 1 month ago

as I wrote, I'm not advocating for the old experience, I like the puzzle especially for operations without an available name (we can't expect folks to enter document ID + partition key, and not what for bulk operations). I think it makes a lot of sense to have this "simplified" experience for the database context. However there are some more caveats:

  1. The Resources setting is speaking of "resource groups": image That's not obvious that it also affects other operations, so we should PR a wording change to Azure Resources
  2. The puzzle dialog should also have a link to the settings page, since it's mentioning that the behaviour can be changed in settings.
tnaum-ms commented 3 weeks ago

Updated settings, the "quiz" is not in there, just the two basic ones as in "Azure Resources":

image

This is the behavior. I'm using "Drop Container" from vCore:

Word Confirmation image

Button Click image

tnaum-ms commented 3 weeks ago

We have a simple helper function that takes care of reading settings and showing an appropriate warning:

/**
 * Prompts the user for a confirmation based on the configured confirmation style.
 *
 * @param title - The title of the confirmation dialog.
 * @param message - The message to display in the confirmation dialog. This message will be suffixed with instructions for a specific prompt.
 * @param expectedConfirmationWord - The word that the user must type to confirm the action when the confirmation style is set to 'Word Confirmation'.
 * @returns A promise that resolves to a boolean indicating whether the user confirmed the action.
 */
export async function getConfirmationAsInSettings(
    title: string,
    message: string,
    expectedConfirmationWord: string,
): Promise<boolean> {
    const deleteConfirmation: string | undefined = vscode.workspace
        .getConfiguration()
        .get<string>(ext.settingsKeys.confirmationStyle);

    if (deleteConfirmation === 'Word Confirmation') {
        return await getConfirmationWithWordQuestion(title, message, expectedConfirmationWord);
    }
    return await getConfirmationWithClick(title, message);
}
tnaum-ms commented 3 weeks ago

to be continued here: https://github.com/microsoft/vscode-cosmosdb/pull/2377