Feat/library

[MaryWylde]

No description provided.

[MaryWylde]

@claude

[claude]

Claude finished @MaryWylde's task in 2m 37s —— View job

---

PR Review: Feat/library

• Gather PR context
• Review API layer (src/api/library/)
• Review component architecture (atoms, molecules, contexts)
• Review config/infrastructure changes (next.config.js, package.json)
• Review modifications to existing components (Header, Navbar, UserProfile)
• Post comprehensive review feedback

---

Good foundational work — the atomic component hierarchy, Zod+RHF form validation, FLIP animation on card reorder, and the optimistic-with-rollback pattern in AddObjectModal are all well-done. Below are the issues worth resolving before merge.

---

Bugs

1. AuthContext — logout doesn't reset in-memory state

handleLogout clears the cookie but never calls setToken(null) or setAccountData(null). Any component reading token or accountData from context will continue to see the logged-in user until a hard refresh. Fix this →

2. GlobalStateContext — double fetch of libraries on first authenticated load

Two useEffects both call refetchLibraries() on mount when a token exists. The unconditional first effect ([refetchLibraries]) fires and doesn't set didAttemptLibrariesLoad.current, so the second effect's guard doesn't fire, and it calls refetchLibraries() a second time. Fix this →

3. AddObjectModal — "Select tag to reorder" dropdown is dead UI

The tag-filter dropdown in step 2 renders with no onChange or value, so it neither reads nor writes any state. It does nothing.

// src/components/library/organisms/AddObjectModal/AddObjectModal.tsx
<Dropdown
  options={selectedTags.map(t => ({ value: String(t.id), label: t.name }))}
  === 0 ? 'Pick tags above first' : 'Select a tag to filter'}
  disabled={selectedTags.length === 0}
  // ← no onChange or value
/>

Either wire it up to filter reorderItems or remove it until the feature is ready. Fix this →

---

Type Safety

4. GlobalStateContext — libraries typed as unknown

libraries: unknown | null;  // consumers must cast every time

getLibrariesList() returns StrapiLibrariesResponse | null when called from this context. Narrowing the type here eliminates downstream casts. Fix this →

5. createTag.ts — missing return type

createTag has no explicit return type annotation, unlike every other API function in this module. This leaves its resolved shape as any. Fix this →

---

Architecture / Consistency

6. getTagsList — App Router SSR branch in a Pages Router project

getTagsList has a typeof window === 'undefined' guard that dynamically imports next/headers (an App Router API). This project uses Pages Router (useRouter from next/router). In Pages Router SSR (getServerSideProps), next/headers is not available and this branch would throw. The comment says "when called from a Server Component" — but Pages Router has no Server Components.

If this is intentional preparation for an App Router migration, add a clear comment. If not, remove the dead branch.

7. reorderObjects / reorderShelves — no error handling

All other API functions in this module wrap their calls in try/catch and return null on failure. reorderObjects, reorderShelves, createObject, and uploadFile don't, so callers must handle every error path themselves. AddObjectModal does wrap the call in try/catch, but the inconsistency is a footgun for future callers. Fix this →

8. AuthContext — manual cookie manipulation instead of js-cookie

js-cookie is already a declared dependency. handleProviderSignIn and handleLogout manipulate document.cookie directly — including constructing Secure and SameSite attributes by hand. This is fragile and inconsistent with the getCookie helper used in useEffect. Use Cookies.remove() from js-cookie instead.

---

Minor / Cosmetic

9. UserProfile.tsx — LibraryIcon has an unusual aspect ratio

The icon is rendered at width={20} height={11}, which is not square and doesn't match the other menu icons in the dropdown (Settings uses Image with no explicit non-square dimensions). Confirm this is intentional for the SVG's natural aspect ratio.

10. Incomplete hy locale in UserProfile

The hy (Armenian) locale label for myLibrary is 'My Library' (English), while ru already has the correct translation. This was likely an oversight. Fix this →

11. Three open TODOs blocking completeness

// TODO: hook up URL metadata auto-fetch (YouTube / Spotify / generic OG)
// TODO: drop the client-side `slug` once Strapi makes `object.slug` optional
// TODO: drop once Strapi sets `owner` in `beforeCreate`

All are in AddObjectModal.tsx. The slug and owner hacks are server-side dependencies — fine to ship with comments. The URL metadata fetch is a UI gap (the "Search" button does nothing); consider removing the button until it's wired up, so users don't click a no-op control.

---