Skip to content

refactor(api): genres → src/api modules (Effort 1)#60

Open
chiptus wants to merge 1 commit into
mainfrom
claude/effort1-genres
Open

refactor(api): genres → src/api modules (Effort 1)#60
chiptus wants to merge 1 commit into
mainfrom
claude/effort1-genres

Conversation

@chiptus

@chiptus chiptus commented Jun 25, 2026

Copy link
Copy Markdown
Owner

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

Manual verification

  • Genre badges and the genre filter render on the Artists tab as before.
  • In admin, add a genre and edit an artist's genres — both work.
  • pnpm run lint && pnpm test && pnpm run build all pass.

Generated by Claude Code

Mechanical move of the genres feature from src/hooks/queries/genres to
src/api/genres, 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:27am

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

refactor(api): move genres queries/mutations to src/api/genres
✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Grey Divider

Description

• Move the genres data-access hooks from hooks/queries into src/api/genres.
• Add shared query-key and queryOptions factories for consistent React Query usage.
• Update UI and tests to import genres hooks from the new module paths.
Diagram

graph TD
  A["UI components/pages"] --> B["Genres API hooks"] --> C["React Query cache"] --> D{{"Supabase"}}

  subgraph Legend
    direction LR
    _ui["UI/module"] ~~~ _ext{{"External service"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a src/api/genres/index.ts barrel
  • ➕ Shorter, more stable imports (e.g., '@/api/genres') across the app
  • ➕ Easier future reshuffles inside the module without touching call sites
  • ➖ Adds an extra file/indirection that some teams prefer to avoid
  • ➖ May encourage overly broad imports if not kept curated
2. Keep query keys as const values (no factory)
  • ➕ Slightly simpler call sites (no parentheses)
  • ➕ Less chance of accidentally creating variant keys (if only one key exists)
  • ➖ Doesn’t scale as well when parameterized keys are added later
  • ➖ Inconsistent with common TanStack Query key-factory patterns

Recommendation: Current approach is solid for a modular API layer: centralizing Genre types and using a query key factory + queryOptions improves consistency and sets up future parameterized keys. If this pattern will expand, consider adding a small barrel export (index.ts) to reduce repeated deep import paths and make future moves even more mechanical.

Files changed (9) +28 / -22

Refactor (8) +26 / -20
types.tsAdd shared Genre type and genres query key factory +8/-0

Add shared Genre type and genres query key factory

• Introduces the Genre type and a genresKeys helper for consistent React Query keys. Establishes a single source of truth for key construction used by queries and mutations.

src/api/genres/types.ts

useCreateGenreMutation.tsInvalidate genres cache via genresKeys factory +2/-2

Invalidate genres cache via genresKeys factory

• Switches invalidation from a hard-coded query key to genresKeys.all(). Keeps mutation behavior the same while aligning with the new shared key helper.

src/api/genres/useCreateGenreMutation.ts

useGenres.tsAdd genresQuery() queryOptions factory and move keys/types +11/-13

Add genresQuery() queryOptions factory and move keys/types

• Refactors genres fetching to use a dedicated genresQuery() that returns queryOptions, then consumes it from useGenresQuery(). Moves the query key definition into types.ts and tightens the fetch return type to Genre[].

src/api/genres/useGenres.ts

GenreBadge.tsxUse genres hook from src/api/genres +1/-1

Use genres hook from src/api/genres

• Updates the GenreBadge component to import useGenres from the new src/api/genres module path.

src/components/GenreBadge.tsx

FilterSortControls.tsxRepoint artists filter to genres hook under src/api +1/-1

Repoint artists filter to genres hook under src/api

• Switches the Artists tab filter controls to import useGenres from src/api/genres instead of hooks/queries.

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

AddGenreDialog.tsxImport create-genre mutation from src/api/genres +1/-1

Import create-genre mutation from src/api/genres

• Updates the AddGenreDialog to use the relocated useCreateGenreMutation hook under src/api/genres.

src/pages/admin/AddGenreDialog.tsx

AddArtistDialog.tsxImport useGenresQuery from src/api/genres in admin add-artist flow +1/-1

Import useGenresQuery from src/api/genres in admin add-artist flow

• Repoints the admin AddArtistDialog to import useGenresQuery from src/api/genres, preserving existing behavior for genre selection.

src/pages/admin/ArtistsManagement/AddArtistDialog.tsx

GenresCell.tsxUpdate bulk editor genres cell to new useGenresQuery path +1/-1

Update bulk editor genres cell to new useGenresQuery path

• Switches the bulk editor GenresCell to import useGenresQuery from the new src/api/genres location.

src/pages/admin/ArtistsManagement/BulkEditor/GenresCell.tsx

Tests (1) +2 / -2
GenreBadge.test.tsxUpdate GenreBadge test mocks to new genres hook path +2/-2

Update GenreBadge test mocks to new genres hook path

• Repoints imports and vi.mock targets from the old hooks/queries path to the new api/genres module location.

src/components/GenreBadge.test.tsx

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 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. genresKeys.all uses arrow 📘 Rule violation ⚙ Maintainability
Description
genresKeys.all is implemented as an arrow function, but this rule requires named/exported
functions to use function declarations. Keeping arrow functions for named utilities can undermine
consistent refactor style and enforcement.
Code

src/api/genres/types.ts[R6-8]

+export const genresKeys = {
+  all: () => ["genres"] as const,
+};
Evidence
PR Compliance ID 721308 requires named/exported functions to be written as function declarations. In
src/api/genres/types.ts, genresKeys.all is an exported, reusable named function implemented as
an arrow function.

Rule 721308: Use function declarations for components and named functions
src/api/genres/types.ts[6-8]

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

## Issue description
`genresKeys.all` is a named/exported function implemented as an arrow function inside an exported object. The compliance rule requires function declaration syntax (`function Name(...) {}`) for named/exported functions.

## Issue Context
This module is part of the new `src/api/genres` structure; keep query key factories compliant while preserving the existing `genresKeys.all()` call sites.

## Fix Focus Areas
- src/api/genres/types.ts[6-8]

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


2. Test mocks useGenres hook 📜 Skill insight ▣ Testability
Description
GenreBadge.test.tsx mocks the internal useGenres hook module, which is an internal collaborator
rather than an external system boundary. This makes the test brittle to refactors (e.g., changing
which hook/component provides genres) while keeping behavior the same.
Code

src/components/GenreBadge.test.tsx[R4-6]

+import * as useGenresModule from "@/api/genres/useGenres";

-vi.mock("@/hooks/queries/genres/useGenres");
+vi.mock("@/api/genres/useGenres");
Evidence
PR Compliance ID 730914 forbids mocking internal collaborators. The changed test file directly mocks
the internal @/api/genres/useGenres module via vi.mock(...), which is not an external boundary
(e.g., network/Supabase).

src/components/GenreBadge.test.tsx[4-6]
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 module (`@/api/genres/useGenres`) instead of mocking only at a system boundary. This violates the testing compliance requirement and makes the test depend on implementation details.

## Issue Context
`GenreBadge` uses `useGenres()` internally. Instead of mocking that hook, prefer testing through the component’s observable behavior by providing the real hook with a `QueryClientProvider` and seeding React Query data (or mocking the Supabase client / network boundary).

## Fix Focus Areas
- src/components/GenreBadge.test.tsx[4-6]

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


Grey Divider

Qodo Logo

Comment thread src/api/genres/types.ts
Comment on lines +6 to +8
export const genresKeys = {
all: () => ["genres"] as const,
};

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

1. genreskeys.all uses arrow 📘 Rule violation ⚙ Maintainability

genresKeys.all is implemented as an arrow function, but this rule requires named/exported
functions to use function declarations. Keeping arrow functions for named utilities can undermine
consistent refactor style and enforcement.
Agent Prompt
## Issue description
`genresKeys.all` is a named/exported function implemented as an arrow function inside an exported object. The compliance rule requires function declaration syntax (`function Name(...) {}`) for named/exported functions.

## Issue Context
This module is part of the new `src/api/genres` structure; keep query key factories compliant while preserving the existing `genresKeys.all()` call sites.

## Fix Focus Areas
- src/api/genres/types.ts[6-8]

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

Comment on lines +4 to +6
import * as useGenresModule from "@/api/genres/useGenres";

vi.mock("@/hooks/queries/genres/useGenres");
vi.mock("@/api/genres/useGenres");

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 usegenres hook 📜 Skill insight ▣ Testability

GenreBadge.test.tsx mocks the internal useGenres hook module, which is an internal collaborator
rather than an external system boundary. This makes the test brittle to refactors (e.g., changing
which hook/component provides genres) while keeping behavior the same.
Agent Prompt
## Issue description
The test mocks an internal module (`@/api/genres/useGenres`) instead of mocking only at a system boundary. This violates the testing compliance requirement and makes the test depend on implementation details.

## Issue Context
`GenreBadge` uses `useGenres()` internally. Instead of mocking that hook, prefer testing through the component’s observable behavior by providing the real hook with a `QueryClientProvider` and seeding React Query data (or mocking the Supabase client / network boundary).

## Fix Focus Areas
- src/components/GenreBadge.test.tsx[4-6]

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

chiptus commented Jun 25, 2026

Copy link
Copy Markdown
Owner Author

Triage of Qodo's two findings — both pre-existing/codebase-consistent, no behavior impact; keeping #60 a mechanical move:

1. genresKeys.all arrow function (rule 721308). Every key factory in the codebase uses this exact shape — festivalsKeys.all, editionsKeys.all, setsKeys all do all: () => [...] as const. The CLAUDE.md "use function declarations" rule targets components and standalone named functions, not arrow values inside a key-factory object (a function declaration can't be an object property anyway). Converting only genres would make it inconsistent with every other feature's keys. Declining to keep the pattern uniform — switching key factories to method shorthand would be a codebase-wide change, not this slice.

2. Test mocks internal useGenres (tdd skill insight). Pre-existing — the diff shows this PR only repointed the existing vi.mock(...) path (@/hooks/queries/genres/useGenres@/api/genres/useGenres); the internal-collaborator mock predates the move. Reworking GenreBadge's test to render through a QueryClientProvider (instead of mocking the hook) is a genuine test-quality improvement, but it's its own change, out of scope for a mechanical move.

No code changes here.


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