fix: use case insensitive sorting for name of users, labs and orgs#267
fix: use case insensitive sorting for name of users, labs and orgs#267PurcyTwoBulls merged 4 commits intomainfrom
Conversation
| export function caseInsensitiveSortFn(a: any, b: any, direction: 'asc' | 'desc') { | ||
| if (a.toLowerCase() === b.toLowerCase()) { | ||
| // If idenical ignoring case, sort by case for consistency of results | ||
| if (direction === 'asc') { | ||
| return a < b ? -1 : 1; | ||
| } else { | ||
| return a > b ? -1 : 1; | ||
| } | ||
| return 0; | ||
| } | ||
| if (direction === 'asc') { | ||
| return a.toLowerCase() < b.toLowerCase() ? -1 : 1; | ||
| } else { | ||
| return a.toLowerCase() > b.toLowerCase() ? -1 : 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Thanks @PurcyTwoBulls for working on this fix. This function needs some improvement.
Organization Names, Laboratory Names, and also User Emails are enforced to be unique within their respective DynamoDB tables with the use of the supporting unique-reference-table DynamoDB table.
Have a look through the unique-reference-table DynamoDB table records to see the unique references - you will see that Laboratory Names are unique within their parent Organization.
So, if there is an existing Laboratory named 'ABC' the backend logic will never allow a new Laboratory named 'abc' to be created - as the lower-case uniqueness record already exists in the unique-reference-table.
As a result, this function can be simplified to:
export function caseInsensitiveSortFn(a: string, b: string, direction: 'asc' | 'desc'): number {
const orderAB = a.toLowerCase() < b.toLowerCase() ? -1 : 1;
return direction === 'desc' ? orderAB * -1 : orderAB; // If desc, invert orderAB
}
Also we should not ever need to use the any type, as we are using Typescript instead of Javascript so we should be explicitly defining the types.
There was a problem hiding this comment.
Thanks @zigtan
For users, emails have uniqueness but the users first and last name and combinations do not. If we want to make the decision that we are not tackling that edge case then we don't need to, but given it is user entry it is probable edge case.
I have updated the function signature.
There was a problem hiding this comment.
@PurcyTwoBulls , a User's names (FirstName, LastName) do not have to be unique in the system.
For all User's their FirstName, LastName are just their personal details.
Internally we reference all User's by their primary UUID.
The User's Email is used as a Global Secondary Index (GSI) for efficient lookups in DynamoDB.
The rule to ensure each User's Email is unique is a logical security design and also support the multi-tenant architecture where a User can invited to one or more Organizations (using the same email).
There was a problem hiding this comment.
I understand that, but what is being used here for displaying user is their display name or their email address, so we are using First and Last name in these sorted fields.
There was a problem hiding this comment.
Thanks @PurcyTwoBulls for highlighting the First Name and Last Name sorting issue you are trying to tackle.
I think this is a deeper problem seeming from our UI design.
We will need to circle back to improve the UI design to separately list Users' Email, and Full Name in separate columns and have each be sortable. The current design of showing a single column for Email or Full Name complicates the sorting behaviour and solution.
zigtan
left a comment
There was a problem hiding this comment.
Thanks @PurcyTwoBulls . Just need to update the FE sort-utils caseInsensitiveSortFn() logic.
Overrides the sort function to apply a case insensitive sort (with a case sensitive sort for consistent results if they match).
This is applied to all sortable text fields in tables.