[BI-1202] Germplasm Table: Add sorting to all columns #197
Conversation
src/store/sorting/index.ts
Outdated
| // program table | ||
| germplasmSort: new GermplasmSort(GermplasmSortField.AccessionNumber, SortOrder.Ascending) |
There was a problem hiding this comment.
just a code comment left over from the copy/paste that should be changed to read germplasm
src/store/sorting/mutation-types.ts
Outdated
| // germplasm tbale | ||
| export const UPDATE_GERMPLASM_SORT = 'updateGermplasmSort'; |
There was a problem hiding this comment.
not a big deal, but another typo to fix if you are already updating the previous code comment.
src/store/sorting/mutations.ts
Outdated
| //program table | ||
| [UPDATE_GERMPLASM_SORT](state: SortState, sort: GermplasmSort) { | ||
| state.germplasmSort.field = sort.field; | ||
| state.germplasmSort.order = sort.order; |
There was a problem hiding this comment.
another code comment correction
| // Set sort | ||
| const params = { | ||
| sortField: this.germplasmSort.field, | ||
| sortOrder: this.germplasmSort.order | ||
| }; | ||
| // GET call | ||
| const response = await BrAPIService.get(BrAPIType.GERMPLASM, params, this.activeProgram!.id!, | ||
| this.paginationController.pageSize, this.paginationController.currentPage - 1); |
There was a problem hiding this comment.
Using a BrAPIService and passing in the enum to specify the BrAPI variable makes a lot of sense. Since we already have the PaginationQuery and Sort models it would also be good to type the service args to use these, too.
A generic could be used for the sort params type, so the call would look like this await BrAPIService.get<GermplasmSort>(BrAPIType.GERMPLASM, params, this.activeProgram!.id!, {this.paginationController.pageSize, this.pagnationCOntroller.currentPage - 1}); where const params: GermplasmSort = {...}; is as in line 132.
Also, instead of throwing errors if either the pagination query or sort query is undefined, default values could be used. For example, like in ProgramLocationService.ts,
static getAll(programId: string, paginationQuery: PaginationQuery = new PaginationQuery(1, 50, true), sort: LocationSort = new LocationSort(LocationSortField.Name, SortOrder.Ascending)): Promise<[ProgramLocation[], Metadata]> {...}
There was a problem hiding this comment.
Good idea! I made the sort params generic and made the pagination be passed in as an object.
I removed those pagination checks, but didn't add defaults. Default sort fields and default pagination is handled on the backend so it might be best to have only one place for defaults.
| public static async get(type: BrAPIType, programId: string, sort: { field: string, order: SortOrder }, | ||
| pagination: {pageSize: number, page: number}): Promise<BiResponse> { |
There was a problem hiding this comment.
just use here a generic T for the sort field type.
public static async get<T>(type: BrAPIType, programId: string, sort: { field: T, order: SortOrder }, pagination: {pageSize: number, page: number}): Promise<BiResponse> {
| // GET call | ||
| const response = await BrAPIService.get(BrAPIType.GERMPLASM, this.activeProgram!.id!, this.germplasmSort, | ||
| { pageSize: this.paginationController.pageSize, page: this.paginationController.currentPage - 1 }); |
There was a problem hiding this comment.
and assign the GermplasmSortField type at the method call.
const response = await BrAPIService.get<GermplasmSortField>(BrAPIType.GERMPLASM, this.activeProgram!.id!, this.germplasmSort, { pageSize: this.paginationController.pageSize, page: this.paginationController.currentPage - 1 });
Description
Story: https://breedinginsight.atlassian.net/jira/software/c/projects/BI/boards/1?modal=detail&selectedIssue=BI-1202
Made all germplasm table columns sortable and hooked them up to the backend sorting. Test was added on backend.
Dependencies
biapi: BI-1202
Testing
See bi-api PR for testing details.
Checklist: