Skip to content

[COMMS-934] Adjust WorkPackageAtTimestampRepresenter with target_versions - #24642

Merged
akabiru merged 9 commits into
devfrom
implementation/comms-934-adjust-workpackageattimerepresenter-with-target_verisons
Aug 7, 2026
Merged

[COMMS-934] Adjust WorkPackageAtTimestampRepresenter with target_versions#24642
akabiru merged 9 commits into
devfrom
implementation/comms-934-adjust-workpackageattimerepresenter-with-target_verisons

Conversation

@akabiru

@akabiru akabiru commented Aug 6, 2026

Copy link
Copy Markdown
Member

https://community.openproject.org/wp/COMMS-934

Introduce targetVersions to WorkPackageAtTimestampRepresenter which is primarily used for the Baseline comparison feature. It only adds a multi-value targetVersions node and retains the backwards compatible single-value versions as per agreed convention to avoid breaking existing consumers.

📸 Baseline "since yesterday" with a single version swap (Sprint 1 to Sprint 2) and a multiple target version addition (Sprint 1 to Sprint 1 + Sprint 2):

baseline_target_versions

@akabiru akabiru changed the title Implementation/COMMS-934: Adjust WorkPackageAtTimestampRepresenter with target_versions [COMMS-934] Adjust WorkPackageAtTimestampRepresenter with target_versions Aug 6, 2026
@akabiru
akabiru requested a balanced review from Copilot August 6, 2026 16:53
@akabiru akabiru self-assigned this Aug 6, 2026
@akabiru akabiru added this to the 17.8.x milestone Aug 6, 2026

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/work_packages/table/queries/parent_field_ranking_spec.rb[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 #24642, linked for reference only):

- `rspec ./spec/features/work_packages/table/queries/parent_field_ranking_spec.rb[1:1]`

Treat this as a standalone task, unrelated to PR #24642. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24642 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
akabiru marked this pull request as ready for review August 7, 2026 09:58
akabiru added 9 commits August 7, 2026 13:04
Historic work packages now load target_versions from the journal
snapshots, and baseline change detection derives version changes from
the target version sets instead of the deprecated version_id mirror.
The attributesByTimestamp entries now include the targetVersions links
when the target version set changed between the compared timestamps.
When the only requested timestamp is historic, the work package renders
with its attributes of that time; the target_versions association now
follows along instead of exposing the current join rows.
Version snapshots have no foreign key so they outlive deleted versions;
resolving those rows yielded nils that crashed the association loading.
The nil-vs-empty-hash sentinel conflated class capability with per-record
emptiness; the guard now runs once per load instead of per record.
Change detection was only tested in the additive direction; add removal
of a secondary version, removal of all versions, and the empty-set
targetVersions render.
The rename in at_timestamp put the method on dirty-rubocop's radar; the
attribute hash assembly moves to its own helper.
The primary-version notion is only inferred during the migration
period; describe scenarios via the single-value version field instead.
@akabiru
akabiru force-pushed the implementation/comms-934-adjust-workpackageattimerepresenter-with-target_verisons branch from fac1a39 to eb7e12a Compare August 7, 2026 10:04
@akabiru
akabiru requested review from a team and brunopagno August 7, 2026 10:06
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/work_packages/table/queries/parent_field_ranking_spec.rb[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 #24642, linked for reference only):

- `rspec ./spec/features/work_packages/table/queries/parent_field_ranking_spec.rb[1:1]`

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

I did not test locally, but the the code looks great. Not a big change. Great job keeping it concise. 🎉

Comment on lines +122 to +134
def historic_attributes(journal, timestamp)
attributes = journal.data.attributes.merge(
"id" => id,
"created_at" => created_at,
"updated_at" => journal.updated_at,
"timestamp" => timestamp,
"journal_id" => journal.id
)
self.class.column_names_missing_in_journal.each do |missing_column_name|
attributes[missing_column_name] = nil
end
attributes
end

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.

❤️

@akabiru
akabiru merged commit 36d730d into dev Aug 7, 2026
14 checks passed
@akabiru
akabiru deleted the implementation/comms-934-adjust-workpackageattimerepresenter-with-target_verisons branch August 7, 2026 14:34
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants