Bug/57550 custom field with format version are ordered as strings
Ticket
What are you trying to accomplish?
Order versions as versions instead of as strings
What approach did you choose and why?
Using collation that treats runs of digits as one number rather than as separate digits.
Questionable:
- Creating collation with
IF NOT EXISTSto reduce small chance of conflict. Also not dropping it in down direction - I'm not sure if keeping
order_by_semver_nameis meaningful - It is also now using
orderinstead ofreorder, no complaints from tests about that
Merge checklist
- [x] Added/updated tests
- [ ] Added/updated documentation in Lookbook (patterns, previews, etc)
- [ ] Tested major browsers (Chrome, Firefox, Edge, ...)
Currently there is a discrepancy between ordering a work package list by a version custom field as opposed to ordering it by the hard wired version property. For the later, the null values are always placed last. I propose to remove this in the same PR, or at least to do it right after. It would only require to remove the
null_handlingstatement defined for version inproperty_select.rb.
Related discussion about ordering of absent values for custom fields. What about start date and due date columns? The PR #8000 and OP#32156 don't describe the reason for adding null_handling that puts empty values always at the end for 3 properties.
The other thing I would include in this PR is the cleanup of the
Version.sort_by_namescope which I believe has no more merit.
You mean Version.order_by_name (or Version.order_by_semver_name)?
Just saw that Version.order_by_name orders by lowered name which according to few tests preserves collation, so still orders versions not as plain strings. We can change the collation to also be not case sensitive, but it will make it non deterministic
Lastly, it is a bit weird that there is now a
nameand asemver_nameorder available in the API which basically do the same but I would still keep the two to not break potential integrations.
I'm wondering how does Queries::Versions::Orders::NameOrder work, if it overrides private order to receive no arguments while in base it receives one, so apply_to will raise
Related discussion about ordering of absent values for custom fields. What about start date and due date columns? The PR https://github.com/opf/openproject/pull/8000 and OP#32156 don't describe the reason for adding null_handling that puts empty values always at the end for 3 properties.
I'd just remove it for versions so that it the same whenever one orders by version name, regardless of whether that is done on a custom field or on the hard wired property. But it is also not super important so if you want it done in the scope of the null value order feature that would also be fine with me. And apparently there is also the same discrepancy between date custom fields and due and start date so I definitely see the benefit in having all of them harmonized.
You mean Version.order_by_name (or Version.order_by_semver_name)?
Yes, I mean the order_by_name method. Sorry for the confusion.
Just saw that Version.order_by_name orders by lowered name which according to few tests preserves collation, so still orders versions not as plain strings. We can change the collation to also be not case sensitive, but it will make it non deterministic
At least in my test, the change in the collation already seems to do that even without the transformation to LOWER. I did not check why:
I'm wondering how does Queries::Versions::Orders::NameOrder work, if it overrides private order to receive no arguments while in base it receives one, so apply_to will raise
It fails of course :). Apparently it is just broken. Could be fixed in a separate PR but since the change of simply removing the method override is simple, I'd include it.
At least in my test, the change in the collation already seems to do that even without the transformation to LOWER. I did not check why:
Just to share here the learning that it is caused by using ICU collation: it is not ignoring the case, but ICU collation is more sophisticated than just comparing character codes while applying some language rules - it compares characters on multiple levels and case difference is less important than case-ignoring difference. So following is the correct collation: a 0, A 0, à 0, a 1, A 1, à 1