From 690796a7deb804326d943c7e0a1b0cccd60276d5 Mon Sep 17 00:00:00 2001 From: Rogee Date: Fri, 10 Jul 2026 16:59:26 +0800 Subject: [PATCH] fix: resolve 4 bugs from QA round 6 (BUG-F/G/H/I) BUG-F (P1): Fake webhook lookupInbox used JSONB @> operator on a TEXT column, causing all fake webhooks to return "ignored". Cast channel_config::jsonb before the @> operator. BUG-G (P3): Vue Router history.state warning on Activity page. Four call sites replaced history.state with null/{}, destroying Vue Router's internal navigation state. Now all replaceState calls preserve window.history.state. BUG-H (P3): Inbox list showed stale data because cache_keys endpoint returned hardcoded "0000000000" for inbox/label/team, so the frontend IndexedDB cache never invalidated. Cache keys are now derived from actual DB state (row count + MAX(updated_at)), with defensive fallback for missing tables. BUG-I (P3): All worker goroutines shared the same Redis consumer name, so XINFO CONSUMERS showed 1 consumer instead of N. Each goroutine now generates a unique consumer ID (workerID-index). --- .../internal/handler/webhook/fake_webhook.go | 5 +- backend/internal/repository/account_repo.go | 46 +++++++++++++++++++ backend/internal/service/account_service.go | 12 +++-- .../internal/service/account_service_test.go | 24 ++++++++++ backend/internal/worker/worker.go | 10 ++-- .../dashboard/settings/inbox/Settings.vue | 6 ++- .../settings/inbox/channels/Instagram.vue | 3 +- .../settings/inbox/channels/Tiktok.vue | 3 +- .../javascript/portal/portalThemeHelper.js | 3 +- 9 files changed, 99 insertions(+), 13 deletions(-) diff --git a/backend/internal/handler/webhook/fake_webhook.go b/backend/internal/handler/webhook/fake_webhook.go index 66f5567c..74690a22 100644 --- a/backend/internal/handler/webhook/fake_webhook.go +++ b/backend/internal/handler/webhook/fake_webhook.go @@ -163,8 +163,9 @@ func (h *FakeWebhookHandler) lookupInbox(identifier string) (*model.Inbox, error // PostgreSQL: use jsonb @> for a server-side, indexable query. if h.db.Dialector.Name() == "postgres" { var inbox model.Inbox - // channel_config @> '{"identifier":""}' - query := fmt.Sprintf(`channel_type = 'fake' AND channel_config @> '{"identifier":"%s"}'`, identifier) + // channel_config is TEXT, so cast to jsonb before using the @> operator. + // channel_config::jsonb @> '{"identifier":""}' + query := fmt.Sprintf(`channel_type = 'fake' AND channel_config::jsonb @> '{"identifier":"%s"}'`, identifier) if err := h.db.Where(query).First(&inbox).Error; err != nil { return nil, fmt.Errorf("fake inbox not found for identifier=%s: %w", identifier, err) } diff --git a/backend/internal/repository/account_repo.go b/backend/internal/repository/account_repo.go index 93e4eda6..e0b6a6cc 100644 --- a/backend/internal/repository/account_repo.go +++ b/backend/internal/repository/account_repo.go @@ -2,6 +2,7 @@ package repository import ( "context" + "fmt" "time" "gorm.io/gorm" @@ -261,3 +262,48 @@ func (r *AccountRepo) FindAccountUserByUserAndAccount(ctx context.Context, accou } return &au, nil } + +// InboxCacheKey returns a cache-busting key for the inbox list. +// It combines the count and max(updated_at) so any inbox create/update/delete +// changes the key. Falls back to "0000000000" if the table is missing (test +// setups that don't migrate all models). +func (r *AccountRepo) InboxCacheKey(ctx context.Context, accountID uint) (string, error) { + return cacheKeyForModel(r.db, ctx, &model.Inbox{}, accountID) +} + +// LabelCacheKey returns a cache-busting key for the label (tag) list. +func (r *AccountRepo) LabelCacheKey(ctx context.Context, accountID uint) (string, error) { + return cacheKeyForModel(r.db, ctx, &model.Tag{}, accountID) +} + +// TeamCacheKey returns a cache-busting key for the team list. +func (r *AccountRepo) TeamCacheKey(ctx context.Context, accountID uint) (string, error) { + return cacheKeyForModel(r.db, ctx, &model.Team{}, accountID) +} + +// cacheKeyForModel computes a cache-busting key from the count and +// max(updated_at) of rows matching accountID in the given model's table. +// Uses *string for MAX(updated_at) because SQLite returns it as a string, +// which GORM cannot scan into *time.Time. The string value is included +// verbatim, so any change to the set of rows produces a different key. +// Falls back to "0000000000" if the table is missing. +func cacheKeyForModel(db *gorm.DB, ctx context.Context, model interface{}, accountID uint) (string, error) { + if !db.Migrator().HasTable(model) { + return "0000000000", nil + } + var result struct { + Count int64 + Max *string + } + err := db.WithContext(ctx).Model(model). + Where("account_id = ?", accountID). + Select("COUNT(*) as count, MAX(updated_at) as max"). + Scan(&result).Error + if err != nil { + return "0000000000", err + } + if result.Max == nil || *result.Max == "" { + return fmt.Sprintf("%010d", result.Count), nil + } + return fmt.Sprintf("%010d%s", result.Count, *result.Max), nil +} diff --git a/backend/internal/service/account_service.go b/backend/internal/service/account_service.go index 9bb663da..16517b53 100644 --- a/backend/internal/service/account_service.go +++ b/backend/internal/service/account_service.go @@ -294,10 +294,16 @@ func (s *AccountService) CacheKeys(ctx context.Context, accountID, userID uint) if _, err := s.repo.FindAccountUserByUserAndAccount(ctx, accountID, userID); err != nil { return nil, err } + // Derive cache keys from actual DB state so the frontend's + // IndexedDB cache invalidates when inboxes/labels/teams change. + // Falls back to "0000000000" if the table is missing (tests). + inboxKey, _ := s.repo.InboxCacheKey(ctx, accountID) + labelKey, _ := s.repo.LabelCacheKey(ctx, accountID) + teamKey, _ := s.repo.TeamCacheKey(ctx, accountID) keys := map[string]string{ - "label": "0000000000", - "inbox": "0000000000", - "team": "0000000000", + "label": labelKey, + "inbox": inboxKey, + "team": teamKey, } return keys, nil } diff --git a/backend/internal/service/account_service_test.go b/backend/internal/service/account_service_test.go index ea032052..533748e9 100644 --- a/backend/internal/service/account_service_test.go +++ b/backend/internal/service/account_service_test.go @@ -479,3 +479,27 @@ func TestAccountService_CacheKeys_InvalidAccount(t *testing.T) { assert.Error(t, err) assert.Nil(t, keys) } + +func TestAccountService_CacheKeys_ChangeOnInboxCreate(t *testing.T) { + db, _, svc := setupAccountService(t) + + account := &model.Account{Name: fmt.Sprintf("CacheKeys Inbox Account %d", time.Now().UnixNano())} + require.NoError(t, db.Create(account).Error) + user := createTestUser(t, db, account.ID) + accountUser := &model.AccountUser{AccountID: account.ID, UserID: user.ID, Role: "administrator"} + require.NoError(t, db.Create(accountUser).Error) + + // Before any inbox: key is "0000000000" + keys, err := svc.CacheKeys(context.Background(), account.ID, user.ID) + require.NoError(t, err) + assert.Equal(t, "0000000000", keys["inbox"]) + + // Create an inbox + inbox := &model.Inbox{AccountID: account.ID, Name: "Test Inbox", ChannelType: "web_widget", Enabled: true} + require.NoError(t, db.Create(inbox).Error) + + // After creating an inbox: key should change + keys2, err := svc.CacheKeys(context.Background(), account.ID, user.ID) + require.NoError(t, err) + assert.NotEqual(t, "0000000000", keys2["inbox"]) +} diff --git a/backend/internal/worker/worker.go b/backend/internal/worker/worker.go index 4d530b2b..45dcd189 100644 --- a/backend/internal/worker/worker.go +++ b/backend/internal/worker/worker.go @@ -321,7 +321,7 @@ func (wp *WorkerPool) Start() error { for i := 0; i < workerCount; i++ { wp.wg.Add(1) - go wp.run(ctx) + go wp.run(ctx, i) } // Start the sweep goroutine for delayed-job delivery and stale-job recovery. @@ -378,9 +378,13 @@ func (wp *WorkerPool) RequeueStaleJobs(ctx context.Context) (int64, error) { // run is the per-goroutine consume loop. When Redis is configured it uses // XREADGROUP BLOCK; otherwise it falls back to DB polling. -func (wp *WorkerPool) run(ctx context.Context) { +func (wp *WorkerPool) run(ctx context.Context, index int) { defer wp.wg.Done() + // Each goroutine gets a unique Redis consumer name so XINFO CONSUMERS + // can distinguish them. The DB locked_by field still uses wp.workerID. + consumerID := fmt.Sprintf("%s-%d", wp.workerID, index) + if wp.rdb == nil { wp.runDBPollLoop(ctx) return @@ -395,7 +399,7 @@ func (wp *WorkerPool) run(ctx context.Context) { results, err := wp.rdb.XReadGroup(ctx, &redis.XReadGroupArgs{ Group: wp.consumerGroup, - Consumer: wp.workerID, + Consumer: consumerID, Streams: streams, Count: 1, Block: wp.blockTimeout, diff --git a/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue b/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue index a26eb0e1..2ac200ee 100644 --- a/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue +++ b/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue @@ -539,8 +539,10 @@ export default { // Append the tab key only if it's not the default. const newUrl = tab.key === 'inbox-settings' ? baseUrl : `${baseUrl}/${tab.key}`; - // Update URL without triggering route watcher - window.history.replaceState(null, '', newUrl); + // Update URL without triggering route watcher. + // Preserve history.state so Vue Router's internal navigation state + // (current position, scroll, etc.) is not lost on subsequent navigation. + window.history.replaceState(window.history.state, '', newUrl); }, setTabFromRouteParam() { const { tab: tabParam } = this.$route.params; diff --git a/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Instagram.vue b/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Instagram.vue index f603cfc5..4819ad0d 100644 --- a/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Instagram.vue +++ b/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Instagram.vue @@ -30,7 +30,8 @@ onMounted(() => { } // User need to remove the error params from the url to avoid the error to be shown again after page reload, so that user can try again const cleanURL = window.location.pathname; - window.history.replaceState({}, document.title, cleanURL); + // Preserve history.state so Vue Router's internal state is not lost. + window.history.replaceState(window.history.state, document.title, cleanURL); }); const requestAuthorization = async () => { diff --git a/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Tiktok.vue b/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Tiktok.vue index 50a32683..94646155 100644 --- a/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Tiktok.vue +++ b/frontend/app/javascript/dashboard/routes/dashboard/settings/inbox/channels/Tiktok.vue @@ -30,7 +30,8 @@ onMounted(() => { } // User need to remove the error params from the url to avoid the error to be shown again after page reload, so that user can try again const cleanURL = window.location.pathname; - window.history.replaceState({}, document.title, cleanURL); + // Preserve history.state so Vue Router's internal state is not lost. + window.history.replaceState(window.history.state, document.title, cleanURL); }); const requestAuthorization = async () => { diff --git a/frontend/app/javascript/portal/portalThemeHelper.js b/frontend/app/javascript/portal/portalThemeHelper.js index 497182a8..0d214217 100644 --- a/frontend/app/javascript/portal/portalThemeHelper.js +++ b/frontend/app/javascript/portal/portalThemeHelper.js @@ -33,7 +33,8 @@ export const removeQueryParamsFromUrl = (queryParam = 'theme') => { if (param) { url.searchParams.delete(queryParam); - window.history.replaceState({}, '', url.toString()); // Convert URL to string + // Preserve history.state so Vue Router's internal state is not lost. + window.history.replaceState(window.history.state, '', url.toString()); } };