[COMMS-916] Pin the primary target version convention to the lowest version id - #24486
Conversation
14dc9b3 to
42a5c2f
Compare
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy 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. |
42a5c2f to
56488a4
Compare
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy 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. |
brunopagno
left a comment
There was a problem hiding this comment.
I see the change in meaning here, using min instead of first. I have two thoughts:
- Are we introducing mixed behaviour? As in, are there other places in the codebase that rely on
target_versions.firstwhich we didn't adjust?
I think this is low risk, because even if we are using different approach in different places, all of them should have only a single version associated and consistently return the correct values.
- Introducing the concept of a "primary" target version can be misleading to whoever is maintaining this code in the future. Because we never defined it in documentation, and because essentially there is no such thing as a primary target version.
I think this is low risk as well. What we're calling primary here is more like a single value representation fallback of target versions, but less of a mouthful.
So all good to go from my side 🚢 🚢 🚢
@brunopagno Indeed, "primary" is one word for it- definitely open to other better fitting options. I do think however, that's it warrants an identity as of it's been loosely inferred as the first target versions when the multiple versions feature is OFF. |
The interim primary version is target_versions.first, which reads the lowest version id, while the mirror column followed assignment order. The two diverged whenever versions were assigned out of id order, so legacy version_id readers disagreed with target_versions readers.
target_versions.first is the interim primary version. Unloaded reads already order by id; an explicit order makes preloaded reads agree instead of returning insertion order.
The mirror column now stores the lowest target version id, so the contradiction check must compare against the same value; comparing against the first assigned id rejected consistent writes and let contradicting ones through whenever the order differed.
With leftover multi-target rows and the feature off, the version column picked its representative row by name while the mirror column, API and cost reports name the lowest version id; the same work package grouped under different versions per surface. The groupable subquery needs no versions join once the pick is by id.
56488a4 to
6ab962d
Compare
https://community.openproject.org/wp/COMMS-916
Follows #24432
This is an attempt to tighten deduction of the deprecated version_id primary version from a plain
target_versions.first- which is not a consistent read tomin(target_versions.id)that returns the version with the lowest ID consistently.