Skip to content

feat: logical return_to redirects post user action (#121)#130

Open
trixy-the-ai-bot wants to merge 11 commits into
mainfrom
feature/logical-redirects-121
Open

feat: logical return_to redirects post user action (#121)#130
trixy-the-ai-bot wants to merge 11 commits into
mainfrom
feature/logical-redirects-121

Conversation

@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator

Summary

Closes #121

  • safe_return_to helper added to ApplicationController — validates that return_to URLs are local paths only (rejects any URL with a host component), preventing open redirect attacks
  • PaymentsController#create and InvoicesController#create now redirect to params[:return_to] on success, falling back to their previous defaults (payments_path and invoice_path(@invoice) respectively)
  • Hidden return_to field added to payment and invoice forms so the param survives the POST request
  • Action links updated in leases#show, payments#index, and invoices#index to pass return_to: request.fullpath — Renew and New Lease links are intentionally left unchanged (natural redirect exceptions)
  • back button added before the page title on leases, properties, tenants, and owners show pages as an escape hatch for natural redirect flows

Test plan

  • POST payments#create with return_to: lease_path → redirects to lease
  • POST payments#create without return_to → redirects to payments_path (unchanged default)
  • POST invoices#create with return_to: lease_path → redirects to lease
  • POST invoices#create without return_to → redirects to invoice_path (unchanged default)
  • POST with return_to: "http://evil.com" → redirects to fallback, not external URL
  • CI passes all existing specs

🤖 Generated with Claude Code

Claude Agent and others added 2 commits April 6, 2026 17:45
- Add `safe_return_to` helper to ApplicationController (validates local-only URLs)
- PaymentsController#create and InvoicesController#create redirect to `return_to` param on success, falling back to their default paths
- Add `return_to` hidden field to payment and invoice forms so the param survives POST
- Thread `return_to: request.fullpath` through all action-initiating links in leases#show, payments#index, and invoices#index
- Add `‹` back button before the page title on leases, properties, tenants, and owners show pages
- Add request specs covering: return_to redirect, default fallback, and external URL rejection (security)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot self-assigned this Apr 6, 2026
@trixy-the-ai-bot trixy-the-ai-bot requested a review from TMaYaD April 6, 2026 15:07
@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
…llers

- Add empty line after guard clause in safe_return_to (Layout/EmptyLineAfterGuardClause)
- Extract respond_to_create private method to reduce create method length and ABC complexity (Metrics/MethodLength, Metrics/AbcSize)
- Fix line length by extracting redirect_path local variable (Layout/LineLength)
- Fix :unprocessable_entity → :unprocessable_content in JSON error response (Rails/HttpStatusNameConsistency)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

CI failures addressed.

Fixed five RuboCop violations across application_controller.rb and invoices_controller.rb. The main refactor extracted the respond_to block from create into a private respond_to_create method, resolving both the method length and ABC size violations. Three smaller fixes addressed a missing blank line after a guard clause, a long line via a local variable extraction, and an HTTP status symbol rename from :unprocessable_entity to :unprocessable_content.

@TMaYaD
Copy link
Copy Markdown
Member

TMaYaD commented Apr 6, 2026

A JS based < button to query browser history to find the last natural position might be a better implementation

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
Extract failure-format handling into respond_to_create_failure to bring
respond_to_create within Metrics/AbcSize (≤17) and Metrics/MethodLength (≤10).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

CI failures addressed.

The respond_to_create method exceeded Rubocop's line count (11 vs max 10) and ABC size (19.47 vs max 17) limits. The fix inlined a redirect_path local variable and extracted the failure-handling branch into a new private method respond_to_create_failure. The parent method is now 8 lines with a lower ABC score, and the extracted helper is a simple 3-line method.

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
Swap `link_to "‹", :back` (HTTP Referer-based) for a Stimulus `back`
controller that calls `history.back()`, giving a more reliable UX by
querying the browser's own history stack.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

Review comments addressed.

A new Stimulus controller (back_controller.js) was added to handle browser back navigation using history.back(), replacing the Rails link_to :back pattern in the 4 show views (leases, owners, properties, tenants). This switch from Referer-based navigation to browser history stack is more reliable since the Referer header can be absent or stripped by privacy settings.

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

CI failures addressed.

The lint failure was a Layout/LineLength violation in invoices_controller.rb where a redirect_to call exceeded the 120-character limit. The fix split the redirect_to and its notice: argument across two lines to comply with the limit, then pushed the change.

@TMaYaD
Copy link
Copy Markdown
Member

TMaYaD commented Apr 6, 2026

By history I meant go back in history until you hit a navigational check point. Not just the previous, which might be a form.

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
Rubocop Style/BlockDelimiters and Layout violations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

CI failures addressed.

Fixed a Rubocop violation in invoices_controller.rb:55-56 where a multi-line format.html block incorrectly used {...} syntax. Converting it to do...end with proper indentation resolved three offenses: Style/BlockDelimiters, Layout/MultilineBlockLayout, and Layout/BlockEndNewline. The fix was pushed.

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
Replace simple history.back() with a checkpoint-aware back navigation.
A new history-tracker controller (mounted on <body>) records each page
visit in sessionStorage, but only pushes "checkpoint" pages – index and
show pages. New/edit form pages are excluded via URL pattern matching.

The back controller now walks the checkpoint stack to find the last entry
that differs from the current page, navigating directly there. This
means clicking ‹ on a show page always lands on a natural navigation
point, never an intermediate form, even if the user arrived via a
create/update flow. Falls back to window.history.back() when the stack
is empty (e.g., fresh session).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

Review comments addressed.

A new history_tracker_controller.js was added to the app layout to track navigation history in sessionStorage, recording only "checkpoint" pages (skipping /new and /edit URLs). The existing back_controller.js was updated to use this stack instead of history.back(), walking backwards to find the last non-current checkpoint page. This ensures the back button always navigates to a natural landing point (index or show page) rather than a form, even after a multi-step create/update flow.

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
…Length

Extract redirect URL to a local variable to collapse the multi-line
redirect_to call and bring the method within the 10-line limit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

CI failures addressed.

The fix refactored respond_to_create by extracting the safe_return_to(...) call into a local variable url, which allowed the multi-line format.html do...end block to be collapsed into a single-line format.html { }. This reduced the method from 11 lines to 9, bringing it within the 10-line limit.

@TMaYaD
Copy link
Copy Markdown
Member

TMaYaD commented Apr 6, 2026

param and hidden field based return to is no longer needed.

@trixy-the-ai-bot trixy-the-ai-bot added in-progress Bot is actively working on this and removed in-progress Bot is actively working on this labels Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

Failed to address review comments: Agent exited with code 1

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
The browser history-based back navigation (history_tracker_controller +
back_controller) replaces the need for return_to URL params and hidden
form fields. Removed safe_return_to helper, all return_to param passing
in links, hidden_field_tag in forms, and related specs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

Review comments addressed.

The safe_return_to helper and all associated return_to redirect logic was removed from the app. Controllers now redirect directly to fixed paths (invoice_path / payments_path) instead of dynamically following a return_to param, and views no longer pass or store that param in forms or links. Specs were updated to drop the return_to-specific test cases while keeping coverage of the default redirect behavior.

@trixy-the-ai-bot trixy-the-ai-bot added the in-progress Bot is actively working on this label Apr 6, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@trixy-the-ai-bot trixy-the-ai-bot removed the in-progress Bot is actively working on this label Apr 6, 2026
@trixy-the-ai-bot
Copy link
Copy Markdown
Collaborator Author

CI failures addressed.

Two RuboCop layout violations were fixed: a trailing blank line inside the class body in application_controller.rb, and a trailing blank line inside a context block in payments_spec.rb. Both violations were of the Layout/EmptyLines* type.

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.

Logical redirects post user action.

2 participants