Skip to content

feat(series): implement series creation and partial update functionality#225

Merged
tenkus47 merged 1 commit into
devfrom
subtitle_add
Jun 10, 2026
Merged

feat(series): implement series creation and partial update functionality#225
tenkus47 merged 1 commit into
devfrom
subtitle_add

Conversation

@tenkus47

@tenkus47 tenkus47 commented Jun 9, 2026

Copy link
Copy Markdown
Member
  • 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.

- 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.
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The 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

Comment on lines +365 to +370
return buildSeriesPartialUpdateBody(
data,
options.original,
featured,
options.originalFeatured ?? featured,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Comment on lines +328 to +332
const currentImageKey = current.image_url.trim();
const originalImageKey = original.image_url.trim();
if (currentImageKey !== originalImageKey && currentImageKey) {
body.image_key = currentImageKey;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Comment on lines +85 to +101

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",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

@tenkus47 tenkus47 merged commit 9696d9f into dev Jun 10, 2026
2 of 3 checks passed
@tenkus47 tenkus47 deleted the subtitle_add branch June 10, 2026 04:48
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