Skip to content

refactor(api): custom-links → src/api modules (Effort 1)#61

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

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

Conversation

@chiptus

@chiptus chiptus commented Jun 25, 2026

Copy link
Copy Markdown
Owner

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

Manual verification

  • Website/social links render on the festival selection page and the edition Info tab as before.
  • In admin, add/edit a festival custom link — it saves and displays.
  • pnpm run lint && pnpm test && pnpm run build all pass.

Generated by Claude Code

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

refactor(api): move custom-links queries to src/api with queryOptions factories
✨ Enhancement 🕐 10-20 Minutes

Grey Divider

Description

• Move custom-links query/mutation logic from hooks area into src/api/custom-links.
• Extract shared CustomLink type and query keys into a dedicated types module.
• Add a queryOptions factory and update all consumers to the new import paths.
Diagram

graph TD
  A["UI pages/admin"] --> B["useCustomLinksQuery()"] --> C["customLinksQuery()"] --> D["Supabase client"] --> E[("custom_links table")]
  F["useBulkUpdateCustomLinksMutation()"] --> D
  subgraph Legend
    direction LR
    _ui["UI component"] ~~~ _api["API module"] ~~~ _ext{{"External service"}} ~~~ _db[("Database")]
  end
Loading
High-Level Assessment

This is a clean, low-risk refactor: consolidating feature-specific data access under src/api/custom-links and adding a queryOptions factory improves consistency and reuse without changing behavior. Alternatives (like leaving hooks under src/hooks/queries and re-exporting) don’t materially improve maintainability versus the chosen structure.

Files changed (8) +26 / -19

Refactor (8) +26 / -19
types.tsAdd shared CustomLink type and query key helpers +9/-0

Add shared CustomLink type and query key helpers

• Introduces a CustomLink type alias based on Supabase table typing and centralizes React Query key generation for the custom-links feature.

src/api/custom-links/types.ts

useCustomLinks.tsExtract queryOptions factory and reuse it in the query hook +10/-12

Extract queryOptions factory and reuse it in the query hook

• Moves type/key definitions to the new types module and adds customLinksQuery() to return queryOptions. Updates useCustomLinksQuery() to spread the factory output while preserving conditional enablement.

src/api/custom-links/useCustomLinks.ts

useCustomLinksMutation.tsRepoint mutation invalidation to shared keys module +1/-1

Repoint mutation invalidation to shared keys module

• Updates the import of customLinksKeys to come from types.ts so query invalidation stays consistent after the module split.

src/api/custom-links/useCustomLinksMutation.ts

EditionLayout.tsxUpdate custom-links hook import to src/api +1/-1

Update custom-links hook import to src/api

• Switches useCustomLinksQuery import from the hooks/queries path to the new api/custom-links location.

src/pages/EditionView/EditionLayout.tsx

InfoTab.tsxUpdate custom-links hook import to src/api +1/-1

Update custom-links hook import to src/api

• Repoints the Info tab’s custom link fetching hook to the new api/custom-links module path.

src/pages/EditionView/tabs/InfoTab.tsx

FestivalSelection.tsxUpdate custom-links hook import to src/api +1/-1

Update custom-links hook import to src/api

• Updates the festival selection page to import the custom-links query hook from src/api/custom-links.

src/pages/FestivalSelection.tsx

FestivalLinksField.tsxUpdate admin field imports to new custom-links API modules +2/-2

Update admin field imports to new custom-links API modules

• Replaces imports of CustomLink and the bulk update mutation hook to come from src/api/custom-links, matching the feature relocation.

src/pages/admin/festivals/info/FestivalFields/FestivalLinksField.tsx

FestivalInfoDetails.tsxUpdate custom-links hook import to src/api +1/-1

Update custom-links hook import to src/api

• Switches the admin festival info view’s useCustomLinksQuery import to the new src/api/custom-links module.

src/pages/admin/festivals/info/FestivalInfoDetails.tsx

@qodo-code-review

qodo-code-review Bot commented Jun 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

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

Grey Divider


Remediation recommended

1. customLinksKeys.byFestival arrow function 📘 Rule violation ⚙ Maintainability
Description
The exported customLinksKeys object defines byFestival as an arrow function rather than a
function declaration. This violates the requirement that named/exported functions use `function
Name(...) { ... }` declarations.
Code

src/api/custom-links/types.ts[R5-8]

+export const customLinksKeys = {
+  all: ["customLinks"] as const,
+  byFestival: (festivalId: string) =>
+    [...customLinksKeys.all, festivalId] as const,
Evidence
PR Compliance ID 721308 requires named/exported functions to be declared using `function Name(...) {
... } rather than arrow functions. In src/api/custom-links/types.ts, customLinksKeys.byFestival`
is defined as an arrow function and exported as part of customLinksKeys.

Rule 721308: Use function declarations for components and named functions
src/api/custom-links/types.ts[5-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
`customLinksKeys.byFestival` is a named exported function defined as an arrow function inside an exported object literal. The codebase standard requires function declarations for named/exported functions.

## Issue Context
`customLinksKeys` is exported and consumed across modules; keeping named functions as `function` declarations improves consistency and avoids introducing arrow-function named APIs.

## Fix Focus Areas
- src/api/custom-links/types.ts[5-8]

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


2. Undefined query key ✓ Resolved 🐞 Bug ☼ Reliability
Description
useCustomLinksQuery calls customLinksQuery(festivalId!) even when festivalId is undefined,
so the query is registered under customLinksKeys.byFestival(undefined) and the queryFn closes over
an undefined id (even though it won’t run while enabled is false). This makes cache keys
inconsistent and easier to misuse in future call sites/loaders compared to the repo’s pattern of
normalizing missing ids (e.g., editionId || "").
Code

src/api/custom-links/useCustomLinks.ts[R21-32]

+export function customLinksQuery(festivalId: string) {
+  return queryOptions({
+    queryKey: customLinksKeys.byFestival(festivalId),
+    queryFn: () => fetchCustomLinks(festivalId),
+  });
+}
+
export function useCustomLinksQuery(festivalId: string | undefined) {
  return useQuery({
-    queryKey: customLinksKeys.byFestival(festivalId || ""),
-    queryFn: () => fetchCustomLinks(festivalId!),
+    ...customLinksQuery(festivalId!),
    enabled: !!festivalId,
  });
Evidence
The hook currently constructs query options using festivalId! prior to applying enabled, so at
runtime an undefined id flows into the queryKey/queryFn. Another API module demonstrates the
established normalization approach (editionId || "") to avoid undefined keys while still disabling
execution via enabled.

src/api/custom-links/useCustomLinks.ts[21-33]
src/api/sets/useSetsByEdition.ts[49-61]

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

## Issue description
`useCustomLinksQuery` spreads `customLinksQuery(festivalId!)` even when `festivalId` is undefined, producing a queryKey containing `undefined` and a queryFn that would call `fetchCustomLinks(undefined)` if ever executed.

## Issue Context
Other query hooks in `src/api` avoid `undefined` in query keys by normalizing to an empty string when disabled.

## Fix Focus Areas
- src/api/custom-links/useCustomLinks.ts[21-33]

## Suggested fix
Change the hook to normalize the id before calling the factory, e.g.:
- `...customLinksQuery(festivalId ?? "")`
- keep `enabled: !!festivalId`

Optionally, if all callers always have an id, consider tightening the hook signature to `festivalId: string` and removing the `enabled` gate (or keep it but drop `undefined` from the API).

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


Grey Divider

Qodo Logo

The mechanical move changed customLinksKeys.byFestival(festivalId || "") to
customLinksQuery(festivalId!), shifting the disabled-state cache key from
["customLinks", ""] to ["customLinks", undefined]. Restore the original
normalization via ?? "" to keep the move behavior-neutral.

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 two findings:

1. customLinksKeys.byFestival arrow function (rule 721308) — declined. Every key factory in the codebase (festivalsKeys, editionsKeys, setsKeys, …) uses arrow values inside the key-factory object; converting only this one would be inconsistent. Switching key factories to method-shorthand is a codebase-wide decision, not this slice.

2. customLinksQuery(festivalId!) → undefined key — valid, fixed in 857da9e. Good catch. The original on main normalized the key with customLinksKeys.byFestival(festivalId || ""); the refactor changed it to festivalId!, shifting the disabled-state key from ["customLinks", ""] to ["customLinks", undefined]. No runtime impact (the query is disabled and never runs), but it broke the "no behavior change" guarantee — so I restored the original normalization with ...customLinksQuery(festivalId ?? ""). Lint/build/290 tests green.


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