Skip to content

feat(vps): port favorites to HttpApiBuilder.group (Step 6)#166

Merged
guidefari merged 3 commits into
migration/effect-http-apifrom
migration/6-favorites-group
Jul 11, 2026
Merged

feat(vps): port favorites to HttpApiBuilder.group (Step 6)#166
guidefari merged 3 commits into
migration/effect-http-apifrom
migration/6-favorites-group

Conversation

@guidefari

Copy link
Copy Markdown
Owner

Why

Step 6 continues: port favorites (POST /api/favorites,
DELETE /api/favorites/:audioId, DELETE /api/favorites/show/:showId,
GET /api/favorites) from the Hono fallback onto the Effect router.

What to look for

  • Handlers go through the existing FavoriteService
    (apps/vps/src/services/favorite.service.ts), which already returns
    typed tagged errors (NotFoundError, ConflictError, DatabaseError) --
    this is the cleanest port so far, matching the search/profile/resolve
    shape rather than admin/newsletter's raw-Drizzle-in-the-handler style.
  • The old zod schema's cross-field .refine() ("either audioId or
    showId must be provided") has no direct Effect Schema equivalent used
    elsewhere in this migration. Kept both fields optional in the payload
    schema (packages/api/src/favorites.ts's AddFavoriteInput) and moved
    the check into the handler (400 if neither is present), rather than
    inventing a new cross-field schema pattern for one endpoint.
  • FavoriteWithContent's schema is written against FavoriteService's
    real return type, which includes a show field --
    apps/www/src/lib/http.ts's own Favorite type currently omits show
    entirely. Frontend-side gap, not corrected in this PR (backend now
    correctly serves the field either way).
  • Uses the shared dieOnDatabaseError helper.

Test evidence

All four endpoints have real apps/www consumers
(FavoritesSection.tsx, FavoriteButton.tsx, MixMenu.tsx,
TrackContextMenu.tsx), so this isn't screenshot-exempt.

Verified via an authenticated in-page fetch (not a screenshot, since the
test account has zero favorites and an empty-list screenshot can't
distinguish "endpoint works, no data" from "endpoint broken") against the
real dev server:

fetch('/api/favorites?limit=5', { credentials: 'include' })
  .then(r => r.json())
// => {"success":true,"favorites":[],"total":0}

Response shape matches the new GetFavoritesResponse schema exactly.
Given an earlier live-send mistake this session (an invite email
misfired to the wrong user on PR #164), no favorite/unfavorite mutation
was triggered live -- auth-gating on all four endpoints is covered by 4
new blackbox tests instead.

bun run typecheck (vps and api packages): clean.

vitest run --config vitest.unit.config.ts (vps): added 4 new blackbox
cases (401 for all four endpoints without a session cookie), all pass.
Pre-existing DB-connectivity failures (8, same as
migration/effect-http-api's current tip) are unchanged by this PR.

@guidefari

Copy link
Copy Markdown
Owner Author

Fixed: restored UUID format validation on audioId/showId (add payload + both delete params) using the same Schema.String.pipe(Schema.check(Schema.isPattern(...))) pattern as newsletter's fix in #165. Typecheck clean, same pre-existing DB-connectivity failure count.

Ports POST /api/favorites, DELETE /api/favorites/:audioId,
DELETE /api/favorites/show/:showId, GET /api/favorites from the Hono
fallback onto the Effect router. All four are gated by
.middleware(AuthMiddleware).

Handlers go through the existing FavoriteService
(apps/vps/src/services/favorite.service.ts), which already returns typed
tagged errors (NotFoundError, ConflictError, DatabaseError) -- the
cleanest port so far, matching the search/profile/resolve shape rather
than admin/newsletter's raw-Drizzle-in-the-handler style.

The old zod schema's cross-field .refine() ("either audioId or showId
must be provided") has no direct Effect Schema equivalent used elsewhere
in this migration; kept both fields optional in the payload schema and
moved the check into the handler (400 if neither is present), rather
than inventing a new schema pattern for one endpoint.

Schema written against FavoriteService's real return type, which
includes a show field that apps/www/src/lib/http.ts's own Favorite type
currently omits -- a frontend-side gap, not corrected here.

Uses the shared dieOnDatabaseError helper.

Deletes the superseded routes/favorites/{favorites.routes,favorites.index,favorites.handlers}.ts
Hono trio and its app.ts mount.
Adversarial review found the same class of regression as newsletter's
#165: the old zod schema validated audioId/showId as z.string().uuid()
(on the add payload and both delete params), and the new Schema.String
dropped that constraint entirely. Added a shared Uuid schema
(Schema.String.pipe(Schema.check(Schema.isPattern(...)))) and applied it
everywhere audioId/showId appear in packages/api/src/favorites.ts.
@guidefari guidefari force-pushed the migration/6-favorites-group branch from d3cdad9 to 1f05dda Compare July 11, 2026 17:08
@guidefari guidefari merged commit ff7e75e into migration/effect-http-api Jul 11, 2026
@guidefari guidefari deleted the migration/6-favorites-group branch July 11, 2026 17:08
guidefari added a commit that referenced this pull request Jul 11, 2026
Adversarial review found a third occurrence of the input-validation
regression (after newsletter #165, favorites #166): the old zod schema
used z.string().min(1) for bucketName/key/sourceBucket/destinationBucket,
and the new Schema.String dropped that constraint. Switched to
Schema.NonEmptyString (already used in search.ts's q field) for all four.

Lower severity than the prior two cases -- an empty string here reaches
S3Service/AWS SDK calls and surfaces as a 500 via the caught S3Error
path, not a DB write or enumeration signal -- but fixed anyway per the
now-mandatory checklist item in docs/migration-effect-http-api-process.md.
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.

1 participant