diff --git a/CHANGELOG.md b/CHANGELOG.md index 025c278..b85d562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Building toward 1.0.0 release. Using 0.x versions during active development. +**Fixed:** +- `PagesController` now reads the page slug from the route's `defaults: {page: ...}` (via `params[:page]`) instead of parsing `request.path`, fixing 404s for root-path pages (`bunko_page :home, path: "/"`) and pages with custom paths (`bunko_page :about, path: "about-us"`) (#57) + ## [0.2.0] - 2025-11-14 First functional release of Bunko - a lightweight Rails CMS based on the "one model, infinite collections" philosophy. diff --git a/CLAUDE.md b/CLAUDE.md index 54c6e7f..9eea729 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -144,6 +144,14 @@ Integration tests run against a minimal Rails app in `test/dummy/`: - Tests use **Minitest** framework - CI runs on GitHub Actions (`.github/workflows/main.yml`) and executes `bundle exec rake` +### Linking Work to GitHub Issues + +When working on a GitHub issue/ticket, always include a closing keyword so the issue closes automatically on merge: + +- Add `Closes #xx` (or `Fixes #xx`) to the **pull request description** — this is what GitHub uses to auto-close the issue when the PR merges +- Also include it in the commit message body when the commit resolves the issue +- Use one line per issue if a change resolves multiple issues (e.g., `Closes #12`, `Closes #34`) + ## Current Features (Implemented) **Milestone 1 - Post Model Behavior:** diff --git a/lib/tasks/templates/controllers/pages_controller.rb.tt b/lib/tasks/templates/controllers/pages_controller.rb.tt index 3772490..f6b280a 100644 --- a/lib/tasks/templates/controllers/pages_controller.rb.tt +++ b/lib/tasks/templates/controllers/pages_controller.rb.tt @@ -2,10 +2,16 @@ class PagesController < ApplicationController def show - # Extract page slug from request path (not user-controllable params) - # This prevents path traversal attacks via query string manipulation - # e.g., GET /about?page=../../admin/users - page_slug = request.path.split("/").reject(&:empty?).last + # The page slug comes from the route's defaults (bunko_page sets + # defaults: {page: ...}). Route defaults are path parameters, which + # take precedence over query string params in Rails, so + # e.g. GET /about?page=../../admin/users cannot override the slug. + page_slug = params[:page].to_s + + # Format guard in case this action is wired up without a :page default + unless page_slug.match?(/\A[a-z0-9-]+\z/) + raise ActiveRecord::RecordNotFound, "Page not found" + end @post = Post.published.find_by( post_type: PostType.find_by(name: "pages"), diff --git a/test/controllers/pages_controller_test.rb b/test/controllers/pages_controller_test.rb index db6f8e4..b329e48 100644 --- a/test/controllers/pages_controller_test.rb +++ b/test/controllers/pages_controller_test.rb @@ -149,4 +149,92 @@ class PagesControllerTest < ActionDispatch::IntegrationTest assert_match "Our privacy policy", response.body assert_no_match "Our terms", response.body end + + # Regression tests for slug derivation (GitHub issue #57) + # The slug must come from the route's defaults, not from request.path + + test "bunko_page with root path renders the home page" do + Post.create!( + title: "Home", + slug: "home", + content: "Welcome to the home page", + post_type: @pages_type, + status: "published", + published_at: 1.day.ago + ) + + draw_dummy_routes_with do + bunko_page :home, path: "/" + end + + get "/" + assert_response :success + assert_match "Welcome to the home page", response.body + ensure + Rails.application.reload_routes! + end + + test "bunko_page with custom path finds post by page name, not path" do + Post.create!( + title: "About", + slug: "about", + content: "All about the company", + post_type: @pages_type, + status: "published", + published_at: 1.day.ago + ) + + draw_dummy_routes_with do + bunko_page :about, path: "company-info" + end + + get "/company-info" + assert_response :success + assert_match "All about the company", response.body + ensure + Rails.application.reload_routes! + end + + test "query param cannot override route default on custom-path page" do + Post.create!( + title: "About", + slug: "about", + content: "All about the company", + post_type: @pages_type, + status: "published", + published_at: 1.day.ago + ) + + draw_dummy_routes_with do + bunko_page :about, path: "about-us" + end + + get "/about-us?page=privacy-policy" + assert_response :success + # Route default (page: "about") wins over the query string param + assert_match "All about the company", response.body + assert_no_match "Our privacy policy", response.body + ensure + Rails.application.reload_routes! + end + + private + + # Redraws the dummy app's routes with extra routes prepended, so tests can + # exercise bunko_page options (root path, custom paths) the committed routes + # file doesn't use. The shared nav partial links to every collection, so the + # full set from test/dummy/config/routes.rb must stay present. + def draw_dummy_routes_with(&extra) + Rails.application.routes.draw do + instance_exec(&extra) + + root "blog#index" + bunko_collection :blog + bunko_collection :docs + bunko_collection :articles + bunko_collection :videos + bunko_collection :long_reads + bunko_collection :all_content + end + end end diff --git a/test/dummy/app/controllers/legal/pages_controller.rb b/test/dummy/app/controllers/legal/pages_controller.rb index 33584a3..57570c9 100644 --- a/test/dummy/app/controllers/legal/pages_controller.rb +++ b/test/dummy/app/controllers/legal/pages_controller.rb @@ -3,10 +3,17 @@ module Legal class PagesController < ApplicationController def show - # Extract page slug from request path (not user-controllable params) - # This prevents path traversal attacks via query string manipulation - # e.g., GET /legal/privacy-policy?page=../../admin/users - page_slug = request.path.split("/").reject(&:empty?).last + # The page slug comes from the route's defaults (bunko_page sets + # defaults: {page: ...}). Route defaults are path parameters, which + # take precedence over query string params in Rails, so + # e.g. GET /legal/privacy-policy?page=../../admin/users cannot + # override the slug. + page_slug = params[:page].to_s + + # Format guard in case this action is wired up without a :page default + unless page_slug.match?(/\A[a-z0-9-]+\z/) + raise ActiveRecord::RecordNotFound, "Page not found" + end @post = Post.published.find_by( post_type: PostType.find_by(name: "pages"),