Skip to content

fix(catalog): normalize missing product prices - #2322

Merged
charlietlamb merged 1 commit into
devfrom
charlie/normalize-product-response-dev
Jul 20, 2026
Merged

fix(catalog): normalize missing product prices#2322
charlietlamb merged 1 commit into
devfrom
charlie/normalize-product-response-dev

Conversation

@charlietlamb

@charlietlamb charlietlamb commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Normalize missing product prices at catalog response boundaries so incomplete historical products cannot crash catalog analysis.


Summary by cubic

Normalize product responses to treat missing prices as empty arrays. Prevents catalog analysis from crashing on historical products and keeps the updateable flag safe.

  • Bug Fixes
    • Default prices to [] when mapping items in getPlanResponse and getProductResponse.
    • Guard updateable with (product.prices ?? []).some(...) to avoid undefined access.

Written for commit ad32b7c. Summary will update on new commits.

Review in cubic

Greptile Summary

This PR adds ?? [] null-coalescing guards at three call sites where product.prices is passed into mapToProductItems or iterated, preventing crashes when historical products have missing price arrays. The fix is applied consistently to both getPlanResponse.ts and getProductResponse.ts.

  • [Bug fixes] getPlanResponse.ts: guards product.prices before mapToProductItems, preventing a runtime crash for historical products with no price data.
  • [Bug fixes] getProductResponse.ts: guards product.prices in the mapToProductItems call and the updateable .some() check; however, the same null/undefined product.prices will still crash when getProductProperties calls productToEffectivePrices({ product }), which spreads product.prices directly without a guard.

Confidence Score: 3/5

The fix is incomplete — getProductResponse still has an unguarded spread of product.prices inside productToEffectivePrices, which will throw for the same historical products this PR intends to protect.

Two of the three guarded call sites are correct, but getProductProperties calls productToEffectivePrices({ product }), which spreads product.prices directly. A historical product with null prices will still crash at is_free/is_one_off computation in getProductResponse, making the catalog analysis fix only partially effective.

server/src/internal/products/productUtils/productResponseUtils/getProductResponse.ts — the getProductProperties function and its downstream call to productToEffectivePrices need the same null guard treatment.

Important Files Changed

Filename Overview
server/src/internal/products/productUtils/productResponseUtils/getPlanResponse.ts Adds ?? [] null guard to product.prices before passing to mapToProductItems; the change is minimal and correct for this file.
server/src/internal/products/productUtils/productResponseUtils/getProductResponse.ts Adds ?? [] guards to the mapToProductItems call and the updateable .some() check, but getProductProperties still calls productToEffectivePrices({ product }) which spreads product.prices without a null guard — the same crash scenario is still reachable via is_free/is_one_off computation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[getProductResponse / getPlanResponse] --> B["mapToProductItems\n(product.prices ?? [] ✅ fixed)"]
    A --> C[getProductProperties]
    C --> D["updateable: (product.prices ?? []).some(...)\n✅ fixed"]
    C --> E["productToEffectivePrices({ product })\n...product.prices ❌ still unguarded"]
    E --> F["isFreeProduct / isOneOffProduct / getLargestInterval"]
    E --> G["TypeError if product.prices is null"]
Loading
%%{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[getProductResponse / getPlanResponse] --> B["mapToProductItems\n(product.prices ?? [] ✅ fixed)"]
    A --> C[getProductProperties]
    C --> D["updateable: (product.prices ?? []).some(...)\n✅ fixed"]
    C --> E["productToEffectivePrices({ product })\n...product.prices ❌ still unguarded"]
    E --> F["isFreeProduct / isOneOffProduct / getLargestInterval"]
    E --> G["TypeError if product.prices is null"]
Loading

Comments Outside Diff (1)

  1. server/src/internal/products/productUtils/productResponseUtils/getProductResponse.ts, line 141-158 (link)

    P1 productToEffectivePrices still spreads product.prices without a null guard

    getProductProperties calls productToEffectivePrices({ product }) on line 141, which does [...product.prices, ...] (spread of product.prices). For the same historical products this PR targets — where product.prices can be null/undefined — this will still throw a TypeError ("null is not iterable") before the is_free, is_one_off, interval_group, and has_trial properties can be computed. The three ?? [] guards added here and in getPlanResponse.ts do not cover this code path; productToEffectivePrices itself would need the same normalization.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: server/src/internal/products/productUtils/productResponseUtils/getProductResponse.ts
    Line: 141-158
    
    Comment:
    **`productToEffectivePrices` still spreads `product.prices` without a null guard**
    
    `getProductProperties` calls `productToEffectivePrices({ product })` on line 141, which does `[...product.prices, ...]` (spread of `product.prices`). For the same historical products this PR targets — where `product.prices` can be null/undefined — this will still throw a TypeError ("null is not iterable") before the `is_free`, `is_one_off`, `interval_group`, and `has_trial` properties can be computed. The three `?? []` guards added here and in `getPlanResponse.ts` do not cover this code path; `productToEffectivePrices` itself would need the same normalization.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
server/src/internal/products/productUtils/productResponseUtils/getProductResponse.ts:141-158
**`productToEffectivePrices` still spreads `product.prices` without a null guard**

`getProductProperties` calls `productToEffectivePrices({ product })` on line 141, which does `[...product.prices, ...]` (spread of `product.prices`). For the same historical products this PR targets — where `product.prices` can be null/undefined — this will still throw a TypeError ("null is not iterable") before the `is_free`, `is_one_off`, `interval_group`, and `has_trial` properties can be computed. The three `?? []` guards added here and in `getPlanResponse.ts` do not cover this code path; `productToEffectivePrices` itself would need the same normalization.

Reviews (1): Last reviewed commit: "fix(catalog): normalize missing product ..." | Re-trigger Greptile

@capy-ai

capy-ai Bot commented Jul 20, 2026

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@charlietlamb
charlietlamb merged commit f78679c into dev Jul 20, 2026
16 checks passed
@charlietlamb
charlietlamb deleted the charlie/normalize-product-response-dev branch July 20, 2026 21:12
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