kasugaijin / baja-pet-rescue

Kasugaijin's open-source Ruby on Rails production app that enables dog rescue organisation staff in Mexico to post dogs and receive applications for adoption from users in USA and Canada..
27 stars 9 forks source link

Run traceroute to look for any dead routes or unused controller actions #157

Closed kasugaijin closed 1 year ago

kasugaijin commented 1 year ago

Per this article best practice is to ensure controller methods are exposed only as needed. There should be no public controller method that does not support a route.

This ticket is to perform the exercise of looking for dead routes and/or inappropriately exposed controller methods. This can be done by hand or using a gem like traceroute.

Overlandcoder commented 1 year ago

This sounds interesting, I'd love to take this on.

kasugaijin commented 1 year ago

All yours @Overlandcoder Apologies for the slow response. My github notifications go to a peripheral inbox that I don't check daily.

Overlandcoder commented 1 year ago

@kasugaijin Here are the 197 unreachable action methods

Unreachable action methods (197): application#dog_in_same_organization? application#check_if_adopter application#verified_staff application#adopter_with_profile active_storage/blobs/proxy#send_stream active_storage/representations/proxy#send_stream successes#dog_in_same_organization? successes#check_if_adopter successes#verified_staff successes#adopter_with_profile static_pages#contact_us static_pages#dog_in_same_organization? static_pages#check_if_adopter static_pages#verified_staff static_pages#adopter_with_profile profile_reviews#dog_in_same_organization? profile_reviews#check_if_adopter profile_reviews#verified_staff profile_reviews#adopter_with_profile organization_dogs#dog_in_same_organization? organization_dogs#check_if_adopter organization_dogs#verified_staff organization_dogs#adopter_with_profile errors#dog_in_same_organization? errors#check_if_adopter errors#verified_staff errors#adopter_with_profile donations#dog_in_same_organization? donations#check_if_adopter donations#verified_staff donations#adopter_with_profile contacts#dog_in_same_organization? contacts#check_if_adopter contacts#verified_staff contacts#adopter_with_profile attachments#dog_in_same_organization? attachments#check_if_adopter attachments#verified_staff attachments#adopter_with_profile adoptions#dog_in_same_organization? adoptions#check_if_adopter adoptions#verified_staff adoptions#adopter_with_profile adoption_application_reviews#dog_in_same_organization? adoption_application_reviews#check_if_adopter adoption_application_reviews#verified_staff adoption_application_reviews#adopter_with_profile adopter_profiles#dog_in_same_organization? adopter_profiles#check_if_adopter adopter_profiles#verified_staff adopter_profiles#adopter_with_profile adopter_applications#dog_in_same_organization? adopter_applications#check_if_adopter adopter_applications#verified_staff adopter_applications#adopter_with_profile adoptable_dogs#dog_in_same_organization? adoptable_dogs#check_if_adopter adoptable_dogs#verified_staff adoptable_dogs#adopter_with_profile inherited_resources/base#index inherited_resources/base#show inherited_resources/base#edit inherited_resources/base#destroy inherited_resources/base#create inherited_resources/base#update inherited_resources/base#new inherited_resources/base#dog_in_same_organization? inherited_resources/base#check_if_adopter inherited_resources/base#verified_staff inherited_resources/base#adopter_with_profile devise#dog_in_same_organization? devise#check_if_adopter devise#verified_staff devise#adopter_with_profile devise#_prefixes active_admin/base#index active_admin/base#show active_admin/base#edit active_admin/base#destroy active_admin/base#create active_admin/base#update active_admin/base#new active_admin/base#dog_in_same_organization? active_admin/base#check_if_adopter active_admin/base#verified_staff active_admin/base#adopter_with_profile active_admin/page#clear_page_actions! active_admin/page#index active_admin/page#dog_in_same_organization? active_admin/page#check_if_adopter active_admin/page#verified_staff active_admin/page#adopter_with_profile active_admin/resource#batch_action active_admin/resource#index active_admin/resource#show active_admin/resource#edit active_admin/resource#destroy active_admin/resource#create active_admin/resource#update active_admin/resource#new active_admin/resource#dog_in_same_organization? active_admin/resource#check_if_adopter active_admin/resource#verified_staff active_admin/resource#adopter_with_profile admin/dashboard#clear_page_actions! admin/dashboard#dog_in_same_organization? admin/dashboard#check_if_adopter admin/dashboard#verified_staff admin/dashboard#adopter_with_profile admin/comments#scoped_collection admin/comments#batch_action admin/comments#dog_in_same_organization? admin/comments#check_if_adopter admin/comments#verified_staff admin/comments#adopter_with_profile admin/staff_accounts#dog_in_same_organization? admin/staff_accounts#check_if_adopter admin/staff_accounts#verified_staff admin/staff_accounts#adopter_with_profile admin/admin_users#dog_in_same_organization? admin/admin_users#check_if_adopter admin/admin_users#verified_staff admin/admin_users#adopter_with_profile devise/omniauth_callbacks#passthru devise/omniauth_callbacks#failure devise/omniauth_callbacks#dog_in_same_organization? devise/omniauth_callbacks#check_if_adopter devise/omniauth_callbacks#verified_staff devise/omniauth_callbacks#adopter_with_profile devise/confirmations#show devise/confirmations#create devise/confirmations#new devise/confirmations#dog_in_same_organization? devise/confirmations#check_if_adopter devise/confirmations#verified_staff devise/confirmations#adopter_with_profile devise/registrations#cancel devise/registrations#edit devise/registrations#destroy devise/registrations#create devise/registrations#update devise/registrations#new devise/registrations#dog_in_same_organization? devise/registrations#check_if_adopter devise/registrations#verified_staff devise/registrations#adopter_with_profile devise/unlocks#show devise/unlocks#create devise/unlocks#new devise/unlocks#dog_in_same_organization? devise/unlocks#check_if_adopter devise/unlocks#verified_staff devise/unlocks#adopter_with_profile devise/passwords#dog_in_same_organization? devise/passwords#check_if_adopter devise/passwords#verified_staff devise/passwords#adopter_with_profile devise/sessions#dog_in_same_organization? devise/sessions#check_if_adopter devise/sessions#verified_staff devise/sessions#adopter_with_profile active_admin/devise/confirmations#show active_admin/devise/confirmations#create active_admin/devise/confirmations#new active_admin/devise/confirmations#dog_in_same_organization? active_admin/devise/confirmations#check_if_adopter active_admin/devise/confirmations#verified_staff active_admin/devise/confirmations#adopter_with_profile registrations#dog_in_same_organization? registrations#check_if_adopter registrations#verified_staff registrations#adopter_with_profile active_admin/devise/registrations#cancel active_admin/devise/registrations#edit active_admin/devise/registrations#destroy active_admin/devise/registrations#create active_admin/devise/registrations#update active_admin/devise/registrations#new active_admin/devise/registrations#dog_in_same_organization? active_admin/devise/registrations#check_if_adopter active_admin/devise/registrations#verified_staff active_admin/devise/registrations#adopter_with_profile active_admin/devise/unlocks#show active_admin/devise/unlocks#create active_admin/devise/unlocks#new active_admin/devise/unlocks#dog_in_same_organization? active_admin/devise/unlocks#check_if_adopter active_admin/devise/unlocks#verified_staff active_admin/devise/unlocks#adopter_with_profile active_admin/devise/passwords#dog_in_same_organization? active_admin/devise/passwords#check_if_adopter active_admin/devise/passwords#verified_staff active_admin/devise/passwords#adopter_with_profile active_admin/devise/sessions#dog_in_same_organization? active_admin/devise/sessions#check_if_adopter active_admin/devise/sessions#verified_staff active_admin/devise/sessions#adopter_with_profile

kasugaijin commented 1 year ago

Thanks @Overlandcoder Phew that's a lot more than I was expecting! However, most of them are related to gems, so I think I'll focus on the application code we wrote first. On that, most of the methods in that are tied to the application controller methods. I will work my way through this list, and feel free to also review and comment here.

Application Controller

Overlandcoder commented 1 year ago

You're right, after making Application Controller's methods private, unused methods dropped from 197 to 57. And after deleting static_pages#contact_us, we're left with 56, and all tests pass. rake traceroute now gives us:

Unused routes (4): rails/conductor/action_mailbox/inbound_emails#edit rails/conductor/action_mailbox/inbound_emails#update rails/conductor/action_mailbox/inbound_emails#update rails/conductor/action_mailbox/inbound_emails#destroy

Unreachable action methods (56): active_storage/blobs/proxy#send_stream active_storage/representations/proxy#send_stream inherited_resources/base#index inherited_resources/base#show inherited_resources/base#edit inherited_resources/base#destroy inherited_resources/base#create inherited_resources/base#update inherited_resources/base#new devise#_prefixes active_admin/base#index active_admin/base#show active_admin/base#edit active_admin/base#destroy active_admin/base#create active_admin/base#update active_admin/base#new active_admin/page#clear_page_actions! active_admin/page#index active_admin/resource#batch_action active_admin/resource#index active_admin/resource#show active_admin/resource#edit active_admin/resource#destroy active_admin/resource#create active_admin/resource#update active_admin/resource#new admin/dashboard#clear_page_actions! admin/comments#scoped_collection admin/comments#batch_action devise/omniauth_callbacks#failure devise/omniauth_callbacks#passthru devise/confirmations#show devise/confirmations#create devise/confirmations#new devise/registrations#cancel devise/registrations#edit devise/registrations#destroy devise/registrations#create devise/registrations#update devise/registrations#new devise/unlocks#show devise/unlocks#create devise/unlocks#new active_admin/devise/confirmations#show active_admin/devise/confirmations#create active_admin/devise/confirmations#new active_admin/devise/registrations#cancel active_admin/devise/registrations#edit active_admin/devise/registrations#destroy active_admin/devise/registrations#create active_admin/devise/registrations#update active_admin/devise/registrations#new active_admin/devise/unlocks#show active_admin/devise/unlocks#create active_admin/devise/unlocks#new

kasugaijin commented 1 year ago

Thanks for updating the list.

The rest of these actions are related to gems, and come built in. As such, I am inclined to leave as is.

Leaving the unused actions as-is does not cause any immediate issues. They are there to provide a foundation for future functionality, and if we eventually decide to utilize them, they will be readily available. Keeping them allows for easier extension and customization without having to modify the library's code directly.

I am open to suggestions/discussion though.

As is, this issue has provided a good chunk of value by identifying the public application controller methods, which was definitely a good catch.

Overlandcoder commented 1 year ago

@kasugaijin I agree with leaving them as is. I believe that wraps up this issue so I'll go ahead and make a PR.

kasugaijin commented 1 year ago

Thank you, sounds good.