Skip to content

Redirect to @record or path in controller generator#569

Merged
dhh merged 1 commit intorails:mainfrom
jeromedalbert:path-in-controller-generator
Sep 14, 2024
Merged

Redirect to @record or path in controller generator#569
dhh merged 1 commit intorails:mainfrom
jeromedalbert:path-in-controller-generator

Conversation

@jeromedalbert
Copy link
Contributor

@jeromedalbert jeromedalbert commented Sep 1, 2024 •
edited
Loading

Partially addresses rails/rails#52756

Problem

There is some discrepancy between Rails controller generators and jbuilder controller generators when it comes to redirects.

Solution

This PR changes the controller generator to generate redirects with redirect_to @post or _path, in order to match the Rails codebase generators as close as possible.

@jeromedalbert
Copy link
Contributor Author

jeromedalbert commented Sep 1, 2024 •
edited
Loading

Tests fail for Rails 5 because singular_route_name is not available in that version. I am wondering if a better fix would be to stop testing against Ruby version 2.7 and Rails 5 since both version are end of life. If so I can update this PR or make a separate one.

@skipkayhil
Copy link
Member

skipkayhil commented Sep 1, 2024 •
edited
Loading

Would redirect_to @post work in this case? I think that's what the Rails' templates are generating now (however, redirect_to @post is still redirecting with a full URL which maybe(?) DHH is wanting to change)

@dhh
Copy link
Member

dhh commented Sep 1, 2024

Yes, let's change to redirect_to @post so it matches the generator in Rails itself. They should be as close as possible.

@jeromedalbert jeromedalbert force-pushed the path-in-controller-generator branch from b684c81 to 598ea5a Compare September 2, 2024 03:51
@jeromedalbert jeromedalbert changed the title Change controller generators to use _path instead of _url for redirects Redirect to @record or path in controller generator Sep 2, 2024
@jeromedalbert jeromedalbert force-pushed the path-in-controller-generator branch 2 times, most recently from a1c5222 to 298056d Compare September 2, 2024 04:08
@jeromedalbert
Copy link
Contributor Author

jeromedalbert commented Sep 2, 2024 •
edited
Loading

PR updated. Tests are failing for Rails 5.0 and 5.1 because redirect_resource_name looks like it was added in Rails 5.2.

I would suggest not testing against these end of life versions any more. Here is a PR to do so: #570.

This was referenced Sep 2, 2024
Merged
Closed
@jeromedalbert jeromedalbert force-pushed the path-in-controller-generator branch from 298056d to 31330d6 Compare September 2, 2024 21:17
@dhh dhh merged commit 8dbc23f into rails:main Sep 14, 2024
@jeromedalbert jeromedalbert deleted the path-in-controller-generator branch September 14, 2024 19:15
derricklannaman referenced this pull request in powerhome/power-web-development-interview Sep 24, 2024
@renovate
Update dependency jbuilder to v2.13.0 (#173)
e1728d1 
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [jbuilder](https://redirect.github.com/rails/jbuilder)
([changelog](https://redirect.github.com/rails/jbuilder/releases/tag/v2.13.0))
| `2.12.0` -> `2.13.0` |
[![age](https://developer.mend.io/api/mc/badges/age/rubygems/jbuilder/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/rubygems/jbuilder/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/rubygems/jbuilder/2.12.0/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/rubygems/jbuilder/2.12.0/2.13.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>rails/jbuilder (jbuilder)</summary>

###
[`v2.13.0`](https://redirect.github.com/rails/jbuilder/releases/tag/v2.13.0)

[Compare
Source](https://redirect.github.com/rails/jbuilder/compare/v2.12.0...v2.13.0)

#### What's Changed

- Redirect to `@record` or path in controller generator by
[@&#8203;jeromedalbert](https://redirect.github.com/jeromedalbert) in
[https://github.com/rails/jbuilder/pull/569](https://redirect.github.com/rails/jbuilder/pull/569)
- Return early from collection partial rendering if blank by
[@&#8203;tylerjc](https://redirect.github.com/tylerjc) in
[https://github.com/rails/jbuilder/pull/560](https://redirect.github.com/rails/jbuilder/pull/560)
- Add missing ':see_other' status code in generated destroy controller
method by [@&#8203;ldeld](https://redirect.github.com/ldeld) in
[https://github.com/rails/jbuilder/pull/538](https://redirect.github.com/rails/jbuilder/pull/538)
- Remove OpenStruct references from Jbuilder by
[@&#8203;mtsmfm](https://redirect.github.com/mtsmfm) in
[https://github.com/rails/jbuilder/pull/567](https://redirect.github.com/rails/jbuilder/pull/567)
- Use new `params.expect` syntax instead of `params.require` by
[@&#8203;jeromedalbert](https://redirect.github.com/jeromedalbert) in
[https://github.com/rails/jbuilder/pull/573](https://redirect.github.com/rails/jbuilder/pull/573)

#### New Contributors

- [@&#8203;jeromedalbert](https://redirect.github.com/jeromedalbert)
made their first contribution in
[https://github.com/rails/jbuilder/pull/570](https://redirect.github.com/rails/jbuilder/pull/570)
- [@&#8203;tylerjc](https://redirect.github.com/tylerjc) made their
first contribution in
[https://github.com/rails/jbuilder/pull/560](https://redirect.github.com/rails/jbuilder/pull/560)
- [@&#8203;ldeld](https://redirect.github.com/ldeld) made their first
contribution in
[https://github.com/rails/jbuilder/pull/538](https://redirect.github.com/rails/jbuilder/pull/538)
- [@&#8203;mtsmfm](https://redirect.github.com/mtsmfm) made their first
contribution in
[https://github.com/rails/jbuilder/pull/567](https://redirect.github.com/rails/jbuilder/pull/567)

**Full Changelog**:
rails/jbuilder@v2.12.0...v2.13.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/powerhome/power-web-development-interview).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC43NC4xIiwidXBkYXRlZEluVmVyIjoiMzguNzQuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This was referenced May 29, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jeromedalbert @skipkayhil @dhh