From 9177fffa71adc2b5afba913614f3bb6c053561af Mon Sep 17 00:00:00 2001
From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
Date: Tue, 4 Aug 2026 19:59:05 +0530
Subject: [PATCH] fix: enforce required conversation attributes on resolve for
 macros (#15232)

---
 .../ConversationResolveAttributesModal.vue    |  11 +-
 .../useConversationRequiredAttributes.spec.js |  22 ++--
 .../useConversationRequiredAttributes.js      |   5 +-
 .../dashboard/i18n/locale/en/macros.json      |   3 +-
 .../dashboard/conversation/Macros/List.vue    | 104 +++++++++++++++++-
 .../conversation/Macros/MacroItem.vue         |  35 +-----
 .../store/modules/conversations/actions.js    |   2 +-
 app/services/macros/execution_service.rb      |   2 +
 .../enterprise/macros/execution_service.rb    |  32 ++++++
 .../services/macros/execution_service_spec.rb |  96 ++++++++++++++++
 10 files changed, 264 insertions(+), 48 deletions(-)
 create mode 100644 enterprise/app/services/enterprise/macros/execution_service.rb
 create mode 100644 spec/enterprise/services/macros/execution_service_spec.rb

diff --git a/app/javascript/dashboard/components-next/ConversationWorkflow/ConversationResolveAttributesModal.vue b/app/javascript/dashboard/components-next/ConversationWorkflow/ConversationResolveAttributesModal.vue
index 12e13f11e..7603a6013 100644
--- a/app/javascript/dashboard/components-next/ConversationWorkflow/ConversationResolveAttributesModal.vue
+++ b/app/javascript/dashboard/components-next/ConversationWorkflow/ConversationResolveAttributesModal.vue
@@ -11,7 +11,7 @@ import Input from 'dashboard/components-next/input/Input.vue';
 import ChoiceToggle from 'dashboard/components-next/input/ChoiceToggle.vue';
 import { ATTRIBUTE_TYPES } from './constants';
 
-const emit = defineEmits(['submit']);
+const emit = defineEmits(['submit', 'close']);
 
 const { t } = useI18n();
 
@@ -105,8 +105,6 @@ const comboBoxOptions = computed(() => {
 
 const close = () => {
   dialogRef.value?.close();
-  conversationContext.value = null;
-  v$.value.$reset();
 };
 
 const open = (attributes = [], initialValues = {}, context = null) => {
@@ -135,6 +133,12 @@ const open = (attributes = [], initialValues = {}, context = null) => {
   dialogRef.value?.open();
 };
 
+const handleClose = () => {
+  conversationContext.value = null;
+  v$.value.$reset();
+  emit('close');
+};
+
 const handleConfirm = async () => {
   v$.value.$touch();
   if (v$.value.$invalid) {
@@ -167,6 +171,7 @@ defineExpose({ open, close });
     "
     :disable-confirm-button="!isFormComplete"
     @confirm="handleConfirm"
+    @close="handleClose"
   >
     <div class="flex flex-col gap-4">
       <div
diff --git a/app/javascript/dashboard/composables/spec/useConversationRequiredAttributes.spec.js b/app/javascript/dashboard/composables/spec/useConversationRequiredAttributes.spec.js
index a269f3d66..4131fafff 100644
--- a/app/javascript/dashboard/composables/spec/useConversationRequiredAttributes.spec.js
+++ b/app/javascript/dashboard/composables/spec/useConversationRequiredAttributes.spec.js
@@ -304,19 +304,21 @@ describe('useConversationRequiredAttributes', () => {
       expect(result.missing[0].value).toBe('category');
     });
 
-    it('should consider undefined checkbox values as present when key exists', () => {
+    it('should detect missing checkbox when the key exists without a value', () => {
       const { checkMissingAttributes } = useConversationRequiredAttributes();
 
-      const customAttributes = {
-        priority: 'High',
-        category: 'Bug Report',
-        is_urgent: undefined, // key exists but value is undefined - still considered "filled" for checkbox
-      };
+      // The backend reads nil as missing, so the prompt has to open for it too.
+      [null, undefined].forEach(unset => {
+        const result = checkMissingAttributes({
+          priority: 'High',
+          category: 'Bug Report',
+          is_urgent: unset,
+        });
 
-      const result = checkMissingAttributes(customAttributes);
-
-      expect(result.hasMissing).toBe(false);
-      expect(result.missing).toEqual([]);
+        expect(result.hasMissing).toBe(true);
+        expect(result.missing).toHaveLength(1);
+        expect(result.missing[0].value).toBe('is_urgent');
+      });
     });
 
     it('should return no missing when no attributes are required', () => {
diff --git a/app/javascript/dashboard/composables/useConversationRequiredAttributes.js b/app/javascript/dashboard/composables/useConversationRequiredAttributes.js
index 7b6ab29b1..51c69ea3b 100644
--- a/app/javascript/dashboard/composables/useConversationRequiredAttributes.js
+++ b/app/javascript/dashboard/composables/useConversationRequiredAttributes.js
@@ -72,9 +72,10 @@ export function useConversationRequiredAttributes() {
     const missing = requiredAttributes.value.filter(attribute => {
       const value = conversationCustomAttributes[attribute.value];
 
-      // For checkbox/boolean attributes, only check if the key exists
+      // A checkbox is filled once true or false is picked. An explicit null is
+      // an unanswered prompt, which is how the modal and the backend read it.
       if (attribute.type === ATTRIBUTE_TYPES.CHECKBOX) {
-        return !(attribute.value in conversationCustomAttributes);
+        return value == null;
       }
 
       // For other attribute types, only consider null, undefined, empty string, or whitespace-only as missing
diff --git a/app/javascript/dashboard/i18n/locale/en/macros.json b/app/javascript/dashboard/i18n/locale/en/macros.json
index e51975921..342a7ae51 100644
--- a/app/javascript/dashboard/i18n/locale/en/macros.json
+++ b/app/javascript/dashboard/i18n/locale/en/macros.json
@@ -82,7 +82,8 @@
     "EXECUTE": {
       "BUTTON_TOOLTIP": "Execute",
       "PREVIEW": "Preview Macro",
-      "EXECUTED_SUCCESSFULLY": "Macro executed successfully"
+      "EXECUTED_SUCCESSFULLY": "Macro executed successfully",
+      "EXECUTED_WITHOUT_RESOLVING": "Macro executed. The conversation was not resolved because required attributes are missing."
     },
     "ERRORS": {
       "ATTRIBUTE_KEY_REQUIRED": "Attribute key is required",
diff --git a/app/javascript/dashboard/routes/dashboard/conversation/Macros/List.vue b/app/javascript/dashboard/routes/dashboard/conversation/Macros/List.vue
index 7903ae751..07fdaa4c8 100644
--- a/app/javascript/dashboard/routes/dashboard/conversation/Macros/List.vue
+++ b/app/javascript/dashboard/routes/dashboard/conversation/Macros/List.vue
@@ -1,15 +1,20 @@
 <script setup>
 import { computed, onMounted, ref } from 'vue';
+import { useI18n } from 'vue-i18n';
+import { useAlert, useTrack } from 'dashboard/composables';
 import { useStore, useMapGetter } from 'dashboard/composables/store';
 import { useAccount } from 'dashboard/composables/useAccount';
 import { useUISettings } from 'dashboard/composables/useUISettings';
+import { useConversationRequiredAttributes } from 'dashboard/composables/useConversationRequiredAttributes';
+import { CONVERSATION_EVENTS } from 'dashboard/helper/AnalyticsHelper/events';
 
 import Draggable from 'vuedraggable';
 import Spinner from 'dashboard/components-next/spinner/Spinner.vue';
 import MacroItem from './MacroItem.vue';
 import NextButton from 'dashboard/components-next/button/Button.vue';
+import ConversationResolveAttributesModal from 'dashboard/components-next/ConversationWorkflow/ConversationResolveAttributesModal.vue';
 
-defineProps({
+const props = defineProps({
   conversationId: {
     type: [Number, String],
     required: true,
@@ -17,13 +22,19 @@ defineProps({
 });
 
 const store = useStore();
+const { t } = useI18n();
 const { accountScopedUrl } = useAccount();
 const { uiSettings, updateUISettings } = useUISettings();
+const { checkMissingAttributes } = useConversationRequiredAttributes();
 
 const dragging = ref(false);
+const executingMacroId = ref(null);
+const pendingExecution = ref(null);
+const resolveAttributesModalRef = ref(null);
 
 const macros = useMapGetter('macros/getMacros');
 const uiFlags = useMapGetter('macros/getUIFlags');
+const conversationById = useMapGetter('getConversationById');
 
 const MACROS_ORDER_KEY = 'macros_display_order';
 
@@ -60,6 +71,89 @@ const onDragEnd = () => {
   dragging.value = false;
 };
 
+const customAttributesFor = conversationId =>
+  conversationById.value(conversationId)?.custom_attributes || {};
+
+// change_status is not offered by the macro builder, but the API accepts it and
+// it resolves the conversation just like resolve_conversation does. Its param is
+// stored as raw JSON, so the status can be the enum name or its integer value.
+const RESOLVED_STATUSES = ['resolved', 1];
+
+const resolvesConversation = macro =>
+  macro.actions.some(
+    ({ action_name: name, action_params: params }) =>
+      name === 'resolve_conversation' ||
+      (name === 'change_status' && RESOLVED_STATUSES.includes(params?.[0]))
+  );
+
+const runMacro = async ({ macro, conversationId }, skippedResolve = false) => {
+  try {
+    executingMacroId.value = macro.id;
+    await store.dispatch('macros/execute', {
+      macroId: macro.id,
+      conversationIds: [conversationId],
+    });
+    useTrack(CONVERSATION_EVENTS.EXECUTED_A_MACRO);
+    useAlert(
+      skippedResolve
+        ? t('MACROS.EXECUTE.EXECUTED_WITHOUT_RESOLVING')
+        : t('MACROS.EXECUTE.EXECUTED_SUCCESSFULLY')
+    );
+  } catch (error) {
+    useAlert(t('MACROS.ERROR'));
+  } finally {
+    executingMacroId.value = null;
+  }
+};
+
+const onExecuteMacro = macro => {
+  const execution = { macro, conversationId: props.conversationId };
+
+  if (!resolvesConversation(macro)) {
+    runMacro(execution);
+    return;
+  }
+
+  const customAttributes = customAttributesFor(execution.conversationId);
+  const { hasMissing, missing } = checkMissingAttributes(customAttributes);
+  if (!hasMissing) {
+    runMacro(execution);
+    return;
+  }
+
+  pendingExecution.value = execution;
+  resolveAttributesModalRef.value?.open(missing, customAttributes);
+};
+
+const onAttributesSubmit = async ({ attributes }) => {
+  const execution = pendingExecution.value;
+  pendingExecution.value = null;
+
+  try {
+    await store.dispatch('updateCustomAttributes', {
+      conversationId: execution.conversationId,
+      customAttributes: {
+        ...customAttributesFor(execution.conversationId),
+        ...attributes,
+      },
+    });
+  } catch (error) {
+    useAlert(t('CUSTOM_ATTRIBUTES.FORM.UPDATE.ERROR'));
+    return;
+  }
+
+  runMacro(execution);
+};
+
+// Dismissing the modal still runs the macro, the backend leaves the
+// conversation unresolved while the required attributes are empty.
+const onAttributesClose = () => {
+  if (!pendingExecution.value) return;
+
+  runMacro(pendingExecution.value, true);
+  pendingExecution.value = null;
+};
+
 onMounted(() => {
   store.dispatch('macros/get');
 });
@@ -103,10 +197,16 @@ onMounted(() => {
         <MacroItem
           :key="element.id"
           :macro="element"
-          :conversation-id="conversationId"
+          :is-executing="executingMacroId === element.id"
+          @execute="onExecuteMacro(element)"
         />
       </template>
     </Draggable>
+    <ConversationResolveAttributesModal
+      ref="resolveAttributesModalRef"
+      @submit="onAttributesSubmit"
+      @close="onAttributesClose"
+    />
   </div>
 </template>
 
diff --git a/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroItem.vue b/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroItem.vue
index b6de05d51..709440d97 100644
--- a/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroItem.vue
+++ b/app/javascript/dashboard/routes/dashboard/conversation/Macros/MacroItem.vue
@@ -1,47 +1,24 @@
 <script setup>
 import { ref } from 'vue';
-import { useI18n } from 'vue-i18n';
-import { useAlert } from 'dashboard/composables';
-import { useStore } from 'dashboard/composables/store';
-import { CONVERSATION_EVENTS } from '../../../../helper/AnalyticsHelper/events';
-import { useTrack } from 'dashboard/composables';
 
 import NextButton from 'dashboard/components-next/button/Button.vue';
 import MacroPreview from './MacroPreview.vue';
 
-const props = defineProps({
+defineProps({
   macro: {
     type: Object,
     required: true,
   },
-  conversationId: {
-    type: [Number, String],
-    required: true,
+  isExecuting: {
+    type: Boolean,
+    default: false,
   },
 });
 
-const store = useStore();
-const { t } = useI18n();
+defineEmits(['execute']);
 
-const isExecuting = ref(false);
 const showPreview = ref(false);
 
-const executeMacro = async macro => {
-  try {
-    isExecuting.value = true;
-    await store.dispatch('macros/execute', {
-      macroId: macro.id,
-      conversationIds: [props.conversationId],
-    });
-    useTrack(CONVERSATION_EVENTS.EXECUTED_A_MACRO);
-    useAlert(t('MACROS.EXECUTE.EXECUTED_SUCCESSFULLY'));
-  } catch (error) {
-    useAlert(t('MACROS.ERROR'));
-  } finally {
-    isExecuting.value = false;
-  }
-};
-
 const toggleMacroPreview = () => {
   showPreview.value = !showPreview.value;
 };
@@ -77,7 +54,7 @@ const closeMacroPreview = () => {
         faded
         xs
         :is-loading="isExecuting"
-        @click="executeMacro(macro)"
+        @click="$emit('execute')"
       />
     </div>
     <transition name="menu-slide">
diff --git a/app/javascript/dashboard/store/modules/conversations/actions.js b/app/javascript/dashboard/store/modules/conversations/actions.js
index f8fdecc36..47481b697 100644
--- a/app/javascript/dashboard/store/modules/conversations/actions.js
+++ b/app/javascript/dashboard/store/modules/conversations/actions.js
@@ -495,7 +495,7 @@ const actions = {
         customAttributes: custom_attributes,
       });
     } catch (error) {
-      // Handle error
+      throw new Error(error);
     }
   },
 
diff --git a/app/services/macros/execution_service.rb b/app/services/macros/execution_service.rb
index df82d4c6f..6a32ac078 100644
--- a/app/services/macros/execution_service.rb
+++ b/app/services/macros/execution_service.rb
@@ -68,3 +68,5 @@ class Macros::ExecutionService < ActionService
     WebhookJob.perform_later(webhook_url.first, payload)
   end
 end
+
+Macros::ExecutionService.include_mod_with('Macros::ExecutionService')
diff --git a/enterprise/app/services/enterprise/macros/execution_service.rb b/enterprise/app/services/enterprise/macros/execution_service.rb
new file mode 100644
index 000000000..51ba4817d
--- /dev/null
+++ b/enterprise/app/services/enterprise/macros/execution_service.rb
@@ -0,0 +1,32 @@
+module Enterprise::Macros::ExecutionService
+  def resolve_conversation(_params)
+    return if required_attributes_missing?
+
+    super
+  end
+
+  def change_status(status)
+    return if resolved_status?(status.first) && required_attributes_missing?
+
+    super
+  end
+
+  private
+
+  # Action params are stored as raw JSON, so the status arrives either as the enum
+  # name or as its integer value depending on how the macro was created.
+  def resolved_status?(status)
+    ['resolved', Conversation.statuses['resolved']].include?(status)
+  end
+
+  def required_attributes_missing?
+    return false unless @account.feature_enabled?('conversation_required_attributes')
+
+    defined_keys = @account.custom_attribute_definitions.conversation_attribute.pluck(:attribute_key)
+    required_keys = @account.conversation_required_attributes.to_a & defined_keys
+    return false if required_keys.blank?
+
+    custom_attributes = @conversation.custom_attributes || {}
+    required_keys.any? { |key| custom_attributes[key].to_s.blank? }
+  end
+end
diff --git a/spec/enterprise/services/macros/execution_service_spec.rb b/spec/enterprise/services/macros/execution_service_spec.rb
new file mode 100644
index 000000000..227bcb80e
--- /dev/null
+++ b/spec/enterprise/services/macros/execution_service_spec.rb
@@ -0,0 +1,96 @@
+require 'rails_helper'
+
+describe Macros::ExecutionService, type: :service do
+  let(:account) { create(:account) }
+  let(:user) { create(:user, account: account) }
+  let(:inbox) { create(:inbox, account: account) }
+  let(:conversation) { create(:conversation, account: account, inbox: inbox, status: :open) }
+  let(:macro) do
+    create(:macro, account: account, created_by: user, updated_by: user,
+                   actions: [{ 'action_name' => 'resolve_conversation', 'action_params' => [] }])
+  end
+
+  before do
+    create(:inbox_member, user: user, inbox: inbox)
+    create(:custom_attribute_definition, account: account, attribute_key: 'priority_level',
+                                         attribute_model: 'conversation_attribute')
+    account.enable_features('conversation_required_attributes')
+    account.update!(conversation_required_attributes: ['priority_level'])
+  end
+
+  it 'skips the resolve action when a required attribute is missing' do
+    described_class.new(macro, conversation, user).perform
+
+    expect(conversation.reload.status).to eq('open')
+  end
+
+  it 'resolves when all required attributes are filled' do
+    conversation.update!(custom_attributes: { 'priority_level' => 'high' })
+
+    described_class.new(macro, conversation, user).perform
+
+    expect(conversation.reload.status).to eq('resolved')
+  end
+
+  it 'resolves when the feature is disabled' do
+    account.disable_features('conversation_required_attributes')
+
+    described_class.new(macro, conversation, user).perform
+
+    expect(conversation.reload.status).to eq('resolved')
+  end
+
+  it 'ignores required keys whose attribute definition no longer exists' do
+    account.update!(conversation_required_attributes: %w[priority_level deleted_key])
+    conversation.update!(custom_attributes: { 'priority_level' => 'high' })
+
+    described_class.new(macro, conversation, user).perform
+
+    expect(conversation.reload.status).to eq('resolved')
+  end
+
+  context 'when the macro uses a change_status action' do
+    let(:macro) do
+      create(:macro, account: account, created_by: user, updated_by: user,
+                     actions: [{ 'action_name' => 'change_status', 'action_params' => [status] }])
+    end
+
+    context 'with a resolved status' do
+      let(:status) { 'resolved' }
+
+      it 'skips the action when a required attribute is missing' do
+        described_class.new(macro, conversation, user).perform
+
+        expect(conversation.reload.status).to eq('open')
+      end
+    end
+
+    context 'with the resolved enum value' do
+      let(:status) { Conversation.statuses['resolved'] }
+
+      it 'skips the action when a required attribute is missing' do
+        described_class.new(macro, conversation, user).perform
+
+        expect(conversation.reload.status).to eq('open')
+      end
+    end
+
+    context 'with a status other than resolved' do
+      let(:status) { 'pending' }
+
+      it 'applies the status even when a required attribute is missing' do
+        described_class.new(macro, conversation, user).perform
+
+        expect(conversation.reload.status).to eq('pending')
+      end
+    end
+  end
+
+  it 'treats a false checkbox value as filled' do
+    conversation.update!(custom_attributes: { 'priority_level' => false })
+
+    described_class.new(macro, conversation, user).perform
+
+    expect(conversation.reload.status).to eq('resolved')
+  end
+end