FIX: Force composer markdown mode in email template editing (#34668)

For now, we want to force the markdown mode for email template
editing for admins, as this is a niche area and doesn't much
benefit from RTE for now...we may want to revert this decision
in future and do a better solution here.

This commit also fixes an issue where, for templates like
system_messages.reviewables_reminder which have a `one` and
`other` key for their text body (and thus "multiple bodies"),
we were showing `object Object` in the editor. Instead, we
want to do the same thing we do for multiple subjects, which
is provide a link to site texts with the text:

> This email template has multiple bodies.

There is only one known case of this happening so far, but at least
this minimally handles the issue for now.

app/assets/javascripts/admin/addon/controllers/admin-email-templates-edit.js

@@ -6,6 +6,7 @@ import { service } from "@ember/service";
 import BufferedProxy from "ember-buffered-proxy/proxy";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import discourseComputed from "discourse/lib/decorators";
+import { isObject } from "discourse/lib/object";
 import { i18n } from "discourse-i18n";
 
 export default class AdminEmailTemplatesEditController extends Controller {
@@ -39,6 +40,15 @@ export default class AdminEmailTemplatesEditController extends Controller {
     }
   }
 
+  @discourseComputed("buffered")
+  hasMultipleBodyTemplates(buffered) {
+    if (!isObject(buffered.getProperties("body")["body"])) {
+      return false;
+    } else {
+      return buffered.getProperties("id")["id"];
+    }
+  }
+
   @action
   saveChanges() {
     this.set("saved", false);

app/assets/javascripts/admin/addon/templates/email-templates-edit.gjs

@@ -23,16 +23,35 @@ export default RouteTemplate(
         <h3><LinkTo
             @route="adminSiteText"
             @query={{hash q=@controller.hasMultipleSubjects}}
+            class="email-template__has-multiple-subjects"
           >{{i18n
               "admin.customize.email_templates.multiple_subjects"
             }}</LinkTo></h3>
       {{else}}
-        <Input @value={{@controller.buffered.subject}} />
+        <Input
+          @value={{@controller.buffered.subject}}
+          class="email-template__subject"
+        />
       {{/if}}
       <br />
 
       <label>{{i18n "admin.customize.email_templates.body"}}</label>
-      <DEditor @value={{@controller.buffered.body}} />
+
+      {{#if @controller.hasMultipleBodyTemplates}}
+        <h3><LinkTo
+            @route="adminSiteText"
+            @query={{hash q=@controller.hasMultipleBodyTemplates}}
+            class="email-template__has-multiple-bodies"
+          >{{i18n
+              "admin.customize.email_templates.multiple_bodies"
+            }}</LinkTo></h3>
+      {{else}}
+        <DEditor
+          @value={{@controller.buffered.body}}
+          @forceEditorMode="markdown"
+          class="email-template__body"
+        />
+      {{/if}}
 
       <SaveControls
         @model={{@controller.emailTemplate}}

app/assets/javascripts/discourse/app/components/d-editor.gjs

@@ -23,6 +23,7 @@ import EmojiPickerDetached from "discourse/components/emoji-picker/detached";
 import UpsertHyperlink from "discourse/components/modal/upsert-hyperlink";
 import PluginOutlet from "discourse/components/plugin-outlet";
 import PopupInputTip from "discourse/components/popup-input-tip";
+import concatClass from "discourse/helpers/concat-class";
 import renderEmojiAutocomplete from "discourse/lib/autocomplete/emoji";
 import userAutocomplete from "discourse/lib/autocomplete/user";
 import Toolbar from "discourse/lib/composer/toolbar";
@@ -94,18 +95,7 @@ export default class DEditor extends Component {
     this.register = getRegister(this);
 
     this.setupToolbar();
-
-    if (this.siteSettings.rich_editor) {
-      // TODO (martin) Remove this once we are sure all users have migrated
-      // to the new rich editor preference, or a few months after the 3.5 release.
-      await this.handleOldRichEditorPreference();
-
-      if (this.currentUser.useRichEditor) {
-        this.editorComponent = await loadRichEditor();
-      }
-    }
-
-    this.editorComponent ??= TextareaEditor;
+    this.setupEditorMode();
   }
 
   setupToolbar() {
@@ -121,6 +111,29 @@ export default class DEditor extends Component {
     }
   }
 
+  async setupEditorMode() {
+    if (this.forceEditorMode) {
+      if (this.forceEditorMode === USER_OPTION_COMPOSITION_MODES.rich) {
+        this.editorComponent = await loadRichEditor();
+      } else {
+        this.editorComponent = TextareaEditor;
+      }
+      return;
+    }
+
+    if (this.siteSettings.rich_editor) {
+      // TODO (martin) Remove this once we are sure all users have migrated
+      // to the new rich editor preference, or a few months after the 3.5 release.
+      await this.handleOldRichEditorPreference();
+
+      if (this.currentUser.useRichEditor) {
+        this.editorComponent = await loadRichEditor();
+      }
+    }
+
+    this.editorComponent ??= TextareaEditor;
+  }
+
   async handleOldRichEditorPreference() {
     const oldValue = this.keyValueStore.get("d-editor-prefers-rich-editor");
 
@@ -145,6 +158,11 @@ export default class DEditor extends Component {
     return null;
   }
 
+  @discourseComputed("siteSettings.rich_editor", "forceEditorMode")
+  showEditorModeToggle() {
+    return this.siteSettings.rich_editor && !this.forceEditorMode;
+  }
+
   _readyNow() {
     this.set("ready", true);
 
@@ -158,6 +176,14 @@ export default class DEditor extends Component {
     this._previewMutationObserver = this._disablePreviewTabIndex();
   }
 
+  get editorContainerModeClass() {
+    if (this.isRichEditorEnabled) {
+      return "--rich-editor-enabled";
+    } else {
+      return "--markdown-editor-enabled";
+    }
+  }
+
   get keymap() {
     const keymap = {};
 
@@ -211,7 +237,7 @@ export default class DEditor extends Component {
     // itsatrap expects the return value to be false to prevent default
     keymap["tab"] = () => !this.textManipulation.indentSelection("right");
     keymap["shift+tab"] = () => !this.textManipulation.indentSelection("left");
-    if (this.siteSettings.rich_editor) {
+    if (this.siteSettings.rich_editor && !this.forceEditorMode) {
       keymap["ctrl+m"] = () => this.toggleRichEditor();
     }
 
@@ -604,6 +630,11 @@ export default class DEditor extends Component {
 
   @action
   async toggleRichEditor() {
+    // Can't toggle if only rich/markdown is allowed.
+    if (this.forceEditorMode) {
+      return;
+    }
+
     // The ProsemirrorEditor component is loaded here, adding this comment because
     // otherwise it's hard to find where the component is rendered by name.
     this.editorComponent = this.isRichEditorEnabled
@@ -739,8 +770,7 @@ export default class DEditor extends Component {
 
   <template>
     <div
-      class="d-editor-container
-        {{if this.siteSettings.rich_editor '--rich-editor-enabled'}}"
+      class={{concatClass "d-editor-container" this.editorContainerModeClass}}
     >
       <div class="d-editor-textarea-column">
         {{yield}}
@@ -768,7 +798,7 @@ export default class DEditor extends Component {
             </div>
           {{else}}
             <div class="d-editor-button-bar" role="toolbar">
-              {{#if this.siteSettings.rich_editor}}
+              {{#if this.showEditorModeToggle}}
                 <ToggleSwitch
                   @preventFocus={{true}}
                   @disabled={{@disableSubmit}}

app/assets/javascripts/discourse/app/lib/object.js

@@ -1,4 +1,4 @@
-function isObject(obj) {
+export function isObject(obj) {
   return obj && typeof obj === "object";
 }
 

config/locales/client.en.yml

@@ -6884,6 +6884,7 @@ en:
           title: "Email"
           subject: "Subject"
           multiple_subjects: "This email template has multiple subjects."
+          multiple_bodies: "This email template has multiple bodies."
           body: "Body"
           revert: "Revert Changes"
           revert_confirm: "Are you sure you want to revert your changes?"

plugins/chat/spec/system/rich_editor_extension_transcript_spec.rb

@@ -37,7 +37,7 @@ describe "chat transcripts in rich editor", type: :system do
 
     cdp.copy_paste(
       markdown,
-      css_selector: PageObjects::Components::Composer::COMPOSER_INPUT_SELECTOR,
+      css_selector: PageObjects::Components::Composer.new.composer_input_selector,
     )
 
     expect(rich).to have_css(".chat-transcript", text: channel.name)
@@ -62,7 +62,7 @@ describe "chat transcripts in rich editor", type: :system do
 
     cdp.copy_paste(
       markdown,
-      css_selector: PageObjects::Components::Composer::COMPOSER_INPUT_SELECTOR,
+      css_selector: PageObjects::Components::Composer.new.composer_input_selector,
     )
 
     expect(rich).to have_css(".chat-transcript.chat-transcript-chained", count: 2)

spec/system/admin_email_templates_spec.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+describe "Admin Email Templates", type: :system do
+  fab!(:admin)
+
+  let(:email_templates_page) { PageObjects::Pages::AdminEmailTemplates.new }
+  let(:composer) { PageObjects::Components::Composer.new(".email-template__body") }
+
+  before { sign_in(admin) }
+
+  it "forces markdown mode and doesn't allow the user to toggle the rich text editor" do
+    email_templates_page.visit_template("user_notifications.account_exists")
+
+    expect(composer).to have_markdown_editor_active
+    expect(composer).to have_no_toggle_switch
+  end
+
+  it "can edit an email template" do
+    email_templates_page.visit_template("user_notifications.account_exists")
+
+    subject_text = "Modified test subject #{SecureRandom.hex(8)}"
+    email_templates_page.edit_subject(subject_text)
+
+    body_text =
+      "This is a modified test body with some **markdown** formatting #{SecureRandom.hex(8)}"
+    email_templates_page.edit_body(body_text)
+
+    expect(email_templates_page).to have_preview_content(
+      "This is a modified test body with some markdown formatting",
+    )
+    expect(page).to have_css(".d-editor-preview strong", text: "markdown")
+
+    email_templates_page.save_changes
+    expect(page).to have_css(".save-button .saved")
+
+    email_templates_page.visit_template("user_notifications.account_exists")
+    expect(email_templates_page).to have_subject_value(subject_text)
+    expect(composer).to have_value(body_text)
+  end
+
+  it "shows link to site texts for template with multiple subjects" do
+    email_templates_page.visit_template("system_messages.pending_users_reminder")
+    expect(email_templates_page).to have_multiple_subjects_link(
+      "#{Discourse.base_url}/admin/customize/site_texts?q=system_messages.pending_users_reminder",
+    )
+  end
+
+  it "shows link to site texts for template with multiple bodies" do
+    email_templates_page.visit_template("system_messages.reviewables_reminder")
+    expect(email_templates_page).to have_multiple_bodies_link(
+      "#{Discourse.base_url}/admin/customize/site_texts?q=system_messages.reviewables_reminder",
+    )
+  end
+end

spec/system/page_objects/components/composer.rb

@@ -10,6 +10,10 @@ module PageObjects
       RICH_EDITOR = ".d-editor-input.ProseMirror"
       POST_LANGUAGE_SELECTOR = ".post-language-selector"
 
+      def initialize(composer_id = COMPOSER_ID)
+        @composer_id = composer_id
+      end
+
       def rich_editor
         find(RICH_EDITOR)
       end
@@ -23,15 +27,15 @@ module PageObjects
       end
 
       def opened?
-        page.has_css?("#{COMPOSER_ID}.open")
+        page.has_css?("#{@composer_id}.open")
       end
 
       def closed?
-        page.has_css?("#{COMPOSER_ID}.closed", visible: :all)
+        page.has_css?("#{@composer_id}.closed", visible: :all)
       end
 
       def minimized?
-        page.has_css?("#{COMPOSER_ID}.draft")
+        page.has_css?("#{@composer_id}.draft")
       end
 
       def open_composer_actions
@@ -49,22 +53,22 @@ module PageObjects
       end
 
       def focus
-        find(COMPOSER_INPUT_SELECTOR).click
+        find(composer_input_selector).click
         self
       end
 
       def fill_title(title)
-        find("#{COMPOSER_ID} #reply-title").fill_in(with: title)
+        find("#{@composer_id} #reply-title").fill_in(with: title)
         self
       end
 
       def fill_content(content)
-        find("#{COMPOSER_ID} .d-editor .d-editor-input").fill_in(with: content)
+        find("#{@composer_id} .d-editor .d-editor-input").fill_in(with: content)
         self
       end
 
       def minimize
-        find("#{COMPOSER_ID} .toggle-minimize").click
+        find("#{@composer_id} .toggle-minimize").click
         self
       end
 
@@ -106,11 +110,11 @@ module PageObjects
       end
 
       def reply_button_focused?
-        page.has_css?("#{COMPOSER_ID} .btn-primary:focus")
+        page.has_css?("#{@composer_id} .btn-primary:focus")
       end
 
       def create
-        find("#{COMPOSER_ID} .btn-primary").click
+        find("#{@composer_id} .btn-primary").click
       end
 
       def action(action_title)
@@ -118,11 +122,11 @@ module PageObjects
       end
 
       def button_label
-        find("#{COMPOSER_ID} .btn-primary .d-button-label")
+        find("#{@composer_id} .btn-primary .d-button-label")
       end
 
       def emoji_picker
-        find("#{COMPOSER_ID} .emoji-picker")
+        find("#{@composer_id} .emoji-picker")
       end
 
       def emoji_autocomplete
@@ -134,7 +138,7 @@ module PageObjects
       end
 
       def locale
-        find("#{COMPOSER_ID} #{POST_LANGUAGE_SELECTOR}")
+        find("#{@composer_id} #{POST_LANGUAGE_SELECTOR}")
       end
 
       def set_locale(locale)
@@ -148,7 +152,7 @@ module PageObjects
       end
 
       def preview
-        find("#{COMPOSER_ID} .d-editor-preview-wrapper")
+        find("#{@composer_id} .d-editor-preview-wrapper")
       end
 
       def has_discard_draft_modal?
@@ -193,30 +197,32 @@ module PageObjects
         page.has_no_css?(emoji_preview_selector(emoji))
       end
 
-      COMPOSER_INPUT_SELECTOR = "#{COMPOSER_ID} .d-editor-input"
+      def composer_input_selector
+        "#{@composer_id} .d-editor-input"
+      end
 
       def has_no_composer_input?
-        page.has_no_css?(COMPOSER_INPUT_SELECTOR)
+        page.has_no_css?(composer_input_selector)
       end
 
       def has_composer_input?
-        page.has_css?(COMPOSER_INPUT_SELECTOR)
+        page.has_css?(composer_input_selector)
       end
 
       def has_composer_preview?
-        page.has_css?("#{COMPOSER_ID} .d-editor-preview-wrapper")
+        page.has_css?("#{@composer_id} .d-editor-preview-wrapper")
       end
 
       def has_no_composer_preview?
-        page.has_no_css?("#{COMPOSER_ID} .d-editor-preview-wrapper")
+        page.has_no_css?("#{@composer_id} .d-editor-preview-wrapper")
       end
 
       def has_composer_preview_toggle?
-        page.has_css?("#{COMPOSER_ID} .toggle-preview")
+        page.has_css?("#{@composer_id} .toggle-preview")
       end
 
       def has_no_composer_preview_toggle?
-        page.has_no_css?("#{COMPOSER_ID} .toggle-preview")
+        page.has_no_css?("#{@composer_id} .toggle-preview")
       end
 
       def has_form_template?
@@ -268,11 +274,11 @@ module PageObjects
       end
 
       def composer_input
-        find("#{COMPOSER_ID} .d-editor .d-editor-input")
+        find("#{@composer_id} .d-editor-input")
       end
 
       def composer_popup
-        find("#{COMPOSER_ID} .composer-popup")
+        find("#{@composer_id} .composer-popup")
       end
 
       def form_template_field(field)
@@ -282,7 +288,7 @@ module PageObjects
       def move_cursor_after(text)
         execute_script(<<~JS, text)
           const text = arguments[0];
-          const composer = document.querySelector("#{COMPOSER_ID} .d-editor-input");
+          const composer = document.querySelector("#{@composer_id} .d-editor-input");
           const index = composer.value.indexOf(text);
           const position = index + text.length;
 
@@ -292,12 +298,12 @@ module PageObjects
       end
 
       def select_all
-        find(COMPOSER_INPUT_SELECTOR).send_keys([PLATFORM_KEY_MODIFIER, "a"])
+        find(composer_input_selector).send_keys([PLATFORM_KEY_MODIFIER, "a"])
       end
 
       def select_range(start_index, length)
         execute_script(<<~JS, text)
-          const composer = document.querySelector("#{COMPOSER_ID} .d-editor-input");
+          const composer = document.querySelector("#{@composer_id} .d-editor-input");
           composer.focus();
           composer.setSelectionRange(#{start_index}, #{length});
         JS
@@ -309,23 +315,23 @@ module PageObjects
       end
 
       def submit
-        find("#{COMPOSER_ID} .save-or-cancel .create").click
+        find("#{@composer_id} .save-or-cancel .create").click
       end
 
       def discard
-        find("#{COMPOSER_ID} .discard-button").click
+        find("#{@composer_id} .discard-button").click
       end
 
       def close
-        find("#{COMPOSER_ID} .toggle-save-and-close").click
+        find("#{@composer_id} .toggle-save-and-close").click
       end
 
       def has_no_in_progress_uploads?
-        find("#{COMPOSER_ID}").has_no_css?("#file-uploading")
+        find("#{@composer_id}").has_no_css?("#file-uploading")
       end
 
       def has_in_progress_uploads?
-        find("#{COMPOSER_ID}").has_css?("#file-uploading")
+        find("#{@composer_id}").has_css?("#file-uploading")
       end
 
       def select_pm_user(username)
@@ -337,11 +343,11 @@ module PageObjects
       end
 
       def has_rich_editor_active?
-        find("#{COMPOSER_ID}").has_css?(".composer-toggle-switch.--rte")
+        find("#{@composer_id}").has_css?(".d-editor-container.--rich-editor-enabled")
       end
 
       def has_no_rich_editor_active?
-        find("#{COMPOSER_ID}").has_css?(".composer-toggle-switch.--markdown")
+        find("#{@composer_id}").has_css?(".d-editor-container.--markdown-editor-enabled")
       end
 
       def has_markdown_editor_active?
@@ -362,8 +368,16 @@ module PageObjects
         self
       end
 
+      def has_toggle_switch?
+        page.has_css?("#{@composer_id} .composer-toggle-switch")
+      end
+
+      def has_no_toggle_switch?
+        page.has_no_css?("#{@composer_id} .composer-toggle-switch")
+      end
+
       def editor_toggle_switch
-        find("#{COMPOSER_ID} .composer-toggle-switch")
+        find("#{@composer_id} .composer-toggle-switch")
       end
 
       private

spec/system/page_objects/pages/admin_email_templates.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class AdminEmailTemplates < PageObjects::Pages::AdminBase
+      def visit
+        page.visit("/admin/email/templates")
+        self
+      end
+
+      def visit_template(template_id)
+        page.visit("/admin/email/templates/#{template_id}")
+        self
+      end
+
+      def has_template?(template_name)
+        has_css?("td", text: template_name)
+      end
+
+      def click_template(template_name)
+        find("td", text: template_name).click
+        self
+      end
+
+      def edit_subject(text)
+        find("input.email-template__subject").fill_in(with: text)
+        self
+      end
+
+      def has_subject_value?(value)
+        try_until_success { expect(find("input.email-template__subject").value).to eq(value) }
+      end
+
+      def edit_body(text)
+        find(".d-editor-input").fill_in(with: text)
+        self
+      end
+
+      def has_preview_content?(text)
+        has_css?(".d-editor-preview", text: text)
+      end
+
+      def save_changes
+        find(".save-changes").click
+        self
+      end
+
+      def has_multiple_subjects_link?(href)
+        link = find(".email-template__has-multiple-subjects")
+        expect(link[:href]).to eq(href)
+        expect(link.text).to eq(
+          I18n.t("admin_js.admin.customize.email_templates.multiple_subjects"),
+        )
+      end
+
+      def has_multiple_bodies_link?(href)
+        link = find(".email-template__has-multiple-bodies")
+        expect(link[:href]).to eq(href)
+        expect(link.text).to eq(I18n.t("admin_js.admin.customize.email_templates.multiple_bodies"))
+      end
+    end
+  end
+end