Skip to content

Conversation

@AnthonyRonning
Copy link
Contributor

@AnthonyRonning AnthonyRonning commented Feb 9, 2026

Summary

This PR implements the approved version check for the API Management button in the sidebar, following the same logic used for the pricing routes.

Changes

  • Modified frontend/src/components/AccountMenu.tsx to add version checking logic
  • API Management button now shows on desktop/web (always)
  • On iOS/Android, it only shows if the app version is approved by the server
  • Fetches products with version to check is_available status

Testing

  • ✓ Prettier formatting passed
  • ✓ ESLint linting passed
  • ✓ TypeScript build passed
  • ✓ All pre-commit hooks passed

Resolves #419

Generated with Claude Code


Open with Devin

Summary by CodeRabbit

  • Improvements
    • API Management menu is now platform-aware: on desktop it remains visible consistently; on mobile it appears only when relevant product(s) are available, preventing empty or inapplicable menu items.
    • Enhances reliability of menu visibility across devices for a clearer navigation experience.

…roid

- API Management button now follows same version check logic as pricing routes
- On iOS/Android: only shows if app version is approved by server
- On desktop/web: always shows (no restrictions)
- Fetches products with version to check is_available status
- Resolves #419

Co-authored-by: Anthony <AnthonyRonning@users.noreply.github.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

Adds platform-aware visibility for the API Management menu item in AccountMenu: detects iOS, imports app version from package.json, queries mobile-specific products filtered by version, and sets showApiManagement based on fetched product availability; desktop remains always visible.

Changes

Cohort / File(s) Summary
Account menu & mobile product gating
frontend/src/components/AccountMenu.tsx, package.json
Introduce platform detection (iOS), import app version from package.json, add a useQuery call to fetch mobile products with version filtering, derive showApiManagement from query results, replace previous hard-coded desktop-only check, and add basic error handling for the fetch.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Ios plans disabled #140 — Implements similar iOS-aware product fetching using the app version and versioned product availability checks; likely the upstream counterpart.
  • (none) — No other strongly related PRs identified.

Poem

🐇 I nibbled code by moonlit log,
Found versions tucked inside the package bog,
For iOS I fetched the goods with care,
Now API Management shows when it's fair,
A little hop, a joyous debug dance ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding version checking for the API Management button on iOS/Android platforms to control visibility based on approved versions.
Linked Issues check ✅ Passed The PR implements the core requirement from #419: checking approved versions before showing the API Management button on mobile platforms, matching the logic used in pricing routes.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the version-check requirement for API Management visibility on iOS/Android, with no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/issue-419-20260209-2113

No actionable comments were generated in the recent review. 🎉


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@frontend/src/components/AccountMenu.tsx`:
- Line 54: Replace the relative import of package.json in AccountMenu.tsx with
an alias-based module: create a small module (e.g., export const appVersion in
src/config/appVersion.ts) that exposes the version from a build-time env or
generated constant, then update AccountMenu.tsx to import the version via the
path alias (e.g., import { appVersion } from '@/config/appVersion'); ensure no
direct ../../package.json imports remain and the new module is used wherever
version is needed.
- Around line 159-173: The current showApiManagement IIFE uses isMobile() which
treats mobile web as mobile and hides API Management; change the platform check
to only treat native iOS/Android as restricted so web (including mobile web)
always shows the item. Update the top-level condition in the showApiManagement
logic (the IIFE using isMobile()) to check native platform flags (e.g., isIos()
|| isAndroid() or a dedicated isNativeMobile() helper) so that if the runtime is
not native iOS/Android you return true, and keep the existing
products/product.is_available checks for the native branch.

import type { TeamStatus } from "@/types/team";
import { TeamManagementDialog } from "@/components/team/TeamManagementDialog";
import { ApiKeyManagementDialog } from "@/components/apikeys/ApiKeyManagementDialog";
import packageJson from "../../package.json";
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace the relative package.json import with an alias-based source.
The new relative import breaks the repo rule for TS/TSX imports and pulls from outside src. Consider exposing the version via a src module (or build-time env) and import it via @/….

♻️ Suggested update
-import packageJson from "../../package.json";
+import { appVersion } from "@/config/appVersion";

You can back this with a small module like frontend/src/config/appVersion.ts that reads from a build-time env or a generated constant.
As per coding guidelines, "Use path aliases (@/* maps to ./src/*) for imports in TypeScript/React files".

🤖 Prompt for AI Agents
In `@frontend/src/components/AccountMenu.tsx` at line 54, Replace the relative
import of package.json in AccountMenu.tsx with an alias-based module: create a
small module (e.g., export const appVersion in src/config/appVersion.ts) that
exposes the version from a build-time env or generated constant, then update
AccountMenu.tsx to import the version via the path alias (e.g., import {
appVersion } from '@/config/appVersion'); ensure no direct ../../package.json
imports remain and the new module is used wherever version is needed.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 9, 2026

Greptile Overview

Greptile Summary

This PR changes the sidebar AccountMenu to gate the API Management menu item based on a server-side “approved version” check on iOS, while keeping it always visible on other platforms.

Implementation-wise, it adds a new React Query call to billingService.getProducts() (sending v${packageJson.version} on iOS) and derives showApiManagement from the returned products’ is_available flags. This mirrors the pricing route’s general approach of passing app version to product fetching, but applies the result to menu visibility.

Confidence Score: 2/5

  • This PR has correctness risks on iOS due to platform initialization assumptions and query error handling that can permanently hide the API Management menu.
  • The change is small, but isIOS() can throw during render if platform initialization order is ever violated, the products query caches null as a successful result (making transient failures sticky), and the approval predicate treats missing is_available as approved, which can show the menu when it should be hidden.
  • frontend/src/components/AccountMenu.tsx

Important Files Changed

Filename Overview
frontend/src/components/AccountMenu.tsx Adds an iOS-only products query and uses it to gate the API Management menu entry; current implementation can crash if platform isn’t initialized, can cache null on fetch errors (hiding menu indefinitely), and treats missing is_available as approved.

Sequence Diagram

sequenceDiagram
  participant U as User
  participant AM as AccountMenu
  participant P as platform.ts
  participant RQ as React Query
  participant BS as BillingService
  participant API as Billing API

  U->>AM: Open account dropdown
  AM->>P: isIOS()
  P-->>AM: boolean (throws if not initialized)

  alt iOS platform
    AM->>RQ: useQuery(products-version-check)
    RQ->>BS: getProducts("v<package version>")
    BS->>API: fetchProducts(version)
    API-->>BS: BillingProduct[]
    BS-->>RQ: BillingProduct[]
    RQ-->>AM: products
    AM->>AM: showApiManagement = products.some(is_available != false)
  else non-iOS
    AM->>AM: showApiManagement = true
  end

  AM-->>U: Render API Management item (or hide)
Loading

@AnthonyRonning
Copy link
Contributor Author

@claude is this comment valid or not? we should have our existing ways that we handle useQuery and should have been using it for pricing related api calls throughout the pricing page. I care more about our existing standards and correctness and whether or not our approach works, vs whatever this bot is saying. research this in our codebase. ignore with an explanation if it's invalid. or make the fix if it is valid:


🟡 Shared query key with error-swallowing queryFn can poison the pricing page's product cache

The new useQuery in AccountMenu uses query key ["products", isIOSPlatform, isAndroidPlatform] with a queryFn that catches errors and returns null as successful data. The pricing page at frontend/src/routes/pricing.tsx:267 uses the exact same query key but its queryFn properly throws errors so React Query can show an error state.

Detailed Explanation of Cache Poisoning
On mobile, if AccountMenu's product fetch fails (e.g., network issue), the catch block at AccountMenu.tsx:147-149 returns null, which React Query caches as successful data under the shared key ["products", true, false] (iOS) or ["products", false, true] (Android).

When the user then navigates to the pricing page, React Query serves the cached null immediately as stale data. The pricing page receives products = null, productsError = undefined, and productsLoading = false. This bypasses both the loading skeleton (pricing.tsx:649) and the error UI (pricing.tsx:682), resulting in an empty pricing page with no products and no error message shown to the user.

While React Query's default staleTime: 0 means a background refetch will start, during the window before it completes the user sees a broken UI. If the refetch also fails, the pricing page's queryFn would properly set the error state, but the initial flash of empty content is still a poor user experience.

Fix: Use a distinct query key (e.g., ["products-version-check", ...]) for the AccountMenu query, or avoid swallowing errors and instead let React Query handle the error state naturally.

@claude
Copy link
Contributor

claude bot commented Feb 9, 2026

Claude finished @AnthonyRonning's task in 3m 42s —— View job


Analysis Complete: Bot's Concern is Valid ✓

After researching the codebase's React Query patterns, the bot's cache poisoning concern is valid and has been fixed.

The Problem

AccountMenu.tsx and pricing.tsx shared the same query key ["products", isIOSPlatform, isAndroidPlatform] but handled errors differently:

  1. AccountMenu.tsx (lines 136-153): Catches errors and returns null as "successful" data
  2. pricing.tsx (lines 262-285): Throws errors properly for React Query to handle

Why This Causes Cache Poisoning

If the AccountMenu's product fetch fails:

  1. The catch block returns null
  2. React Query caches null as successful data under the shared key
  3. When user navigates to pricing page, React Query serves the cached null
  4. Pricing page receives: products = null, productsError = undefined, productsLoading = false
  5. At pricing.tsx:806-811, products?.find(...) returns undefined for all plans
  6. All paid plan cards return null (don't render), only Free plan shows
  7. Result: User sees broken pricing page with no paid plans and no error message

Codebase Pattern Analysis

I found that Marketing.tsx uses a different query key ["marketing-products"] even though it also fetches products. This confirms unique keys are the correct pattern when queries have different error handling.

The Fix

Changed AccountMenu's query key from ["products", ...] to ["products-version-check", ...] to prevent cache collision.

Committed and pushed: Changed query key in AccountMenu.tsx
All tests passed: Prettier, ESLint, TypeScript build, bun test, pre-commit hooks

Summary

The bot was correct about the cache poisoning risk. This is a real bug that would cause the pricing page to show incorrectly when navigating from AccountMenu after a failed product fetch. The fix ensures each query maintains its own cache entry, preventing cross-contamination.

--- | Branch with fix

…ing page

The bot's concern was valid: AccountMenu and pricing page shared the same
query key ["products", isIOSPlatform, isAndroidPlatform] but handled errors
differently. AccountMenu caught errors and returned null as successful data,
which could poison React Query's cache. When the user navigated to pricing,
they'd see no paid plans and no error message.

Changed AccountMenu's query key to ["products-version-check", ...] to
prevent cache collision. This follows the pattern used by Marketing.tsx
which uses a separate ["marketing-products"] key.

Co-authored-by: Anthony <AnthonyRonning@users.noreply.github.com>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 9, 2026

Deploying maple with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9449a88
Status: ✅  Deploy successful!
Preview URL: https://76d8fbd9.maple-ca8.pages.dev
Branch Preview URL: https://claude-issue-419-20260209-21.maple-ca8.pages.dev

View logs

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@AnthonyRonning
Copy link
Contributor Author

@claude

Greptile brought up a good point:

Android gating incorrect

showApiManagement hides API Management on all mobile (isMobile()), but the PR description says it should only be gated on iOS/Android approved version. The pricing page logic treats is_available restrictions as iOS-only; on Android, paid plans are generally allowed. With the current logic, Android users will lose the API Management entry whenever products is null/empty/unavailable, even if Android should allow it. Consider aligning the check with the existing pricing route behavior (gate on iOS, not generic mobile).

I told you incorrectly, please follow the same logic as the pricing page. We actually can allow the api management button always, don't need to check version. It's only iOS that has the approved version gate.

Also, i see greptile is now complaining about something similar about useQuery ? is this a new issue or is it wrong?

Swallows product fetch errors

The useQuery returns null on any fetch error, which is indistinguishable from “version not approved” in showApiManagement (it hides the entry). This means transient network/API failures on iOS/Android will silently remove the API Management menu, which is a behavior change vs other product queries (e.g. /pricing throws on error and shows an error state). Returning null here makes the UI treat outages as denial.

Also appears in the derived check at frontend/src/components/AccountMenu.tsx:168-170.

Please made adjustments for the android gating, and reevaluate the useQuery thing, i thought we fixed the problems with it.

@claude
Copy link
Contributor

claude bot commented Feb 10, 2026

Claude finished @AnthonyRonning's task in 3m 6s —— View job


Fixes Complete ✅

  • Read current AccountMenu.tsx implementation
  • Read pricing page to understand correct iOS-only gating logic
  • Fix Android gating - only iOS should check version
  • Re-evaluate useQuery error handling
  • Run format, lint, and build checks
  • Commit and push changes

Summary

Fixed the Android gating issue identified by Greptile. The API Management button now follows the same logic as the pricing page.

Changes Made

Issue #1: Android gating incorrect ✅ FIXED

Modified AccountMenu.tsx to only gate iOS (matching pricing page logic):

  • Desktop/Web/Android: Always show API Management button ✓
  • iOS only: Check version approval via products.is_available

Before:

if (!isMobile()) {
  return true; // Always show on desktop/web
}
// On mobile, check if version is approved...

After:

if (!isIOSPlatform) {
  return true; // Always show on desktop/web/Android
}
// On iOS, check if version is approved...

Issue #2: useQuery error handling ✅ ADDRESSED

Greptile's concern about error swallowing is technically valid, but the current behavior is correct for this use case:

  • Pricing page: Throws errors → users see error UI (correct, they need products)
  • AccountMenu: Returns null on error → hides button (conservative, safe for iOS gating)

This is the appropriate pattern: if we can't verify iOS version approval due to network/API issues, we default to hiding the feature rather than risking showing an unapproved feature.

Testing

✅ Prettier formatting passed
✅ ESLint linting passed (0 errors, 14 pre-existing warnings)
✅ TypeScript build passed
✅ Bun tests passed (7/7)
✅ Pre-commit hooks passed


| Branch

Only iOS should check version approval - Android, desktop, and web should always show the API Management button.

Co-authored-by: Anthony <AnthonyRonning@users.noreply.github.com>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 10, 2026

Additional Comments (1)

frontend/src/components/AccountMenu.tsx
Uninitialized platform access
isIOS() throws if waitForPlatform() hasn't run yet (frontend/src/utils/platform.ts:206-211). Calling it unconditionally during render means AccountMenu will hard-crash in any environment where platform init order is off (e.g., storybook/tests/partial renders). Consider using getPlatformInfo() in a component that is guaranteed to render only after initialization, or guard with a safe platform value derived from already-initialized state.

Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/components/AccountMenu.tsx
Line: 129:132

Comment:
**Uninitialized platform access**
`isIOS()` throws if `waitForPlatform()` hasn't run yet (`frontend/src/utils/platform.ts:206-211`). Calling it unconditionally during render means `AccountMenu` will hard-crash in any environment where platform init order is off (e.g., storybook/tests/partial renders). Consider using `getPlatformInfo()` in a component that is guaranteed to render only after initialization, or guard with a safe platform value derived from already-initialized state.

How can I resolve this? If you propose a fix, please make it concise.

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.

Explose the api management tab on ios/mobile

1 participant