feat: add hard paywall in hosted Hopp#349
Conversation
✅ Deploy Preview for hoppdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a hard paywall cutoff for new teams, Stripe trial onboarding and gating, a team rename endpoint, trial-specific billing/email handling, onboarding UI flow, and removes Rewardful tracking from checkout and app startup. ChangesHard Paywall, Trial Onboarding, and Rewardful Removal
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/internal/handlers/billing.go`:
- Around line 336-345: The subscription response for post-cutoff teams with no
subscription is still being serialized through the earlier subscription==nil
fallback, which incorrectly reports status=trialing even when
RequiresPaymentMethod is set. Update the response-building logic in the billing
handler so hard-paywall teams with no subscription are returned as a
blocked/no-subscription state instead of trialing, and make sure the branch that
sets subscriptionResponse.RequiresPaymentMethod and the surrounding subscription
serialization stay consistent with backend/internal/models/user.go’s
IsPro/IsTrial behavior.
- Around line 512-517: The cancellation flow in billing handling leaves the
pending-cancel flag set after a subscription is fully deleted. Update the
subscription teardown path in the billing handler logic that sets dbSub.Status
and dbSub.CanceledAt to also clear dbSub.CancelAtPeriodEnd when the final
deletion is processed, so GET /billing/subscription no longer reports a
scheduled cancellation for an already canceled subscription.
- Around line 528-545: The fallback in mapStripeSubscriptionStatus currently
maps unrecognized Stripe subscription states to models.StatusActive, which can
incorrectly keep access enabled. Update mapStripeSubscriptionStatus in
billing.go to fail closed by mapping unknown or paused statuses to
models.StatusCanceled or a dedicated unknown state, and ensure any entitlement
checks via models.Subscription.IsActive() no longer treat unrecognized upstream
states as active.
In `@backend/internal/handlers/handlers.go`:
- Around line 369-380: Wrap the placeholder team creation and the subsequent
user creation in the same transaction inside the signup flow handled by
h.DB.Create and the surrounding Team/User setup logic. Start a transaction
before creating the Team, reuse that transaction for both the Team insert and
the later h.DB.Create(u) call, and roll back on any failure so a user-create
error cannot leave an orphan Team behind. If the transaction succeeds, commit
once both inserts are successful; keep the existing error handling/messages but
make sure they are returned from the transactional path.
- Around line 688-691: The unauthorized branch in handlers.go for the
/api/auth/team flow currently returns plain text from
getAuthenticatedUserFromJWT, but the documented contract expects a JSON Error
shape. Update the 401 path in the relevant handler to return the same
application/json error payload used elsewhere in the API, keeping the response
consistent with the generated clients and OpenAPI definition.
In `@backend/test/integration/paywall_test.go`:
- Around line 82-89: The post-cutoff paywall fixtures are relying on
createTestTeam’s default created_at instead of stamping a known post-cutoff
time. Update newAdminWithTeam to explicitly set the team’s created_at to a
post-cutoff value, mirroring the existing preCutoffDate handling, so
TestHardPaywall_PostCutoffAccessByStatus,
TestHardPaywall_InvitedUserInheritsAccess, TestHardPaywall_RoomJoinBlocked, and
TestHardPaywall_RequiresPaymentMethod always exercise the intended path.
In `@backend/web/emails/hopp-subscription-trial.html`:
- Around line 111-113: The GitHub CTA link in the email template is missing its
URL scheme, so mail clients may not recognize it as a valid absolute link.
Update the href in the subscription trial HTML template to use an absolute
https:// GitHub URL, and make sure the anchor inside the CTA block points to the
correct GitHub repository target.
- Around line 24-25: The subscription trial email template still hardcodes
“14-day” in the preview, heading, and body even though the Stripe trial length
is now configurable. Update the relevant text in hopp-subscription-trial.html to
use the configured trial period from config.go instead of a fixed value, and
make sure the preview/heading/body sections stay consistent with the runtime
Stripe trial settings.
In `@web-app/src/App.tsx`:
- Around line 58-72: The subscription gate in App.tsx can fail open when
useQuery("/api/auth/billing/subscription") errors because subscription becomes
undefined and mustOnboard stays false, allowing dashboard content to render.
Update the App component’s query handling to include the error state (from
useQuery) and keep blocking navigation/content until onboarding eligibility is
known, either by rendering an error/retry view or treating the failed fetch as a
blocking state. Use the existing mustOnboard, isLoading, and navigate logic to
ensure the gate only opens when the subscription result is successfully
resolved.
- Around line 52-63: The checkout-success bypass in App.tsx is currently tied
directly to the URL param, so `justSubscribed` can keep suppressing
`mustOnboard` indefinitely. Update the `justSubscribed` logic around `useQuery`
and `mustOnboard` to make the bypass one-time only: either clear
`subscription_success` from the URL after the first render or use a ref/state
flag that is consumed once and then re-evaluates the fresh subscription data.
Keep the fix localized to `App` so the onboarding gate resumes normally after
the initial post-checkout render.
- Line 12: The App.tsx import still uses a relative path for a src module, so
update the import of QueryProvider and useAPI from useQueryClients to use the @
alias instead of ./hooks/useQueryClients. Keep the change limited to the App
component’s import statement and align it with the existing Vite alias
convention used across the web-app src code.
In `@web-app/src/components/SubscriptionSuccessModal.tsx`:
- Line 30: The success message in SubscriptionSuccessModal is hard-coded to “14
days” instead of using the configured trial duration. Update the text in
SubscriptionSuccessModal to pull the same backend/config-derived trial length
used by the onboarding payment step, and reuse that shared value wherever the
trial duration is displayed so the copy stays consistent with the configured
Stripe trial period.
In `@web-app/src/pages/Onboarding.tsx`:
- Line 18: The onboarding draft storage in Onboarding.tsx is using a single
origin-wide key, which can leak one account’s draft into another on the same
browser. Update the ONBOARDING_DRAFT_KEY usage so the draft is scoped by a
non-sensitive account/team identifier in the onboarding flow, or clear the
stored draft when auth/account context changes; make sure the save/load logic
around the onboarding draft state uses the same scoped key everywhere.
- Around line 29-34: The loadDraft() helper in Onboarding.tsx currently trusts
parsed localStorage data too much, which can let malformed values reach form
state. Update loadDraft() to normalize the returned draft before using it,
especially ensuring pairingTool is always an array (and other fields match
expected types) even when localStorage contains stale or invalid JSON. Keep the
fix localized to the loadDraft function so downstream components do not receive
unsafe form values.
- Line 16: The onboarding billing copy is hard-coded to “14 days” instead of
using the configurable Stripe trial period. Update Onboarding and the related
copy around the referenced trial messaging to read the duration from
backend/config or the shared generated contract, using the existing TRIAL_DAYS
constant only if it is sourced from that shared value, so the UI always matches
the actual billing behavior.
- Around line 130-132: The checkout flow in Onboarding.tsx leaves
checkoutLoading stuck true when the API response has no checkout_url, blocking
retries. Update the handler around the checkout_url redirect so the loading
state is always cleared on the non-redirect path as well, and only keep it
active while a valid redirect is happening. Use the existing Onboarding
component flow and the response.checkout_url check to locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b60c279a-fb4b-4166-9571-0212114ab97b
📒 Files selected for processing (21)
backend/api-files/openapi.yamlbackend/internal/config/config.gobackend/internal/email/email.gobackend/internal/handlers/billing.gobackend/internal/handlers/billing_test.gobackend/internal/handlers/handlers.gobackend/internal/models/user.gobackend/internal/server/server.gobackend/sql/mock_data.sqlbackend/test/integration/paywall_test.gobackend/web/emails/hopp-subscription-trial.htmltauri/src/openapi.d.tsweb-app/src/App.tsxweb-app/src/components/OnboardingModal.tsxweb-app/src/components/SubscriptionSuccessModal.tsxweb-app/src/lib/rewardful.jsweb-app/src/main.tsxweb-app/src/openapi.d.tsweb-app/src/pages/Login.tsxweb-app/src/pages/Onboarding.tsxweb-app/src/pages/Subscription.tsx
💤 Files with no reviewable changes (2)
- web-app/src/lib/rewardful.js
- web-app/src/main.tsx
| dbSub.Status = models.StatusCanceled | ||
| canceledAt := time.Now() | ||
| if subscription.CanceledAt != 0 { | ||
| canceledAt = time.Unix(subscription.CanceledAt, 0) | ||
| } | ||
| dbSub.CanceledAt = &canceledAt |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Clear CancelAtPeriodEnd when the subscription is fully deleted.
This path marks the row canceled but leaves the pending-cancel flag untouched. For subscriptions that were already scheduled to end, GET /billing/subscription can keep returning cancel_at_period_end=true after the deletion is final.
Proposed fix
dbSub.Status = models.StatusCanceled
+ dbSub.CancelAtPeriodEnd = false
canceledAt := time.Now()
if subscription.CanceledAt != 0 {
canceledAt = time.Unix(subscription.CanceledAt, 0)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dbSub.Status = models.StatusCanceled | |
| canceledAt := time.Now() | |
| if subscription.CanceledAt != 0 { | |
| canceledAt = time.Unix(subscription.CanceledAt, 0) | |
| } | |
| dbSub.CanceledAt = &canceledAt | |
| dbSub.Status = models.StatusCanceled | |
| dbSub.CancelAtPeriodEnd = false | |
| canceledAt := time.Now() | |
| if subscription.CanceledAt != 0 { | |
| canceledAt = time.Unix(subscription.CanceledAt, 0) | |
| } | |
| dbSub.CanceledAt = &canceledAt |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/handlers/billing.go` around lines 512 - 517, The
cancellation flow in billing handling leaves the pending-cancel flag set after a
subscription is fully deleted. Update the subscription teardown path in the
billing handler logic that sets dbSub.Status and dbSub.CanceledAt to also clear
dbSub.CancelAtPeriodEnd when the final deletion is processed, so GET
/billing/subscription no longer reports a scheduled cancellation for an already
canceled subscription.
| // mapStripeSubscriptionStatus converts a Stripe subscription status into our | ||
| // internal SubscriptionStatus. Unknown statuses default to active so access is | ||
| // not accidentally revoked. | ||
| func mapStripeSubscriptionStatus(status stripe.SubscriptionStatus) models.SubscriptionStatus { | ||
| switch status { | ||
| case stripe.SubscriptionStatusTrialing: | ||
| return models.StatusTrialing | ||
| case stripe.SubscriptionStatusActive: | ||
| return models.StatusActive | ||
| case stripe.SubscriptionStatusPastDue: | ||
| return models.StatusPastDue | ||
| case stripe.SubscriptionStatusCanceled, stripe.SubscriptionStatusUnpaid: | ||
| return models.StatusCanceled | ||
| case stripe.SubscriptionStatusIncomplete, stripe.SubscriptionStatusIncompleteExpired: | ||
| return models.StatusIncomplete | ||
| default: | ||
| return models.StatusActive | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In Stripe's subscriptions API, what does the pausedstatus mean, and shouldpaused or unknown subscription statuses be treated as active for access control?
💡 Result:
In Stripe’s Subscription API, the paused status indicates that a subscription has been explicitly paused, which stops both service delivery and billing [1][2]. When a subscription enters the paused status, it stops generating invoices and does not advance to new billing periods [3][1]. It is important to distinguish the paused status from the pause_collection feature [3][1]: - paused status: The subscription is effectively inactive; Stripe stops billing and service is typically revoked [1][2]. - pause_collection: The subscription status remains active, but payment collection is frozen [4][3][1]. Because the status remains active during pause_collection, your application logic must independently check the pause_collection field to determine if billing or service access should be restricted [5][6]. Regarding access control, you should generally not treat paused subscriptions as active [1][2]. For subscriptions that are active but have pause_collection enabled, access control should be determined by your specific business policy (e.g., read-only access, full access, or limited access) rather than relying solely on the subscription status [5][6]. For any unknown or unhandled subscription statuses, the safest approach for access control is to default to the most restrictive state (e.g., deny access) to prevent unintended service usage until the status is explicitly accounted for in your application logic. Always rely on the authoritative status field provided by the Stripe API and supplement it with checks for pause_collection or other metadata as required by your product's billing policy [5][6].
Citations:
- 1: https://docs.stripe.com/billing/subscriptions/pause
- 2: https://docs.stripe.com/billing/subscriptions/pause.md
- 3: https://docs.stripe.com/api/subscriptions/pause?api-version=2026-04-22.preview
- 4: https://docs.stripe.com/api/subscriptions/object
- 5: https://goosekit.dev/blog/stripe-pause-collection-supabase-access/
- 6: https://savemrr.co/how-to-create-pause-flow-stripe
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant handler and nearby access-control logic.
git ls-files backend/internal/handlers/billing.go backend/internal/models backend/internal | sed -n '1,120p'
echo '--- billing.go around mapStripeSubscriptionStatus ---'
sed -n '480,600p' backend/internal/handlers/billing.go
echo '--- search for SubscriptionStatus usage ---'
rg -n "StatusActive|StatusCanceled|StatusTrialing|StatusPastDue|StatusIncomplete|mapStripeSubscriptionStatus|SubscriptionStatus" backend/internal -g '!**/*_test.go'
echo '--- search for Stripe status fields and any paused handling ---'
rg -n "paused|pause_collection|stripe.SubscriptionStatus" backend -g '!**/*_test.go'Repository: gethopp/hopp
Length of output: 9299
Fail closed for unrecognized Stripe subscription states Map unknown or paused Stripe statuses to canceled (or a dedicated unknown state) instead of active; models.Subscription.IsActive() treats active/trialing as entitled, so the current default can grant Pro access on non-billable upstream states.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/handlers/billing.go` around lines 528 - 545, The fallback in
mapStripeSubscriptionStatus currently maps unrecognized Stripe subscription
states to models.StatusActive, which can incorrectly keep access enabled. Update
mapStripeSubscriptionStatus in billing.go to fail closed by mapping unknown or
paused statuses to models.StatusCanceled or a dedicated unknown state, and
ensure any entitlement checks via models.Subscription.IsActive() no longer treat
unrecognized upstream states as active.
| user, isAuthenticated := h.getAuthenticatedUserFromJWT(c) | ||
| if !isAuthenticated { | ||
| return c.String(http.StatusUnauthorized, "Unauthorized") | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return the documented JSON error shape on 401.
Lines 689-690 return plain text, but /api/auth/team is documented in backend/api-files/openapi.yaml and both generated clients as application/json Error for 401. That breaks the new typed contract on expired/missing-token paths.
Proposed fix
user, isAuthenticated := h.getAuthenticatedUserFromJWT(c)
if !isAuthenticated {
- return c.String(http.StatusUnauthorized, "Unauthorized")
+ return echo.NewHTTPError(http.StatusUnauthorized, "Unauthorized")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user, isAuthenticated := h.getAuthenticatedUserFromJWT(c) | |
| if !isAuthenticated { | |
| return c.String(http.StatusUnauthorized, "Unauthorized") | |
| } | |
| user, isAuthenticated := h.getAuthenticatedUserFromJWT(c) | |
| if !isAuthenticated { | |
| return echo.NewHTTPError(http.StatusUnauthorized, "Unauthorized") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/handlers/handlers.go` around lines 688 - 691, The
unauthorized branch in handlers.go for the /api/auth/team flow currently returns
plain text from getAuthenticatedUserFromJWT, but the documented contract expects
a JSON Error shape. Update the 401 path in the relevant handler to return the
same application/json error payload used elsewhere in the API, keeping the
response consistent with the generated clients and OpenAPI definition.
| const title = isTrial ? "🚢 Welcome onboard to Hopp" : "🎉 Welcome back to Hopp"; | ||
| const subtitle = | ||
| isTrial ? | ||
| "Your trial to Hopp just started. Enjoy all the premium features for 14 days." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use the configured trial duration here too.
This hard-codes “14 days” even though the Stripe trial period is configurable in this PR stack. Reuse the same backend/config-derived value as the onboarding payment step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-app/src/components/SubscriptionSuccessModal.tsx` at line 30, The success
message in SubscriptionSuccessModal is hard-coded to “14 days” instead of using
the configured trial duration. Update the text in SubscriptionSuccessModal to
pull the same backend/config-derived trial length used by the onboarding payment
step, and reuse that shared value wherever the trial duration is displayed so
the copy stays consistent with the configured Stripe trial period.
|
|
||
| import { CompanySizeSelect, PairingToolMultiSelect, ReferralSourceSelect } from "@/components/OnboardingModal"; | ||
|
|
||
| const TRIAL_DAYS = 14; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Don’t hard-code the trial duration in billing copy.
The PR stack makes the Stripe trial period configurable, but this page always tells users “14 days.” Pull this value from backend/config or a shared generated contract so UI copy matches actual billing behavior.
Also applies to: 220-230
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-app/src/pages/Onboarding.tsx` at line 16, The onboarding billing copy is
hard-coded to “14 days” instead of using the configurable Stripe trial period.
Update Onboarding and the related copy around the referenced trial messaging to
read the duration from backend/config or the shared generated contract, using
the existing TRIAL_DAYS constant only if it is sourced from that shared value,
so the UI always matches the actual billing behavior.
| if (response?.checkout_url) { | ||
| window.location.href = response.checkout_url; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle a missing checkout URL before leaving the button disabled.
If the API returns without checkout_url, checkoutLoading stays true and the user cannot retry.
Proposed fix
- if (response?.checkout_url) {
- window.location.href = response.checkout_url;
- }
+ if (!response?.checkout_url) {
+ toast.error("Failed to start your trial. Please try again.");
+ setCheckoutLoading(false);
+ return;
+ }
+
+ window.location.href = response.checkout_url;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response?.checkout_url) { | |
| window.location.href = response.checkout_url; | |
| } | |
| if (!response?.checkout_url) { | |
| toast.error("Failed to start your trial. Please try again."); | |
| setCheckoutLoading(false); | |
| return; | |
| } | |
| window.location.href = response.checkout_url; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-app/src/pages/Onboarding.tsx` around lines 130 - 132, The checkout flow
in Onboarding.tsx leaves checkoutLoading stuck true when the API response has no
checkout_url, blocking retries. Update the handler around the checkout_url
redirect so the loading state is always cleared on the non-redirect path as
well, and only keep it active while a valid redirect is happening. Use the
existing Onboarding component flow and the response.checkout_url check to locate
the fix.
konsalex
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web-app/src/pages/Onboarding.tsx (1)
127-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
subscriptionvariable shadows the outer billing subscription.The
form.store.subscribe(...)return value is namedsubscription, which shadows the outerconst subscription = subscriptionData?.subscription;(Line 58) used throughout the component. This works today only because the effect body doesn't reference the outer variable, but it's a landmine for future edits — anyone extending this effect to reference the Stripe subscription would silently get the wrong value.♻️ Suggested rename
useEffect(() => { if (!userId) return; - const subscription = form.store.subscribe(() => { + const unsubscribe = form.store.subscribe(() => { localStorage.setItem(draftKey(userId), JSON.stringify(form.store.state.values)); }); - return () => subscription.unsubscribe(); + return () => unsubscribe.unsubscribe(); }, [form, userId]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-app/src/pages/Onboarding.tsx` around lines 127 - 133, The local `subscription` declared inside `Onboarding`’s `useEffect` shadows the outer Stripe subscription variable from `subscriptionData?.subscription`, which can cause accidental misuse later. Rename the `form.store.subscribe(...)` result to a distinct name in `Onboarding` and update the cleanup to use that new identifier so the effect stays clear and non-conflicting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-app/src/components/onboarding-fields.tsx`:
- Around line 71-88: The remove control inside the selected tool Badge is
mouse-only because Button with asChild renders a span instead of a focusable
control. Update the remove action in onClick/removeSelection to use a real
button element, or add the required accessibility props and keyboard handlers so
it can receive focus and activate with Enter/Space. Keep the fix localized to
the Badge/Button/XIcon remove control in onboarding-fields.tsx.
---
Nitpick comments:
In `@web-app/src/pages/Onboarding.tsx`:
- Around line 127-133: The local `subscription` declared inside `Onboarding`’s
`useEffect` shadows the outer Stripe subscription variable from
`subscriptionData?.subscription`, which can cause accidental misuse later.
Rename the `form.store.subscribe(...)` result to a distinct name in `Onboarding`
and update the cleanup to use that new identifier so the effect stays clear and
non-conflicting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2158a15a-5985-4fcb-82bc-c182eced3d4c
📒 Files selected for processing (12)
backend/internal/handlers/billing.gobackend/internal/handlers/handlers.gobackend/test/integration/paywall_test.gobackend/web/emails/hopp-subscription-trial.htmlbackend/web/emails/hopp-subscription.htmlbackend/web/emails/hopp-unsubscribe.htmlbackend/web/emails/hopp-welcome.htmlweb-app/src/components/OnboardingModal.tsxweb-app/src/components/SignInSuccessModal.tsxweb-app/src/components/onboarding-fields.tsxweb-app/src/pages/Dashboard.tsxweb-app/src/pages/Onboarding.tsx
💤 Files with no reviewable changes (3)
- web-app/src/components/SignInSuccessModal.tsx
- web-app/src/pages/Dashboard.tsx
- web-app/src/components/OnboardingModal.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/web/emails/hopp-subscription-trial.html
- backend/internal/handlers/handlers.go
- backend/test/integration/paywall_test.go
- backend/internal/handlers/billing.go
| <Badge key={val} variant="outline"> | ||
| {tool.label} | ||
| <Button | ||
| type="button" | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-4" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| removeSelection(val); | ||
| }} | ||
| asChild | ||
| > | ||
| <span> | ||
| <XIcon className="size-3" /> | ||
| </span> | ||
| </Button> | ||
| </Badge> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the Button component's asChild/Slot implementation for focus/keyboard handling
fd -a button.tsx web-app/src/components/ui | xargs cat -nRepository: gethopp/hopp
Length of output: 5484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== web-app/src/components/ui/button.tsx =="
cat -n web-app/src/components/ui/button.tsx
echo
echo "== web-app/src/components/onboarding-fields.tsx (relevant range) =="
sed -n '60,100p' web-app/src/components/onboarding-fields.tsx | cat -nRepository: gethopp/hopp
Length of output: 5110
Remove control is mouse-only. Button with asChild renders the inner <span> directly, so this icon never gets focus or Enter/Space activation. Use a real <button> here, or add role, tabIndex, and keyboard handling, so selected tools can be removed from the keyboard too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-app/src/components/onboarding-fields.tsx` around lines 71 - 88, The
remove control inside the selected tool Badge is mouse-only because Button with
asChild renders a span instead of a focusable control. Update the remove action
in onClick/removeSelection to use a real button element, or add the required
accessibility props and keyboard handlers so it can receive focus and activate
with Enter/Space. Keep the fix localized to the Badge/Button/XIcon remove
control in onboarding-fields.tsx.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tauri/src/components/sidebar/Sidebar.tsx (2)
122-129: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFloating promises now unhandled — restore
voidor add.catch().
installAndRelaunch()anddownloadAndRelaunch()both return promises (pertauri/src/update.ts) and are called withoutawait,void, or.catch(). Any rejection (failed install/download/relaunch) becomes an unhandled promise rejection instead of being intentionally discarded.♻️ Restore explicit handling
if (OS === "macos" && hasPendingUpdate()) { - installAndRelaunch(); + installAndRelaunch().catch((err) => console.error("Failed to install and relaunch:", err)); return; } - downloadAndRelaunch(); + downloadAndRelaunch().catch((err) => console.error("Failed to download and relaunch:", err));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tauri/src/components/sidebar/Sidebar.tsx` around lines 122 - 129, The sidebar update click handler is dropping returned promises from `installAndRelaunch()` and `downloadAndRelaunch()`, which can lead to unhandled rejections. Update the `onClick` callback in `Sidebar` to explicitly handle these async calls by prefixing them with `void` or by chaining a `.catch()` handler, while keeping the existing `setUpdateInProgress` and `hasPendingUpdate()` flow intact. Use the `installAndRelaunch` and `downloadAndRelaunch` calls as the fix points.
182-196: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSame floating-promise concern for
openUrl(...).
openUrlreturnsPromise<void>(per@tauri-apps/plugin-opener), and the call at line 187 no longer discards or handles the promise. This is inside a throttled click handler, so a rejection would surface as an unhandled rejection on every click.♻️ Restore explicit handling
if (user.is_admin) { - openUrl(new URL("/subscription", Constants.webAppUrl).toString()); + openUrl(new URL("/subscription", Constants.webAppUrl).toString()).catch((err) => + console.error("Failed to open subscription URL:", err), + ); } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tauri/src/components/sidebar/Sidebar.tsx` around lines 182 - 196, The throttled click handler in Sidebar’s handleClick is triggering a floating-promise issue because openUrl returns a Promise<void> and its result is not handled. Update the user.is_admin branch to explicitly discard or handle the promise from openUrl, keeping the behavior inside the throttle wrapper unchanged and ensuring any rejection is not left unhandled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tauri/src/components/sidebar/Sidebar.tsx`:
- Around line 122-129: The sidebar update click handler is dropping returned
promises from `installAndRelaunch()` and `downloadAndRelaunch()`, which can lead
to unhandled rejections. Update the `onClick` callback in `Sidebar` to
explicitly handle these async calls by prefixing them with `void` or by chaining
a `.catch()` handler, while keeping the existing `setUpdateInProgress` and
`hasPendingUpdate()` flow intact. Use the `installAndRelaunch` and
`downloadAndRelaunch` calls as the fix points.
- Around line 182-196: The throttled click handler in Sidebar’s handleClick is
triggering a floating-promise issue because openUrl returns a Promise<void> and
its result is not handled. Update the user.is_admin branch to explicitly discard
or handle the promise from openUrl, keeping the behavior inside the throttle
wrapper unchanged and ensuring any rejection is not left unhandled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a26afc4a-950c-4f9f-b1d1-08f07f94738a
📒 Files selected for processing (4)
backend/internal/models/user.gobackend/test/integration/paywall_test.gotauri/src/components/sidebar/Sidebar.tsxweb-app/src/pages/Onboarding.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/internal/models/user.go
- web-app/src/pages/Onboarding.tsx
- backend/test/integration/paywall_test.go
1 participant
This PR introduces hard paywall for hosted service of Hopp.
Also some other things:
Summary by CodeRabbit
Summary
New Features
Bug Fixes
Changes