-
Couldn't load subscription status.
- Fork 9
Enabled the new sortings for application table #229
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
Enabled the new sortings for application table #229
Conversation
…datatarget-id-to-error-message
…e/IOT-1380-update-gateway-eui-input
…/IOT-1486-add-error-message-to-email-in-use
…if no user admins found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestions
| (query.orderOn == "id" || | ||
| query.orderOn == "name" || | ||
| query.orderOn == "updatedAt") | ||
| (query.orderOn == "id" || query.orderOn == "name" || query.orderOn == "updatedAt") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make these === instead of == ? Even though I know it's not your code..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (left != null checks as this also checks for undefined)
| } | ||
| const order: "DESC" | "ASC" = | ||
| query?.sort?.toLocaleUpperCase() == "DESC" ? "DESC" : "ASC"; | ||
| const order: "DESC" | "ASC" = query?.sort?.toLocaleUpperCase() == "DESC" ? "DESC" : "ASC"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same (it's pretty general in this file, sadly..)
| private getSortingForApplications(query: ListAllEntitiesDto): Record<string, string | number> { | ||
| const sorting: Record<string, string | number> = {}; | ||
| if ( | ||
| // TODO: Make this nicer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be removed or is it for the future? :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future, a bit more research for a nice way of doing it if it is possible
…umn-options-on-application-table
No description provided.