Skip to content

feat: cache notifications in IndexedDB via storageManager#73

Open
gushishang wants to merge 1 commit into
mainfrom
codex/store-notifications-in-indexeddb
Open

feat: cache notifications in IndexedDB via storageManager#73
gushishang wants to merge 1 commit into
mainfrom
codex/store-notifications-in-indexeddb

Conversation

@gushishang
Copy link
Copy Markdown
Member

Motivation

  • Notification payloads are effectively immutable and can be large, so persisting them in IndexedDB improves durability and performance without changing the API contract.
  • The goal is to keep all storage access behind the app's storage middleware instead of using browser APIs directly.

Description

  • Extended localStorages with notificationsCache and added IndexedDB helpers openIDB, idbGet, and idbSet to src/services/storage/index.ts.
  • Exposed middleware-level methods getObjFromIDB and setObjToIDB on storageManager to read/write structured entries via IndexedDB.
  • Updated src/components/messages/NotificationList.vue to restore cached notifications on startup using the cache key notifications:${CategoryID}:${locale.value}, merge fetched pages with existing items, deduplicate by ID, and persist merged results to IndexedDB.

Testing

  • Ran a full production build with npm run build (includes vue-tsc type check) and the build completed successfully.
  • Verified the application rebuilt with only the intended files changed and no build-time errors were produced.

Codex Task

@wsxiaolin
Copy link
Copy Markdown
Member

@codex

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@gushishang
Copy link
Copy Markdown
Member Author

@codex

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d61ee5154

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CategoryID: number
}>()

const notificationCacheKey = `notifications:${CategoryID}:${locale.value}`
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include user identity in notification cache key

The new cache key only uses CategoryID and locale, so cached notifications are shared across accounts on the same browser profile. On a logout/login switch, restoreNotificationsFromCache() will hydrate another user's messages before any API response arrives, which is a privacy leak and can also leave the list offset (skip) inconsistent for the new account. Add a stable per-user identifier to the key (or clear this store on auth change) to keep notification data isolated.

Useful? React with 👍 / 👎.

const cached = await storageManager.getObjFromIDB<NotificationMessage[]>(notificationCacheKey)
if (cached.status !== 'success' || cached.value == null || cached.value.length === 0) return
items.value = cached.value
skip.value = cached.value.length
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reset offset when refreshing after restoring cached items

Setting skip to cached.value.length causes offset pagination to miss newly arrived notifications. If new messages were inserted at the top since the cache was written, the next /Messages/GetMessages call (which uses Skip: skip.value) skips over those unseen entries, so users can miss fresh notifications indefinitely. After restoring cache, refresh should fetch from the head (or use a cursor/anchor) instead of reusing list length as an offset.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants