SC-399 - paginate ByUser page. Includes fix for race condition#854
SC-399 - paginate ByUser page. Includes fix for race condition#854nbeatty-gpa wants to merge 15 commits intomasterfrom
Conversation
| IEnumerable<DataRow> filteredRows = dataTable.AsEnumerable(); | ||
| IEnumerable<SQLSearchFilter> searchesToApply = postData.Searches.Where(flt => !IsInDatabase(flt.FieldName)); | ||
| foreach (SQLSearchFilter search in searchesToApply) | ||
| { | ||
| string wildcardPattern = Regex.Escape(search.SearchText.ToLower()).Replace(@"\*", ".*"); | ||
| switch (search.Operator) | ||
| { | ||
| case "=": | ||
| filteredRows = filteredRows.Where((row) => row.Field<string>(search.FieldName).ToLower() == search.SearchText.ToLower()); | ||
| break; | ||
| case "LIKE": | ||
| filteredRows = filteredRows.Where((row) => Regex.IsMatch(row.Field<string>(search.FieldName).ToLower(), wildcardPattern)); | ||
| break; | ||
| case "NOT LIKE": | ||
| filteredRows = filteredRows.Where((row) => !Regex.IsMatch(row.Field<string>(search.FieldName).ToLower(), wildcardPattern)); | ||
| break; | ||
| case "IN": | ||
| List<string> groupTypes = search.SearchText.Trim('(', ')').Split(',').ToList(); | ||
| filteredRows = filteredRows.Where((row) => groupTypes.Contains(row.Field<string>(search.FieldName))); | ||
| break; | ||
| default: | ||
| throw new Exception("Operator not found for Filter."); | ||
| } | ||
| } |
There was a problem hiding this comment.
I would create a static method that both SecurityGroup/UserAccount can use if they are the exact same logic.
| React.useEffect(() => { | ||
| if (currentPage >= totalPages) { } | ||
| if (currentPage >= totalPages && totalPages > 0) | ||
| setPage(totalPages) | ||
| }, [totalPages]) |
There was a problem hiding this comment.
It seems as this logic is repeated multiple times in different components with varying logic in them. At the very least they should be sharing the same logic if they are trying to do the same thing. Even better would be to create a reusable hook.
For example check -- Source/Applications/SystemCenter/wwwroot/Scripts/TSX/SystemCenter/Settings/Setting.tsx
It has different logic than whats above but seem to be attempting to do the same thing.
| React.useEffect(() => { | ||
| if (currentPage >= totalPages) | ||
| setPage(totalPages) | ||
| else | ||
| setPage(1) | ||
| }, [totalPages]) |
There was a problem hiding this comment.
Same as above comment, same intended logic but different code. Would benefit from a reusable hook also missing a dep
| if (searchStatus == 'uninitiated' || searchStatus == 'changed') | ||
| dispatch(SecurityGroupSlice.DBSearch({ sortField, ascending, filter: search })) | ||
| dispatch(SecurityGroupSlice.PagedSearch({ sortField, ascending, filter: search, page: currentPage - 1 })) | ||
| }, [searchStatus]) |
There was a problem hiding this comment.
missing multiple deps here
| setPage(totalPages) | ||
| else | ||
| setPage(1) | ||
| }, [totalPages]) |
There was a problem hiding this comment.
same comment as above, as well as this is missing currentPage dep.
| React.useEffect(() => { | ||
| if (currentPage >= totalPages && totalPages > 0) | ||
| setPage(totalPages) | ||
| }, [totalPages]) |
There was a problem hiding this comment.
ssame as above comment also missing deps
| if (currentPage >= totalPages && totalPages > 0) | ||
| setPage(totalPages) | ||
| } | ||
| else | ||
| setPage(0) |
There was a problem hiding this comment.
Same as above, and missing dep
paginate user account page, and guard against race conditions that could cause negative pages.
SC-399