Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approved
Static review found no blocking issues in this focused removal; regression coverage is added, but tests were not executed.
0 open findings
What changed in this PR
Removes obsolete standalone creation forms while preserving the existing book-control creation flow.
Changes:
- Removes the shared
newaction, routes, and three templates. - Adds regression tests for retired URLs and picture creation.
- Documents the supported creation flow and HTTP 404 behavior.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| test/controllers/sections_controller_test.rb | Tests retired URL returns 404. |
| test/controllers/pictures_controller_test.rb | Tests retired URL and direct creation. |
| test/controllers/pages_controller_test.rb | Tests retired URL returns 404. |
| README.md | Documents supported creation flow. |
| config/routes.rb | Removes three new routes. |
| app/views/sections/new.html.erb | Deletes obsolete section form. |
| app/views/pictures/new.html.erb | Deletes obsolete picture form. |
| app/views/pages/new.html.erb | Deletes obsolete page form. |
| app/controllers/leafables_controller.rb | Removes unused new action. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The book controls create a Leaf and its content record directly. They do not use the separate
newpages.The old routes remained after the UI changed. On the checked branch base (
057f098), all three creation pages failed. The action assigned@leafable, but the templates expected@page,@picture, or@section. The shared forms also required an existing Leaf and submitted PUT requests.This PR removes the broken flow.
History
Dates below are the upstream PR merge dates.
newroutes with forms that create pages directly from the type picker.4774da5book_part_create_button. The PR describes click-to-create as existing behavior.a6710d0LeafablesController. Thenewaction used@leafable, while the templates retained their named variables.0f45743leafable_path(record.leaf), which requires an existing Leaf.20a6da8PR #56 states the intended flow:
Compatibility
The former
/books/:book_id/pages/new,/books/:book_id/pictures/new, and/books/:book_id/sections/newURLs return HTTP 404.The history shows that the current UI does not require these URLs.