gsadmin/libredesk
1
0
Fork 0
mirror of https://github.com/abhinavxd/libredesk.git synced 2026-09-24 19:32:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix automation rule feedback loops and scope the starts with operator

Browse Source 
An automation rule that listens on an event and then writes the same field
re-fires its own event, so the rule matches again and loops forever. Status
already returned early on a no-op write, but priority and user assignment did
not, so those two could spin a worker doing a DB write per lap. Add the same
unchanged-value guard to both.

Suppression while the engine applies actions was a plain flag in a sync.Map,
so with more than one worker on the same conversation the first one to finish
deleted the key while the other was still applying actions. Make it a refcount
so each worker releases only its own claim.

The starts with operator is only implemented by the automation evaluator, not
the SQL filter builder, so give automation text fields their own operator list
instead of adding it to the shared one.

Also switch the notify action to its own recipients field type, fix the snooze
duration hint since the backend only takes Go duration units, trim and
lowercase notify recipient entries, and treat a missing previous value as no
match rather than an empty string.
This commit is contained in:
Abhinav Raut
2026-08-05 18:01:20 +05:30
parent fa591deb7a
commit aa0deedd48
10 changed files with 118 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/apps/main/src/composables/useConversationFilters.js
+6 -6
View File
@@ -29,11 +29,11 @@ export function useConversationFilters () {
}
const customAttributeDataTypeToFieldOperators = {
'text': FIELD_OPERATORS.TEXT,
'text': FIELD_OPERATORS.TEXT_AUTOMATION,
'number': FIELD_OPERATORS.NUMBER,
'checkbox': FIELD_OPERATORS.BOOLEAN,
'date': FIELD_OPERATORS.DATE,
'link': FIELD_OPERATORS.TEXT,
'link': FIELD_OPERATORS.TEXT_AUTOMATION,
'list': FIELD_OPERATORS.SELECT,
}
@@ -164,17 +164,17 @@ export function useConversationFilters () {
contact_email: {
label: t('globals.terms.email'),
type: FIELD_TYPE.TEXT,
operators: FIELD_OPERATORS.TEXT
operators: FIELD_OPERATORS.TEXT_AUTOMATION
},
content: {
label: t('globals.terms.content'),
type: FIELD_TYPE.TEXT,
operators: FIELD_OPERATORS.TEXT
operators: FIELD_OPERATORS.TEXT_AUTOMATION
},
subject: {
label: t('globals.terms.subject'),
type: FIELD_TYPE.TEXT,
operators: FIELD_OPERATORS.TEXT
operators: FIELD_OPERATORS.TEXT_AUTOMATION
},
status: {
label: t('globals.terms.status'),
@@ -336,7 +336,7 @@ export function useConversationFilters () {
},
notify: {
label: t('actions.notify'),
type: FIELD_TYPE.TAG
type: FIELD_TYPE.RECIPIENTS
},
snooze: {
label: t('actions.snooze'),
frontend/apps/main/src/constants/filterConfig.js
+10
View File
@@ -16,6 +16,7 @@ export const FIELD_TYPE = {
BOOLEAN: 'boolean',
DATE: 'date',
WEBHOOK: 'webhook',
RECIPIENTS: 'recipients',
}
export const OPERATOR = {
@@ -46,6 +47,15 @@ export const FIELD_OPERATORS = {
SELECT: [OPERATOR.EQUALS, OPERATOR.NOT_EQUALS, OPERATOR.SET, OPERATOR.NOT_SET],
BOOLEAN: [OPERATOR.EQUALS, OPERATOR.NOT_EQUALS],
TEXT: [
OPERATOR.EQUALS,
OPERATOR.NOT_EQUALS,
OPERATOR.SET,
OPERATOR.NOT_SET,
OPERATOR.CONTAINS,
OPERATOR.NOT_CONTAINS
],
// "starts with" is only implemented by the automation evaluator, not the SQL filter builder.
TEXT_AUTOMATION: [
OPERATOR.EQUALS,
OPERATOR.NOT_EQUALS,
OPERATOR.SET,
frontend/apps/main/src/features/admin/automation/ActionBox.vue
+5 -6
View File
@@ -34,11 +34,7 @@
<!-- Value -->
<div
v-if="
action.type &&
conversationActions[action.type]?.type === 'tag' &&
action.type !== 'notify'
"
v-if="action.type && conversationActions[action.type]?.type === 'tag'"
class="w-full"
>
<SelectTag
@@ -48,7 +44,10 @@
/>
</div>
<div v-if="action.type === 'notify'" class="w-full">
<div
v-if="action.type && conversationActions[action.type]?.type === 'recipients'"
class="w-full"
>
<TagsInput
:modelValue="action.value || []"
@update:modelValue="(value) => handleNotifyChange(value, index)"
frontend/apps/main/src/stores/webhook.js
+3 -3
View File
@@ -1,9 +1,9 @@
import { ref, computed } from 'vue'
import { defineStore } from 'pinia'
import { handleHTTPError } from '@shared-ui/utils/http.js'
import { useEmitter } from '../composables/useEmitter'
import { EMITTER_EVENTS } from '../constants/emitterEvents'
import api from '../api'
import { useEmitter } from '@main/composables/useEmitter'
import { EMITTER_EVENTS } from '@main/constants/emitterEvents'
import api from '@main/api'
export const useWebhookStore = defineStore('webhook', () => {
const webhooks = ref([])
i18n/en-US.json
+1 -1
View File
@@ -1163,7 +1163,7 @@
"placeholders.selectType": "Select type",
"placeholders.selectValue": "Select value",
"placeholders.selectWebhook": "Select webhook",
"placeholders.snoozeDuration": "e.g. 3h, 2h30m, 1d",
"placeholders.snoozeDuration": "e.g. 30m, 3h, 2h30m",
"placeholders.startConversation": "Start conversation",
"placeholders.tellUsAboutYourself": "Tell us about yourself",
"placeholders.webhookEventName": "e.g. priority.escalated",
internal/automation/automation.go
+42 -12
View File
@@ -47,17 +47,20 @@ type ConversationTask struct {
}
type Engine struct {
rules []models.Rule
rulesMu sync.RWMutex
q queries
lo *logf.Logger
i18n *i18n.I18n
conversationStore conversationStore
taskQueue chan ConversationTask
closed bool
closedMu sync.RWMutex
wg sync.WaitGroup
suppressAutomation sync.Map
rules []models.Rule
rulesMu sync.RWMutex
q queries
lo *logf.Logger
i18n *i18n.I18n
conversationStore conversationStore
taskQueue chan ConversationTask
closed bool
closedMu sync.RWMutex
wg sync.WaitGroup
// Refcounted per conversation as multiple workers can apply actions on the same conversation concurrently.
suppressed map[string]int
suppressedMu sync.Mutex
}
type Opts struct {
@@ -300,7 +303,7 @@ func (e *Engine) EvaluateConversationUpdateRules(conversation cmodels.Conversati
e.lo.Error("error evaluating conversation update rules: eventType is empty")
return
}
if _, suppressed := e.suppressAutomation.Load(conversation.UUID); suppressed {
if e.isSuppressed(conversation.UUID) {
e.lo.Debug("automation suppressed for conversation, skipping update rule evaluation", "uuid", conversation.UUID, "event_type", eventType)
return
}
@@ -379,6 +382,33 @@ func (e *Engine) handleTimeTrigger() {
}
}
// suppress marks a conversation as having automation actions in flight.
func (e *Engine) suppress(conversationUUID string) {
e.suppressedMu.Lock()
defer e.suppressedMu.Unlock()
if e.suppressed == nil {
e.suppressed = map[string]int{}
}
e.suppressed[conversationUUID]++
}
// unsuppress releases one in-flight claim on a conversation.
func (e *Engine) unsuppress(conversationUUID string) {
e.suppressedMu.Lock()
defer e.suppressedMu.Unlock()
if e.suppressed[conversationUUID] <= 1 {
delete(e.suppressed, conversationUUID)
return
}
e.suppressed[conversationUUID]--
}
func (e *Engine) isSuppressed(conversationUUID string) bool {
e.suppressedMu.Lock()
defer e.suppressedMu.Unlock()
return e.suppressed[conversationUUID] > 0
}
// queryRules fetches automation rules from the database.
func (e *Engine) queryRules() []models.Rule {
var (
internal/automation/evaluator.go
+11 -5
View File
@@ -35,13 +35,13 @@ func (e *Engine) evalConversationRules(rules []models.Rule, conversation cmodels
if evaluateFinalResult(groupEvalResults, rule.GroupOperator) {
e.lo.Debug("all rules within groups evaluated successfully, executing actions", "conversation_uuid", conversation.UUID)
e.suppressAutomation.Store(conversation.UUID, struct{}{})
e.suppress(conversation.UUID)
for _, action := range rule.Actions {
if err := e.conversationStore.ApplyAction(action, conversation, umodels.User{}); err != nil {
e.lo.Error("error applying action on conversation", "action", action, "conversation_uuid", conversation.UUID, "error", err)
}
}
e.suppressAutomation.Delete(conversation.UUID)
e.unsuppress(conversation.UUID)
if rule.ExecutionMode == models.ExecutionModeFirstMatch {
e.lo.Debug("automation is first match rule execution mode, breaking out of rule evaluation", "conversation_uuid", conversation.UUID)
break
@@ -153,7 +153,13 @@ func (e *Engine) evaluateRule(rule models.RuleDetail, conversation cmodels.Conve
valueToCompare = strconv.Itoa(conversation.InboxID)
case models.ConversationPreviousStatus, models.ConversationPreviousPriority,
models.ConversationPreviousAssignedUser, models.ConversationPreviousAssignedTeam:
valueToCompare = previousValues[rule.Field]
// An absent key is not the same as an empty previous value.
previous, ok := previousValues[rule.Field]
if !ok {
e.lo.Debug("no previous value available for field, skipping rule", "field", rule.Field, "conversation_uuid", conversation.UUID)
return false
}
valueToCompare = previous
default:
e.lo.Error("error unrecognized conversation field", "field", rule.Field, "field_type", rule.FieldType, "conversation_uuid", conversation.UUID)
return false
@@ -230,7 +236,7 @@ func (e *Engine) evaluateRule(rule models.RuleDetail, conversation cmodels.Conve
for _, ruleValue := range ruleValues {
// Normalize rule value by collapsing multiple spaces
normalizedRuleValue := strings.Join(strings.Fields(ruleValue), " ")
// Respect CaseSensitiveMatch flag
if rule.CaseSensitiveMatch {
if strings.Contains(normalizedInputText, normalizedRuleValue) {
@@ -256,7 +262,7 @@ func (e *Engine) evaluateRule(rule models.RuleDetail, conversation cmodels.Conve
for _, ruleValue := range ruleValues {
// Normalize rule value by collapsing multiple spaces
normalizedRuleValue := strings.Join(strings.Fields(ruleValue), " ")
// Respect CaseSensitiveMatch flag
if rule.CaseSensitiveMatch {
if strings.Contains(normalizedInputText, normalizedRuleValue) {
internal/automation/evaluator_test.go
+1 -1
View File
@@ -1594,4 +1594,4 @@ func TestTriggerWebhookAction_PassedThrough(t *testing.T) {
mockStore.AssertExpectations(t)
assert.Equal(t, 1, mockStore.callCount)
}
}
internal/conversation/conversation.go
+38 -9
View File
@@ -732,12 +732,21 @@ func (c *Manager) UpdateConversationWaitingSince(conversationUUID string, at *ti
// UpdateConversationUserAssignee sets the assignee of a conversation to a specifc user.
func (c *Manager) UpdateConversationUserAssignee(uuid string, assigneeID int, actor umodels.User) error {
previousConversation, _ := c.GetConversation(0, uuid, "")
previousConversation, err := c.GetConversation(0, uuid, "")
if err != nil {
return err
}
previousUserID := ""
if previousConversation.AssignedUserID.Valid {
previousUserID = strconv.Itoa(previousConversation.AssignedUserID.Int)
}
// Return early on a no-op write, else automation rules acting on the user assigned event re-trigger themselves.
if previousConversation.AssignedUserID.Valid && previousConversation.AssignedUserID.Int == assigneeID {
c.lo.Debug("no assignee update: conversation assignee unchanged", "uuid", uuid, "assignee_id", assigneeID)
return nil
}
if err := c.updateAssignee(uuid, assigneeID, models.AssigneeTypeUser); err != nil {
return envelope.NewError(envelope.GeneralError, c.i18n.T("globals.messages.somethingWentWrong"), nil)
}
@@ -765,7 +774,9 @@ func (c *Manager) ClaimUnassignedConversation(uuid string, assigneeID, expectedT
}
c.broadcastReassignment(uuid, prev, prevErr)
return c.afterUserAssignedHooks(uuid, assigneeID, actor, nil)
return c.afterUserAssignedHooks(uuid, assigneeID, actor, map[string]string{
amodels.ConversationPreviousAssignedUser: "",
})
}
// afterUserAssignedHooks runs the side-effects shared by every user-assignment path.
@@ -864,8 +875,12 @@ func (c *Manager) UpdateConversationTeamAssignee(uuid string, teamID int, actor
}
}
previousTeamID := ""
if previousAssignedTeamID > 0 {
previousTeamID = strconv.Itoa(previousAssignedTeamID)
}
c.automation.EvaluateConversationUpdateRules(conversation, amodels.EventConversationTeamAssigned, map[string]string{
amodels.ConversationPreviousAssignedTeam: strconv.Itoa(previousAssignedTeamID),
amodels.ConversationPreviousAssignedTeam: previousTeamID,
})
}
@@ -893,7 +908,10 @@ func (c *Manager) broadcastReassignment(uuid string, prev models.ConversationLis
// UpdateConversationPriority updates the priority of a conversation.
func (c *Manager) UpdateConversationPriority(uuid string, priorityID int, priority string, actor umodels.User) error {
previousConversation, _ := c.GetConversation(0, uuid, "")
previousConversation, err := c.GetConversation(0, uuid, "")
if err != nil {
return err
}
previousPriorityID := ""
if previousConversation.PriorityID.Valid {
previousPriorityID = strconv.Itoa(previousConversation.PriorityID.Int)
@@ -906,6 +924,13 @@ func (c *Manager) UpdateConversationPriority(uuid string, priorityID int, priori
}
priority = p.Name
}
// Return early on a no-op write, else automation rules acting on the priority change event re-trigger themselves.
if previousConversation.Priority.String == priority {
c.lo.Debug("no priority update: conversation priority unchanged", "uuid", uuid, "priority", priority)
return nil
}
if _, err := c.q.UpdateConversationPriority.Exec(uuid, priority); err != nil {
c.lo.Error("error updating conversation priority", "error", err)
return envelope.NewError(envelope.GeneralError, c.i18n.T("globals.messages.somethingWentWrong"), nil)
@@ -1477,7 +1502,10 @@ func (m *Manager) ApplyAction(action amodels.RuleAction, conv models.Conversatio
case amodels.ActionSendCSAT:
return m.SendCSATReply(user.ID, conv)
case amodels.ActionSnooze:
return m.UpdateConversationStatus(conv.UUID, 0, models.StatusSnoozed, action.Value[0], user)
if len(action.Value) == 0 || strings.TrimSpace(action.Value[0]) == "" {
return fmt.Errorf("snooze action requires a duration")
}
return m.UpdateConversationStatus(conv.UUID, 0, models.StatusSnoozed, strings.TrimSpace(action.Value[0]), user)
case amodels.ActionTriggerWebhook:
if len(action.Value) < 2 {
return fmt.Errorf("trigger webhook action requires a webhook and an event name")
@@ -1572,12 +1600,13 @@ func (m *Manager) resolveNotifyRecipients(entries []string, conv models.Conversa
}
func parseNotifyRecipient(entry string) (string, int) {
parts := strings.SplitN(entry, ":", 2)
parts := strings.SplitN(strings.TrimSpace(entry), ":", 2)
kind := strings.ToLower(strings.TrimSpace(parts[0]))
if len(parts) == 1 {
return parts[0], 0
return kind, 0
}
id, _ := strconv.Atoi(parts[1])
return parts[0], id
id, _ := strconv.Atoi(strings.TrimSpace(parts[1]))
return kind, id
}
// RemoveConversationAssignee removes assigned user from a conversation.
internal/webhook/webhook.go
+1 -1
View File
@@ -66,7 +66,7 @@ type Opts struct {
type DeliveryTask struct {
Event models.WebhookEvent
Payload any
// WebhookID targets a single webhook instead of fanning out to event subscribers.
// Zero means fan out to all subscribers of Event.
WebhookID int
}