Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set request full path with request url #1893

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

nachiket87
Copy link
Contributor

@nachiket87 nachiket87 commented Nov 1, 2023

What are you trying to accomplish?

Fixing #1867

What approach did you choose and why?

Issue may be a Rails issue here: https://github.com/rails/rails/blob/8e73e620b1072c831b81604449385dfbb60bdb63/actionpack/lib/action_dispatch/http/request.rb#L262

Where @fullpath is cached from previous request. To get around this, I have manually set the values of @fullpath and @original_fullpath in the test helper.

A similar issue was fixed here: 4241a14

@nachiket87 nachiket87 force-pushed the fix-request-fullpath branch 3 times, most recently from 344d6e4 to f9e2d4b Compare November 1, 2023 19:09
@nachiket87 nachiket87 force-pushed the fix-request-fullpath branch from f9e2d4b to 96a8b71 Compare November 1, 2023 19:12
@nachiket87 nachiket87 changed the title set request full path manually for tests with request url Set request full path manually for tests with request url Nov 1, 2023
@nachiket87 nachiket87 force-pushed the fix-request-fullpath branch 3 times, most recently from e78c014 to 7bffc84 Compare November 1, 2023 19:16
@nachiket87 nachiket87 force-pushed the fix-request-fullpath branch from 7bffc84 to 65b63c4 Compare November 1, 2023 19:17
@nachiket87 nachiket87 changed the title Set request full path manually for tests with request url Set request full path with request url Nov 1, 2023
Copy link
Contributor

@camertron camertron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you @nachiket87 😄 I'm fine merging this, but I would heartily recommend submitting a patch upstream as well.

@camertron camertron merged commit 9d1ecad into ViewComponent:main Nov 1, 2023
23 checks passed
claudiob pushed a commit to claudiob/view_component that referenced this pull request Dec 22, 2023
claudiob pushed a commit to claudiob/view_component that referenced this pull request Jan 3, 2024
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.

2 participants