Skip to content

Conversation

hunchr
Copy link
Member

@hunchr hunchr commented Jun 27, 2025

  • a separate route was generated for each model/sheet. this can be simplified using a scope.
  • add home controller, so that not everything is in sheets

@hunchr hunchr self-assigned this Jun 27, 2025
@hunchr hunchr requested a review from a team as a code owner June 27, 2025 09:25
@hunchr hunchr added the feature label Jun 27, 2025
Copy link
Member

@sislr sislr left a comment

Choose a reason for hiding this comment

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

Nice refactoring!

Comment on lines -6 to 8
Hotsheet.sheets.each_key do |sheet_name|
resources sheet_name, sheet_name:, controller: :sheets, only: %i[index update]
scope ":sheet_name" do
resources controller: :sheets, only: %i[index update], as: :sheets
end
Copy link
Member

Choose a reason for hiding this comment

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

nice refactoring!

Hotsheet.sheets.each_key do |sheet_name|
resources sheet_name, sheet_name:, controller: :sheets, only: %i[index update]
scope ":sheet_name" do
resources controller: :sheets, only: %i[index update], as: :sheets
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
resources controller: :sheets, only: %i[index update], as: :sheets
resources :sheets, only: %i[index update]

Copy link
Member Author

Choose a reason for hiding this comment

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

i tried this too, but unfortunately it doesn't work (/sheets/):

Routes for Hotsheet::Engine:
  root GET   /                                 hotsheet/home#show
sheets GET   /:sheet_name/sheets(.:format)     hotsheet/sheets#index
 sheet PUT /:sheet_name/sheets/:id(.:format) hotsheet/sheets#update

@hunchr hunchr merged commit 474301b into main Jun 27, 2025
2 checks passed
@hunchr hunchr deleted the feature/routes branch June 27, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants