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).
This commit is contained in:
@@ -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":"<id>"}'
|
||||
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":"<id>"}'
|
||||
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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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"])
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
+2
-1
@@ -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 () => {
|
||||
|
||||
+2
-1
@@ -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 () => {
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user