Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Contributor
📝 WalkthroughWalkthroughRemoved Pace.js and its inline error-handling patch; replaced a jQuery UI pricing slider and stylesheet with native range+number inputs, refactored client-side pricing and checkout JS (Stripe/Paddle), added server-side input validation for pack counts, and adjusted a yearly pricing tier’s marginal slot size. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Client as Subscription Page (Client JS)
participant Server as Web Server (AJAX)
participant Stripe as Stripe API
participant Paddle as Paddle API
User->>Client: adjust range / number
Client->>Client: updatePricing() → recalc amount, packs, toggle buttons
User->>Client: click Buy (Stripe or Paddle)
Client->>Server: POST /create-checkout (amount, packs)
alt Stripe flow
Server->>Stripe: create checkout/session
Stripe-->>Server: session/url
Server-->>Client: { success:true, url }
Client->>Stripe: redirectToCheckout(url)
else Paddle flow
Server->>Paddle: create/get checkout (PriceId or params)
Paddle-->>Server: response or error
Server-->>Client: { success:true, url } or { success:false, message }
Client->>Paddle: open checkout URL or show error swal
end
Note over Client,Server: AJAX .fail() shows "Connection Error" swal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
Web/Resgrid.Web/Startup.cs (1)
394-407: Pace.js bundle removal looks correct.Removing
lib/pace/pace.jsfrom/js/int-bundle.jsis consistent with the stated cleanup and is safe in this segment. Optional follow-up: remove the stale IntelliSense reference inWeb/Resgrid.Web/wwwroot/_references.js(Line 1) to keep references in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Startup.cs` around lines 394 - 407, The int-bundle was updated to remove lib/pace/pace.js, but the stale IntelliSense entry remains; open _references.js and remove the reference to lib/pace/pace.js (the old Pace IntelliSense line) so editor references match the pipeline.AddJavaScriptBundle("/js/int-bundle.js") contents, then save and verify no other Pace references remain in _references.js.Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml (1)
132-143: Collapse duplicated Razor branches for buy buttons.Both
@if (Model.IsPaddleDepartment)branches render identical anchors. Removing the conditional here reduces noise and future drift risk.Suggested refactor
-@if (Model.IsPaddleDepartment) { - <a id="buyYearlyButton" title="Buy Yearly" class="btn btn-primary" href="#" style="display:none;">Buy Yearly</a> -} else { - <a id="buyYearlyButton" title="Buy Yearly" class="btn btn-primary" href="#" style="display:none;">Buy Yearly</a> -} +<a id="buyYearlyButton" title="Buy Yearly" class="btn btn-primary" href="#" style="display:none;">Buy Yearly</a> ... -@if (Model.IsPaddleDepartment) { - <a id="buyMonthlyButton" title="Buy Monthly" class="btn btn-primary" href="#" style="display:none;">Buy Monthly</a> -} else { - <a id="buyMonthlyButton" title="Buy Monthly" class="btn btn-primary" href="#" style="display:none;">Buy Monthly</a> -} +<a id="buyMonthlyButton" title="Buy Monthly" class="btn btn-primary" href="#" style="display:none;">Buy Monthly</a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml` around lines 132 - 143, The two Razor conditional blocks that check Model.IsPaddleDepartment and render identical anchors (the buyYearlyButton and buyMonthlyButton anchors) should be collapsed into a single unconditional rendering to remove duplicate branches; update SelectRegistrationPlan view to remove both `@if` (Model.IsPaddleDepartment) { ... } else { ... } wrappers and render the anchor tags for buyYearlyButton and buyMonthlyButton once each (keeping their id/title/class/href/style attributes intact) so the buttons remain functionally identical without duplicated code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml`:
- Around line 118-127: The markup appends a static ".00" while the server-side
output for the monthly/yearly values may already include decimals, causing
malformed output (e.g., "15.5.00"); update the rendering logic that writes the
numeric values for the elements with ids monthly-label and yearly-label so it
outputs a fixed two-decimal formatted string (e.g., using ToString("F2") or
String.Format("{0:F2}", value)) and remove the hard-coded ".00" in the markup so
the displayed price is always a single correctly formatted monetary value.
- Around line 190-200: The AJAX call to GetStripeSession and the subsequent
stripe.redirectToCheckout only use .done and don't handle network/API failures
or missing session IDs; add .fail(...) handlers to the $.ajax calls that surface
a user-friendly error (e.g., swal with a clear message) and add a fallback in
the .done handler to check for data and data.SessionId before calling
stripe.redirectToCheckout, showing an error if missing; also add error handling
on the promise returned by stripe.redirectToCheckout (handle result.error) and
apply the same changes to the other checkout block around lines 212-235 where
GetStripeSession and stripe.redirectToCheckout are used.
- Around line 102-109: Add explicit, associated labels for the range slider and
numeric input to improve accessibility: create <label> elements tied to the
existing ids "entity-slider" and "amount-input" (using for="entity-slider" and
for="amount-input"), place them next to the descriptive text ("Entities" /
"Users or Units sold in packs of 10"), and ensure each input also has an
aria-label or aria-describedby pointing to the descriptive text if visual
placement differs; update any JavaScript that queries by label text to use the
ids if needed.
- Around line 249-254: The yearly pricing tiers array pricingTiersYearly
contains a duplicated marginalUserSlots: 1000 entry (with costPerUser 15.0 and
10.0); update the second occurrence to marginalUserSlots: 5000 (to match the
monthly progression 5 → 100 → 1000 → 5000 → 999999999) in both
SelectRegistrationPlan and Index views so the yearly tier breakpoints are
consistent with monthly tiers.
---
Nitpick comments:
In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml`:
- Around line 132-143: The two Razor conditional blocks that check
Model.IsPaddleDepartment and render identical anchors (the buyYearlyButton and
buyMonthlyButton anchors) should be collapsed into a single unconditional
rendering to remove duplicate branches; update SelectRegistrationPlan view to
remove both `@if` (Model.IsPaddleDepartment) { ... } else { ... } wrappers and
render the anchor tags for buyYearlyButton and buyMonthlyButton once each
(keeping their id/title/class/href/style attributes intact) so the buttons
remain functionally identical without duplicated code.
In `@Web/Resgrid.Web/Startup.cs`:
- Around line 394-407: The int-bundle was updated to remove lib/pace/pace.js,
but the stale IntelliSense entry remains; open _references.js and remove the
reference to lib/pace/pace.js (the old Pace IntelliSense line) so editor
references match the pipeline.AddJavaScriptBundle("/js/int-bundle.js") contents,
then save and verify no other Pace references remain in _references.js.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7c1fdfc-f752-4105-82ce-09237e2966f4
📒 Files selected for processing (3)
Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtmlWeb/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtmlWeb/Resgrid.Web/Startup.cs
💤 Files with no reviewable changes (1)
- Web/Resgrid.Web/Areas/User/Views/Shared/_UserLayout.cshtml
Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml
Outdated
Show resolved
Hide resolved
Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml
Outdated
Show resolved
Hide resolved
Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml
Show resolved
Hide resolved
Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml (1)
133-145: Remove redundant conditionals; both branches render identical markup.The
@if (Model.IsPaddleDepartment)blocks for both buttons produce the same HTML regardless of the condition. The checkout logic is already handled in JavaScript via theisPaddlevariable (line 310). Simplify by removing the conditionals.♻️ Suggested simplification
<div class="col-sm-6 text-center"> - `@if` (Model.IsPaddleDepartment) { - <a id="buyYearlyButton" title="Buy Yearly" class="btn btn-primary" href="#" style="display:none;">Buy Yearly</a> - } else { - <a id="buyYearlyButton" title="Buy Yearly" class="btn btn-primary" href="#" style="display:none;">Buy Yearly</a> - } + <a id="buyYearlyButton" title="Buy Yearly" class="btn btn-primary" href="#" style="display:none;">Buy Yearly</a> </div> <div class="col-sm-6 text-center"> - `@if` (Model.IsPaddleDepartment) { - <a id="buyMonthlyButton" title="Buy Monthly" class="btn btn-primary" href="#" style="display:none;">Buy Monthly</a> - } else { - <a id="buyMonthlyButton" title="Buy Monthly" class="btn btn-primary" href="#" style="display:none;">Buy Monthly</a> - } + <a id="buyMonthlyButton" title="Buy Monthly" class="btn btn-primary" href="#" style="display:none;">Buy Monthly</a> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml` around lines 133 - 145, The two conditional blocks using Model.IsPaddleDepartment render identical anchors; remove the redundant `@if` branches and render each button once (keep the existing <a> elements with ids buyYearlyButton and buyMonthlyButton and their attributes), relying on the existing client-side checkout logic that uses the isPaddle variable (referenced around line 310) to handle behavior; ensure you delete the conditional wrappers around buyYearlyButton and buyMonthlyButton while leaving their markup and IDs unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml`:
- Around line 185-211: The GetStripeSession and GetPaddleCheckout actions in
SubscriptionController accept the query parameter count directly and need
server-side bounds checking to prevent bypassing the UI; in each action
(SubscriptionController.GetStripeSession and
SubscriptionController.GetPaddleCheckout) validate and normalize the incoming
count to ensure it is an integer >= 1 (reflecting the frontend requirement that
amount > 10 yields at least one pack), optionally enforce an upper limit (e.g.,
a MAX_PACKS constant) and return a BadRequest/400 when the value is invalid, or
clamp it before passing to the service layer so only safe values are forwarded.
---
Nitpick comments:
In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml`:
- Around line 133-145: The two conditional blocks using Model.IsPaddleDepartment
render identical anchors; remove the redundant `@if` branches and render each
button once (keep the existing <a> elements with ids buyYearlyButton and
buyMonthlyButton and their attributes), relying on the existing client-side
checkout logic that uses the isPaddle variable (referenced around line 310) to
handle behavior; ensure you delete the conditional wrappers around
buyYearlyButton and buyMonthlyButton while leaving their markup and IDs
unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7df0d929-ebe1-4318-84e1-85cd3b5db1c5
📒 Files selected for processing (2)
Web/Resgrid.Web/Areas/User/Views/Subscription/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml
Comment on lines
185
to
211
| function stripeCheckout(id) { | ||
| const amount = slider == 1 ? val : $("#amount").val(); | ||
| var amount = parseInt(document.getElementById('amount-input').value) || 0; | ||
|
|
||
| if (amount && amount > 10) { | ||
| const packs = (amount / 10) - 1; | ||
| if (amount > 10) { | ||
| var packs = (amount / 10) - 1; | ||
|
|
||
| $.ajax({ | ||
| url: resgrid.absoluteBaseUrl + '/User/Subscription/GetStripeSession?id=' + id + '&count=' + packs + (discountCode ? '&discountCode=' + encodeURIComponent(discountCode) : ''), | ||
| contentType: 'application/json', | ||
| type: 'GET' | ||
| }).done(function (data) { | ||
| if (data) { | ||
| if (data.SessionId) { | ||
| stripe.redirectToCheckout({ | ||
| sessionId: data.SessionId | ||
| }).then(function (result) { | ||
| swal({ | ||
| title: "Purchase Error", | ||
| text: "Error redirecting to Stripe for checkout. Stripe error: " + result.error.message, | ||
| icon: "error", | ||
| buttons: true, | ||
| dangerMode: false | ||
| }); | ||
| }); | ||
| } | ||
| if (data && data.SessionId) { | ||
| stripe.redirectToCheckout({ sessionId: data.SessionId }).then(function (result) { | ||
| if (result.error) { | ||
| swal({ title: "Purchase Error", text: "Error redirecting to Stripe: " + result.error.message, icon: "error", buttons: true, dangerMode: false }); | ||
| } | ||
| }); | ||
| } else { | ||
| swal({ title: "Checkout Error", text: "Unable to create a checkout session. Please try again.", icon: "error", buttons: true, dangerMode: false }); | ||
| } | ||
| }).fail(function () { | ||
| swal({ title: "Connection Error", text: "Unable to reach the server. Please check your connection and try again.", icon: "error", buttons: true, dangerMode: false }); | ||
| }); | ||
| } else { | ||
| swal({ | ||
| title: "Cannot Purchase", | ||
| text: "Please select more than 10 entities to purchase a plan.", | ||
| icon: "warning", | ||
| buttons: true, | ||
| dangerMode: false | ||
| }); | ||
| swal({ title: "Cannot Purchase", text: "Please select more than 10 entities to purchase a plan.", icon: "warning", buttons: true, dangerMode: false }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if GetStripeSession and GetPaddleCheckout validate the count parameter
rg -n -A10 'GetStripeSession|GetPaddleCheckout' --type=cs -g '!*Test*' | head -80Repository: Resgrid/Core
Length of output: 3130
🏁 Script executed:
#!/bin/bash
# Find the implementations of CreateStripeSessionForSub and CreatePaddleCheckoutForSub
rg -n 'CreateStripeSessionForSub|CreatePaddleCheckoutForSub' --type=cs -g '!*Test*' | grep -E '(public|private|async).*CreateStripe|CreatePaddle'Repository: Resgrid/Core
Length of output: 1792
🏁 Script executed:
#!/bin/bash
# Search for CreateStripeSessionForSub implementation with validation context
rg -n -B2 -A15 'CreateStripeSessionForSub.*{' --type=cs -g '!*Test*' | head -100Repository: Resgrid/Core
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Read the CreateStripeSessionForSub method implementation
sed -n '1151,1180p' Core/Resgrid.Services/SubscriptionsService.csRepository: Resgrid/Core
Length of output: 1845
🏁 Script executed:
#!/bin/bash
# Read the CreatePaddleCheckoutForSub method implementation
sed -n '1215,1240p' Core/Resgrid.Services/SubscriptionsService.csRepository: Resgrid/Core
Length of output: 1775
Add count parameter validation to GetStripeSession and GetPaddleCheckout actions.
The frontend correctly validates amount > 10 and calculates packs, but the count parameter is passed directly to the API without validation. Users can bypass the UI by crafting direct requests with arbitrary count values (negative, zero, or excessively large), potentially causing unintended charges or API errors.
Add bounds validation in SubscriptionController.GetStripeSession (line 688) and GetPaddleCheckout (line 726) before forwarding to the service layer. Ensure count reflects the frontend's constraint (minimum 1 pack from amount > 10).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Subscription/SelectRegistrationPlan.cshtml`
around lines 185 - 211, The GetStripeSession and GetPaddleCheckout actions in
SubscriptionController accept the query parameter count directly and need
server-side bounds checking to prevent bypassing the UI; in each action
(SubscriptionController.GetStripeSession and
SubscriptionController.GetPaddleCheckout) validate and normalize the incoming
count to ensure it is an integer >= 1 (reflecting the frontend requirement that
amount > 10 yields at least one pack), optionally enforce an upper limit (e.g.,
a MAX_PACKS constant) and return a BadRequest/400 when the value is invalid, or
clamp it before passing to the service layer so only safe values are forwarded.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs (1)
690-692: Consider centralizing pack-count bounds to avoid drift.The same
1..200constraint is duplicated in both endpoints. Prefer a shared constant or private validator helper so Stripe/Paddle paths cannot diverge accidentally.♻️ Suggested refactor
+ private const int MinEntityPackCount = 1; + private const int MaxEntityPackCount = 200; + + private static bool IsValidEntityPackCount(int count) => + count >= MinEntityPackCount && count <= MaxEntityPackCount; ... - if (count < 1 || count > 200) + if (!IsValidEntityPackCount(count)) return BadRequest("Invalid entity pack count."); ... - if (count < 1 || count > 200) + if (!IsValidEntityPackCount(count)) return BadRequest("Invalid entity pack count.");Also applies to: 731-733
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs` around lines 690 - 692, The pack-count bounds (1..200) are duplicated in SubscriptionController; introduce a single shared definition and use it in both places: add private const int PackCountMin = 1 and private const int PackCountMax = 200 (or a private bool ValidatePackCount(int count) method) in the SubscriptionController class, then replace the literal checks (the if (count < 1 || count > 200) in the Stripe and Paddle endpoints) to reference PackCountMin/PackCountMax or call ValidatePackCount to centralize the constraint and prevent drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs`:
- Around line 690-692: The pack-count bounds (1..200) are duplicated in
SubscriptionController; introduce a single shared definition and use it in both
places: add private const int PackCountMin = 1 and private const int
PackCountMax = 200 (or a private bool ValidatePackCount(int count) method) in
the SubscriptionController class, then replace the literal checks (the if (count
< 1 || count > 200) in the Stripe and Paddle endpoints) to reference
PackCountMin/PackCountMax or call ValidatePackCount to centralize the constraint
and prevent drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3681fdfe-858e-439f-9fa9-f7bcfe6c3070
📒 Files selected for processing (1)
Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Merged
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 by CodeRabbit
Refactor
Bug Fixes