Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+74
to
+78
| const handleBeforeCommit = () => { | ||
| if (!showVolumePricingToggle) return; | ||
| const cleaned = cleanTiersForMode({ item, mode: volumePricingMode }); | ||
| setItem(cleaned); | ||
| }; |
Contributor
There was a problem hiding this comment.
flat_amount not cleaned when tiers are reduced to one in flat mode
handleBeforeCommit guards on showVolumePricingToggle (which requires both isVolumeBased && isMultiTier). If a user is in flat mode with 2+ tiers and then removes tiers down to one, showVolumePricingToggle becomes false and the cleanup is skipped. The submitted single-tier item then retains a stale flat_amount even though the UI was presenting it as a per-unit (amount-based) field.
Example failure:
- User edits a volume-based item with 2 tiers in flat mode → sets
flat_amounton both tiers - User deletes one tier → now 1 tier,
showVolumePricingToggle = false - User clicks "Update Plan Feature"
handleBeforeCommitreturns early without cleanup- Backend receives single-tier item with
flat_amountstill populated (incorrect)
Fix: Also clean when the item is volume-based and in flat mode, regardless of tier count:
const handleBeforeCommit = () => {
if (!isVolumeBased) return;
if (!showVolumePricingToggle) {
// Single-tier volume-based: always force per_unit cleanup
const cleaned = cleanTiersForMode({ item, mode: "per_unit" });
setItem(cleaned);
return;
}
const cleaned = cleanTiersForMode({ item, mode: volumePricingMode });
setItem(cleaned);
};Prompt To Fix With AI
This is a comment left during a code review.
Path: vite/src/views/products/plan/components/edit-plan-feature/EditPlanFeatureSheet.tsx
Line: 74-78
Comment:
**`flat_amount` not cleaned when tiers are reduced to one in flat mode**
`handleBeforeCommit` guards on `showVolumePricingToggle` (which requires both `isVolumeBased && isMultiTier`). If a user is in flat mode with 2+ tiers and then removes tiers down to one, `showVolumePricingToggle` becomes `false` and the cleanup is skipped. The submitted single-tier item then retains a stale `flat_amount` even though the UI was presenting it as a per-unit (amount-based) field.
**Example failure:**
1. User edits a volume-based item with 2 tiers in flat mode → sets `flat_amount` on both tiers
2. User deletes one tier → now 1 tier, `showVolumePricingToggle = false`
3. User clicks "Update Plan Feature"
4. `handleBeforeCommit` returns early without cleanup
5. Backend receives single-tier item with `flat_amount` still populated (incorrect)
**Fix:** Also clean when the item is volume-based and in flat mode, regardless of tier count:
```tsx
const handleBeforeCommit = () => {
if (!isVolumeBased) return;
if (!showVolumePricingToggle) {
// Single-tier volume-based: always force per_unit cleanup
const cleaned = cleanTiersForMode({ item, mode: "per_unit" });
setItem(cleaned);
return;
}
const cleaned = cleanTiersForMode({ item, mode: volumePricingMode });
setItem(cleaned);
};
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+43
to
+50
| const [volumePricingMode, setVolumePricingMode] = useState<VolumePricingMode>( | ||
| () => { | ||
| const hasFlatAmount = item?.tiers?.some( | ||
| (t) => t.flat_amount != null && t.flat_amount > 0, | ||
| ); | ||
| return hasFlatAmount ? "flat" : "per_unit"; | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
volumePricingMode not re-initialized when the active item changes
useState lazy initializers run only once per component mount. In the editor context, EditPlanFeatureSheet is not unmounted when users switch between items — only the context item value changes via setCurrentItem. This means if a user edits item A (with flat pricing → volumePricingMode = "flat"), then switches to item B (no flat pricing) without closing the sheet, the toggle will wrongly default to flat mode.
Example failure:
- User edits item A which has
flat_amountvalues →volumePricingModeinitialized to"flat" - User closes item A and opens item B (no
flat_amount) - The sheet is not remounted, so the lazy initializer doesn't run
volumePricingModeremains"flat"even though item B has no flat pricing- UI toggle displays incorrect state for item B
Fix: Add a useEffect to reset the mode whenever the item changes:
useEffect(() => {
const hasFlatAmount = item?.tiers?.some(
(t) => t.flat_amount != null && t.flat_amount > 0,
);
setVolumePricingMode(hasFlatAmount ? "flat" : "per_unit");
}, [item?.feature_id]); // reset whenever the edited item changesPrompt To Fix With AI
This is a comment left during a code review.
Path: vite/src/views/products/plan/components/edit-plan-feature/EditPlanFeatureSheet.tsx
Line: 43-50
Comment:
**`volumePricingMode` not re-initialized when the active item changes**
`useState` lazy initializers run only once per component mount. In the editor context, `EditPlanFeatureSheet` is not unmounted when users switch between items — only the context `item` value changes via `setCurrentItem`. This means if a user edits item A (with flat pricing → `volumePricingMode = "flat"`), then switches to item B (no flat pricing) without closing the sheet, the toggle will wrongly default to flat mode.
**Example failure:**
1. User edits item A which has `flat_amount` values → `volumePricingMode` initialized to `"flat"`
2. User closes item A and opens item B (no `flat_amount`)
3. The sheet is not remounted, so the lazy initializer doesn't run
4. `volumePricingMode` remains `"flat"` even though item B has no flat pricing
5. UI toggle displays incorrect state for item B
**Fix:** Add a `useEffect` to reset the mode whenever the item changes:
```tsx
useEffect(() => {
const hasFlatAmount = item?.tiers?.some(
(t) => t.flat_amount != null && t.flat_amount > 0,
);
setVolumePricingMode(hasFlatAmount ? "flat" : "per_unit");
}, [item?.feature_id]); // reset whenever the edited item changes
```
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Greptile Summary
This PR adds a volume flat amount UI for volume-based, multi-tier feature pricing. Users can now toggle between a per-unit (
amount) view and a flat amount (flat_amount) view via a new segmented control that appears alongside the existing tier-behavior selector. The feature is backed by a newcleanTiersForModeutility that zeroes out unused fields before commit, and is well-covered by a new test file.Key changes:
Selectinto a newPriceSectionTitlecomponent and adds aPer Unit/Flat AmountIconCheckboxtoggle that is only shown for volume-based, multi-tier items.PriceTiersto accept avolumePricingModeprop, switching the editable field betweenamountandflat_amountand hiding theBillingUnitslabel in flat mode.cleanTiersForModeutility intierUtils.tsand ahandleBeforeCommithook inEditPlanFeatureSheetto ensure stale field values are zeroed/nulled before the item is committed.onBeforeCommitcallback toSheetFooterActionsthat is invoked synchronously beforehandleUpdateProductItem, preserving the existing commit flow.cleanTiersForModeandupdateTier(flat_amount path) intierUtils.test.ts.Two critical issues in
EditPlanFeatureSheetshould be addressed before merging:Stale
flat_amountwhen reducing tiers to one in flat mode — If a user edits a multi-tier volume-based item in flat mode and then removes tiers down to one, thehandleBeforeCommitguard condition skips cleanup. The single-tier item is submitted with leftoverflat_amountvalues that contradict the per-unit UI state shown to the user.volumePricingModestate doesn't reset when switching items — TheuseStatelazy initializer runs only once per mount. If users edit item A (with flat pricing) then switch to item B (without flat pricing) without closing the sheet, the toggle state remains frozen at item A's value, causing incorrect UI state for item B.Confidence Score: 2/5
cleanTiersForMode,updateTier) is correct and well-tested. UI components are cleanly structured. However,EditPlanFeatureSheethas two critical bugs: (1)handleBeforeCommitskips cleanup when users reduce a multi-tier volume item to single-tier in flat mode, leaving staleflat_amountin the submitted data; (2)volumePricingModestate won't reset when switching between items without remounting, causing incorrect toggle state. Both bugs lead to silent data corruption and should be fixed before shipping.Sequence Diagram
sequenceDiagram participant User participant PriceSectionTitle participant EditPlanFeatureSheet participant PriceTiers participant SheetFooterActions participant tierUtils User->>PriceSectionTitle: Select "Volume-based" tier behavior PriceSectionTitle->>EditPlanFeatureSheet: onTierBehaviorChange("VolumeBased") EditPlanFeatureSheet->>EditPlanFeatureSheet: setItem({ tier_behavior: VolumeBased }) Note over EditPlanFeatureSheet: showVolumePricingToggle = true<br/>(isVolumeBased && isMultiTier) User->>PriceSectionTitle: Click "Flat Amount" toggle PriceSectionTitle->>EditPlanFeatureSheet: onVolumePricingModeChange("flat") EditPlanFeatureSheet->>EditPlanFeatureSheet: setVolumePricingMode("flat") EditPlanFeatureSheet->>PriceTiers: volumePricingMode="flat" PriceTiers->>User: Shows flat_amount field (hides BillingUnits) User->>PriceTiers: Enter flat amount value PriceTiers->>tierUtils: updateTier({ field: "flat_amount", value }) tierUtils->>EditPlanFeatureSheet: setItem(updatedItem) User->>SheetFooterActions: Click "Update Plan Feature" SheetFooterActions->>EditPlanFeatureSheet: onBeforeCommit() EditPlanFeatureSheet->>tierUtils: cleanTiersForMode({ item, mode: "flat" }) tierUtils-->>EditPlanFeatureSheet: cleaned item (amount=0 preserved) EditPlanFeatureSheet->>EditPlanFeatureSheet: setItem(cleaned) SheetFooterActions->>SheetFooterActions: handleUpdateProductItem() Note over SheetFooterActions: commitItemDraft() or closeSheet() User->>PriceSectionTitle: Select "Graduated" tier behavior PriceSectionTitle->>EditPlanFeatureSheet: onTierBehaviorChange("Graduated") EditPlanFeatureSheet->>EditPlanFeatureSheet: setVolumePricingMode("per_unit") EditPlanFeatureSheet->>EditPlanFeatureSheet: clear flat_amount on all tiers EditPlanFeatureSheet->>EditPlanFeatureSheet: setItem(newItem)Last reviewed commit: c62fc87
Context used:
dashboard- CLAUDE.md (source)