FEATURE: Introduce site settings which require confirmation

[martin-brennan]

Many site settings can be destructive or have huge side-effects
for a site that the admin may not be aware of when changing it.

This commit introduces a requires_confirmation attribute that
can be added to any site setting. When it is simple, a confirmation
dialog will open if that setting is changed in the admin UI,
optionally with a custom message that is defined in client.en.yml.

I have decided to make requires_confirmation a kind of enum
because we also have an existing system of confirmation for many
"default" site settings that affect user options, e.g. the ones defined
at

discourse/app/controllers/admin/site_settings_controller.rb

Lines 248 to 274 in bc7c7a8

	def user_options
	{
	default_email_mailing_list_mode: "mailing_list_mode",
	default_email_mailing_list_mode_frequency: "mailing_list_mode_frequency",
	default_email_level: "email_level",
	default_email_messages_level: "email_messages_level",
	default_topics_automatic_unpin: "automatically_unpin_topics",
	default_email_previous_replies: "email_previous_replies",
	default_email_in_reply_to: "email_in_reply_to",
	default_other_enable_quoting: "enable_quoting",
	default_other_enable_defer: "enable_defer",
	default_other_external_links_in_new_tab: "external_links_in_new_tab",
	default_other_dynamic_favicon: "dynamic_favicon",
	default_other_new_topic_duration_minutes: "new_topic_duration_minutes",
	default_other_auto_track_topics_after_msecs: "auto_track_topics_after_msecs",
	default_other_notification_level_when_replying: "notification_level_when_replying",
	default_other_like_notification_frequency: "like_notification_frequency",
	default_other_skip_new_user_tips: "skip_new_user_tips",
	default_email_digest_frequency: "digest_after_minutes",
	default_include_tl0_in_digests: "include_tl0_in_digests",
	default_text_size: "text_size_key",
	default_title_count_mode: "title_count_mode_key",
	default_hide_profile_and_presence: "hide_profile_and_presence",
	default_sidebar_link_to_filtered_list: "sidebar_link_to_filtered_list",
	default_sidebar_show_count_of_new_items: "sidebar_show_count_of_new_items",
	}
	end

. This pops a modal like so:

In a future PR I would like to consolidate these two "requires confirmation"
systems into one for site settings.

If the admin does not confirm, we reset the setting to its previous
clean value and do not save the new value.

[martin-brennan]

Need actual copy for these 3

[martin-brennan]

Note to self, we already have some form of this for certain settings which relate to user options / preferences, maybe we could consolidate this into one thing

[CvX]

now it's awaiting AND returning 🤪

anyway, I'd be good if dialog.alert would return an object that had a response promise on it, like:

const dialogInstance = await this.dialog.alert({
  buttons: [
    { label: "ok", action: () => true },
    { label: "cancel", action () => false },
  ]
});

return await dialogInstance.response;

[martin-brennan]

Yes that would be nicer 😂 Until then what should have I done here based on Joffrey's feedback?

[CvX]

Well, you could implement that 😜 but the next best thing would be to choose between async/await (no return) or return (w/o async and await) 😛

[martin-brennan]

Sorry can you clarify this a bit more? I am doing this:

async confirmChanges(settingKey) {
  return new Promise((resolve) => {
    // Fallback is needed in case the setting does not have a custom confirmation
    // prompt/confirm defined.
    this.dialog.alert({
      message: I18n.t(
        `admin.site_settings.requires_confirmation_messages.${settingKey}.prompt`,
        {
          translatedFallback: I18n.t(
            "admin.site_settings.requires_confirmation_messages.default.prompt"
          ),
        }
      ),
      buttons: [
        {
          label: I18n.t(
            `admin.site_settings.requires_confirmation_messages.${settingKey}.confirm`,
            {
              translatedFallback: I18n.t(
                "admin.site_settings.requires_confirmation_messages.default.confirm"
              ),
            }
          ),
          class: "btn-primary",
          action: () => resolve(true),
        },
        {
          label: I18n.t("no_value"),
          class: "btn-default",
          action: () => resolve(false),
        },
      ],
    });
  });
},

// further down
confirm = await this.confirmChanges(key);

What specifically should I change without changing what dialog.confirm returns?

[martin-brennan]

Like, I don't know how removing return helps in confirmChanges

[martin-brennan]

Are you saying to do:

const confirmed =  new Promise((resolve) => { //blah };
return confirmed;

?

[CvX]

ah, sorry if you need the return value then this is the way to go:

function () {
  return new Promise(…

i.e. no async and no await

[martin-brennan]

Ah cool, thanks!