Add Guestbook Response Sorting by fields - #12568
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
6403b3f to
ec51106
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
| Order order; | ||
| String orderField = (sortField == null) ? "" : sortField.toLowerCase(); | ||
| switch(orderField) { | ||
| case "dataset": |
There was a problem hiding this comment.
OK _ I see what you're concerned about - not the CriteriaBuilder per se, but the fact that it's a really complex join to sort by dataset name (for example). It sounds like denormalization or a materialized view might be required to be efficient (according to AI). I'm not sure I know enough to decide what the tradeoffs might be there, so maybe some tech hr discussion needed. Alternately - if this looks like more work than expected, perhaps going back to the UI side and asking if sorting on these is really useful versus filtering or allowing guestbook response viewing on a dataset of file page (so one could select a given dataset name and filter by it, with the query underneath just being adding a where dataset_id=X clause (or file, or user, etc.)


What this PR does / why we need it: Modern UI needs to sort the output of the get guestbook response api
Which issue(s) this PR closes:#12524
Special notes for your reviewer:
Suggestions on how to test this: create multiple Datasets using a Guestbook and populate responses. Using the API retrieve the responses with various sort/order query parameters. Test with large numbers for performance hits due to the added db table joins.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?: included
Additional documentation: