Feature/COMMS-1001: Labels data model

[akabiru]

https://community.openproject.org/wp/79593

First slice of labels for work packages: a global Label whose names are case-insensitively unique across the instance, attached through a polymorphic labelings join so other entities can be labeled later without a schema change.

Labels are hard deletable and deleting one cascades to its labelings. This is the data model only; the API, journaling and admin UI land in their own PRs.

[Copilot]

🟢 Approval recommended

The remaining findings are minor test-coverage nits with no blocking issues identified.

Pull request overview

Adds the foundational global label data model, including case-insensitive unique labels and polymorphic associations for work packages and future entities.

Changes:

• Creates labels and labelings tables with indexes and cascade constraints.
• Adds Label, Labeling, and Labelable models and associations.
• Adds factories and model coverage for validations, ordering, and deletion behavior.

File summaries

File	Reviewed changes
spec/models/work_package/work_package_labels_spec.rb	Tests work package label behavior.
spec/models/labeling_spec.rb	Tests labeling validation behavior.
spec/models/label_spec.rb	Tests label validation and behavior.
spec/factories/labeling_factory.rb	Adds labeling factory.
spec/factories/label_factory.rb	Adds label factory.
db/migrate/20260914120000_create_labels.rb	Creates tables, indexes, and foreign keys. Nit (1 vote): add direct foreign-key cascade coverage. Nit (1 vote): add database-level duplicate-labeling coverage.
app/models/work_package.rb	Enables labels for work packages.
app/models/labeling.rb	Defines the polymorphic join model.
app/models/label.rb	Defines labels, validations, and associations.
app/models/concerns/labelable.rb	Adds labelable associations.

Review details

Suppressed comments (2)

db/migrate/20260914120000_create_labels.rb:40

• This test calls label.destroy!, so Rails' dependent: :delete_all removes the labelings before the database foreign-key cascade is exercised. The migration explicitly promises ON DELETE CASCADE, which is also needed for direct SQL/Label.delete paths; add a regression example that deletes the label without the association callback (or asserts the foreign key's on_delete value), similar to spec/models/work_package_semantic_alias_spec.rb:121-126.

      t.references :label, null: false, foreign_key: { on_delete: :cascade }, index: false

db/migrate/20260914120000_create_labels.rb:46

• The duplicate-labeling example only builds the second record, so it verifies the ActiveRecord validation but never verifies this database unique index. Since the index is the protection against concurrent inserts, add a database-level duplicate insert/save-with-validation-disabled example expecting ActiveRecord::RecordNotUnique, as the label-name index is tested in spec/models/label_spec.rb:53-58.

    add_index :labelings, %i[labelable_type labelable_id label_id],
              unique: true,
              name: "index_labelings_on_labelable_and_label"

• Files reviewed: 10/10 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Warning

Flaky specs

• rspec ./modules/meeting/spec/features/meeting_notifications_spec.rb[1:2:2]
• rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]
• rspec ./spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb[1:1]
• rspec ./spec/features/work_packages/new/attributes_from_filter_spec.rb[1:3:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25319, linked for reference only):

- `rspec ./modules/meeting/spec/features/meeting_notifications_spec.rb[1:2:2]`
- `rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]`
- `rspec ./spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb[1:1]`
- `rspec ./spec/features/work_packages/new/attributes_from_filter_spec.rb[1:3:1]`

Treat this as a standalone task, unrelated to PR #25319. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25319 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

[brunopagno]

This is pretty much it. I left some questions because I don't fully understand some details, but the PR seems in a good shape.

One extra thing though. We will need some way of identifying the author/creator of a label as per specs on the EPIC

An admin-created label has the particularity that, unlike user-created ones, it is visible on the list even if no work packages currently use it. (Else it would disappear on creation)

So we either define it in this step or we will need another migration as a follow up. I am good with either case.

[github-actions]

Warning

Flaky specs

• rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]
• rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]
• rspec ./spec/features/roles/report_spec.rb[1:1]
• rspec ./spec/features/roles/report_spec.rb[1:2]
• rspec ./spec/features/roles/report_spec.rb[1:3]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25319, linked for reference only):

- `rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]`
- `rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]`
- `rspec ./spec/features/roles/report_spec.rb[1:1]`
- `rspec ./spec/features/roles/report_spec.rb[1:2]`
- `rspec ./spec/features/roles/report_spec.rb[1:3]`

Treat this as a standalone task, unrelated to PR #25319. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25319 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

[akabiru]

One extra thing though. We will need some way of identifying the author/creator of a label as per specs on the EPIC

Thanks for the reminder, last I checked we didn't have this spec! 😄

[github-actions]

1 Warning

⚠️

Attention developer & reviewer: Files with potential user references found: db/migrate/20260914120000_create_labels.rb (migration with user reference) Please make sure: You've added proper relationships (has_many, belongs_to, …) and their inverse in your models You deal with model destruction dependencies: dependent: :destroy or dependent: :delete_all You add behavior and tests for the Principal::DeleteJob (app/workers/principals/delete_job.rb) You replace references to users with deleted user in Principal::ReplaceReferencesService (app/services/principals/replace_references_service.rb) by adding to the replacements initailizer (config/initializers/replace_references_service.rb) for the core, or using the replace_principal_references helper in modules/plugins. You test the above behaviors with an integration test (e.g., like this one to confirm deletion of users is possible. This helps prevent dangling database objects when users are deleted and resulting bugs.

Generated by 🚫 Danger

[akabiru]

@brunopagno addressed the following:

• Added generic lookup methods: (a) Labelable.labeled_with(label) (b) Label.with_usage_count - for the admin dash usage counts
• Authorship tracking - no special distinction as to whether user is admin or not; that can be resolved at implementation time
• Archival status archived_at with corresponding scopes active and archived

[brunopagno]

As discussed in sync, we will remove the archive behaviour, but other than that we're good to go, so approving it!

Briliant work! 🚢 🚢 🚢

[github-actions]

Warning

Flaky specs

• rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25319, linked for reference only):

- `rspec ./spec/features/notifications/navigation_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #25319. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25319 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @akabiru to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @akabiru, and request a review from @akabiru.
On every commit, set @akabiru as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.