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

Remove 4 unroutable routes from ActionMailbox #45230

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

natematykiewicz
Copy link
Contributor

@natematykiewicz natematykiewicz commented May 31, 2022

Summary

I was auditing which routes in my app do not actually a have corresponding controller action. ActionMailbox's inbound_emails has these 4 routes defined, which do not actually work.

                            Prefix Verb   URI Pattern                                                       Controller#Action
edit_rails_conductor_inbound_email GET    /rails/conductor/action_mailbox/inbound_emails/:id/edit(.:format) rails/conductor/action_mailbox/inbound_emails#edit
                                   PATCH  /rails/conductor/action_mailbox/inbound_emails/:id(.:format)      rails/conductor/action_mailbox/inbound_emails#update
                                   PUT    /rails/conductor/action_mailbox/inbound_emails/:id(.:format)      rails/conductor/action_mailbox/inbound_emails#update
                                   DELETE /rails/conductor/action_mailbox/inbound_emails/:id(.:format)      rails/conductor/action_mailbox/inbound_emails#destroy

Other Information

Here's the corresponding controller, where you can see that only 4 actions are defined, not 7.
https://github.com/rails/rails/blob/ef52dd906ed4ef9f1578119e27b712d11e68009d/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb

I was auditing which routes in my app do not actually a
have corresponding controller action. ActionMailbox's
inbound_mails has these 4 routes defined, which do not
actually work.

GET /rails/conductor/action_mailbox/inbound_emails/:id/edit
PATCH /rails/conductor/action_mailbox/inbound_emails/:id
PUT /rails/conductor/action_mailbox/inbound_emails/:id
DELETE /rails/conductor/action_mailbox/inbound_emails/:id
@natematykiewicz natematykiewicz force-pushed the actionmailbox_unroutable_routes branch from cd44860 to c9e48fe Compare May 31, 2022 20:38
@etiennebarrie
Copy link
Contributor

Maybe we could also set format: false to all these routes since the controllers don't use the format? But that might be better in another PR.

👍 cc @byroot

@byroot byroot merged commit 920fcce into rails:main Jun 1, 2022
@byroot
Copy link
Member

byroot commented Jun 1, 2022

Thanks.

@natematykiewicz natematykiewicz deleted the actionmailbox_unroutable_routes branch June 1, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants