[COMMS-1022] Journal work package labels by akabiru · Pull Request #25320 · opf/openproject

akabiru · 2026-09-14T18:12:09Z

Ticket

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

What are you trying to accomplish?

Label changes on a work package were invisible in its history. Each journal now snapshots the work package's label set, so the activity tab renders "Labels: set to A, B" and "Labels: changed from A, B to A", and baseline comparison loads the historic set for a timestamp.

A label that no longer exists renders as "(deleted label)" instead of silently dropping out of the entry.

Screenshots

Label changes in the activity tab

All labels removed

Baseline comparison through the API (the work package table gains a Labels column in a later PR)

What approach did you choose and why?

The snapshot lives in its own table, label_journals, one row per journal and label, the same shape the versions snapshot uses. It keeps no foreign key to labels, so history survives a hard delete; the formatter looks the label up live and falls back to a placeholder, which is how deleted versions and custom options already behave.

Label names in history resolve live, so a renamed label shows its current name in old entries, the same trade-off versions make. Existing journals get no label snapshot: labels are unreleased and behind a feature flag, so no database carries label history from before this table.

AI involvement

Collaborative – AI generated a substantial part of the code; I reviewed and understand every line.

Merge checklist

Added/updated tests
Added/updated documentation in Lookbook (patterns, previews, etc)
Tested major browsers (Chrome, Firefox, Edge, ...)

github-actions · 2026-09-16T13:25:50Z

Warning

Flaky specs

rspec ./modules/wikis/spec/features/admin/internal_provider_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 #25320, linked for reference only):

- `rspec ./modules/wikis/spec/features/admin/internal_provider_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 #25320. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25320 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 · 2026-09-16T17:54:42Z

Note for the admin side of labels (COMMS-1025 / COMMS-1026, PR 2 of the labels stack): deleting a label from Administration → Labels removes its labelings rows directly through Labels::DeleteService and does not create a journal on the affected work packages.

Consequences for this PR's model:

label_journals keeps the deleted label's id (no FK, as intended here), so existing history still renders.
The removal only becomes visible in each work package's next journal, attributed to whoever edits it next, as "Label X removed".
No updated_at bump, no notification at deletion time.

That is acceptable for the first iteration; the epic's journaling wording ("Label {name} removed by {author}") assumes a per-work-package change, and an admin delete is a bulk operation. If a dedicated entry at deletion time turns out to be wanted, the way to do it is a background job iterating the affected work package ids and calling Journals::CreateService with a cause such as label_deleted, which can build on this PR without schema changes.

Flagging it here so the two PRs stay consistent; nothing to change in this one.

akabiru · 2026-09-17T12:47:08Z

Follow-up to the note above, after checking the epic text: admin deletion of a label (COMMS-1026, #25394) is allowed while work packages still carry it and cascades to the labelings. The epic only defines work-package-level entries ("Label {name} added / removed by {author}") and is silent on admin deletion, so #25394 ships without journaling it.

Decided to handle the journaling side here rather than in the admin PR. Two things this model needs to be able to show a deletion in history: a journal on each affected work package at deletion time (a job with the admin as author and a dedicated cause), and the label name surviving the delete, either as a snapshot column on label_journals or by soft-deleting labels. Without the second, the formatter can only render a placeholder for the removed label.

Snapshots the label set per journal so the activity tab renders label changes and baseline comparison loads historic labels. Snapshot rows keep no foreign key to labels so history survives a hard delete. https://community.openproject.org/wp/79963

Keeps the migration ordered after the ones merged since the branch was opened.

A label can be hard-deleted while journals still reference it. Dropping it made the entry read as if the labels had been cleared.

github-actions · 2026-10-21T07:41:12Z

Warning

Flaky specs

rspec ./spec/features/components/lookbook_accessibility_spec.rb[1:14: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 #25320, linked for reference only):

- `rspec ./spec/features/components/lookbook_accessibility_spec.rb[1:14:1]`

Treat this as a standalone task, unrelated to PR #25320. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25320 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.

Copilot

🟡 Changes recommended

Upgrade baselines, activity filtering, timestamped API integration, and snapshot preloading remain incomplete.

5 open issues

What changed in this PR

Adds label snapshots to work package journals, supporting activity history and historic label comparisons.

Changes:

Adds label snapshot storage and journal change detection.
Adds label formatting with deleted-label placeholders.
Loads historic label sets and tests journaling, aggregation, and comparisons.

File	Description
spec/services/journals/create_service/association_spec.rb	Tests label association registration.
spec/models/work_package/work_package_acts_as_journalized_spec.rb	Tests label journaling and aggregation.
spec/models/journal/label_journal_spec.rb	Tests snapshot persistence after deletion.
spec/models/journable/with_historic_attributes_spec.rb	Tests historic labels and comparisons.
spec/lib/open_project/journal_formatter/labels_spec.rb	Tests label history formatting.
spec/factories/journal/label_journal_factory.rb	Adds a label snapshot factory.
lib/open_project/journal_formatter/labels.rb	Formats label changes and deleted labels.
lib/open_project/journal_formatter/joined_versions.rb	Reuses shared association formatting.
lib/open_project/journal_formatter/joined_association.rb	Extracts joined-association formatting.
lib_static/plugins/acts_as_journalized/lib/journal_changes.rb	Computes label-set changes.
db/migrate/20261021090000_create_label_journals.rb	Creates label snapshot storage.
config/locales/en.yml	Adds label translations.
app/services/journals/create_service/labelable.rb	Snapshots labels and detects changes.
app/services/journals/create_service/association.rb	Registers label snapshot handling.
app/models/work_package/journalized.rb	Registers label change formatting.
app/models/journal/label_journal.rb	Defines the label snapshot model.
app/models/journal.rb	Registers label formatting and snapshots.
app/models/journable/with_historic_attributes/loader.rb	Loads historic label associations.
app/models/journable/with_historic_attributes.rb	Compares historic and current labels.

_{Review effort: Balanced}

Existing journals predate the snapshot table. Without a baseline, the next unchanged save of a labelled work package journals a spurious change and a first-edit removal of all labels journals nothing.

Label-only journals were hidden because the filter compared every other snapshot table with the predecessor but not the label snapshots.

Rendering label changes otherwise queries label_journals once per entry and once per predecessor.

The baseline comparison computed the label change but the timestamp representer dropped it, since labels were not among its supported links.

When the last requested timestamp is historic, the work package took its labels from the current record while every other attribute came from the journal.

The historic last-timestamp case was covered; the baseline comparison entry was not.

dev gained a migration with the version 20261021090000 after the rebase, and CI refuses duplicate versions.

Labels are unreleased and behind a feature flag, so no database has label history from before the snapshot table.

github-actions · 2026-10-21T12:53:04Z

Warning

Flaky specs

rspec ./modules/bim/spec/features/viewer/create_viewpoint_spec.rb[1:2:1:1]
rspec ./spec/features/components/lookbook_accessibility_spec.rb[1:14: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 #25320, linked for reference only):

- `rspec ./modules/bim/spec/features/viewer/create_viewpoint_spec.rb[1:2:1:1]`
- `rspec ./spec/features/components/lookbook_accessibility_spec.rb[1:14:1]`

Treat this as a standalone task, unrelated to PR #25320. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25320 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

I think this is good to go. I don't see big risks as most of it is pretty similar to what we've done with target/observed_in versions. But journals is always somewhat energy consuming 😅

Comments are just observations, probably nothing that needs to be handled in this PR

brunopagno · 2026-10-21T13:53:11Z

+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+class OpenProject::JournalFormatter::JoinedAssociation < JournalFormatter::NamedAssociation


brunopagno · 2026-10-21T13:58:35Z

+      override_association(work_package, source, :target_versions)
+      override_association(work_package, source, :labels)


Did we forget to include observed_in_versions here in the past? 👀

Yes, it seems so. Tracked separately as COMMS-1068 - raising a PR shortly

→ [COMMS-1068] Copy historic observed-in versions onto the work package at a past timestamp #26208

brunopagno · 2026-10-21T14:01:34Z

+        .includes(:data, :customizable_journals, :attachable_journals, :target_version_journals, :label_journals,
                  :observed_in_version_journals, :bcf_comment)


This is one of the unnecessariest things ever, but it feels incorrect to have :label between :target_version and :observed_in_version 😅

Some say some distance is healthy in relationships 😄

Also contemplating moving these values into a constant as this list is fairly large

Covers the callback order that lets the snapshot see labels written in the same save, and drops two examples that only restated Rails conventions.

github-actions · 2026-10-21T14:31:20Z

Warning

Flaky specs

rspec ./spec/features/work_packages/table/delete_work_packages_with_descendants_spec.rb[1:2: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 #25320, linked for reference only):

- `rspec ./spec/features/work_packages/table/delete_work_packages_with_descendants_spec.rb[1:2:1]`

Treat this as a standalone task, unrelated to PR #25320. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25320 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 added this pull request to stack #25321 September 14, 2026 18:12

akabiru mentioned this pull request Sep 14, 2026

Feature/COMMS-1001: Labels data model #25319

Merged

akabiru changed the title ~~Journal work package labels~~ Feature/COMMS-1022: Journal work package labels Sep 14, 2026

akabiru self-assigned this Sep 14, 2026

akabiru force-pushed the implementation/comms-1022-labels-journaling branch from c5c66f6 to 4b766c9 Compare September 14, 2026 18:49

Base automatically changed from implementation/comms-1001-labels-data-model to dev September 16, 2026 12:59

akabiru force-pushed the implementation/comms-1022-labels-journaling branch from 4b766c9 to d2db840 Compare September 16, 2026 12:59

akabiru removed this pull request from stack #25321 September 16, 2026 14:13

akabiru mentioned this pull request Sep 16, 2026

Feature/COMMS-1025: Introduce the work package labels feature flag #25393

Merged

akabiru changed the title ~~Feature/COMMS-1022: Journal work package labels~~ [COMMS-1022]: Journal work package labels Sep 16, 2026

akabiru mentioned this pull request Sep 16, 2026

Feature/COMMS-1025: Labels administration #25394

Merged

akabiru mentioned this pull request Sep 29, 2026

Implementation/COMMS-1048: Labels field on the work package view with inline creation #25554

Merged

akabiru added 3 commits October 21, 2026 10:07

Journal work package labels

3cb950a

Snapshots the label set per journal so the activity tab renders label changes and baseline comparison loads historic labels. Snapshot rows keep no foreign key to labels so history survives a hard delete. https://community.openproject.org/wp/79963

Move label journals migration after the current dev migrations

1c544f8

Keeps the migration ordered after the ones merged since the branch was opened.

Render a placeholder for deleted labels in journal entries

963aae4

A label can be hard-deleted while journals still reference it. Dropping it made the entry read as if the labels had been cleared.

akabiru force-pushed the implementation/comms-1022-labels-journaling branch from d2db840 to 963aae4 Compare October 21, 2026 07:27

akabiru marked this pull request as ready for review October 21, 2026 07:27

akabiru mentioned this pull request Oct 21, 2026

[COMMS-1050] Remove unused inline-created labels #26187

Open

1 of 3 tasks

akabiru added this pull request to stack #26188 October 21, 2026 07:35

akabiru mentioned this pull request Oct 21, 2026

[COMMS-1067] Journal admin deletion of a label #26191

Draft

1 of 3 tasks

akabiru removed this pull request from stack #26188 October 21, 2026 08:19

akabiru added this pull request to stack #26192 October 21, 2026 08:19

github-actions Bot added the ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. label Oct 21, 2026

akabiru changed the title ~~[COMMS-1022]: Journal work package labels~~ [COMMS-1022] Journal work package labels Oct 21, 2026

github-actions Bot added ai: Collaborative 💻 AI generated a substantial part of the code; A human reviewed and understands every line. and removed ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. labels Oct 21, 2026

akabiru requested a balanced review from Copilot October 21, 2026 08:37

Copilot started reviewing on behalf of akabiru October 21, 2026 08:37

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

akabiru added 9 commits October 21, 2026 12:06

Snapshot current labels onto the latest journal when adding the table

b25b8e7

Existing journals predate the snapshot table. Without a baseline, the next unchanged save of a labelled work package journals a spurious change and a first-edit removal of all labels journals nothing.

Include label changes in the activity tab's only-changes filter

e916b99

Label-only journals were hidden because the filter compared every other snapshot table with the predecessor but not the label snapshots.

Preload label snapshots with the other journal associations

14fffab

Rendering label changes otherwise queries label_journals once per entry and once per predecessor.

Render historic labels in the timestamp API

6ef5885

The baseline comparison computed the label change but the timestamp representer dropped it, since labels were not among its supported links.

Copy historic labels onto the work package at a past timestamp

a67f730

When the last requested timestamp is historic, the work package took its labels from the current record while every other attribute came from the journal.

Cover the baseline entry's label links in the timestamp API spec

5dd5b34

The historic last-timestamp case was covered; the baseline comparison entry was not.

Trim comments in the label journals migration and paginator spec

1597509

Move the label journals migration past the newest one on dev

60eeef1

dev gained a migration with the version 20261021090000 after the rebase, and CI refuses duplicate versions.

Drop the label snapshot backfill from the migration

604dd85

Labels are unreleased and behind a feature flag, so no database has label history from before the snapshot table.

akabiru requested a review from a team October 21, 2026 13:10

brunopagno approved these changes Oct 21, 2026

View reviewed changes

akabiru added 2 commits October 21, 2026 17:08

Pin label journaling through the label_id_replacements path

5b32c80

Covers the callback order that lets the snapshot see labels written in the same save, and drops two examples that only restated Rails conventions.

Drop a comment that only restated the class name

65b57c6

akabiru merged commit 28b0e16 into dev Oct 21, 2026
17 of 18 checks passed

akabiru deleted the implementation/comms-1022-labels-journaling branch October 21, 2026 15:05

github-actions Bot locked and limited conversation to collaborators Oct 21, 2026

		override_association(work_package, source, :target_versions)
		override_association(work_package, source, :labels)

		.includes(:data, :customizable_journals, :attachable_journals, :target_version_journals, :label_journals,
		:observed_in_version_journals, :bcf_comment)

Conversation

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

Uh oh!

Ticket

What are you trying to accomplish?

Screenshots

What approach did you choose and why?

AI involvement

Merge checklist

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

akabiru commented Sep 16, 2026

Uh oh!

akabiru commented Sep 17, 2026

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

brunopagno left a comment

Choose a reason for hiding this comment

Uh oh!

brunopagno Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

brunopagno Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

akabiru Oct 21, 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!

brunopagno Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

akabiru Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

akabiru Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 21, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

akabiru commented Sep 14, 2026 •

edited

Loading

akabiru Oct 21, 2026 •

edited

Loading