Fix versioned license pricing in plan dashboard - #2291
Merged
Conversation
Contributor
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
45
to
+53
| const licenseById = new Map( | ||
| products.map((license) => [license.id, license]), | ||
| ); | ||
| for (const license of products) { | ||
| const latest = licenseById.get(license.id); | ||
| if (!latest || license.version > latest.version) { | ||
| licenseById.set(license.id, license); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
new Map(products.map(...)) already populates every ID, so !latest in the loop is always false (dead code). The first pass builds a map where the last item for each duplicate ID wins arbitrarily; the subsequent loop then fully rewrites it to highest-version-wins anyway. A single loop is both clearer and avoids the intermediate allocation.
Suggested change
| const licenseById = new Map( | |
| products.map((license) => [license.id, license]), | |
| ); | |
| for (const license of products) { | |
| const latest = licenseById.get(license.id); | |
| if (!latest || license.version > latest.version) { | |
| licenseById.set(license.id, license); | |
| } | |
| } | |
| const licenseById = new Map<string, (typeof products)[0]>(); | |
| for (const license of products) { | |
| const latest = licenseById.get(license.id); | |
| if (!latest || license.version > latest.version) { | |
| licenseById.set(license.id, license); | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: vite/src/views/products/plan/components/plan-licenses/useResolvedPlanLicenses.ts
Line: 45-53
Comment:
Redundant two-pass map construction — `new Map(products.map(...))` already populates every ID, so `!latest` in the loop is always `false` (dead code). The first pass builds a map where the last item for each duplicate ID wins arbitrarily; the subsequent loop then fully rewrites it to highest-version-wins anyway. A single loop is both clearer and avoids the intermediate allocation.
```suggestion
const licenseById = new Map<string, (typeof products)[0]>();
for (const license of products) {
const latest = licenseById.get(license.id);
if (!latest || license.version > latest.version) {
licenseById.set(license.id, license);
}
}
```
How can I resolve this? If you propose a fix, please make it concise.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!
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.
Summary
plans.getValidation
bun test tests/views/products/plan/resolve-plan-license-product.test.tsbun run tsknipand scoped Vite typecheckSummary by cubic
Fixes plan dashboard license pricing by resolving linked license products using the plan-pinned version. Staged links still use the latest version so previews stay current.
plans.getviaVersionedPlanLicense.id+versionusingresolvePlanLicenseProductanduseProductsQuery({ allVersions: true }).Written for commit fcc09de. Summary will update on new commits.
Greptile Summary
This PR fixes incorrect pricing shown for versioned license products in the plan dashboard by preserving the version number returned by
plans.getand using it to resolve persisted license links to their pinned product version rather than an arbitrary latest entry.usePlanLicensesQuerynow mapslink.versioninto a newVersionedPlanLicensetype, anduseResolvedPlanLicensespasses that version to a newresolvePlanLicenseProducthelper that matches on bothidandversion— ensuring the dashboard shows the price configured at the time the link was created.licenseByIdmap built from the fullallVersions: trueproduct list, which was already a supported query option with its own cache key.resolvePlanLicenseProduct.Confidence Score: 4/5
licenseByIdmap is built with a redundant first pass that makes the!latestbranch unreachable, andResolvedPlanLicense.planLicenseis still typed as the basePlanLicenserather thanVersionedPlanLicense, hiding the.versionfield from downstream consumers without a cast.Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[useResolvedPlanLicenses] --> B{pageCatalogLicenses?} B -- Yes --> C[Use server-resolved licenses] B -- No --> D[usePlanLicensesQuery returns VersionedPlanLicense array] D --> E[resolvePlanLicenseProduct - match by id AND version] E --> F[persistedLicenses with version-pinned ProductV2] A --> G[useProductsQuery with allVersions true] G --> H[licenseById Map - id to latest version ProductV2] A --> I[pendingLicenseIds + initialPatches] I --> J[stagedIds not in persistedIds] J --> K[licenseById.get - latest version ProductV2] K --> L[pendingLicenses] F --> M[return persistedLicenses + pendingLicenses] C --> M L --> M%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[useResolvedPlanLicenses] --> B{pageCatalogLicenses?} B -- Yes --> C[Use server-resolved licenses] B -- No --> D[usePlanLicensesQuery returns VersionedPlanLicense array] D --> E[resolvePlanLicenseProduct - match by id AND version] E --> F[persistedLicenses with version-pinned ProductV2] A --> G[useProductsQuery with allVersions true] G --> H[licenseById Map - id to latest version ProductV2] A --> I[pendingLicenseIds + initialPatches] I --> J[stagedIds not in persistedIds] J --> K[licenseById.get - latest version ProductV2] K --> L[pendingLicenses] F --> M[return persistedLicenses + pendingLicenses] C --> M L --> MComments Outside Diff (1)
vite/src/views/products/plan/components/plan-licenses/useResolvedPlanLicenses.ts, line 17-21 (link)ResolvedPlanLicense.planLicenseis still typed as the basePlanLicense, not the narrowerVersionedPlanLicense. Persisted licenses are now resolved fromfallbackPlanLicenses(typeVersionedPlanLicense[]), so the runtime object does carry.version— but it is invisible to callers through this interface. Any downstream component that needs the pinned version for display or comparison would have to cast. Consider widening the type toPlanLicense | VersionedPlanLicenseor replacing it withVersionedPlanLicenseto make the contract explicit.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(dashboard): resolve pinned license p..." | Re-trigger Greptile