Skip to content

refactor: introduce storage abstraction layer behind TodoStore - #63

Merged
xiaomi7732 merged 7 commits into
mainfrom
feature/storage-abstraction
May 16, 2026
Merged

refactor: introduce storage abstraction layer behind TodoStore#63
xiaomi7732 merged 7 commits into
mainfrom
feature/storage-abstraction

Conversation

@xiaomi7732

Copy link
Copy Markdown
Owner

Summary

Closes part of #62 — refactors the existing Calendar-based data access behind a TodoStore interface so future backends can be added without touching the UI.

This is a strict refactor. No user-visible behavior change, no new dependencies, no new MSAL scopes. The bottom-line recommendation from the decision document was to stay on Calendar and prepare the abstraction layer as a low-risk hedge — this PR is that hedge.

Why now (and why not Excel)

The original direction was to add Excel as a second backend. Validation against Microsoft's own docs confirmed Excel REST API doesn't support personal Microsoft accounts (consumers authority), which is what this app uses. The abstraction ships anyway — it's a small, low-risk refactor that opens the door for future backends (work/school Excel, client-side .xlsx upload, Google Sheets with a future auth abstraction, etc.) and cleans up the data layer regardless.

Architecture

lib/
  graphService.ts          # trimmed to createGraphClient + getUserInfo
  store/
    types.ts               # TodoStore, Book, ListItemsOptions, ID prefix helpers
    multiStore.ts          # MultiBackendStore (router; today routes only to Calendar)
    useStore.ts            # React hook constructing the store
    calendar/
      CalendarStore.ts     # implements TodoStore using Calendar API
      utils.ts             # ' by arrange' suffix logic
      bump.ts              # date-bump helpers
      types.ts             # Graph response shapes

lib/todoDataService.ts and lib/calendarUtils.ts removed — their logic moved into the store.

Key design points (per rubber-duck feedback)

  • ID prefixing at the storage boundary: book IDs are cal:<calendarId>. Routing dispatches on the prefix. Item IDs are bare backend IDs (they only exist within a Book).
  • Explicit listItems ranges{ range: 'all' | 'window' }. No implicit defaults. Matrix/Scrum use 'window'; Cancelled uses 'all'.
  • createBook takes a backend — required even though only one option today, so callers are future-proof.
  • Sweep stays calendar-specificCalendarStore.sweepStaleItems() is not on the TodoStore interface. Matrix uses store.calendar.sweepStaleItems(...) via the typed accessor.
  • Backward compatibility — unprefixed URLs (/matrix?bookId=<raw>) and unprefixed localStorage values are normalized on read; URL is replaced with the canonical prefixed form.

Diff size

Lines
Added +1089
Removed −1036
Net +53

The bulk of the diff is moving CRUD/serialization code into the new structure. The net line growth is from the new abstraction types and the multi-store router scaffolding.

Acceptance checklist

  • All Graph call sites moved inside CalendarStore.
  • No page imports from graphService for CRUD or from todoDataService/calendarUtils (deleted).
  • Unprefixed URL ?bookId=<raw> is accepted and normalized.
  • Unprefixed localStorage lastBookId is accepted and normalized.
  • npm run build passes.
  • npm run lint shows no new errors/warnings (pre-existing ones unchanged).
  • Manual smoke test (handed off in PR review) — sign in, create book, drag items, status change, sweep, sign out/in.

Out of scope

  • No new MSAL scopes.
  • No Excel/Google Sheets/etc. backend.
  • No AuthClient abstraction (storage only, per scope decision).
  • No new test framework.

The interface is designed to slot in additional backends later if the account model or Microsoft's Excel API support changes.

Refactors the existing Calendar-based data access behind a TodoStore
interface so future backends can be added without touching consumers.
Strict refactor — no user-visible behavior change, no new dependencies,
no new MSAL scopes.

Architecture:
- lib/store/types.ts: TodoStore, Book, ListItemsOptions, ID prefix helpers
- lib/store/multiStore.ts: MultiBackendStore (router; today routes only
  to Calendar via 'cal:' prefix)
- lib/store/useStore.ts: React hook constructing the store
- lib/store/calendar/CalendarStore.ts: implements TodoStore against
  Microsoft Graph Calendar API (was graphService.ts CRUD + todoDataService.ts)
- lib/store/calendar/utils.ts: ' by arrange' suffix logic, display name
- lib/store/calendar/bump.ts: date-bump helpers (calendar-specific)
- lib/store/calendar/types.ts: Graph response shapes

Other changes:
- graphService.ts trimmed to createGraphClient + getUserInfo
- todoDataService.ts and calendarUtils.ts removed (logic moved into store)
- Book IDs are now prefixed (cal:<id>) at the storage boundary
- Backward compat: unprefixed URLs and localStorage values are normalized
  on read and the URL is replaced with the canonical prefixed form
- listItems now requires an explicit { range: 'all' | 'window' } option
  (no implicit defaults — caller must specify)
- createBook takes { backend } so callers are future-proof

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 16, 2026 01:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Arrange’s calendar-backed TODO data access behind a new TodoStore abstraction while keeping the current Microsoft Graph Calendar backend.

Changes:

  • Introduces store interfaces, ID prefix helpers, MultiBackendStore, and CalendarStore.
  • Moves calendar CRUD, TODO serialization/parsing, suffix utilities, and date-bump logic into lib/store/calendar.
  • Updates pages/components to consume books/items through the store abstraction instead of direct Graph/todo services.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/arrange-v4/lib/todoDataService.ts Removed legacy TODO/calendar event data service.
src/arrange-v4/lib/calendarUtils.ts Removed legacy Arrange-calendar helper module.
src/arrange-v4/lib/graphService.ts Trimmed to Graph client creation and user info lookup.
src/arrange-v4/lib/store/types.ts Adds store interfaces, TODO/book types, and book ID prefix helpers.
src/arrange-v4/lib/store/useStore.ts Adds React hook for constructing the multi-backend store.
src/arrange-v4/lib/store/multiStore.ts Adds backend router implementation for TodoStore.
src/arrange-v4/lib/store/calendar/types.ts Adds narrow Graph calendar/event response shapes.
src/arrange-v4/lib/store/calendar/utils.ts Adds calendar suffix filtering/display and Graph date conversion helpers.
src/arrange-v4/lib/store/calendar/bump.ts Moves calendar-specific stale date bumping logic.
src/arrange-v4/lib/store/calendar/CalendarStore.ts Implements TodoStore using Microsoft Graph Calendar APIs.
src/arrange-v4/lib/hooks/useBookId.ts Refactors selected-book resolution to use store books and prefixed IDs.
src/arrange-v4/components/AddTodoItem.tsx Updates TODO type imports to store types.
src/arrange-v4/components/ViewTodoItem.tsx Updates TODO type imports to store types.
src/arrange-v4/components/ManageTags.tsx Updates TODO type imports to store types.
src/arrange-v4/components/ScrumCard.tsx Updates card props to use required item IDs from store types.
src/arrange-v4/components/CalendarList.tsx Refactors list UI from calendars to abstract books.
src/arrange-v4/app/page.tsx Refactors post-login routing and matrix availability checks through the store.
src/arrange-v4/app/books/page.tsx Refactors book listing/create/delete to use the store.
src/arrange-v4/app/matrix/page.tsx Refactors matrix item CRUD, listing, and sweep logic through the store.
src/arrange-v4/app/scrum/page.tsx Refactors scrum board item CRUD/listing through the store.
src/arrange-v4/app/cancelled/page.tsx Refactors cancelled item listing/deletion through the store.

Comment thread src/arrange-v4/app/page.tsx Outdated
Comment thread src/arrange-v4/lib/store/calendar/CalendarStore.ts Outdated
- app/page.tsx handleLogin: construct a one-shot MultiBackendStore using
  the access token from loginPopup so the listBooks() call doesn't race
  MSAL's React state update (useGraphToken would otherwise throw 'no account')
- CalendarStore.createBook: use the case-insensitive ARRANGE_SUFFIX_REGEX
  (already used elsewhere) instead of an exact-match endsWith, so names
  like 'Project by Arrange' don't get a duplicate ' by arrange' appended

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comment thread src/arrange-v4/app/page.tsx Outdated
Comment thread src/arrange-v4/app/page.tsx Outdated
…uting error

- app/page.tsx background availability check now uses acquireTokenSilent
  directly (no popup fallback). If the silent acquisition fails, we just
  skip showing the matrix buttons instead of opening an unexpected popup.
- handleLogin: a transient listBooks failure no longer leaves the user
  stuck on the landing page. Post-login routing decision wrapped in its
  own try/catch with /books as the fallback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Comment thread src/arrange-v4/app/page.tsx
Comment thread src/arrange-v4/lib/store/calendar/CalendarStore.ts
Comment thread src/arrange-v4/lib/store/types.ts
…ilable

- CalendarStore now deduplicates concurrent token acquisitions via an
  in-flight promise. Prevents the worst-case scenario of N concurrent
  popup attempts (each failing with interaction_in_progress) when a
  bulk operation runs after the silent token has expired.
- parseBookId rejects unknown prefixes (values containing ':' that
  don't match a known backend) instead of silently treating them as
  legacy calendar IDs. Preserves the prefix-dispatch contract.
- app/page.tsx background check now resets matrixAvailable on failure
  so stale data from a previous successful check or account switch
  doesn't linger.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comment thread src/arrange-v4/lib/store/calendar/CalendarStore.ts Outdated
Comment thread src/arrange-v4/lib/hooks/useBookId.ts Outdated
- CalendarStore.getToken: replace void .finally() chain with .then(clear,
  clear) so the chained promise can't surface as an unhandled rejection
  when token acquisition fails
- useBookId: distinguish missing URL bookId from invalid URL bookId.
  Missing → fall back to saved book. Invalid (present but unknown prefix)
  → redirect to /books. Prevents silently loading a different book from
  a malformed link.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@xiaomi7732

Copy link
Copy Markdown
Owner Author

PR Review Loop Summary

  • 5 review rounds completed
  • 8 issues raised across rounds — all fixed:
    • Round 1 (2 issues): Post-loginPopup race with MSAL state (use one-shot store from loginPopup token); case-sensitive suffix check (use shared case-insensitive regex)
    • Round 2 (2 issues): handleLogin error swallowing (wrap routing in own try/catch, fall back to /books); background availability check triggered popup (use acquireTokenSilent directly)
    • Round 3 (3 issues): Background check didn't reset matrixAvailable on failure; per-method token acquisition could cause N concurrent popups (added in-flight token deduplication); parseBookId silently treated unknown prefixes as legacy (now rejects values with unknown prefix)
    • Round 4 (2 issues): void p.finally(...) created unhandled rejection (replaced with then(clear, clear)); useBookId silently fell back to last book on invalid URL (now distinguishes missing from invalid)
    • Round 5: Clean — zero unresolved comments ✅
  • Final status: All review threads resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comment thread src/arrange-v4/lib/store/calendar/CalendarStore.ts Outdated
Comment thread src/arrange-v4/lib/store/calendar/bump.ts Outdated
- CalendarStore.updateItem now honors caller-provided values for
  startDateTime, finishDateTime, originalEtsDateTime, originalEtaDateTime.
  The status-transition side-effects only fill in fields the caller did
  not explicitly set, preserving both the existing 'pass status, store
  derives timestamps' UX and the abstraction's documented contract.
- bump.ts: comment clarified that bumping fires the moment an item
  becomes stale (ETS before today), not only after falling outside the
  ±30-day window.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Comment thread src/arrange-v4/app/page.tsx
Use a 'cancelled' flag in the background availability check effect so
a slow listBooks() response from a previous account can't clobber
matrixAvailable state after the user switches accounts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.

@xiaomi7732

Copy link
Copy Markdown
Owner Author

PR Review Loop Summary (updated)

  • 7 review rounds completed
  • 11 issues raised across rounds — all fixed:
    • Round 1 (2): post-loginPopup MSAL race; case-sensitive suffix check
    • Round 2 (2): login error swallowing; background check triggered popup
    • Round 3 (3): background check didn't reset state on failure; per-method token acquisition could cause N concurrent popups; parseBookId silently treated unknown prefixes as legacy
    • Round 4 (2): void p.finally(...) unhandled rejection; useBookId silently fell back to last book on invalid URL
    • Round 5 (2, late-arriving): updateItem silently dropped caller-provided body-backed fields; bump.ts comment was inaccurate
    • Round 6 (1, late-arriving): availability-check race on account switch (stale response could clobber new state)
    • Round 7: Clean — zero unresolved comments ✅
  • Final status: All review threads resolved

@xiaomi7732
xiaomi7732 merged commit 4b5d5a1 into main May 16, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants