Skip to content
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

[BI-1617] Refactor Ontology table to use Buefy #304

Merged
merged 25 commits into from
Mar 10, 2023
Merged

Conversation

dmeidlin
Copy link
Contributor

@dmeidlin dmeidlin commented Mar 3, 2023

Description

Story: BI-1617, BI-1628, BI-1629

A new component SidePanelTableBuefy.vue was created that uses the Buefy table with search filter inputs at the column headings. The active and archived ontology tables were changed to use the new component. The TraitsService method was changed to search traits using the GET /traits endpoint instead POST /traits/search. The ontology mutations in the vuex sorting module were updated.

Dependencies

bi-api feature/BI-1629

Testing

Please include any details needed for reviewers to test this code

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • I have create/modified unit tests to cover this change
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to documentation
  • I have run TAF: <link to TAF run>

Copy link
Member

@timparsons timparsons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good! Couple of small questions.

src/breeding-insight/service/TraitService.ts Outdated Show resolved Hide resolved
src/components/tables/SidePanelTableBuefy.vue Outdated Show resolved Hide resolved
src/components/tables/SidePanelTableBuefy.vue Outdated Show resolved Hide resolved
Copy link
Member

@timparsons timparsons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, a couple more questions

Comment on lines 187 to 209
private static makeTraitReqConfig(programId: string) {
let config: any = {
url: `${process.env.VUE_APP_BI_API_V1_PATH}/programs/${programId}/traits`,
method: 'get'
};
return config;
}

private static makeTraitParams(sort: OntologySort, pagination: {pageSize: number, page: number}, filters?: any) {
let params: any = {};

if (filters) {
params = filters;
}
if (sort.field) {
params['sortField'] = sort.field;
}
if (sort.order) {
params['sortOrder'] = sort.order;
}
if (pagination.page || pagination.page == 0) { //have to account for 0-index pagination since 0 falsy
params ['page'] = pagination.page;
}
if (pagination.pageSize) {
params['pageSize'] = pagination.pageSize;
}
params.full = true;

return params;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious as to why this block of code is in the service and not the DAO?

v-on:paginate="paginationController.updatePage($event)"
v-on:paginate-toggle-all="paginationController.toggleShowAll(traitsPagination.totalCount.valueOf())"
v-on:paginate-page-size="paginationController.updatePageSize(parseInt($event,10))"
<SidePanelTableNew
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the intent to rename this back to SidePanelTable once the old version has been completely removed from the codebase?

@dmeidlin dmeidlin merged commit de4b002 into develop Mar 10, 2023
@dmeidlin dmeidlin deleted the feature/BI-1617 branch March 10, 2023 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants