Skip to content

feat(global-edition): prefix generated paths only when not excluded#73444

Merged
artem-vavilov merged 1 commit into
stagingfrom
global/url-generation-improvement
Jun 24, 2026
Merged

feat(global-edition): prefix generated paths only when not excluded#73444
artem-vavilov merged 1 commit into
stagingfrom
global/url-generation-improvement

Conversation

@artem-vavilov

@artem-vavilov artem-vavilov commented Jun 24, 2026

Copy link
Copy Markdown
Member

This is a follow-up to the original Global Edition URL generation patch.

The original change added the active Global Edition path prefix by passing it through Rails :script_name URL options. That worked for normal route helper generation, but it also meant Rails treated the GE prefix as a script name override rather than as part of the final generated path.

This change moves the patch point from ActionDispatch::Routing::RouteSet#url_for to ActionDispatch::Http::URL.path_for, so Rails generates the path first, and then Global Edition applies the current region prefix through Cdo::GlobalEdition.path.

This makes generated paths consistently respect the active Global Edition region while also preserving the existing Global Edition path behavior:

Rails.application.routes.url_helpers.home_path
# => '/home'

Cdo::GlobalEdition.current_region = 'in'
I18n.locale = 'hi-IN'

Rails.application.routes.url_helpers.home_path
# => '/in/hi/home'

Rails.application.routes.url_helpers.home_path(script_name: '/custom_script_name')
# => '/in/hi/custom_script_name/home'

Rails.application.routes.url_helpers.home_path(script_name: '/ua')
# => '/in/hi/home'

Excluded routes always generate prefixless paths:

Cdo::GlobalEdition.current_region = 'in'
I18n.locale = 'hi-IN'

Rails.application.routes.url_helpers.health_check_path
# => '/health_check'

Because the final path now goes through Cdo::GlobalEdition.path, excluded paths are still returned without a GE prefix, and paths that already contain another valid GE region prefix are normalized to the current GE region.

Related PR

chatgpt-codex-connector[bot]

This comment was marked as outdated.

Copilot AI 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.

Pull request overview

Moves the Global Edition (GE) URL generation hook from route helper url_for to Rails’ lower-level ActionDispatch::Http::URL.path_for, so paths are generated normally first and then region/locale prefixes are applied via Cdo::GlobalEdition.path, preserving excluded-path behavior and normalizing existing GE prefixes.

Changes:

  • Replace the RouteSet url_for monkeypatch with a ActionDispatch::Http::URL.path_for monkeypatch.
  • Apply GE prefixing via Cdo::GlobalEdition.path after Rails generates the path.
  • Update integration tests to reflect new :script_name behavior and GE-prefix normalization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
lib/cdo/global_edition.rb Changes the patch point to ActionDispatch::Http::URL.path_for and applies Cdo::GlobalEdition.path to the generated path.
dashboard/test/integration/global_edition/route_prefix_test.rb Updates expectations for :script_name and adds coverage around GE prefix normalization.

Comment thread lib/cdo/global_edition.rb
Comment thread dashboard/test/integration/global_edition/route_prefix_test.rb
@artem-vavilov artem-vavilov force-pushed the global/url-generation-improvement branch from 94b3790 to b2b1906 Compare June 24, 2026 10:41
@artem-vavilov artem-vavilov requested review from a team and wilkie June 24, 2026 10:43
@artem-vavilov artem-vavilov force-pushed the global/url-generation-improvement branch from b2b1906 to 0a9c7ea Compare June 24, 2026 14:01
@artem-vavilov artem-vavilov merged commit dafc6e8 into staging Jun 24, 2026
8 checks passed
@artem-vavilov artem-vavilov deleted the global/url-generation-improvement branch June 24, 2026 19: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.

3 participants