Skip to content

refactor(api): stages → src/api modules (Effort 1)#62

Open
chiptus wants to merge 2 commits into
mainfrom
claude/effort1-stages
Open

refactor(api): stages → src/api modules (Effort 1)#62
chiptus wants to merge 2 commits into
mainfrom
claude/effort1-stages

Conversation

@chiptus

@chiptus chiptus commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Mechanical move of the stages feature from src/hooks/queries/stages to src/api/stages, adding queryOptions factories for the query hooks. No behavior change. Part of #52.

Manual verification

  • In an edition, open the Schedule tab — stage lanes and stage filter buttons render as before.
  • In admin Stage management: create, edit, and delete a stage — all succeed; the stage selector and stage pins still work.
  • pnpm run lint && pnpm test && pnpm run build all pass.

Generated by Claude Code

Mechanical move of the stages feature from src/hooks/queries/stages to
src/api/stages, adding queryOptions factories for the query hooks. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K513rsQz6Lg1HbbfYiafrE
@vercel

vercel Bot commented Jun 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
upline Ready Ready Preview, Comment Jun 25, 2026 3:42am

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Refactor stages queries into src/api/stages with queryOptions factories
✨ Enhancement 🕐 20-40 Minutes

Grey Divider

Description

• Move stage query/mutation hooks into src/api/stages modules.
• Add reusable queryOptions factories for stage-by-id and stages-by-edition queries.
• Update all imports across UI, admin, utils, and tests; no behavior change.
Diagram

graph TD
  EditionUI["Edition view UI"] --> StagesAPI["Stages API hooks"] --> ReactQuery["TanStack Query"] --> Supabase["Supabase stages table"]
  AdminUI["Admin stage mgmt"] --> StagesAPI
  StagesAPI --> SharedTypes["Stage type + utils"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Temporary compatibility re-exports in old hooks path
  • ➕ Reduces churn for downstream branches and avoids large import diffs
  • ➕ Enables incremental migration across the codebase
  • ➖ Prolongs dual locations during migration
  • ➖ Adds maintenance overhead and potential circular-dependency risks
2. Add a single barrel entrypoint (src/api/stages/index.ts) and import from it
  • ➕ Stabilizes import paths and reduces future refactor surface area
  • ➕ Improves discoverability for consumers
  • ➖ Can encourage overly broad imports if not disciplined
  • ➖ Still requires a one-time migration of call sites

Recommendation: The PR’s approach is appropriate for consolidating data access under src/api/* and enabling reuse via queryOptions factories. If ongoing migration causes cross-branch friction, consider adding a short-lived re-export shim or a disciplined index.ts barrel to stabilize imports while finishing #52.

Files changed (25) +51 / -40

Refactor (24) +49 / -38
types.tsRe-export Stage type and keep stages query key factory +1/-4

Re-export Stage type and keep stages query key factory

• Replaces the local Supabase Database-derived Stage type with a re-export and keeps the stagesKeys query key factory (all/byEdition/byId).

src/api/stages/types.ts

useCreateStage.tsStage create mutation hook under src/api/stages +0/-0

Stage create mutation hook under src/api/stages

• Implements createStage via Supabase insert (including slug generation) and invalidates stage queries on success with toast feedback.

src/api/stages/useCreateStage.ts

useDeleteStage.tsStage delete (archive) mutation hook under src/api/stages +0/-0

Stage delete (archive) mutation hook under src/api/stages

• Archives a stage via Supabase update and invalidates stage queries on success with toast feedback.

src/api/stages/useDeleteStage.ts

useStageQuery.tsAdd stageQuery() queryOptions factory and export fetchStage() +12/-6

Add stageQuery() queryOptions factory and export fetchStage()

• Exports fetchStage() and introduces stageQuery(stageId) returning queryOptions; useStageQuery now spreads those options and uses enabled to guard missing ids.

src/api/stages/useStageQuery.ts

useStagesByEdition.tsAdd stagesByEditionQuery() factory and export fetchStagesByEdition() +14/-6

Add stagesByEditionQuery() factory and export fetchStagesByEdition()

• Exports fetchStagesByEdition() and introduces stagesByEditionQuery(editionId) returning queryOptions; retains sortStagesByOrder for consistent ordering.

src/api/stages/useStagesByEdition.ts

useUpdateStage.tsStage update mutation hook under src/api/stages +0/-0

Stage update mutation hook under src/api/stages

• Updates stage fields via Supabase update and invalidates stage queries on success with toast feedback.

src/api/stages/useUpdateStage.ts

ReviewStage.tsxSwitch stages imports to src/api/stages +2/-2

Switch stages imports to src/api/stages

• Repoints stagesKeys and useStagesByEditionQuery imports from the prior hooks/queries location to src/api/stages.

src/components/Admin/ScheduleImport/ReviewStage.tsx

StagePin.tsxUpdate useStageQuery import path +1/-1

Update useStageQuery import path

• Repoints StagePin to use useStageQuery from src/api/stages.

src/components/StagePin.tsx

useScheduleData.tsUpdate Stage type import to src/api/stages +1/-1

Update Stage type import to src/api/stages

• Switches the Stage type import used for schedule modeling to @/api/stages/types.

src/hooks/useScheduleData.ts

stageUtils.tsUpdate Stage type import to src/api/stages +1/-1

Update Stage type import to src/api/stages

• Repoints Stage type import for stage sorting utilities to @/api/stages/types.

src/lib/stageUtils.ts

timelineCalculator.tsUpdate Stage type import to src/api/stages +1/-1

Update Stage type import to src/api/stages

• Repoints Stage type import for timeline calculations to @/api/stages/types.

src/lib/timelineCalculator.ts

SetMetadata.tsxUpdate useStageQuery import path +1/-1

Update useStageQuery import path

• Repoints SetMetadata to use useStageQuery from src/api/stages.

src/pages/EditionView/tabs/ArtistsTab/SetCard/SetMetadata.tsx

DesktopFilters.tsxUpdate useStagesByEditionQuery import path (desktop) +1/-1

Update useStagesByEditionQuery import path (desktop)

• Repoints DesktopFilters to use useStagesByEditionQuery from src/api/stages.

src/pages/EditionView/tabs/ArtistsTab/filters/DesktopFilters.tsx

MobileFilters.tsxUpdate useStagesByEditionQuery import path (mobile) +1/-1

Update useStagesByEditionQuery import path (mobile)

• Repoints MobileFilters to use useStagesByEditionQuery from src/api/stages.

src/pages/EditionView/tabs/ArtistsTab/filters/MobileFilters.tsx

StageFilterButtons.tsxUpdate useStagesByEditionQuery import path (schedule filters) +1/-1

Update useStagesByEditionQuery import path (schedule filters)

• Repoints StageFilterButtons to use useStagesByEditionQuery from src/api/stages.

src/pages/EditionView/tabs/ScheduleTab/StageFilterButtons.tsx

Timeline.tsxUpdate useStagesByEditionQuery import path (timeline) +1/-1

Update useStagesByEditionQuery import path (timeline)

• Repoints the horizontal Timeline to use useStagesByEditionQuery from src/api/stages.

src/pages/EditionView/tabs/ScheduleTab/horizontal/Timeline.tsx

ListSchedule.tsxUpdate useStagesByEditionQuery import path (list schedule) +1/-1

Update useStagesByEditionQuery import path (list schedule)

• Repoints ListSchedule to use useStagesByEditionQuery from src/api/stages.

src/pages/EditionView/tabs/ScheduleTab/list/ListSchedule.tsx

SetCardHeader.tsxUpdate useStageQuery import path (explore card) +1/-1

Update useStageQuery import path (explore card)

• Repoints SetCardHeader to use useStageQuery from src/api/stages.

src/pages/ExploreSetPage/SetExploreCard/SetCardHeader.tsx

SetsTable.tsxUpdate useStagesByEditionQuery import path (admin sets) +1/-1

Update useStagesByEditionQuery import path (admin sets)

• Repoints the admin SetsTable to use useStagesByEditionQuery from src/api/stages.

src/pages/admin/festivals/SetsTable.tsx

StageManagement.tsxUpdate stage query/mutation and Stage type imports (admin) +3/-3

Update stage query/mutation and Stage type imports (admin)

• Repoints StageManagement to src/api/stages for list/delete hooks and updates Stage typing to the new location.

src/pages/admin/festivals/StageManagement.tsx

CreateStageDialog.tsxUpdate create mutation import path (admin) +1/-1

Update create mutation import path (admin)

• Repoints CreateStageDialog to use useCreateStageMutation from src/api/stages.

src/pages/admin/festivals/StageManagement/CreateStageDialog.tsx

EditStageDialog.tsxUpdate update mutation and Stage type imports (admin) +2/-2

Update update mutation and Stage type imports (admin)

• Repoints EditStageDialog to src/api/stages for update hook and Stage typing.

src/pages/admin/festivals/StageManagement/EditStageDialog.tsx

StagesTable.tsxUpdate Stage type import path (admin table) +1/-1

Update Stage type import path (admin table)

• Repoints StagesTable to import Stage type from src/api/stages/types.

src/pages/admin/festivals/StageManagement/StagesTable.tsx

StageSelector.tsxUpdate useStagesByEditionQuery import path (admin selector) +1/-1

Update useStagesByEditionQuery import path (admin selector)

• Repoints StageSelector to use useStagesByEditionQuery from src/api/stages.

src/pages/admin/festivals/StageSelector.tsx

Tests (1) +2 / -2
StagePin.test.tsxUpdate mocked useStageQuery import path +2/-2

Update mocked useStageQuery import path

• Updates the module import and vi.mock target to @/api/stages/useStageQuery.

src/components/StagePin.test.tsx

@qodo-code-review

qodo-code-review Bot commented Jun 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (1)

Context used
✅ Compliance rules (platform): 109 rules
✅ Skills: 4 invoked
  supabase
  supabase-postgres-best-practices
  tdd
  improve-codebase-architecture

Grey Divider


Action required

1. Test mocks useStageQuery 📜 Skill insight ▣ Testability
Description
StagePin.test.tsx mocks the internal hook module @/api/stages/useStageQuery, which violates the
requirement to only mock at external system boundaries. This makes the test couple to internal
module structure and breaks under internal refactors.
Code

src/components/StagePin.test.tsx[R4-8]

+import * as useStageQueryModule from "@/api/stages/useStageQuery";

type StageQueryResult = ReturnType<typeof useStageQueryModule.useStageQuery>;

-vi.mock("@/hooks/queries/stages/useStageQuery");
+vi.mock("@/api/stages/useStageQuery");
Evidence
The checklist prohibits mocking internal collaborators; mocks should only be used at system
boundaries. The changed lines show vi.mock("@/api/stages/useStageQuery") and the corresponding
module import, which is an internal project module rather than an external boundary.

src/components/StagePin.test.tsx[4-8]
Skill: tdd

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test mocks an internal collaborator (`@/api/stages/useStageQuery`) via `vi.mock(...)`, which violates the rule that mocks must be used only at system boundaries.

## Issue Context
Instead of mocking `useStageQuery`, render `StagePin` with a real `QueryClientProvider` and seed the query cache for `stagesKeys.byId(stageId)` (or mock the external boundary, e.g., Supabase network calls, via a boundary-level approach).

## Fix Focus Areas
- src/components/StagePin.test.tsx[4-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Stage type re-exported ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
src/api/stages/types.ts uses export type { ... } from ..., which is a re-export pattern
disallowed by the checklist unless explicitly required. This can encourage indirect imports and
obscures the true owning module for Stage.
Code

src/api/stages/types.ts[1]

+export type { Stage } from "@/api/sets/types";
Evidence
The checklist forbids export { ... } from "..." / export * from "..." re-exports unless
explicitly required. The changed code adds a export type { Stage } from "@/api/sets/types"
re-export in src/api/stages/types.ts.

Rule 721321: Avoid export ... from re-exports unless explicitly required
src/api/stages/types.ts[1-1]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/api/stages/types.ts` uses a re-export (`export type { Stage } from ...`), which violates the rule forbidding `export ... from` re-exports unless explicitly required.

## Issue Context
This file can instead define `Stage` directly (as it did previously via `Database[...]`) so consumers import from the concrete owning module without relying on re-export indirection.

## Fix Focus Areas
- src/api/stages/types.ts[1-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Nullish stageId in query ✓ Resolved 🐞 Bug ☼ Reliability
Description
useStageQuery() and useStagesByEditionQuery() accept nullish IDs but still invoke their exported
query factories (stageQuery(stageId!) / stagesByEditionQuery(editionId!)) using non-null
assertions, effectively passing null/undefined into functions typed to require string. This
undermines type-safety and can lead to inconsistent cache keys (e.g., null/undefined vs "")
because the factories build queryKey/queryFn from invalid values.
Code

src/api/stages/useStageQuery.ts[R31-35]

export function useStageQuery(stageId: string | undefined | null) {
  return useQuery({
-    queryKey: stagesKeys.byId(stageId || ""),
-    queryFn: () => fetchStage(stageId!),
+    ...stageQuery(stageId!),
    enabled: !!stageId,
  });
Evidence
Both hooks are typed to accept potentially missing identifiers (the stage hook allows `string |
undefined | null, and the edition hook allows string | undefined`), and there are normal call
paths where callers provide null/undefined (e.g., components passing a string | null stage id
or using optional chaining like edition?.id while data is loading). Despite this, each hook
forwards the value into its respective query factory with a non-null assertion (!), which forces
TypeScript to treat the value as a string and allows null/undefined to flow into a factory
that constructs cache keys and query functions based on that parameter, creating the inconsistency
and weakening the intended string contract.

src/api/stages/useStageQuery.ts[24-36]
src/components/StagePin.tsx[4-6]
src/pages/ExploreSetPage/SetExploreCard/SetCardHeader.tsx[7-15]
src/api/stages/types.ts[3-7]
src/api/stages/useStagesByEdition.ts[24-36]
src/pages/EditionView/tabs/ScheduleTab/StageFilterButtons.tsx[15-17]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`useStageQuery(stageId)` and `useStagesByEditionQuery(editionId)` accept IDs that may be `null`/`undefined`, but they currently call `stageQuery(stageId!)` and `stagesByEditionQuery(editionId!)`, which can forward invalid values into query factories typed as requiring `string`.

## Issue Context
Several call sites intentionally or naturally pass missing IDs (e.g., a `string | null` stage id and `edition?.id` while edition data is loading), so this can occur during normal execution and may produce inconsistent React Query cache keys (missing value vs empty string) while weakening type-safety.

## Fix Focus Areas
- src/api/stages/useStageQuery.ts[24-36]
- src/api/stages/useStagesByEdition.ts[24-36]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/api/stages/types.ts Outdated
Comment on lines +4 to +8
import * as useStageQueryModule from "@/api/stages/useStageQuery";

type StageQueryResult = ReturnType<typeof useStageQueryModule.useStageQuery>;

vi.mock("@/hooks/queries/stages/useStageQuery");
vi.mock("@/api/stages/useStageQuery");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Test mocks usestagequery 📜 Skill insight ▣ Testability

StagePin.test.tsx mocks the internal hook module @/api/stages/useStageQuery, which violates the
requirement to only mock at external system boundaries. This makes the test couple to internal
module structure and breaks under internal refactors.
Agent Prompt
## Issue description
The test mocks an internal collaborator (`@/api/stages/useStageQuery`) via `vi.mock(...)`, which violates the rule that mocks must be used only at system boundaries.

## Issue Context
Instead of mocking `useStageQuery`, render `StagePin` with a real `QueryClientProvider` and seed the query cache for `stagesKeys.byId(stageId)` (or mock the external boundary, e.g., Supabase network calls, via a boundary-level approach).

## Fix Focus Areas
- src/components/StagePin.test.tsx[4-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

… key normalization

Two deviations the mechanical move introduced:
- types.ts re-exported Stage from sets (violates no-export-from rule); restore
  the original local definition (Database stages Row), as on main.
- useStageQuery/useStagesByEdition switched stagesKeys.byId(stageId || "") to
  stageQuery(stageId!); restore ?? "" so the disabled-state key matches main
  (stageId can be null).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K513rsQz6Lg1HbbfYiafrE

chiptus commented Jun 25, 2026

Copy link
Copy Markdown
Owner Author

Triage of Qodo's three findings — two were real deviations the move introduced (now fixed in d8ee4a1), one is pre-existing:

1. export type { Stage } from "@/api/sets/types" (rule 721321 / CLAUDE.md "no export from") — fixed. main's stages/types.ts defined Stage locally (Database["public"]["Tables"]["stages"]["Row"]); the move replaced that with a re-export. Restored the original local definition. (Stage also living in sets/types.ts is pre-existing duplication from the sets slice — a later consolidation can pick one owner; not this mechanical move's job.)

3. stageQuery(stageId!) / stagesByEditionQuery(editionId!) nullish key (bug) — fixed. Same regression as #61: the originals normalized with stagesKeys.byId(stageId || "") / byEdition(editionId || ""); the move changed them to ! (and stageId can be null). Restored ?? "" so the disabled-state cache key matches main. No runtime impact (queries are disabled when id is missing), but it keeps the move behavior-neutral.

2. StagePin.test.tsx mocks internal useStageQuery (tdd skill) — declined (pre-existing). The diff only repoints the existing vi.mock(...) path; the internal-hook mock predates the move. Reworking it to render through a QueryClientProvider is a worthwhile test-quality change, but out of scope here.

Lint/build/290 tests green after the fix.


Generated by Claude Code

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