From 8d002d1dbe1d4bc82b6ee02022059758950c5b52 Mon Sep 17 00:00:00 2001 From: Rogee Date: Thu, 6 Aug 2026 11:19:39 +0800 Subject: [PATCH] =?UTF-8?q?fix(conversations):=20=E4=BF=AE=E5=A4=8D?= =?UTF-8?q?=E4=BC=9A=E8=AF=9D=E6=A0=87=E8=AF=86=E4=B8=8E=E8=A7=A3=E5=86=B3?= =?UTF-8?q?=E5=90=8E=E8=B7=B3=E8=BD=AC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/repository/conversation_repo.go | 20 +++++-- backend/internal/repository/coverage2_test.go | 15 +++++ ...6_repair_conversation_display_ids.down.sql | 7 +++ ...066_repair_conversation_display_ids.up.sql | 57 +++++++++++++++++++ .../dashboard/components/ConversationItem.vue | 2 + .../components/buttons/ResolveAction.vue | 53 ++++++++++++++--- .../helper/conversationNavigation.js | 10 ++++ .../specs/conversationNavigation.spec.js | 21 +++++++ 8 files changed, 173 insertions(+), 12 deletions(-) create mode 100644 backend/migrations/000066_repair_conversation_display_ids.down.sql create mode 100644 backend/migrations/000066_repair_conversation_display_ids.up.sql create mode 100644 frontend/app/javascript/dashboard/helper/conversationNavigation.js create mode 100644 frontend/app/javascript/dashboard/helper/specs/conversationNavigation.spec.js diff --git a/backend/internal/repository/conversation_repo.go b/backend/internal/repository/conversation_repo.go index 7ec217da..42d59d8f 100644 --- a/backend/internal/repository/conversation_repo.go +++ b/backend/internal/repository/conversation_repo.go @@ -272,17 +272,29 @@ func (r *ConversationRepo) Search(ctx context.Context, accountID uint, query str // Create inserts a new conversation. func (r *ConversationRepo) Create(ctx context.Context, conversation *model.Conversation) error { - if conversation.DisplayID == nil || *conversation.DisplayID == 0 { + if conversation.DisplayID != nil && *conversation.DisplayID != 0 { + return r.db.WithContext(ctx).Create(conversation).Error + } + + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + // PostgreSQL needs serialization because MAX(display_id)+1 is otherwise + // racy when a channel imports several conversations concurrently. + if tx.Dialector != nil && tx.Dialector.Name() == "postgres" { + if err := tx.Exec("SELECT pg_advisory_xact_lock(?)", int64(conversation.AccountID)).Error; err != nil { + return err + } + } + var next uint - if err := r.db.WithContext(ctx).Model(&model.Conversation{}). + if err := tx.Model(&model.Conversation{}). Select("COALESCE(MAX(display_id), 0) + 1"). Where("account_id = ?", conversation.AccountID). Scan(&next).Error; err != nil { return err } conversation.DisplayID = &next - } - return r.db.WithContext(ctx).Create(conversation).Error + return tx.Create(conversation).Error + }) } // Update modifies an existing conversation. diff --git a/backend/internal/repository/coverage2_test.go b/backend/internal/repository/coverage2_test.go index c0d9ccfe..d1e9b9e3 100644 --- a/backend/internal/repository/coverage2_test.go +++ b/backend/internal/repository/coverage2_test.go @@ -90,6 +90,21 @@ func TestConversationRepo_FindByAccountAndDisplayIDOrID_PrefersLegacyNullDisplay assert.Nil(t, found.DisplayID) } +func TestConversationRepo_Create_AssignsDistinctSequentialDisplayIDs(t *testing.T) { + db := setupTestDB(t) + repo := NewConversationRepo(db) + ctx := context.Background() + account, inbox, contact := createConvAccount(t, db) + + first := &model.Conversation{AccountID: account.ID, InboxID: inbox.ID, ContactID: contact.ID, Status: "open", ChannelType: "web_widget", Channel: "web_widget"} + second := &model.Conversation{AccountID: account.ID, InboxID: inbox.ID, ContactID: contact.ID, Status: "open", ChannelType: "web_widget", Channel: "web_widget"} + require.NoError(t, repo.Create(ctx, first)) + require.NoError(t, repo.Create(ctx, second)) + require.NotNil(t, first.DisplayID) + require.NotNil(t, second.DisplayID) + assert.Equal(t, *first.DisplayID+1, *second.DisplayID) +} + func TestCov2_ConversationRepo_FindByContact(t *testing.T) { db := setupTestDB(t) repo := NewConversationRepo(db) diff --git a/backend/migrations/000066_repair_conversation_display_ids.down.sql b/backend/migrations/000066_repair_conversation_display_ids.down.sql new file mode 100644 index 00000000..a93affee --- /dev/null +++ b/backend/migrations/000066_repair_conversation_display_ids.down.sql @@ -0,0 +1,7 @@ +DROP INDEX IF EXISTS idx_conversations_account_display_id; +CREATE INDEX idx_conversations_account_display_id + ON conversations(account_id, display_id) + WHERE deleted_at IS NULL; + +-- Repaired display IDs are intentionally retained: their previous duplicate +-- values cannot be restored without reintroducing ambiguous conversation URLs. diff --git a/backend/migrations/000066_repair_conversation_display_ids.up.sql b/backend/migrations/000066_repair_conversation_display_ids.up.sql new file mode 100644 index 00000000..8149a239 --- /dev/null +++ b/backend/migrations/000066_repair_conversation_display_ids.up.sql @@ -0,0 +1,57 @@ +-- Repair duplicate and missing account-scoped conversation display IDs before +-- enforcing the Chatwoot routing invariant. Keep already-unique IDs stable so +-- existing conversation URLs continue to resolve to the same records. +WITH ranked AS ( + SELECT + id, + account_id, + display_id, + ROW_NUMBER() OVER ( + PARTITION BY account_id, display_id + ORDER BY id + ) AS duplicate_rank + FROM conversations + WHERE deleted_at IS NULL + AND display_id IS NOT NULL + AND display_id > 0 +), +needs_repair AS ( + SELECT id, account_id + FROM ranked + WHERE duplicate_rank > 1 + + UNION ALL + + SELECT id, account_id + FROM conversations + WHERE deleted_at IS NULL + AND (display_id IS NULL OR display_id = 0) +), +account_max AS ( + SELECT account_id, COALESCE(MAX(display_id), 0) AS max_display_id + FROM conversations + WHERE deleted_at IS NULL + AND display_id IS NOT NULL + AND display_id > 0 + GROUP BY account_id +), +replacements AS ( + SELECT + repair.id, + COALESCE(account_max.max_display_id, 0) + + ROW_NUMBER() OVER ( + PARTITION BY repair.account_id + ORDER BY repair.id + ) AS display_id + FROM needs_repair AS repair + LEFT JOIN account_max ON account_max.account_id = repair.account_id +) +UPDATE conversations AS conversation +SET display_id = replacements.display_id +FROM replacements +WHERE conversation.id = replacements.id; + +DROP INDEX IF EXISTS idx_conversations_account_display_id; +CREATE UNIQUE INDEX idx_conversations_account_display_id + ON conversations(account_id, display_id) + WHERE deleted_at IS NULL; diff --git a/frontend/app/javascript/dashboard/components/ConversationItem.vue b/frontend/app/javascript/dashboard/components/ConversationItem.vue index 866c8bc8..06047381 100644 --- a/frontend/app/javascript/dashboard/components/ConversationItem.vue +++ b/frontend/app/javascript/dashboard/components/ConversationItem.vue @@ -182,6 +182,7 @@ const onDeleteConversation = () => { { toggleDropdown(true); const currentChat = computed(() => getters.getSelectedChat.value); +const openConversation = conversationId => { + if (!conversationId) { + redirectToConversationList(); + return; + } + const { + params: { accountId, inbox_id: inboxId, label, teamId, id: customViewId }, + name, + } = route; + let conversationType = ''; + if (isOnMentionsView({ route: { name } })) conversationType = 'mention'; + else if (isOnParticipatingView({ route: { name } })) { + conversationType = 'participating'; + } else if (isOnUnattendedView({ route: { name } })) { + conversationType = 'unattended'; + } + router.push({ + path: frontendURL( + conversationUrl({ + accountId, + activeInbox: inboxId, + id: conversationId, + label, + teamId, + foldersId: isOnFoldersView({ route: { name } }) ? customViewId : 0, + conversationType, + }) + ), + }); +}; + const redirectToConversationList = () => { const { params: { accountId, inbox_id: inboxId, label, teamId, id: customViewId }, @@ -112,6 +146,15 @@ const getConversationParams = () => { }; }; +const getAdjacentConversationID = conversationId => { + const conversationIds = [ + ...document.querySelectorAll('.conversations-list .conversation'), + ] + .map(element => Number(element.dataset.conversationId)) + .filter(Boolean); + return getAdjacentConversationId(conversationIds, conversationId); +}; + const openSnoozeModal = () => { const ninja = document.querySelector('ninja-keys'); ninja.open({ parent: 'snooze_conversation' }); @@ -121,9 +164,7 @@ const toggleStatus = (status, snoozedUntil, customAttributes = null) => { closeDropdown(); isLoading.value = true; const conversationId = currentChat.value.id; - const { all, activeIndex, lastIndex } = getConversationParams(); - const adjacentConversation = - activeIndex < lastIndex ? all[activeIndex + 1] : all[activeIndex - 1]; + const adjacentConversationId = getAdjacentConversationID(conversationId); const payload = { conversationId, @@ -140,11 +181,7 @@ const toggleStatus = (status, snoozedUntil, customAttributes = null) => { .then(() => { useAlert(t('CONVERSATION.CHANGE_STATUS')); if (status !== wootConstants.STATUS_TYPE.OPEN) { - if (adjacentConversation) { - adjacentConversation.click(); - } else { - redirectToConversationList(); - } + openConversation(adjacentConversationId); } }) .finally(() => { diff --git a/frontend/app/javascript/dashboard/helper/conversationNavigation.js b/frontend/app/javascript/dashboard/helper/conversationNavigation.js new file mode 100644 index 00000000..db374ea0 --- /dev/null +++ b/frontend/app/javascript/dashboard/helper/conversationNavigation.js @@ -0,0 +1,10 @@ +export const getAdjacentConversationId = (conversationIds, currentId) => { + const currentIndex = conversationIds.indexOf(currentId); + if (currentIndex === -1) return null; + + return ( + conversationIds[currentIndex + 1] ?? + conversationIds[currentIndex - 1] ?? + null + ); +}; diff --git a/frontend/app/javascript/dashboard/helper/specs/conversationNavigation.spec.js b/frontend/app/javascript/dashboard/helper/specs/conversationNavigation.spec.js new file mode 100644 index 00000000..8f623cfc --- /dev/null +++ b/frontend/app/javascript/dashboard/helper/specs/conversationNavigation.spec.js @@ -0,0 +1,21 @@ +import { getAdjacentConversationId } from '../conversationNavigation'; + +describe('getAdjacentConversationId', () => { + const conversationIds = [15, 19, 25]; + + it('selects the immediate next conversation', () => { + expect(getAdjacentConversationId(conversationIds, 15)).toBe(19); + }); + + it('selects the previous conversation for the last item', () => { + expect(getAdjacentConversationId(conversationIds, 25)).toBe(19); + }); + + it('returns null for a single-item list', () => { + expect(getAdjacentConversationId([15], 15)).toBeNull(); + }); + + it('returns null when the current conversation is absent', () => { + expect(getAdjacentConversationId(conversationIds, 99)).toBeNull(); + }); +});