[#71630] POC: leverage friendly_id :history for persistent project identifier URIs by akabiru · Pull Request #22295 · opf/openproject

akabiru · 2026-03-10T18:40:00Z

Ticket

What are you trying to accomplish?

Project identifiers can change. When they do, all previously bookmarked or indexed URLs break with a 404. This PR makes old project identifier URLs redirect permanently (301) to the current identifier, and permanently reserves retired identifiers so no other project can claim them.

Behaviour after this PR:

GET /projects/old-identifier/... → 301 to GET /projects/current-identifier/...
A retired identifier can never be claimed by another project
A project can revert to an identifier it previously held
Numeric ID access (/projects/123/...) is unaffected

What approach did you choose and why?

We leverage the friendly_id gem (already a dependency at ~> 5.6) with its :history extension. Enabling :history on the Project model causes the gem to automatically record every previous identifier value in the friendly_id_slugs table whenever a project is saved with a new identifier.

Historical slugs are used for redirect detection - the actual database lookup still uses the current identifier. If the param is a historical slug, ApplicationController#redirect_if_historical_project_identifier issues a 301 to the same URL with the current identifier substituted in.

Security boundary: redirects are only issued after Project.visible confirms the project exists and is accessible to the current user. Methods that run before authorization deliberately do not redirect, to avoid disclosing that a
historical identifier exists to users who would otherwise be denied access.

Merge checklist

Added/updated unit tests (spec/models/project_spec.rb)
Added redirect coverage in controller specs (shared example: "redirects GET requests using a historical project identifier")
Added/updated documentation or Lookbook previews (no UI changes)
Tested in major browsers
No new N+1 queries introduced

Copilot

Pull request overview

This PR makes project identifier URLs stable across identifier changes by enabling FriendlyId :history for Project, issuing permanent (301) redirects from historical identifiers to the current identifier, and preventing re-use of retired identifiers by other projects.

Changes:

Enable FriendlyId :history on Project and add validation to reserve historical identifiers.
Add ApplicationController#redirect_if_historical_project_identifier and apply it across controllers after project lookup.
Add migrations + specs to create/populate friendly_id_slugs, plus controller/model/service specs for redirects and reservation behavior.

Reviewed changes

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

Show a summary per file

File	Description
spec/support/shared/controllers/stale_project_identifier_redirect.rb	Adds shared examples to assert 301 redirects for stale project identifiers in controller specs
spec/services/projects/set_attributes_service_integration_spec.rb	Ensures service rejects creating a project with an identifier retired by another project
spec/models/project_spec.rb	Adds model coverage for identifier uniqueness vs historical slugs and FriendlyId history behavior
spec/migrations/initialize_historic_identifiers_spec.rb	Adds migration spec coverage for backfilling `friendly_id_slugs` for existing projects
spec/controllers/wiki_controller_spec.rb	Adds redirect coverage for wiki routes accessed via historical identifiers
spec/controllers/versions_controller_spec.rb	Adds redirect coverage for versions routes accessed via historical identifiers
spec/controllers/projects_controller_spec.rb	Adds redirect coverage for copy form when accessed via historical identifier
spec/controllers/projects/identifier_controller_spec.rb	Adds redirect coverage for identifier controller show (and ensures numeric id does not redirect)
spec/controllers/messages_controller_spec.rb	Adds redirect coverage for message/forum routes using historical identifiers
spec/controllers/categories_controller_spec.rb	Adds redirect coverage for category routes using historical identifiers
modules/costs/app/controllers/hourly_rates_controller.rb	Redirects requests using a historical `:project_id` after project lookup
modules/costs/app/controllers/costlog_controller.rb	Redirects requests using a historical `:project_id` after project lookup
modules/backlogs/lib/open_project/backlogs/patches/versions_controller_patch.rb	Applies historical identifier redirect to patched versions controller logic
modules/backlogs/app/controllers/rb_sprints_controller.rb	Redirects requests using a historical `:project_id` after project lookup
modules/backlogs/app/controllers/rb_application_controller.rb	Redirects requests using a historical `:project_id` early in backlogs controller base
db/migrate/20260312121938_initialize_historic_identifiers.rb	Backfills `friendly_id_slugs` with current project identifiers
db/migrate/20260311000000_create_friendly_id_slugs.rb	Introduces the `friendly_id_slugs` table and indexes
app/models/project.rb	Enables FriendlyId history on `identifier` and prevents re-use of retired identifiers
app/controllers/wiki_controller.rb	Redirects stale identifier access before proceeding with wiki lookup
app/controllers/versions_controller.rb	Redirects stale identifier access after project lookup
app/controllers/users/invite_controller.rb	Redirects stale identifier access when invite dialog is scoped to a project
app/controllers/projects_controller.rb	Redirects stale identifier access for actions that find projects without `visible` scope
app/controllers/projects/archive_controller.rb	Redirects stale identifier access for archived/unarchive flows
app/controllers/news/comments_controller.rb	Redirects stale identifier access before loading project news
app/controllers/messages_controller.rb	Redirects stale identifier access before loading forum/messages
app/controllers/categories_controller.rb	Redirects stale identifier access after project lookup
app/controllers/application_controller.rb	Adds shared redirect helper and integrates it into standard project-finding helpers

db/migrate/20260311000000_create_friendly_id_slugs.rb

app/controllers/application_controller.rb

lib/api/helpers/historical_identifier_redirect.rb

…entifier URIs Proof of concept that friendly_id ~> 5.6.0 (already a dependency) can satisfy the requirements of #71630 with minimal changes: - Enable the :history extension on Project (slug_column: :identifier) so old identifiers are recorded in friendly_id_slugs on every save - Add redirect_if_historical_project_identifier in ApplicationController: any GET request using a stale identifier receives a 301 to the current one - Old identifiers are permanently tied to their original project via sluggable_id — they cannot be claimed by another project Key constraints discovered: - acts_as_customizable#method_missing intercepts FriendlyId module-defined methods (friendly_id_status, slug); use @project.identifier directly - FriendlyId's idiomatic redirect pattern (params[:id] != @post.slug) maps cleanly to params[key] != @project.identifier with slug_column: :identifier Resources: * https://norman.github.io/friendly_id/file.Guide.html * https://norman.github.io/friendly_id/FriendlyId/Slugged.html * https://norman.github.io/friendly_id/FriendlyId/History.html

…h numeric IDs Numeric project IDs (e.g. /projects/123/...) resolve via PK lookup; the previous logic compared the raw param against project.identifier, causing an unintended 301 redirect whenever a project was accessed by ID rather than identifier string. Added the /\A\d+\z/ guard so only non-numeric params that differ from the current identifier (i.e. historical slugs recorded in friendly_id_slugs) trigger a redirect. Updated comment to accurately describe the three-case logic.

…ifier on validation failure FriendlyId::Slugged adds after_validation :unset_slug_if_invalid to revert a generated slug when validation fails. With slug_column: :identifier this reverts the identifier to its prior value — nil for new records — when e.g. a uniqueness error fires. Since the identifier is managed by acts_as_url and user input rather than FriendlyId's slug generator, override the method as a no-op on Project. Also fixes wrong route helper (copy_form_project_path → copy_project_path) in projects_controller_spec.

…cal reservation identifier_not_historically_reserved was firing alongside the uniqueness validation for identifiers still actively held by another project, producing two "Identifier has already been taken." errors. Fixed by excluding projects whose current identifier matches — those are already covered by uniqueness. Moved the custom validation adjacent to the uniqueness check with a comment block describing both responsibilities: - uniqueness → "currently owned by another project" - not_historically_reserved → "retired but still reserved via friendly_id_slugs" Replaced the TODO spec with a concrete test: retired identifiers remain reserved when a different project tries to claim them.

Add two examples to `describe "identifier history"` confirming that `identifier_not_historically_reserved` correctly excludes the project's own slug history, so a project can safely change back to an identifier it previously held.

Co-authored-by: Judith Roth <j.roth@openproject.com>

Extends the historical-identifier redirect to every controller method that loads a project from URL params, matching the coverage of PR 22187. Previously only ApplicationController#find_project and The following methods now also redirect GET requests that arrive with a retired identifier: Core controllers: - ApplicationController#find_optional_project - CategoriesController#find_project - VersionsController#find_project - WikiController#find_wiki - MessagesController#find_project_and_forum - News::CommentsController#find_news_and_project - Users::InviteController#set_project - ProjectsController#find_project_including_archived - Projects::ArchiveController#find_project_including_archived Module controllers (backlogs, costs): - RbApplicationController#load_sprint_and_project - RbSprintsController#load_project - Backlogs::VersionsControllerPatch#override_project_from_id - HourlyRatesController#find_project - CostlogController#find_cost_entry_work_package_or_project - Costs::TimeEntriesController#load_and_authorize_optional_project Intentionally excluded: authorization concern (load_and_authorize_in_optional_project runs before auth and uses Project.find without .visible, so redirecting there would disclose retired identifiers to unauthorised users). Adds a shared example "redirects GET requests using a historical project identifier" for DRY controller specs, with coverage for: categories#new, versions#index, wiki#index, messages#new. Co-authored-by: Judith Roth <j.roth@openproject.com>

Replace the manual regex `/\A\d+\z/` in redirect_if_historical_project_identifier with FriendlyId's own `String#friendly_id?` helper, which uses the faster to_i.to_s != to_s comparison. Keeps the check consistent with the gem's own finder logic. Co-authored-by: Judith Roth <j.roth@openproject.com>

…ional_project This method runs before authorization. Redirecting here would disclose a historical identifier (and the current one) to users who would otherwise be denied access — same reason load_and_authorize_in_optional_project in accounts/authorization.rb was excluded from the redirect surface.

…ata from projects https://community.openproject.org/wp/72917

https://community.openproject.org/wp/71630 This one is an interesting case. The API documentation says an identifier should be allowed as ID param here: https://github.com/opf/openproject/blob/dev/docs/api/bcf/bcf-rest-api.md#41-project-services However, the regex prevented that. I removed the regex to be able to add the identifier redirect (and specs for it). I fear that if I don't fix that now, eventually someone else will find this some day, remove the constraint on the ID but will not add the redirect.

Suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

lib/api/v3/work_packages/schema/work_package_schemas_api.rb

modules/bim/lib/api/bim/bcf_xml/v1/bcf_xml_api.rb

judithroth · 2026-03-18T10:39:43Z

lib/api/v3/projects/available_parents_api.rb

          after_validation do
            authorize_globally(:add_project) do
              authorize_in_any_project(%i[add_subprojects edit_project])
            end
+
+            # Handle redirect for historical identifier in :of query param
+            if params[:of]
+              @of_project = Project.find(params[:of])
+              redirect_if_historical_identifier(:of, @of_project)
+            end


We should look into this, probably not using visible was not intentional but we should check.

app/controllers/application_controller.rb

+      # Reconstruct the URL from path + query parameters, and prevent user-supplied
+      # options such as :host or :protocol from influencing the redirect target.
+      safe_path_params  = request.path_parameters.symbolize_keys
+      safe_query_params = request.query_parameters.symbolize_keys
+
+      # Ensure the identifier in the path matches the current project identifier.
+      safe_path_params[identifier_param_key] = @project.identifier
+
+      # Remove any URL option keys that could affect the redirect target.
+      safe_query_params.except!(:host, :protocol, :subdomain, :domain, :port)
+
+      redirect_to url_for(safe_path_params.merge(safe_query_params).merge(only_path: true)),


spec/support/shared/controllers/historic_project_identifier_redirect.rb

+#   it_behaves_like "redirects GET requests using a historical project identifier",
+#                   :index, { project_id: :injected_by_shared_example }
+#
+#   # With extra required params (e.g. forum_id):
+#   it_behaves_like "redirects GET requests using a historical project identifier",
+#                   :index, { forum_id: -> { forum.id } }
+#


lib/api/helpers/historical_identifier_redirect.rb

+            %r{(/)#{Regexp.escape(param_value)}(/|$)},
+            "\\1#{project.identifier}\\2"
+          )
+


thykel

Wooo, coming up nicely 🚀

app/controllers/users/invite_controller.rb

app/controllers/application_controller.rb

app/controllers/categories_controller.rb

app/models/project.rb

thykel · 2026-03-17T16:37:45Z

lib/api/helpers/historical_identifier_redirect.rb

+      #       redirect_if_historical_identifier(:id, @project)
+      #     end
+      #   end
+      def redirect_if_historical_identifier(identifier_param, project)


@judithroth This helper is currently rather raw because Grape doesn't have the same URL construction capabilities as Rails, right?

I might have some good news -- it looks like we have a helper that allows us to construct URLs neatly within Grape as well. 🙂 => quick example here

Lemme know what you think -- at the very least, it looks like it's making tests pass.

I like that solution very much - thanks for suggesting it!

thykel · 2026-03-17T16:46:05Z

app/controllers/application_controller.rb

+    if params[:project_id].present?
+      @project = Project.visible.find(params[:project_id])
+      redirect_if_historical_project_identifier(:project_id)
+      nil if performed?


Suggested change

nil if performed?

nil

Buuut it looks like before_action does not care about the return value anyway, so we could also drop the entire line -- unless it is here for a reason.

Good catch. I thought this was for before_action.
When I try to see this from an point of view not knowing much about project identifiers: My expectation when reading find_optional_project and find_project (etc) would be that they return a project (or nil if it can't be found).
We added the redirect, which was convenient for us but from a code clarity perspective should probably be its own before_action 😕

We decided to have a separate before_action for redirects.

thykel · 2026-03-17T16:48:27Z

lib/api/helpers/historical_identifier_redirect.rb

+              /(\A|&)#{Regexp.escape(identifier_param.to_s)}=#{Regexp.escape(param_value)}(&|\z)/,
+              "\\1#{identifier_param}=#{CGI.escape(project.identifier)}\\2"


I'm gonna admit I'm not bright enough to grasp this on first read. 😶‍🌫️

Suggestion: Could we either unroll this into more lines with comments to make it more descriptive (AI is good at that), or just replace the regex approach entirely with the suggestion I linked above?

I also found the regex based replacement quite difficult to parse- did some AI assisted digging and it seems we might get away with using structured ruby objects:

def redirect_if_historical_identifier(identifier_param, project) param_value = params[identifier_param] return unless request.get? && param_value.friendly_id? && param_value != project.identifier segments = request.path.split("/") index = segments.index(param_value) segments[index] = project.identifier if index new_path = segments.join("/") if request.query_string.present? query_hash = Rack::Utils.parse_query(request.query_string) query_hash[identifier_param.to_s] = project.identifier new_path += "?#{Rack::Utils.build_query(query_hash)}" end redirect new_path, permanent: true end

thykel · 2026-03-17T17:14:57Z

app/controllers/application_controller.rb

  def redirect_if_historical_project_identifier(identifier_param_key)
    param = params[identifier_param_key]
-    if request.get? && request.format.html? && param.friendly_id? && param != @project.identifier
+    if request.get? && request.format.symbol == :html && param.friendly_id? && param != @project.identifier


actionpack detected turbo_stream requests as HTML because the helper is surprisingly vague:

(turbo_stream content type is text/vnd.turbo-stream.html)

Co-authored-by: Tom Hykel <mail@thykel.cz>

app/controllers/versions_controller.rb

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

judithroth · 2026-03-18T10:52:29Z

spec/support/shared/controllers/historic_project_identifier_redirect.rb

+
+  it "redirects to the same action with the current identifier (301)" do
+    resolved = extra_params.transform_values { |v| v.respond_to?(:call) ? instance_exec(&v) : v }
+    get action, params: { project_id: old_identifier }.merge(resolved)


Adjusting the comment could resolve the issue Copilot mentioned.

akabiru · 2026-03-20T09:47:58Z

POC complete 👍🏾 - handled separately in #22397 and subsequent PRs.

akabiru self-assigned this Mar 10, 2026

akabiru added DO NOT MERGE experiment labels Mar 10, 2026

akabiru mentioned this pull request Mar 11, 2026

Feature/71630 prepare project identifiers for semantic work package ids new #22187

Closed

1 task

akabiru removed their assignment Mar 11, 2026

judithroth removed DO NOT MERGE experiment labels Mar 12, 2026

akabiru changed the title ~~[#71630] POC: leverage friendly_id :history for persistent project identifier URIs~~ [#71630] leverage friendly_id :history for persistent project identifier URIs Mar 12, 2026

akabiru assigned judithroth and akabiru Mar 12, 2026

akabiru added the feature label Mar 12, 2026

akabiru added this to the 17.3.x milestone Mar 12, 2026

akabiru requested a review from Copilot March 12, 2026 14:08

Copilot started reviewing on behalf of akabiru March 12, 2026 14:09 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

db/migrate/20260311000000_create_friendly_id_slugs.rb Outdated Show resolved Hide resolved

app/controllers/application_controller.rb Outdated Show resolved Hide resolved

judithroth force-pushed the chore/test-friendly branch from a36186a to 5d7d4f1 Compare March 13, 2026 09:13

judithroth reviewed Mar 13, 2026

View reviewed changes

lib/api/helpers/historical_identifier_redirect.rb Outdated Show resolved Hide resolved

akabiru and others added 13 commits March 16, 2026 16:56

Validate for historic identifiers

01cedde

Fix spec

7c7aa05

WIP: Temporarily remove failing spec to get a feature spec run

a40e47e

Use error exclusion instead of query as the safer method

db8ade3

Co-authored-by: Judith Roth <j.roth@openproject.com>

[#71630] Add migration to initialize friendly_id_slugs with current d…

39c6ff7

…ata from projects https://community.openproject.org/wp/72917

judithroth and others added 8 commits March 16, 2026 17:03

[#71630] Refine wording

1713db5

[#71630] Redirect historic identifiers in the API

51d5ec6

[#71630] Add even more redirects

c42a6d5

[#71630] Rename file for consistency

50b2fdf

[#71630] Ensure redirects without security issues

391e9ec

Suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

[#71630] Use bigint for ID column in friendly_id_slugs

b69328b

Suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

[#71630] Ensure redirects in the API without security issues

f4d42dc

judithroth force-pushed the chore/test-friendly branch from 67dbb67 to f4d42dc Compare March 16, 2026 16:03

judithroth requested a review from thykel March 16, 2026 16:05

akabiru requested a review from Copilot March 17, 2026 04:37

Copilot started reviewing on behalf of akabiru March 17, 2026 04:37 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

thykel reviewed Mar 17, 2026

View reviewed changes

thykel added 4 commits March 17, 2026 17:58

fix: properly detect HTML content type in historic identifier specs

c9fb8f8

fix: extend the path replacement regex to include identifier dashes

e6e67d8

fix: add a missing feature flag setting for rb sprints controller test

8413818

refactor: Appease the Rubocop complexity scanner

bf4fe67

thykel reviewed Mar 17, 2026

View reviewed changes

judithroth and others added 2 commits March 18, 2026 11:21

Apply suggestion from @thykel

c9da7de

Co-authored-by: Tom Hykel <mail@thykel.cz>

Apply suggestion from @thykel

3a8c642

Co-authored-by: Tom Hykel <mail@thykel.cz>

judithroth reviewed Mar 18, 2026

View reviewed changes

app/controllers/versions_controller.rb Show resolved Hide resolved

judithroth and others added 2 commits March 18, 2026 11:44

Potential fix for pull request finding

44f00d8

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

babdc34

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

judithroth reviewed Mar 18, 2026

View reviewed changes

akabiru changed the title ~~[#71630] leverage friendly_id :history for persistent project identifier URIs~~ [#71630] POC: leverage friendly_id :history for persistent project identifier URIs Mar 18, 2026

akabiru closed this Mar 20, 2026

github-actions bot locked and limited conversation to collaborators Mar 20, 2026

akabiru deleted the chore/test-friendly branch March 20, 2026 09:48

		/(\A\|&)#{Regexp.escape(identifier_param.to_s)}=#{Regexp.escape(param_value)}(&\|\z)/,
		"\\1#{identifier_param}=#{CGI.escape(project.identifier)}\\2"

Conversation

akabiru commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

What approach did you choose and why?

Merge checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

judithroth Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

thykel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thykel Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

judithroth Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

judithroth Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

judithroth Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

thykel Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akabiru Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thykel Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

judithroth Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akabiru commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

akabiru commented Mar 10, 2026 •

edited

Loading

thykel Mar 17, 2026 •

edited

Loading

thykel Mar 17, 2026 •

edited

Loading

akabiru Mar 18, 2026 •

edited

Loading

judithroth Mar 18, 2026 •

edited

Loading