fix: uise existing set next sla deadline sql query and remove duplicate query.

- remove previously added `applied_sla_id` column to conversations table as it was causing cyclic dep
2025-10-23 05:11:57 +00:00 · 2025-05-24 23:53:05 +05:30
parent 7e8c9962c3
commit a9fd4fe2b6
7 changed files with 133 additions and 75 deletions
--- a/internal/conversation/conversation.go
+++ b/internal/conversation/conversation.go
@@ -82,7 +82,7 @@ type Manager struct {

 type slaStore interface {
 	ApplySLA(startTime time.Time, conversationID, assignedTeamID, slaID int) (slaModels.SLAPolicy, error)
-	CreateNextResponseSLAEvent(conversationID, appliedSLAID, slaPolicyID, assignedTeamID int) (time.Time, error)
+	CreateNextResponseSLAEvent(conversationID, assignedTeamID int) (time.Time, error)
 	SetLatestSLAEventMetAt(appliedSLAID int, metric string) (time.Time, error)
 }

--- a/internal/conversation/message.go
+++ b/internal/conversation/message.go
@@ -587,6 +587,7 @@ func (m *Manager) processIncomingMessage(in models.IncomingMessage) error {
 	m.automation.EvaluateConversationUpdateRules(in.Message.ConversationUUID, amodels.EventConversationMessageIncoming)

 	// Create SLA event for next response if a SLA is applied and has next response time set, subsequent agent replies will mark this event as met.
+	// This cycle continues for next response time SLA metric.
 	conversation, err := m.GetConversation(in.Message.ConversationID, "")
 	if err != nil {
 		m.lo.Error("error fetching conversation", "conversation_id", in.Message.ConversationID, "error", err)
@@ -595,15 +596,13 @@ func (m *Manager) processIncomingMessage(in models.IncomingMessage) error {
 		m.lo.Info("no SLA policy applied to conversation, skipping next response SLA event creation")
 		return nil
 	}
-	if deadline, err := m.slaStore.CreateNextResponseSLAEvent(conversation.ID, conversation.AppliedSLAID.Int, conversation.SLAPolicyID.Int, conversation.AssignedTeamID.Int); err != nil {
+	if deadline, err := m.slaStore.CreateNextResponseSLAEvent(conversation.ID, conversation.AssignedTeamID.Int); err != nil {
 		m.lo.Error("error creating next response SLA event", "conversation_id", conversation.ID, "error", err)
-	} else {
-		if !deadline.IsZero() {
-			m.lo.Debug("next response SLA event created", "conversation_id", conversation.ID, "deadline", deadline, "applied_sla_id", conversation.AppliedSLAID.Int, "sla_policy_id", conversation.SLAPolicyID.Int)
-			m.BroadcastConversationUpdate(in.Message.ConversationUUID, "next_response_deadline_at", deadline.Format(time.RFC3339))
-			// Clear next response met at timestamp as a new SLA event is created.
-			m.BroadcastConversationUpdate(in.Message.ConversationUUID, "next_response_met_at", nil)
-		}
+	} else if !deadline.IsZero() {
+		m.lo.Debug("next response SLA event created for conversation", "conversation_id", conversation.ID, "deadline", deadline, "sla_policy_id", conversation.SLAPolicyID.Int)
+		m.BroadcastConversationUpdate(in.Message.ConversationUUID, "next_response_deadline_at", deadline.Format(time.RFC3339))
+		// Clear next response met at timestamp as this event was just created.
+		m.BroadcastConversationUpdate(in.Message.ConversationUUID, "next_response_met_at", nil)
 	}
 	return nil
 }
--- a/internal/conversation/models/models.go
+++ b/internal/conversation/models/models.go
@@ -84,7 +84,6 @@ type Conversation struct {
 	LastMessage           null.String     `db:"last_message" json:"last_message"`
 	LastMessageSender     null.String     `db:"last_message_sender" json:"last_message_sender"`
 	Contact               umodels.User    `db:"contact" json:"contact"`
-	AppliedSLAID          null.Int        `db:"applied_sla_id" json:"applied_sla_id"`
 	SLAPolicyID           null.Int        `db:"sla_policy_id" json:"sla_policy_id"`
 	SlaPolicyName         null.String     `db:"sla_policy_name" json:"sla_policy_name"`
 	NextSLADeadlineAt     null.Time       `db:"next_sla_deadline_at" json:"next_sla_deadline_at"`
--- a/internal/migrations/v0.6.0.go
+++ b/internal/migrations/v0.6.0.go
@@ -249,14 +249,6 @@ func V0_6_0(db *sqlx.DB, fs stuffbin.FileSystem, ko *koanf.Koanf) error {
 		return err
 	}

-	// Add applied_sla_id column to conversations table if it doesn't exist
-	_, err = db.Exec(`
-		ALTER TABLE conversations ADD COLUMN IF NOT EXISTS applied_sla_id BIGINT REFERENCES applied_slas(id) ON DELETE SET NULL ON UPDATE CASCADE;
-	`)
-	if err != nil {
-		return err
-	}
-
 	// Create sla_events table if it does not exist
 	_, err = db.Exec(`
 		CREATE TABLE IF NOT EXISTS sla_events (
--- a/internal/sla/queries.sql
+++ b/internal/sla/queries.sql
@@ -38,12 +38,11 @@ WITH new_sla AS (
  ) VALUES ($1, $2, $3, $4)
  RETURNING conversation_id, id
 )
-- update the conversation with the new SLA policy and applied SLA
+-- update the conversation with the new SLA policy and next SLA deadline.
 UPDATE conversations c
 SET 
   sla_policy_id = $2,
-   next_sla_deadline_at = LEAST($3, $4),
-   applied_sla_id = ns.id
+   next_sla_deadline_at = LEAST($3, $4)
 FROM new_sla ns
 WHERE c.id = ns.conversation_id
 RETURNING ns.id;
@@ -70,14 +69,30 @@ UPDATE applied_slas SET
   updated_at = NOW()
 WHERE id = $1;

-- name: set-next-sla-deadline
+-- name: set-conversation-sla-deadline
 UPDATE conversations c
-SET next_sla_deadline_at = CASE 
-    WHEN c.status_id IN (SELECT id from conversation_statuses where name in ('Resolved', 'Closed')) THEN NULL
-    WHEN c.first_reply_at IS NOT NULL AND c.resolved_at IS NULL AND a.resolution_deadline_at IS NOT NULL THEN a.resolution_deadline_at
-    WHEN c.first_reply_at IS NULL AND c.resolved_at IS NULL AND a.first_response_deadline_at IS NOT NULL THEN a.first_response_deadline_at
-    WHEN a.first_response_deadline_at IS NOT NULL AND a.resolution_deadline_at IS NOT NULL THEN LEAST(a.first_response_deadline_at, a.resolution_deadline_at)
-    ELSE NULL
+SET next_sla_deadline_at = CASE
+    -- If resolved or closed, clear the deadline
+    WHEN c.status_id IN (SELECT id FROM conversation_statuses WHERE name IN ('Resolved', 'Closed')) THEN NULL
+
+    -- If an external timestamp ($3) is provided (e.g. next_response), use the earliest of $3.
+    WHEN $3 IS NOT NULL THEN LEAST(
+        $3,
+        CASE
+            WHEN c.first_reply_at IS NOT NULL AND c.resolved_at IS NULL AND a.resolution_deadline_at IS NOT NULL THEN a.resolution_deadline_at
+            WHEN c.first_reply_at IS NULL AND c.resolved_at IS NULL AND a.first_response_deadline_at IS NOT NULL THEN a.first_response_deadline_at
+            WHEN a.first_response_deadline_at IS NOT NULL AND a.resolution_deadline_at IS NOT NULL THEN LEAST(a.first_response_deadline_at, a.resolution_deadline_at)
+            ELSE NULL
+        END
+    )
+
+    -- No $3,
+    ELSE CASE
+        WHEN c.first_reply_at IS NOT NULL AND c.resolved_at IS NULL AND a.resolution_deadline_at IS NOT NULL THEN a.resolution_deadline_at
+        WHEN c.first_reply_at IS NULL AND c.resolved_at IS NULL AND a.first_response_deadline_at IS NOT NULL THEN a.first_response_deadline_at
+        WHEN a.first_response_deadline_at IS NOT NULL AND a.resolution_deadline_at IS NOT NULL THEN LEAST(a.first_response_deadline_at, a.resolution_deadline_at)
+        ELSE NULL
+    END
 END
 FROM applied_slas a
 WHERE a.conversation_id = c.id
@@ -136,6 +151,33 @@ FROM applied_slas a INNER JOIN conversations c on a.conversation_id = c.id
 LEFT JOIN conversation_statuses s ON c.status_id = s.id
 WHERE a.id = $1;

+-- name: get-latest-applied-sla-for-conversation
+SELECT a.id,
+   a.created_at,
+   a.updated_at,
+   a.conversation_id,
+   a.sla_policy_id,
+   a.first_response_deadline_at,
+   a.resolution_deadline_at,
+   a.first_response_met_at,
+   a.resolution_met_at,
+   a.first_response_breached_at,
+   a.resolution_breached_at,
+   a.status,
+   c.first_reply_at as conversation_first_response_at,
+   c.resolved_at as conversation_resolved_at,
+   c.uuid as conversation_uuid,
+   c.reference_number as conversation_reference_number,
+   c.subject as conversation_subject,
+   c.assigned_user_id as conversation_assigned_user_id,
+   s.name as conversation_status
+FROM applied_slas a
+INNER JOIN conversations c ON a.conversation_id = c.id
+LEFT JOIN conversation_statuses s ON c.status_id = s.id
+WHERE a.conversation_id = $1
+ORDER BY a.created_at DESC
+LIMIT 1;
+
 -- name: mark-notification-processed
 UPDATE scheduled_sla_notifications
 SET processed_at = NOW(),
@@ -179,11 +221,6 @@ SELECT id, created_at, updated_at, applied_sla_id, sla_policy_id, type, deadline
 FROM sla_events
 WHERE id = $1;

-- name: update-conversation-next-sla-deadline
-UPDATE conversations
-SET next_sla_deadline_at = LEAST(next_sla_deadline_at, $2)
-WHERE id = $1;
-
 -- name: get-pending-sla-events
 SELECT id
 FROM sla_events
--- a/internal/sla/sla.go
+++ b/internal/sla/sla.go
@@ -103,28 +103,29 @@ type businessHrsStore interface {

 // queries hold prepared SQL queries.
 type queries struct {
-	GetSLA                            *sqlx.Stmt `query:"get-sla-policy"`
-	GetAllSLA                         *sqlx.Stmt `query:"get-all-sla-policies"`
-	GetAppliedSLA                     *sqlx.Stmt `query:"get-applied-sla"`
-	GetSLAEvent                       *sqlx.Stmt `query:"get-sla-event"`
-	GetScheduledSLANotifications      *sqlx.Stmt `query:"get-scheduled-sla-notifications"`
-	InsertScheduledSLANotification    *sqlx.Stmt `query:"insert-scheduled-sla-notification"`
-	InsertSLA                         *sqlx.Stmt `query:"insert-sla-policy"`
-	InsertNextResponseSLAEvent        *sqlx.Stmt `query:"insert-next-response-sla-event"`
-	DeleteSLA                         *sqlx.Stmt `query:"delete-sla-policy"`
-	UpdateSLA                         *sqlx.Stmt `query:"update-sla-policy"`
-	ApplySLA                          *sqlx.Stmt `query:"apply-sla"`
-	GetPendingSLAs                    *sqlx.Stmt `query:"get-pending-slas"`
-	UpdateBreach                      *sqlx.Stmt `query:"update-breach"`
-	UpdateMet                         *sqlx.Stmt `query:"update-met"`
-	UpdateConversationNextSLADeadline *sqlx.Stmt `query:"update-conversation-next-sla-deadline"`
-	SetNextSLADeadline                *sqlx.Stmt `query:"set-next-sla-deadline"`
-	GetPendingSLAEvents               *sqlx.Stmt `query:"get-pending-sla-events"`
-	UpdateSLAStatus                   *sqlx.Stmt `query:"update-sla-status"`
-	MarkNotificationProcessed         *sqlx.Stmt `query:"mark-notification-processed"`
-	MarkSLAEventAsBreached            *sqlx.Stmt `query:"mark-sla-event-as-breached"`
-	MarkSLAEventAsMet                 *sqlx.Stmt `query:"mark-sla-event-as-met"`
-	SetLatestSLAEventMetAt            *sqlx.Stmt `query:"set-latest-sla-event-met-at"`
+	// TODO: name queries better.
+	GetSLA                             *sqlx.Stmt `query:"get-sla-policy"`
+	GetAllSLA                          *sqlx.Stmt `query:"get-all-sla-policies"`
+	GetAppliedSLA                      *sqlx.Stmt `query:"get-applied-sla"`
+	GetSLAEvent                        *sqlx.Stmt `query:"get-sla-event"`
+	GetScheduledSLANotifications       *sqlx.Stmt `query:"get-scheduled-sla-notifications"`
+	GetLatestAppliedSLAForConversation *sqlx.Stmt `query:"get-latest-applied-sla-for-conversation"`
+	InsertScheduledSLANotification     *sqlx.Stmt `query:"insert-scheduled-sla-notification"`
+	InsertSLA                          *sqlx.Stmt `query:"insert-sla-policy"`
+	InsertNextResponseSLAEvent         *sqlx.Stmt `query:"insert-next-response-sla-event"`
+	DeleteSLA                          *sqlx.Stmt `query:"delete-sla-policy"`
+	UpdateSLA                          *sqlx.Stmt `query:"update-sla-policy"`
+	ApplySLA                           *sqlx.Stmt `query:"apply-sla"`
+	GetPendingSLAs                     *sqlx.Stmt `query:"get-pending-slas"`
+	UpdateBreach                       *sqlx.Stmt `query:"update-breach"`
+	UpdateMet                          *sqlx.Stmt `query:"update-met"`
+	SetConversationNextSLADeadline     *sqlx.Stmt `query:"set-conversation-sla-deadline"`
+	GetPendingSLAEvents                *sqlx.Stmt `query:"get-pending-sla-events"`
+	UpdateSLAStatus                    *sqlx.Stmt `query:"update-sla-status"`
+	MarkNotificationProcessed          *sqlx.Stmt `query:"mark-notification-processed"`
+	MarkSLAEventAsBreached             *sqlx.Stmt `query:"mark-sla-event-as-breached"`
+	MarkSLAEventAsMet                  *sqlx.Stmt `query:"mark-sla-event-as-met"`
+	SetLatestSLAEventMetAt             *sqlx.Stmt `query:"set-latest-sla-event-met-at"`
 }

 // New creates a new SLA manager.
@@ -266,19 +267,33 @@ func (m *Manager) ApplySLA(startTime time.Time, conversationID, assignedTeamID,
 }

 // CreateNextResponseSLAEvent creates a next response SLA event for a conversation.
-func (m *Manager) CreateNextResponseSLAEvent(conversationID, appliedSLAID, slaPolicyID, assignedTeamID int) (time.Time, error) {
-	var slaPolicy models.SLAPolicy
-	if err := m.q.GetSLA.Get(&slaPolicy, slaPolicyID); err != nil {
+func (m *Manager) CreateNextResponseSLAEvent(conversationID, assignedTeamID int) (time.Time, error) {
+	// Fetch the latest applied SLA for the conversation.
+	var appliedSLA models.AppliedSLA
+	if err := m.q.GetLatestAppliedSLAForConversation.Get(&appliedSLA, conversationID); err != nil {
 		if err == sql.ErrNoRows {
-			return time.Time{}, fmt.Errorf("SLA policy not found: %d", slaPolicyID)
+			return time.Time{}, fmt.Errorf("no applied SLA found for conversation: %d", conversationID)
+		}
+		m.lo.Error("error fetching latest applied SLA for conversation", "error", err)
+		return time.Time{}, fmt.Errorf("fetching latest applied SLA for conversation: %w", err)
+	}
+
+	var slaPolicy models.SLAPolicy
+	if err := m.q.GetSLA.Get(&slaPolicy, appliedSLA.SLAPolicyID); err != nil {
+		if err == sql.ErrNoRows {
+			return time.Time{}, fmt.Errorf("SLA policy not found: %d", appliedSLA.SLAPolicyID)
 		}
 		m.lo.Error("error fetching SLA policy", "error", err)
 		return time.Time{}, fmt.Errorf("fetching SLA policy: %w", err)
 	}

 	if slaPolicy.NextResponseTime == "" {
-		m.lo.Info("no next response time set for SLA policy, skipping event creation", "conversation_id", conversationID, "policy_id", slaPolicyID)
-		return time.Time{}, fmt.Errorf("no next response time set for SLA policy: %d", slaPolicyID)
+		m.lo.Info("no next response time set for SLA policy, skipping event creation",
+			"conversation_id", conversationID,
+			"policy_id", appliedSLA.SLAPolicyID,
+			"applied_sla_id", appliedSLA.ID,
+		)
+		return time.Time{}, fmt.Errorf("no next response time set for SLA policy: %d, applied_sla: %d", appliedSLA.SLAPolicyID, appliedSLA.ID)
 	}

 	// Calculate the deadline for the next response SLA event.
@@ -289,30 +304,47 @@ func (m *Manager) CreateNextResponseSLAEvent(conversationID, appliedSLAID, slaPo
 	}

 	if deadlines.NextResponse.IsZero() {
-		m.lo.Info("next response deadline is zero, skipping event creation", "conversation_id", conversationID, "policy_id", slaPolicyID)
-		return time.Time{}, fmt.Errorf("next response deadline is zero for conversation: %d and policy: %d", conversationID, slaPolicyID)
+		m.lo.Info("next response deadline is zero, skipping event creation",
+			"conversation_id", conversationID,
+			"policy_id", appliedSLA.SLAPolicyID,
+			"applied_sla_id", appliedSLA.ID,
+		)
+		return time.Time{}, fmt.Errorf("next response deadline is zero for conversation: %d, policy: %d, applied_sla: %d", conversationID, appliedSLA.SLAPolicyID, appliedSLA.ID)
 	}

 	var slaEventID int
-	if err := m.q.InsertNextResponseSLAEvent.QueryRow(appliedSLAID, slaPolicyID, deadlines.NextResponse).Scan(&slaEventID); err != nil {
+	if err := m.q.InsertNextResponseSLAEvent.QueryRow(appliedSLA.ID, appliedSLA.SLAPolicyID, deadlines.NextResponse).Scan(&slaEventID); err != nil {
 		if err == sql.ErrNoRows {
-			m.lo.Debug("unmet sla event for next response already exists, skipping creation", "conversation_id", conversationID, "policy_id", slaPolicyID)
-			return time.Time{}, fmt.Errorf("unmet next response SLA event already exists for conversation: %d and policy: %d", conversationID, slaPolicyID)
+			m.lo.Debug("unmet SLA event for next response already exists, skipping creation",
+				"conversation_id", conversationID,
+				"policy_id", slaPolicy.ID,
+				"applied_sla_id", appliedSLA.ID,
+			)
+			return time.Time{}, fmt.Errorf("unmet next response SLA event already exists for conversation: %d, policy: %d, applied_sla: %d", conversationID, slaPolicy.ID, appliedSLA.ID)
 		}
-		m.lo.Error("error inserting SLA event", "error", err)
-		return time.Time{}, fmt.Errorf("inserting SLA event: %w", err)
+		m.lo.Error("error inserting SLA event",
+			"error", err,
+			"conversation_id", conversationID,
+			"applied_sla_id", appliedSLA.ID,
+		)
+		return time.Time{}, fmt.Errorf("inserting SLA event (applied_sla: %d): %w", appliedSLA.ID, err)
 	}

-	// Update next SLA deadline (sla target) in the conversation.
-	if _, err := m.q.UpdateConversationNextSLADeadline.Exec(conversationID, deadlines.NextResponse); err != nil {
-		m.lo.Error("error updating conversation next SLA deadline", "error", err)
-		return time.Time{}, fmt.Errorf("updating conversation next SLA deadline: %w", err)
+	// Update next SLA deadline (SLA target) in the conversation.
+	if _, err := m.q.SetConversationNextSLADeadline.Exec(conversationID, deadlines.NextResponse); err != nil {
+		m.lo.Error("error updating conversation next SLA deadline",
+			"error", err,
+			"conversation_id", conversationID,
+			"applied_sla_id", appliedSLA.ID,
+		)
+		return time.Time{}, fmt.Errorf("updating conversation next SLA deadline (applied_sla: %d): %w", appliedSLA.ID, err)
 	}

 	// Create notification schedule for the next response SLA event.
 	deadlines.FirstResponse = time.Time{}
 	deadlines.Resolution = time.Time{}
-	m.createNotificationSchedule(slaPolicy.Notifications, appliedSLAID, null.IntFrom(slaEventID), deadlines, Breaches{})
+	m.createNotificationSchedule(slaPolicy.Notifications, appliedSLA.ID, null.IntFrom(slaEventID), deadlines, Breaches{})
+
 	return deadlines.NextResponse, nil
 }

@@ -846,7 +878,7 @@ func (m *Manager) evaluateSLA(sla models.AppliedSLA) error {
 	}

 	// Update the conversation next SLA deadline.
-	if _, err := m.q.SetNextSLADeadline.Exec(sla.ConversationID); err != nil {
+	if _, err := m.q.SetConversationNextSLADeadline.Exec(sla.ConversationID, nil); err != nil {
 		return fmt.Errorf("setting conversation next SLA deadline: %w", err)
 	}

--- a/schema.sql
+++ b/schema.sql
@@ -203,7 +203,6 @@ CREATE TABLE conversations (

 	-- Set to NULL when SLA policy is deleted.
 	sla_policy_id INT REFERENCES sla_policies(id) ON DELETE SET NULL ON UPDATE CASCADE,
-	applied_sla_id BIGINT REFERENCES applied_slas(id) ON DELETE SET NULL ON UPDATE CASCADE,

    -- Cascade deletes when inbox is deleted.
 	inbox_id INT REFERENCES inboxes(id) ON DELETE CASCADE ON UPDATE CASCADE NOT NULL,