-
Notifications
You must be signed in to change notification settings - Fork 9
fix: user officer users and experiment table filter #1232
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
base: develop
Are you sure you want to change the base?
fix: user officer users and experiment table filter #1232
Conversation
…api calls and optimised the handling of search query params
c5e6a2f to
0003da9
Compare
5e8415b to
8ea968b
Compare
|
|
2 similar comments
|
|
|
|
|
|
|
|
|
|
janosbabik
left a comment
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.
Hi @yoganandaness , some comments from my side.
| .orWhereILikeEscaped('lastname', '%?%', searchText); | ||
| }); | ||
| } | ||
| logger.logInfo( |
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.
If this is not important then I would not log the sort information.
| let isMounted = true; | ||
|
|
||
| if (isMounted) { | ||
| if (isMounted && !isFirstRender) { |
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.
could you use the same code what you usied in the other PR:
https://github.com/UserOfficeProject/user-office-core/pull/1231/files#diff-34e1208dfd78a93b7db3c503dc505b5fe67a545a449d7dcc76e6165a6ded3b3bR386
| throw new GraphQLError(`Bad sort field given: ${sortField}`); | ||
| } | ||
| sortField = fieldMap[sortField]; | ||
| query.orderByRaw(`${sortField} ${sortDirection}`); |
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.
Is it possible to validate the user’s input values with GraphQL in this case?
The sortDirection string goes directly into the SQL command which could be harmful.
| @Field(() => String, { nullable: true }) | ||
| orderDirection?: string; | ||
| @Field({ nullable: true }) | ||
| public sortDirection?: string; |
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.
Here, we could use SortDirection enum (asc, desc) for validating the input, I guess.
|
|
|
|
1 similar comment
|
|
Description
Persisting the Experiment table filters and searches in the query params, so that refreshing the page retains the data rather than starting from scratch. In addition to that, additional fetching query have been avoided.
Motivation and Context
Improve usability of experiment table
How Has This Been Tested
Manually
Fixes
Changes
In Experiment Table, query params persisting have been implemented .
Depends on
Tests included/Docs Updated?