Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/57401 sorting by link custom field causes exception #16514

Merged

Conversation

toy
Copy link
Contributor

@toy toy commented Aug 23, 2024

Ticket

OP#57401

What are you trying to accomplish?

Allow ordering by custom field of link format

What approach did you choose and why?

Adding sorting as for string format + add tests for ordering by custom fields of all formats

Merge checklist

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

@toy toy force-pushed the bug/57401-sorting-by-link-custom-field-causes-exception branch from 5b6f1fa to 635774d Compare August 27, 2024 17:42
@toy toy marked this pull request as ready for review August 28, 2024 15:45
@toy toy force-pushed the bug/57401-sorting-by-link-custom-field-causes-exception branch from 635774d to 6b313e4 Compare August 28, 2024 15:46
Copy link
Contributor

@ulferts ulferts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇 for adding all the specs.

The functionality is also fine and works as expected.

What I have been wondering about though, is that the specs are currently work package Query specific. The same code is however also used in the ProjectQuery and there the order statement are included in a different manner. From my point of view it would help to copy over the specs to the ProjectQuery realm as well. That would in my expectation also make the subsequent changes to be carried out for the optimization of the performance a safer change. But this needn't be done in this PR necessarily.

@ulferts ulferts changed the base branch from dev to release/14.5 August 29, 2024 10:00
@toy
Copy link
Contributor Author

toy commented Aug 29, 2024

True, also it looks like only for work packages there is a logic to change the placement of null values (null_handling method). I'll create a maintenance ticket for that.

@toy toy merged commit bf4cacd into release/14.5 Aug 29, 2024
10 checks passed
@toy toy deleted the bug/57401-sorting-by-link-custom-field-causes-exception branch August 29, 2024 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants