Skip to content

url-rewriting - #31

Closed
ljonesfl wants to merge 3 commits into
developfrom
feature/url-rewriting
Closed

ljonesfl wants to merge 3 commits into
developfrom
feature/url-rewriting

Conversation

@ljonesfl

@ljonesfl ljonesfl commented Jan 12, 2026 •

Copy link
Copy Markdown
Member

Note

Introduces centralized routing configuration and cleans up legacy routing.

  • Replaces config/routes.yaml with config/routing.yaml defining rewrites and controller_paths; publishes template in resources/config/routing.yaml
  • Updates installer to create/recognize routing.yaml and adjusts messages; removes legacy routing block from neuron.yaml.example
  • Expands README with attribute-based routing docs, examples, and rewrite behavior
  • Adds unit tests for routing behavior: duplicate route detection, config precedence/empty handling, and URL rewrite log sanitization
  • Removes legacy Home controller and its unit tests

Written by Cursor Bugbot for commit bd351cb. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Refactor

    • Transitioned routing configuration from file-based route definitions to attribute-based routing for improved flexibility and maintainability.
    • Reorganized configuration structure to separate routing settings into a dedicated configuration file.
  • Chores

    • Updated documentation and installer to reflect routing configuration changes.
    • Removed deprecated routing artifacts.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 12, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The changes migrate from a YAML-based route configuration system to attribute-based routing with centralized controller discovery. The legacy config/routes.yaml is removed and replaced with config/routing.yaml that enables automatic controller path discovery. The Home controller and associated tests are deleted, and documentation is updated to reflect the new routing approach.

Changes

Cohort / File(s) Summary
Route Configuration Migration
config/routes.yaml, config/routing.yaml
Deleted comprehensive YAML route definitions (auth, admin, blog, pages, members, events); introduced new routing.yaml with URL rewrites and controller path discovery pointing to CMS controllers namespace.
Configuration & Installation
resources/config/neuron.yaml.example, resources/config/routing.yaml, src/Cms/Cli/Commands/Install/InstallCommand.php
Extracted routing config from neuron.yaml.example into dedicated routing.yaml; updated install command to reference and copy routing.yaml instead of routes.yaml.
Controller Removal
src/Cms/Controllers/Home.php, tests/Unit/Cms/Controllers/HomeTest.php
Deleted Home controller class with index route handler and its corresponding PHPUnit test suite (constructor, rendering, and registration flag tests).
Documentation Updates
readme.md
Updated references from "routes.yaml" to "routing.yaml"; replaced explicit route listing with narrative on attribute-based routing; removed RSS feed reference; updated installation and project structure descriptions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Whiskers twitch with routing delight,
YAML routes fade into the night,
Attributes bloom where controllers play,
Discovery paths light up the way,
A cleaner hop through code so bright!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'url-rewriting' is vague and doesn't clearly convey the main purpose of the changeset, which involves migrating from config/routes.yaml to config/routing.yaml and adopting attribute-based routing with transparent URL rewrites. Consider a more descriptive title such as 'Migrate to attribute-based routing with config/routing.yaml' or 'Implement URL rewriting and refactor routing configuration' to better communicate the scope and intent of the changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jan 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/Cms/Cli/Commands/Install/InstallCommand.php 0.00% 8 Missing ⚠️
Flag Coverage Δ Complexity Δ
cms 73.55% <0.00%> (+0.06%) 2096.00 <0.00> (-3.00) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ Complexity Δ
src/Cms/Cli/Commands/Install/InstallCommand.php 26.58% <0.00%> (+0.33%) 188.00 <0.00> (-1.00) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ljonesfl

Copy link
Copy Markdown
Member Author

@CodeRabbit please review

@coderabbitai

coderabbitai Bot commented Jan 12, 2026

Copy link
Copy Markdown
Contributor

@ljonesfl, I'll review the changes in this PR now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread src/Cms/Cli/Commands/Install/InstallCommand.php

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Cms/Cli/Commands/Install/InstallCommand.php (1)

1634-1634: Stale reference to routes.yaml in installation summary.

Line 1634 still references config/routes.yaml which has been renamed to config/routing.yaml. This will confuse users following the installation instructions.

Proposed fix
-		$this->output->writeln( "  2. Review config/routes.yaml for admin routes" );
+		$this->output->writeln( "  2. Review config/routing.yaml for routing configuration" );
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbd7c4e and 441222f.

📒 Files selected for processing (8)
  • config/routes.yaml
  • config/routing.yaml
  • readme.md
  • resources/config/neuron.yaml.example
  • resources/config/routing.yaml
  • src/Cms/Cli/Commands/Install/InstallCommand.php
  • src/Cms/Controllers/Home.php
  • tests/Unit/Cms/Controllers/HomeTest.php
💤 Files with no reviewable changes (3)
  • tests/Unit/Cms/Controllers/HomeTest.php
  • config/routes.yaml
  • src/Cms/Controllers/Home.php
🔇 Additional comments (6)
config/routing.yaml (1)

1-13: LGTM!

The routing configuration is well-structured for CMS package development. The distinction between this file (using src/Cms/Controllers for local development) and resources/config/routing.yaml (using vendor/neuron-php/cms/... for end-users) is appropriate.

resources/config/neuron.yaml.example (1)

9-10: LGTM!

Clean separation of routing configuration into a dedicated file with appropriate reference comments.

resources/config/routing.yaml (1)

1-31: LGTM!

The routing configuration template is well-documented with clear guidance on URL rewrites and controller path ordering. The example showing how to add custom controllers before CMS controllers is particularly helpful for users who want to override default routes.

src/Cms/Cli/Commands/Install/InstallCommand.php (1)

375-398: LGTM!

The routing configuration logic is correctly updated to use routing.yaml. File path, variable names, and user-facing messages are all consistent with the new naming convention.

readme.md (2)

107-107: LGTM!

The references to configuration files are correctly updated to reflect the new routing.yaml naming convention.

Also applies to: 139-139, 227-227


372-439: Excellent documentation for the new routing system.

The routing section is well-structured with:

  • Clear explanation of URL rewrites and their transparent behavior
  • Practical YAML configuration example
  • Complete PHP controller example for custom landing pages
  • Comprehensive list of available routes

This significantly improves the developer experience for customizing the homepage and understanding the routing architecture.

Comment thread resources/config/routing.yaml
@ljonesfl ljonesfl closed this Jan 12, 2026
@ljonesfl
ljonesfl deleted the feature/url-rewriting branch January 12, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant