Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: [Auto Routing Improved] one controller method has more than one URI when $translateURIDashes is true #7422

Merged
merged 10 commits into from
Jul 3, 2023
Merged

fix: [Auto Routing Improved] one controller method has more than one URI when $translateURIDashes is true #7422

merged 10 commits into from
Jul 3, 2023
+239 −18

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Apr 12, 2023 •
edited
Loading

Needs #7406

Description
When $translateURIDashes is true, two URIs correspond to a single controller method,
one URI for dashes (foo-bar) and one URI for underscores (foo_bar).
This is incorrect behavior, contrary to the design philosophy.

This PR prohibits the URI for underscores (foo_bar) when $translateURIDashes is true.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added bug Verified issues on the current code behavior or pull requests that will fix them 4.4 labels Apr 12, 2023
@kenjis kenjis marked this pull request as draft April 12, 2023 03:03
@kenjis kenjis force-pushed the fix-auto-routing-improved-setTranslateURIDashes-4.4 branch from 2eb3948 to fc5be36 Compare April 12, 2023 07:35
@kenjis kenjis changed the title fix: [Auto Routing Improved] one controller has two URIs when $translateURIDashes is true fix: [Auto Routing Improved] one controller method has two URIs when $translateURIDashes is true Apr 12, 2023
@kenjis kenjis changed the title fix: [Auto Routing Improved] one controller method has two URIs when $translateURIDashes is true fix: [Auto Routing Improved] one controller method has more than one URI when $translateURIDashes is true Apr 12, 2023
@kenjis kenjis force-pushed the fix-auto-routing-improved-setTranslateURIDashes-4.4 branch from fc5be36 to bdc10f1 Compare April 13, 2023 11:34
@kenjis kenjis force-pushed the fix-auto-routing-improved-setTranslateURIDashes-4.4 branch from bdc10f1 to 28350f3 Compare June 2, 2023 08:38
@kenjis kenjis marked this pull request as ready for review June 2, 2023 08:58
@kenjis
Copy link
Member Author

kenjis commented Jun 2, 2023

Rebased and added docs.

@kenjis kenjis force-pushed the fix-auto-routing-improved-setTranslateURIDashes-4.4 branch from 88a8e2e to 78b450c Compare June 14, 2023 01:41
@kenjis
Copy link
Member Author

kenjis commented Jun 14, 2023

Rebased to resolve conflicts.

MGatner
MGatner approved these changes Jul 3, 2023
View reviewed changes
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Very nice testing solution. Really glad you made this class final to begin with! I'd like to make most service classes final in v5

@kenjis kenjis merged commit 8e08820 into codeigniter4:4.4 Jul 3, 2023
@kenjis kenjis deleted the fix-auto-routing-improved-setTranslateURIDashes-4.4 branch July 3, 2023 23:05
@kenjis kenjis mentioned this pull request Dec 11, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MGatner MGatner MGatner approved these changes

Assignees
No one assigned
Labels
4.4 bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kenjis @MGatner