Refactor checkout redirection to use a unified path helper for improv…

[kipsang01]

…ed clarity and maintainability

[willymwai]

/describe

[willymwai]

/review

[willymwai]

/improve

[willymwai]

/improve

[willymwai]

/review

[qodo-code-review]

User description

…ed clarity and maintainability

---

PR Type

Enhancement

---

Description

• Introduce unified checkout_state_path_for helper method across multiple controllers

• Standardize checkout redirection to use order token for improved security

• Replace direct spree.checkout_state_path calls with helper method invocations

• Improve code maintainability by centralizing path generation logic

---

Diagram Walkthrough

flowchart LR
  A["Multiple Controllers<br/>with checkout redirects"] -- "Replace direct<br/>spree.checkout_state_path calls" --> B["Unified<br/>checkout_state_path_for<br/>helper method"]
  B -- "Uses order token<br/>for security" --> C["Standardized<br/>checkout paths"]

Loading

File Walkthrough

Relevant files

Enhancement

ipay_controller.rb Add checkout path helper to API iPay controller                    app/controllers/spree/api/v1/ipay_controller.rb Add private checkout_state_path_for helper method that wraps spree.checkout_state_path with order token Replace three direct spree.checkout_state_path calls with new helper method Support optional state parameter with default to order.state +7/-3      checkout_controller_decorator.rb Add checkout path helper to checkout controller                    app/controllers/spree/checkout_controller_decorator.rb Add private checkout_state_path_for helper method for consistent path generation Replace four direct checkout_state_path calls with helper method invocations Update next_step_url_for method to use new helper for all checkout state paths Maintain backward compatibility with optional state parameter +12/-8    gateway_callbacks_controller.rb Add order token to gateway callback path                                  app/controllers/spree/gateway_callbacks_controller.rb Update spree.checkout_state_path call to include order token parameter Improve security by passing order token explicitly to path generation +1/-1      ipay_controller.rb Add checkout path helper to iPay controller                            app/controllers/spree/ipay_controller.rb Add private checkout_state_path_for helper method with order token support Replace two checkout_state_path calls with new helper method Improve error handling by using ternary operator for fallback to cart path +7/-3      ipay_controller_decorator.rb Add checkout path helper to iPay controller decorator        app/controllers/spree/ipay_controller_decorator.rb Add private checkout_state_path_for helper method for consistent path generation Replace two checkout_state_path calls with helper method invocations Improve error handling with cleaner ternary operator syntax Maintain consistent behavior across decorator and base controller +9/-4

---

[qodo-code-review]

PR Reviewer Guide 🔍

(Review updated until commit 564b334)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 Security concerns Token exposure: Redirect URLs now include order.token in the checkout path. While often required for guest checkout, it can leak via referrers/logs/analytics if users navigate away. Confirm this is intended for these endpoints and that token-based access is acceptable for your threat model.

⚡ Recommended focus areas for review Routing The new checkout_state_path_for helper calls spree.checkout_state_path(order.token, state). Please verify the route signature for spree.checkout_state_path in this app/Spree version actually accepts (token, state) in that order (vs a single state arg, or state plus order_token: param). A mismatch here would break the redirect flow at runtime. redirect_to checkout_state_path_for(order, 'payment'), notice: 'We are still processing your payment. Please check back soon.' return end # If payment failed if @payment.failed? || @payment.void? redirect_to checkout_state_path_for(order, 'payment'), alert: 'Payment was not completed. Please try again or use a different payment method.' return end # Default fallback redirect_to checkout_state_path_for(order), notice: 'Please complete your order.' rescue StandardError redirect_to spree.root_path, alert: 'An error occurred while processing your order. Please contact support if the problem persists.' end # Check payment status endpoint def status payment = Spree::Payment.find(params[:payment_id]) payment_method = payment.payment_method if payment_method.is_a?(Spree::PaymentMethod::Ipay) status_response = payment_method.send(:check_payment_status, payment.response_code) render json: status_response else render json: { status: 'error', message: 'Invalid payment method' }, status: :bad_request end rescue ActiveRecord::RecordNotFound render json: { status: 'error', message: 'Payment not found' }, status: :not_found rescue StandardError render json: { status: 'error', message: 'Internal server error' }, status: :internal_server_error end private def checkout_state_path_for(order, state = order.state) spree.checkout_state_path(order.token, state) end Consistency Several redirects now go through checkout_state_path_for(order, state), but the implementation uses checkout_state_path(order.token, state) while other places use spree.checkout_state_path(...). Confirm both helpers resolve to the same underlying route and behave identically (especially with guest tokens), otherwise redirects could differ between controllers. format.html { redirect_to checkout_state_path_for(@order), error: error_message } format.json { render json: { status: 'error', message: error_message }, status: :unprocessable_entity } end end rescue StandardError => e respond_to do |format| format.html do redirect_to checkout_state_path_for(@order, 'payment'), error: "Payment processing failed: #{e.message}" end format.json do render json: { status: 'error', message: "Payment processing failed: #{e.message}", errors: [e.message] }, status: :unprocessable_entity end end end def generate_ipay_form_html(payment, phone, ipay_method) ipay_method.generate_ipay_form_html(payment, phone) rescue StandardError => e raise "Error generating payment form: #{e.message}" end # Override update action to handle JSON responses def update if @order.update_from_params(params, permitted_checkout_attributes, request.headers.env) respond_to do |format| format.html do if @order.next redirect_to checkout_state_path_for(@order) else redirect_to checkout_state_path_for(@order) end end format.json do if @order.next # Get the next state after the transition next_state = @order.state # Prepare response data response_data = { status: 'success', next_step: next_state, order: { number: @order.number, state: @order.state, total: @order.total.to_f, payment_state: @order.payment_state, shipment_state: @order.shipment_state }, payment_required: @order.payment_required?, checkout_steps: @order.checkout_steps, current_step: next_state, next_step_url: next_step_url_for(@order, next_state) } # Add payment info if in payment state if next_state == 'payment' && @order.payments.any? payment = @order.payments.last response_data[:payment] = { id: payment.id, number: payment.number, state: payment.state, amount: payment.amount.to_f, payment_method_id: payment.payment_method_id, payment_method_type: payment.payment_method&.type } end render json: response_data else render json: { status: 'error', errors: @order.errors.messages, message: @order.errors.full_messages.to_sentence }, status: :unprocessable_entity end end end else respond_to do |format| format.html { render :edit } format.json do render json: { status: 'error', errors: @order.errors.messages, message: @order.errors.full_messages.to_sentence, validation_errors: @order.errors.full_messages }, status: :unprocessable_entity end end end end private def checkout_state_path_for(order, state = order.state) checkout_state_path(order.token, state) end def next_step_url_for(order, next_step) return unless next_step case next_step when 'address' checkout_state_path_for(order, 'address') when 'delivery' checkout_state_path_for(order, 'delivery') when 'payment' checkout_state_path_for(order, 'payment') when 'confirm' checkout_state_path_for(order, 'confirm') when 'complete' Flash key New redirects use error: as a flash key. In Rails defaults, alert:/notice: are standard; error: may not be displayed by existing layouts unless explicitly handled. Validate UI/JSON behavior is still correct and consider standardizing on alert: for errors. format.html { redirect_to checkout_state_path_for(@order), error: error_message } format.json { render json: { status: 'error', message: error_message }, status: :unprocessable_entity } end end rescue StandardError => e respond_to do |format| format.html do redirect_to checkout_state_path_for(@order, 'payment'), error: "Payment processing failed: #{e.message}" end

[qodo-code-review]

Persistent review updated to latest commit 564b334

[deepsource-io]

DeepSource Code Review

We reviewed changes in 83bde25...564b334 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
JavaScript		Apr 16, 2026 10:06a.m.	Review ↗
Ruby		Apr 16, 2026 10:06a.m.	Review ↗

---

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 564b334

Category	Suggestion	Impact
Possible issue	Fix checkout path argument order Fix the argument order in checkout_state_path to pass the state first and the token as a query parameter. app/controllers/spree/api/v1/ipay_controller.rb [223-225] def checkout_state_path_for(order, state = order.state) - spree.checkout_state_path(order.token, state) + spree.checkout_state_path(state, order_token: (order.guest_token || order.token)) end Apply / Chat Suggestion importance[1-10]: 9 __ Why: Identifies a critical routing bug where positional arguments are incorrectly provided, which would cause an exception or generate a broken URL.	High
	Correct token passing in redirects Fix the argument order in checkout_state_path to pass the state first and the token as a query parameter. app/controllers/spree/checkout_controller_decorator.rb [160-162] def checkout_state_path_for(order, state = order.state) - checkout_state_path(order.token, state) + checkout_state_path(state, order_token: (order.guest_token || order.token)) end Apply / Chat Suggestion importance[1-10]: 9 __ Why: Identifies a critical routing bug where positional arguments are incorrectly provided, which would break checkout redirects.	High
	Generate valid checkout return link Fix the argument order in checkout_state_path to pass the state first and the token as a query parameter. app/controllers/spree/gateway_callbacks_controller.rb [123] -esc_payment_path = ERB::Util.html_escape(spree.checkout_state_path(order.token, order.state)) +esc_payment_path = ERB::Util.html_escape( + spree.checkout_state_path(order.state, order_token: (order.guest_token || order.token)) +) Apply / Chat Suggestion importance[1-10]: 9 __ Why: Identifies a critical routing bug where positional arguments are incorrectly provided, which would result in broken continuation links for users.	High
More

---

Previous suggestions

Suggestions up to commit 564b334

Category	Suggestion	Impact
Possible issue	Fix checkout URL generation Update checkout_state_path_for to pass the state as the first positional argument and the order token as a keyword argument. app/controllers/spree/api/v1/ipay_controller.rb [223-225] def checkout_state_path_for(order, state = order.state) - spree.checkout_state_path(order.token, state) + token = order.guest_token || order.token + spree.checkout_state_path(state, order_token: token) end Suggestion importance[1-10]: 9 __ Why: Fixes a critical routing error where passing a string instead of an options hash as the second argument to checkout_state_path will raise an ArgumentError and crash the application.	High
	Correct checkout redirect parameters Correct the checkout_state_path_for method signature by passing the state first and the token via keyword arguments. app/controllers/spree/checkout_controller_decorator.rb [160-162] def checkout_state_path_for(order, state = order.state) - checkout_state_path(order.token, state) + token = order.guest_token || order.token + checkout_state_path(state, order_token: token) end Suggestion importance[1-10]: 9 __ Why: Addresses a critical bug where incorrect positional arguments in checkout_state_path will break the checkout flow and result in server errors.	High
	Fix payment link URL Fix the checkout_state_path invocation by passing the state as the primary argument and the token as a keyword argument. app/controllers/spree/gateway_callbacks_controller.rb [123] -esc_payment_path = ERB::Util.html_escape(spree.checkout_state_path(order.token, order.state)) +esc_payment_path = ERB::Util.html_escape( + spree.checkout_state_path(order.state, order_token: (order.guest_token || order.token)) +) Suggestion importance[1-10]: 9 __ Why: Fixes an error where passing the state as the second argument to checkout_state_path generates an invalid payment link or causes an ArgumentError.	High

[qodo-code-review]

Persistent suggestions updated to latest commit 564b334

[willymwai]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (0)   📎 Requirement gaps (0)

🐞\ ≡ Correctness (1) ⚙ Maintainability (1)

1. Bad checkout URL construction 🐞 ≡

Description

checkout_state_path_for calls checkout_state_path(order.token, state), passing the token as a
positional path segment with no evidence in this repo that checkout_state_path is defined to take
a token. This conflicts with existing iPay flows that use order.guest_token as order_token, so
redirects can land on the wrong checkout state (token treated as state) or produce unusable guest
links.

Code

app/controllers/spree/checkout_controller_decorator.rb[R160-162]

+    def checkout_state_path_for(order, state = order.state)
+      checkout_state_path(order.token, state)
+    end

Relevance

⭐⭐⭐ High

Spree checkout_state_path typically takes state, with order_token as query param; positional
order.token likely breaks redirects.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repo’s Spree engine routes extension does not redefine checkout routes, and all prior/other
usage patterns in this repo pass only a state to checkout_state_path; meanwhile iPay’s existing
guest-access URLs consistently use order.guest_token via the order_token parameter. The new
helper instead uses order.token and passes it positionally, which is inconsistent with the
established URL/token scheme used elsewhere in the same integration.

app/controllers/spree/checkout_controller_decorator.rb[160-162]
app/controllers/spree/api/v1/ipay_controller.rb[223-225]
app/controllers/spree/ipay_controller.rb[34-36]
app/controllers/spree/ipay_controller_decorator.rb[76-78]
app/controllers/spree/gateway_callbacks_controller.rb[114-124]
config/routes.rb[1-23]
app/models/spree/payment_method/ipay.rb[230-237]
app/controllers/spree/api/v1/ipay_controller.rb[166-175]
app/controllers/spree/api/v1/ipay_controller_decorator.rb[118-125]
app/controllers/spree/checkout_controller_decorator.rb[164-178]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`checkout_state_path_for` currently builds checkout URLs as `checkout_state_path(order.token, state)` (and one place uses `spree.checkout_state_path(order.token, order.state)`). In this codebase, the iPay integration uses `order.guest_token` as `order_token` for guest access, and there is no repo-defined checkout route that takes a token as a positional segment.

### Issue Context
Fix should generate checkout URLs by passing the checkout state as the path segment and the guest token as an option (e.g., `order_token:`) consistent with other redirects/return URLs in this repo.

### Fix Focus Areas
- app/controllers/spree/checkout_controller_decorator.rb[160-178]
- app/controllers/spree/ipay_controller.rb[34-36]
- app/controllers/spree/ipay_controller_decorator.rb[76-78]
- app/controllers/spree/api/v1/ipay_controller.rb[223-225]
- app/controllers/spree/gateway_callbacks_controller.rb[120-124]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Helper duplicated across controllers 🐞 ⚙

Description

checkout_state_path_for is redefined in multiple controllers with slightly different
implementations (spree.checkout_state_path vs checkout_state_path). This duplication increases
the chance of inconsistent checkout URL behavior and makes future fixes to the redirect logic easy
to miss.

Code

app/controllers/spree/api/v1/ipay_controller.rb[R223-225]

+        def checkout_state_path_for(order, state = order.state)
+          spree.checkout_state_path(order.token, state)
+        end

Relevance

⭐⭐⭐ High

PR claims “unified helper” but still redefines same method across controllers; likely they’ll want
DRY/concern.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The same helper name is implemented separately in four controllers, and one variant prefixes the
helper with spree. while others do not. This is a direct maintainability risk because any future
change to token/state handling must be applied in multiple places to keep behavior consistent.

app/controllers/spree/api/v1/ipay_controller.rb[223-225]
app/controllers/spree/checkout_controller_decorator.rb[160-162]
app/controllers/spree/ipay_controller.rb[34-36]
app/controllers/spree/ipay_controller_decorator.rb[76-78]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`checkout_state_path_for` is duplicated across multiple controllers, creating a high risk of divergence and missed updates.

### Issue Context
Consolidate the helper into a shared module/concern (e.g., under `app/controllers/concerns/` or `lib/`) and include it where needed, keeping a single implementation for checkout URL generation.

### Fix Focus Areas
- app/controllers/spree/api/v1/ipay_controller.rb[223-225]
- app/controllers/spree/checkout_controller_decorator.rb[160-162]
- app/controllers/spree/ipay_controller.rb[34-36]
- app/controllers/spree/ipay_controller_decorator.rb[76-78]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. Bad checkout url construction 🐞 Bug ≡ Correctness

checkout_state_path_for calls checkout_state_path(order.token, state), passing the token as a
positional path segment with no evidence in this repo that checkout_state_path is defined to take
a token. This conflicts with existing iPay flows that use order.guest_token as order_token, so
redirects can land on the wrong checkout state (token treated as state) or produce unusable guest
links.

Agent Prompt

### Issue description
`checkout_state_path_for` currently builds checkout URLs as `checkout_state_path(order.token, state)` (and one place uses `spree.checkout_state_path(order.token, order.state)`). In this codebase, the iPay integration uses `order.guest_token` as `order_token` for guest access, and there is no repo-defined checkout route that takes a token as a positional segment.

### Issue Context
Fix should generate checkout URLs by passing the checkout state as the path segment and the guest token as an option (e.g., `order_token:`) consistent with other redirects/return URLs in this repo.

### Fix Focus Areas
- app/controllers/spree/checkout_controller_decorator.rb[160-178]
- app/controllers/spree/ipay_controller.rb[34-36]
- app/controllers/spree/ipay_controller_decorator.rb[76-78]
- app/controllers/spree/api/v1/ipay_controller.rb[223-225]
- app/controllers/spree/gateway_callbacks_controller.rb[120-124]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools