Fix/remove duplicate setting links #17
Conversation
…dated in Polar. Select the latest subscription when querying subscription based on userId.
…. priceId is not updated after a user changes the subscription creating confusion. Changes Summary: Checkout Creation (lines 52-54): - REMOVED: priceId: productPriceId, from the metadata object - RESULT: Metadata now only contains userId when creating Polar checkouts Impact: - Polar subscriptions: Will only store userId in metadata (no priceId) - Convex subscriptions: Will only store userId in metadata (no priceId) - Price tracking: Still works via the polarPriceId field in the subscription record - User linking: Preserved via userId in metadata for webhook processing
Changes Made:
- Removed duplicate Settings link from sidebar
navigation
- Eliminated redundant Settings entry in navSecondary
array in app-sidebar.tsx
- Fixed Settings link in user dropdown navigation
- Added proper routing to Settings dropdown item in
nav-user.tsx
- Settings link now navigates to /dashboard/settings
when clicked
- Used asChild pattern with React Router Link for
proper navigation
Result:
- Eliminates confusing duplicate Settings links in
dashboard
- Provides single, functional Settings access through
user dropdown
- Improves user experience with cleaner navigation
structure
|
@pax2678 is attempting to deploy a commit to the Goshen Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughRemoved the Settings link from the sidebar. Converted the user dropdown’s Settings item to a router Link. Updated subscription logic to always use the most recent subscription, removed priceId from checkout metadata, and on webhook subscription.updated, store price_id into polarPriceId. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NavUser as NavUser Dropdown
participant Router as Router
participant Settings as Settings Page
User->>NavUser: Open menu
User->>NavUser: Click "Settings"
NavUser->>Router: Link to /dashboard/settings
Router-->>Settings: Render page
sequenceDiagram
autonumber
participant Client as Client/API
participant Subs as subscriptions.ts
participant DB as Database
Note over Subs: Status fetch
Client->>Subs: checkUserSubscriptionStatus(...)
Subs->>DB: query subscriptions ORDER BY createdAt DESC
DB-->>Subs: latest subscription
Subs-->>Client: status/result
Note over Subs: Checkout creation
Client->>Subs: createCheckout(...)
Subs->>DB: store checkout metadata (without priceId)
DB-->>Subs: ack
Subs-->>Client: checkout link
Note over Subs: Webhook update
participant Webhook as Webhook Handler
Webhook->>Subs: subscription.updated(price_id)
Subs->>DB: update subscription.polarPriceId = price_id
DB-->>Subs: ack
Subs-->>Webhook: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
app/components/dashboard/app-sidebar.tsx (1)
51-53: Render NavSecondary only when items exist.Avoids extra empty markup and future layout quirks at the footer.
<NavMain items={data.navMain} /> - <NavSecondary items={data.navSecondary} className="mt-auto" /> + {data.navSecondary.length > 0 && ( + <NavSecondary items={data.navSecondary} className="mt-auto" /> + )}app/components/dashboard/nav-user.tsx (1)
89-95: Good switch to asChild Link; consider prefetch + icon set consistency.
- Add prefetch for parity with other Links (if supported in your router setup).
- Use the existing Tabler icon set here for consistency and smaller bundles.
- <DropdownMenuItem asChild> - <Link to="/dashboard/settings"> - - <SettingsIcon /> - Settings - </Link> + <DropdownMenuItem asChild> + <Link to="/dashboard/settings" prefetch="intent"> + <IconSettings /> + Settings + </Link> </DropdownMenuItem>Additionally update imports (outside this range):
- IconDotsVertical, - IconLogout, - IconUserCircle, + IconDotsVertical, + IconLogout, + IconUserCircle, + IconSettings,convex/subscriptions.ts (3)
234-242: Repeat: verify ordering + align comment.Same concern as above for Clerk-based lookup; update the comment.
- .order("desc") // Order by createdAt in descending order + .order("desc") // Newest first for this user (descending)
262-269: Repeat: verify ordering + align comment.Ensure this returns the latest subscription; update the comment for clarity.
- .order("desc") // Order by createdAt in descending order + .order("desc") // Newest first for this user (descending)
197-205: Ensure correct index for desired subscription ordering
- The
subscriptionstable currently only hasso.index("userId", ["userId"]).order("desc")falls back to document_id(insertion time). If that matches your “newest subscription” intent, update the inline comment:- .order("desc") // Order by createdAt in descending order + .order("desc") // Newest by creation (_id) first for this user- If you need to sort by a specific timestamp field (e.g.
startedAt), add a composite index and use it in the query:.index("byUserStartedAt", ["userId", "startedAt"]) .withIndex("byUserStartedAt", q => q.eq("userId", user.tokenIdentifier)) .order("desc") .first();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/components/dashboard/app-sidebar.tsx(1 hunks)app/components/dashboard/nav-user.tsx(2 hunks)convex/subscriptions.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/components/dashboard/app-sidebar.tsx (2)
app/components/dashboard/nav-secondary.tsx (2)
NavSecondary(15-58)item(31-53)app/routes/dashboard/settings.tsx (1)
Page(4-16)
app/components/dashboard/nav-user.tsx (2)
app/components/ui/dropdown-menu.tsx (3)
DropdownMenuItem(248-248)DropdownMenuItem(62-83)DropdownMenu(9-13)app/routes/dashboard/settings.tsx (1)
Page(4-16)
convex/subscriptions.ts (2)
app/routes/pricing.tsx (2)
priceId(57-97)handleSubscribe(209-209)app/components/homepage/pricing.tsx (1)
priceId(26-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
app/components/dashboard/app-sidebar.tsx (1)
29-29: LGTM: duplicate Settings removed from sidebar.Change aligns with the PR goal and avoids redundant navigation.
app/components/dashboard/nav-user.tsx (1)
8-8: LGTM: router Link import added.Consistent with Link usage elsewhere in this codebase.
convex/subscriptions.ts (1)
331-345: LGTM: persist latest price on subscription.updated.Storing
polarPriceIdon updates keeps the record consistent with plan changes.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
| const subscription = await ctx.db | ||
| .query("subscriptions") | ||
| .withIndex("userId", (q) => q.eq("userId", user.tokenIdentifier)) | ||
| .order("desc") // Order by createdAt in descending order |
There was a problem hiding this comment.
Selecting the newest subscription record to infer active status can yield incorrect entitlement checks. Filter for an active status when querying instead of relying on recency.
Prompt for AI agents
Address the following comment on convex/subscriptions.ts at line 200:
<comment>Selecting the newest subscription record to infer active status can yield incorrect entitlement checks. Filter for an active status when querying instead of relying on recency.</comment>
<file context>
@@ -194,10 +193,11 @@ export const checkUserSubscriptionStatus = query({
const subscription = await ctx.db
.query("subscriptions")
.withIndex("userId", (q) => q.eq("userId", user.tokenIdentifier))
+ .order("desc") // Order by createdAt in descending order
.first();
</file context>
| .order("desc") // Order by createdAt in descending order | |
| .filter((q) => q.eq(q.field("status"), "active")) |
1 participant
fix: clean up dashboard settings navigation and remove a duplicate setting link
Changes Made:
navigation
array in app-sidebar.tsx
nav-user.tsx
when clicked
proper navigation
Result:
dashboard
user dropdown
structure
Summary by cubic
Removed a duplicate Settings link in the dashboard and fixed the dropdown link to route correctly. Also cleaned up subscription handling to avoid stale price data and always read the latest record.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes