feat(series): implement series creation and partial update functionality#225
Conversation
- Added `buildSeriesCreateBody` function to construct request body for new series creation. - Updated `saveSeriesMutation` to handle series creation and partial updates based on whether the series is new or being updated. - Introduced helper functions to compare series metadata and plans for efficient updates. - Enhanced `buildSeriesUpdateBody` to support partial updates with original data comparison.
Confidence Score: 4/5The create and update paths work correctly for all currently exercised scenarios; the main risk is in buildSeriesPartialUpdateBody, which silently discards image removal and featured-flag changes under certain calling patterns. The two logic gaps in buildSeriesPartialUpdateBody are shielded by the current UI and explicit passing in CreateSeries.tsx, but they make the exported API fragile for future use. src/components/routes/create-series/api/seriesApi.ts — the partial-update body builder has two edge-case gaps worth hardening before the function is called from additional sites. Reviews (1): Last reviewed commit: "feat(series): implement series creation ..." | Re-trigger Greptile |
| return buildSeriesPartialUpdateBody( | ||
| data, | ||
| options.original, | ||
| featured, | ||
| options.originalFeatured ?? featured, | ||
| ); |
There was a problem hiding this comment.
Silent featured-change drop when
originalFeatured is omitted
When a caller passes options.original but omits options.originalFeatured, the fallback options.originalFeatured ?? featured evaluates to the current featured value, making the comparison featured !== featured always false — so any change to the featured flag is silently dropped from the partial-update body. CreateSeries.tsx avoids this by always supplying originalFeatured: seriesData!.featured, but any future caller who follows the looser pattern will silently lose featured-state changes without an error.
| const currentImageKey = current.image_url.trim(); | ||
| const originalImageKey = original.image_url.trim(); | ||
| if (currentImageKey !== originalImageKey && currentImageKey) { | ||
| body.image_key = currentImageKey; | ||
| } |
There was a problem hiding this comment.
Image removal is silently ignored by partial update
The guard currentImageKey !== originalImageKey && currentImageKey skips sending image_key whenever the current value is empty. If a user removes a cover image (image_url becomes ""), the server is never told about the removal and retains the old image. The UI currently prevents saving with an empty imageUrl (via submitEnabled), which hides this, but buildSeriesPartialUpdateBody is an exported API and contains no documentation of this limitation — a future caller or a change to that UI guard would cause silent data-loss for image removal.
|
|
||
| describe("buildSeriesUpdateBody", () => { | ||
| it("returns full payload for create", () => { | ||
| const body = buildSeriesUpdateBody(baseForm, false, { groupId: "group-1" }); | ||
|
|
||
| expect(body).toMatchObject({ | ||
| metadata: [ | ||
| { | ||
| language: "EN", | ||
| title: "Series title", | ||
| description: "Original description", | ||
| }, | ||
| ], | ||
| featured: false, | ||
| plans: { EN: ["plan-a", "plan-b"] }, | ||
| image_key: "series-image-key", | ||
| group_id: "group-1", |
There was a problem hiding this comment.
Misleading test title and missing
buildSeriesCreateBody coverage
The test case "returns full payload for create" calls buildSeriesUpdateBody with a groupId, but creation is now handled exclusively by buildSeriesCreateBody in CreateSeries.tsx. This test is exercising a backward-compatible code path that no longer represents the create flow. Additionally, buildSeriesCreateBody — newly exported in this PR — has no dedicated test, leaving its buildSeriesWriteBody delegation and groupId forwarding unverified.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
buildSeriesCreateBodyfunction to construct request body for new series creation.saveSeriesMutationto handle series creation and partial updates based on whether the series is new or being updated.buildSeriesUpdateBodyto support partial updates with original data comparison.