From 110345d659af8f881acb6500bc2aaa53542e468b Mon Sep 17 00:00:00 2001
From: Kailash Nadh <kailash@nadh.in>
Date: Mon, 7 Apr 2025 14:36:14 +0530
Subject: [PATCH] Refactor and simplify state management in campaign editor.

- Simplify and fix content conversion between formats.
- Fix state management issues.
- Rename `Apply` to `Import` on the visual template UI.
---
 frontend/src/assets/style.scss     |   3 +
 frontend/src/components/Editor.vue | 249 +++++++++++++++--------------
 i18n/en.json                       |   5 +-
 3 files changed, 138 insertions(+), 119 deletions(-)

diff --git a/frontend/src/assets/style.scss b/frontend/src/assets/style.scss
index 00feab9d..ade18198 100644
--- a/frontend/src/assets/style.scss
+++ b/frontend/src/assets/style.scss
@@ -533,6 +533,9 @@ body.is-noscroll {
   }
 }
 
+.button.is-primary[disabled] {
+  border-color: $grey-light;
+}
 .has-addons {
   .controls .button.is-primary {
     border-top-left-radius: 0;
diff --git a/frontend/src/components/Editor.vue b/frontend/src/components/Editor.vue
index 054a5acb..5bc7806f 100644
--- a/frontend/src/components/Editor.vue
+++ b/frontend/src/components/Editor.vue
@@ -4,7 +4,7 @@
     <div class="columns">
       <div class="column is-three-quarters is-inline-flex">
         <b-field :label="$t('campaigns.format')" label-position="on-border" class="mr-4 mb-0">
-          <b-select v-model="contentType">
+          <b-select v-model="contentTypeSel">
             <option :disabled="disabled" name="format" value="richtext" data-cy="check-richtext">
               {{ $t('campaigns.richText') }}
             </option>
@@ -27,10 +27,9 @@
           </b-select>
         </b-field>
 
-        <b-field v-if="computedValue.contentType !== 'visual'" :label="$t('globals.terms.baseTemplate')"
-          label-position="on-border">
+        <b-field v-if="self.contentType !== 'visual'" :label="$tc('globals.terms.template')" label-position="on-border">
           <b-select :placeholder="$t('globals.terms.none')" v-model="templateId" name="template" :disabled="disabled">
-            <template v-for="t in applicableTemplates">
+            <template v-for="t in validTemplates">
               <option :value="t.id" :key="t.id">
                 {{ t.name }}
               </option>
@@ -41,22 +40,23 @@
         <div v-else>
           <b-button v-if="!isVisualTplSelector" @click="onShowVisualTplSelector" type="is-ghost"
             icon-left="file-find-outline" data-cy="btn-select-visual-tpl">
-            {{ $t('globals.terms.copyVisualTemplate') }}
+            {{ $t('campaigns.importVisualTemplate') }}
           </b-button>
 
-          <b-field v-else :label="$t('globals.terms.copyVisualTemplate')" label-position="on-border">
-            <b-select :placeholder="$t('globals.terms.none')" v-model="visualTemplateId" name="template"
-              :disabled="disabled" class="copy-visual-template-list">
-              <template v-for="t in applicableTemplates">
+          <b-field v-else :label="$tc('globals.terms.template')" label-position="on-border">
+            <b-select :placeholder="$t('globals.terms.none')" v-model="visualTemplateId"
+              @input="() => isVisualTplDisabled = false" name="template" :disabled="disabled"
+              class="copy-visual-template-list">
+              <template v-for="t in validTemplates">
                 <option :value="t.id" :key="t.id">
                   {{ t.name }}
                 </option>
               </template>
             </b-select>
 
-            <b-button :disabled="isVisualTplApplied" class="ml-3" @click="onApplyVisualTpl" type="is-primary"
-              icon-left="content-save-outline" data-cy="btn-save-visual-tpl">
-              {{ $t('globals.terms.apply') }}
+            <b-button :disabled="disabled || isVisualTplDisabled" class="ml-3" @click="onImportVisualTpl"
+              type="is-primary" icon-left="content-save-outline" data-cy="btn-save-visual-tpl">
+              {{ $t('globals.terms.import') }}
             </b-button>
           </b-field>
         </div>
@@ -69,25 +69,25 @@
     </div>
 
     <!-- wsywig //-->
-    <richtext-editor v-if="computedValue.contentType === 'richtext'" v-model="computedValue.body" />
+    <richtext-editor v-if="self.contentType === 'richtext'" v-model="self.body" />
 
     <!-- visual editor //-->
-    <visual-editor v-if="computedValue.contentType === 'visual'" :source="computedValue.bodySource"
-      @change="onChangeVisualEditor" height="65vh" />
+    <visual-editor v-if="self.contentType === 'visual'" :source="self.bodySource" @change="onVisualEditorChange"
+      height="65vh" />
 
     <!-- raw html editor //-->
-    <html-editor v-if="computedValue.contentType === 'html'" v-model="computedValue.body" />
+    <html-editor v-if="self.contentType === 'html'" v-model="self.body" />
 
     <!-- markdown editor //-->
-    <markdown-editor v-if="computedValue.contentType === 'markdown'" v-model="computedValue.body" />
+    <markdown-editor v-if="self.contentType === 'markdown'" v-model="self.body" />
 
     <!-- plain text //-->
-    <b-input v-if="computedValue.contentType === 'plain'" v-model="computedValue.body" type="textarea" name="content"
-      ref="plainEditor" class="plain-editor" />
+    <b-input v-if="self.contentType === 'plain'" v-model="self.body" type="textarea" name="content" ref="plainEditor"
+      class="plain-editor" />
 
     <!-- campaign preview //-->
     <campaign-preview v-if="isPreviewing" is-post @close="onTogglePreview" type="campaign" :id="id" :title="title"
-      :content-type="computedValue.contentType" :template-id="templateId" :body="computedValue.body" />
+      :content-type="self.contentType" :template-id="templateId" :body="self.body" />
   </section>
 </template>
 
@@ -118,6 +118,10 @@ export default {
     title: { type: String, default: '' },
     disabled: { type: Boolean, default: false },
     templates: { type: Array, default: null },
+
+    // value is provided by the parent component.
+    // Throught the editor, `this.self` (a mutable clone of `value`) is used,
+    // instead of `this.value` directly.
     value: {
       type: Object,
       default: () => ({
@@ -133,8 +137,8 @@ export default {
     return {
       isPreviewing: false,
       isVisualTplSelector: false,
-      isVisualTplApplied: false,
-      contentType: this.$props.value.contentType,
+      isVisualTplDisabled: false,
+      contentTypeSel: this.$props.value.contentType,
       templateId: '',
       visualTemplateId: '',
     };
@@ -142,79 +146,97 @@ export default {
 
   methods: {
     onContentTypeChange(to, from) {
-      if (this.computedValue.body?.trim() === '') {
-        this.computedValue.contentType = this.contentType;
-        return;
-      }
-
-      // To avoid prompt loop.
-      if (to === this.computedValue.contentType) {
-        return;
-      }
-
-      // Content isn't empty. Warn.
+      // Ask for confirmation as pretty much all conversions are lossy.
+      const msgKey = to === 'visual' ? 'campaigns.confirmOverwriteContent' : 'campaigns.confirmSwitchFormat';
       this.$utils.confirm(
-        this.$t('campaigns.confirmSwitchFormat'),
+        this.$t(msgKey),
         () => {
-          this.computedValue.contentType = this.contentType;
+          this.convertContentType(to, from);
         },
         () => {
-          this.contentType = from;
+          // Cancelled. Reset the <select> to the last value.
+          this.contentTypeSel = from;
         },
       );
     },
 
     convertContentType(to, from) {
-      let body;
+      let body = this.self.body ?? '';
+
+      // Skip UI update (markdown => richtext, html requires a backenbd call).
       let skip = false;
 
-      if ((from === 'richtext' || from === 'html') && to === 'plain') {
-        // richtext, html => plain
-
-        // Preserve line breaks when converting HTML to plaintext.
+      // If `from` is HTML content, strip out `<body>..` etc. and keep the beautified HTML.
+      let isHTML = false;
+      if (from === 'richtext' || from === 'html' || from === 'visual') {
         const d = document.createElement('div');
-        d.innerHTML = this.beautifyHTML(this.computedValue.body);
-        body = this.trimLines(d.innerText.trim(), true);
-      } else if ((from === 'richtext' || from === 'html') && to === 'markdown') {
-        // richtext, html => markdown
-        body = turndown.turndown(this.computedValue.body).replace(/\n\n+/ig, '\n\n');
-      } else if (from === 'plain' && (to === 'richtext' || to === 'html')) {
-        // plain => richtext, html
-        body = this.computedValue.body.replace(/\n/ig, '<br>\n');
-      } else if (from === 'richtext' && to === 'html') {
-        // richtext => html
-        body = this.beautifyHTML(this.computedValue.body);
+        d.innerHTML = body;
+        body = this.beautifyHTML(d.innerHTML.trim());
+        isHTML = true;
+      }
+
+      // HTML => Non-HTML.
+      if (isHTML) {
+        switch (to) {
+          case 'plain': {
+            const d = document.createElement('div');
+            d.innerHTML = body;
+            body = this.trimLines(d.innerText.trim(), true);
+            break;
+          }
+
+          case 'markdown': {
+            body = turndown.turndown(body).replace(/\n\n+/ig, '\n\n');
+            break;
+          }
+
+          default:
+            // Switching between HTML formats, no need to do anything further
+            // as body is already beautified.
+            // richtext|html => visual, the contents are simply lost.
+            break;
+        }
+
+        // Markdown to HTML requires a backend call.
       } else if (from === 'markdown' && (to === 'richtext' || to === 'html')) {
-        // Skip default update.
         skip = true;
-        // markdown => richtext, html.
         this.$api.convertCampaignContent({
-          id: 1, body: this.computedValue.body, from, to,
+          id: 1, body, from, to,
         }).then((data) => {
           this.$nextTick(() => {
-            this.computedValue.body = this.beautifyHTML(data.trim());
-            this.computedValue.bodySource = null;
+            // Both type + body should be updated in one cycle to avoid firing
+            // multiple events.
+            this.self.contentType = to;
+            this.self.body = this.beautifyHTML(data.trim());
           });
         });
+
+        // Plain to an HTML type, change plain line breaks to HTML breaks.
+      } else if (from === 'plain' && (to === 'richtext' || to === 'html')) {
+        body = body.replace(/\n/ig, '<br>\n');
       }
 
-      if (!skip) {
-        // Update the current body.
-        this.$nextTick(() => {
-          this.computedValue.body = body;
-
-          // If not visual editor then set bodySource to null
-          // this makes sure previous bodySource is not used when switching to visual editor.
-          if (to !== 'visual') {
-            this.computedValue.bodySource = null;
-          }
-        });
+      // =======================================================================
+      // If the target is visual, empty the visual editor's block content source.
+      if (to !== 'visual') {
+        this.self.bodySource = null;
       }
 
-      // Reset template ID only if its converted to or from visual template.
+      // Reset the campaign template ID if its converted to or from visual template.
       if (to === 'visual' || from === 'visual') {
         this.templateId = null;
-        this.computedValue.templateId = null;
+        this.self.templateId = null;
+      }
+
+      // =======================================================================
+      // Apply the conversion on the editor UI.
+      if (!skip) {
+        this.$nextTick(() => {
+          // Both type + body should be updated in one cycle to avoid firing
+          // multiple events.
+          this.self.contentType = to;
+          this.self.body = body;
+        });
       }
     },
 
@@ -229,9 +251,9 @@ export default {
       }
     },
 
-    onChangeVisualEditor({ body, source }) {
-      this.computedValue.body = body;
-      this.computedValue.bodySource = source;
+    onVisualEditorChange({ body, source }) {
+      this.self.body = body;
+      this.self.bodySource = source;
     },
 
     beautifyHTML(str) {
@@ -267,33 +289,33 @@ export default {
       this.setDefaultTemplate();
     },
 
-    onApplyVisualTpl() {
+    onImportVisualTpl() {
       this.$utils.confirm(
-        this.$t('campaigns.confirmApplyVisualTemplate'),
+        this.$t('campaigns.confirmOverwriteContent'),
         () => {
           let found = false;
           this.templates.forEach((t) => {
             if (t.id === this.visualTemplateId) {
               found = true;
-              this.computedValue.body = t.body;
-              this.computedValue.bodySource = t.bodySource;
+              this.self.body = t.body;
+              this.self.bodySource = t.bodySource;
 
               // Deplay update so that applied template is propogated to visual editor
               // and it doesn't enable the apply button again. Delay here is arbitrary.
               setTimeout(() => {
-                this.isVisualTplApplied = true;
+                this.isVisualTplDisabled = true;
               }, 250);
             }
           });
 
           if (!found) {
-            this.computedValue.body = '';
-            this.computedValue.bodySource = null;
+            this.self.body = '';
+            this.self.bodySource = null;
 
             // Deplay update so that applied template is propogated to visual editor
             // and it doesn't enable the apply button again. Delay here is arbitrary.
             setTimeout(() => {
-              this.isVisualTplApplied = true;
+              this.isVisualTplDisabled = true;
             }, 250);
           }
         },
@@ -301,18 +323,18 @@ export default {
     },
 
     setDefaultTemplate() {
-      if (this.computedValue.contentType === 'visual') {
-        this.visualTemplateId = this.applicableTemplates[0]?.id || null;
+      if (this.self.contentType === 'visual') {
+        this.visualTemplateId = this.validTemplates[0]?.id || null;
       } else {
-        const defaultTemplate = this.applicableTemplates.find((t) => t.isDefault === true);
-        this.templateId = defaultTemplate?.id || this.applicableTemplates[0]?.id || null;
+        const defaultTemplate = this.validTemplates.find((t) => t.isDefault === true);
+        this.templateId = defaultTemplate?.id || this.validTemplates[0]?.id || null;
       }
     },
   },
 
   mounted() {
     // Set initial content type for the selector.
-    this.contentType = this.value.contentType;
+    this.contentTypeSel = this.value.contentType;
     this.templateId = this.value.templateId;
 
     window.addEventListener('keydown', this.onPreviewShortcut);
@@ -325,53 +347,48 @@ export default {
   computed: {
     ...mapState(['serverConfig']),
 
-    computedValue: {
+    // This is a clone of the incoming `value` prop that's mutated here.
+    self: {
       get() {
         return this.value;
       },
-      set(newValue) {
-        this.$emit('input', newValue);
+
+      // Any change to the local copy, emit it to the parent.
+      set(val) {
+        this.$emit('input', val);
       },
     },
 
-    applicableTemplates() {
-      if (this.computedValue.contentType === 'visual') {
-        return this.templates.filter((t) => t.type === 'campaign_visual');
-      }
-      return this.templates.filter((t) => t.type === 'campaign');
+    // Returns the list of valid (visual vs. normal) templates for the template dropdown.
+    validTemplates() {
+      const typ = this.self.contentType === 'visual' ? 'campaign_visual' : 'campaign';
+      return this.templates.filter((t) => (t.type === typ));
     },
   },
 
   watch: {
-    contentType(to, from) {
-      this.onContentTypeChange(to, from, true);
-    },
-
-    // eslint-disable-next-line func-names
-    'computedValue.contentType': function (to, from) {
-      this.convertContentType(to, from);
-    },
-
-    applicableTemplates() {
+    validTemplates() {
+      // When the filtered list of validTemplates changes (visual vs. regular),
+      // select the appropriate 'default' in the template select list.
       this.setDefaultTemplate();
     },
 
+    contentTypeSel(to, from) {
+      // Show the conversion prompt if the value in the dropdown isn't the same
+      // as the current selection. This happens when eg: contentTypeSel = html -> visual happens
+      // in the selector, the prompt is shown, and Cancel is clicked,
+      // at which point, contentTypeSel = html again, which triggers this event.
+      if (from !== to && to !== this.self.contentType) {
+        this.onContentTypeChange(to, from);
+      }
+    },
+
     templateId(to) {
-      if (this.computedValue.templateId === to) {
+      if (this.self.templateId === to) {
         return;
       }
-      this.computedValue.templateId = to;
-    },
 
-    // eslint-disable-next-line func-names
-    'computedValue.bodySource': function (to, from) {
-      this.isVisualTplApplied = !(JSON.stringify(to) !== JSON.stringify(from));
-    },
-
-    visualTemplateId(to, from) {
-      if (from && from !== to) {
-        this.isVisualTplApplied = false;
-      }
+      this.self.templateId = to;
     },
   },
 };
diff --git a/i18n/en.json b/i18n/en.json
index 9f0044e3..fbeb23aa 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -74,6 +74,7 @@
     "campaigns.rawHTML": "Raw HTML",
     "campaigns.removeAltText": "Remove alternate plain text message",
     "campaigns.richText": "Rich text",
+    "campaigns.importVisualTemplate": "Import visual template",
     "campaigns.visual": "Visual",
     "campaigns.format": "Format",
     "campaigns.schedule": "Schedule campaign",
@@ -239,13 +240,11 @@
     "globals.terms.tags": "Tags",
     "globals.terms.template": "Template | Templates",
     "globals.terms.templates": "Templates",
-    "globals.terms.baseTemplate": "Base template",
-    "globals.terms.applyVisualTemplate": "Apply visual template",
     "globals.terms.tx": "Transactional | Transactional",
     "globals.terms.user": "User | Users",
     "globals.terms.users": "Users",
     "globals.terms.year": "Year | Years",
-    "globals.terms.apply": "Apply",
+    "globals.terms.import": "Import",
     "import.alreadyRunning": "An import is already running. Wait for it to finish or stop it before trying again.",
     "import.blocklist": "Blocklist",
     "import.csvDelim": "CSV delimiter",