Skip to content

BI-1721 - Show All - Breeding Methods table#303

Merged
timparsons merged 3 commits intodevelopfrom
bug/BI-1721
Mar 16, 2023
Merged

BI-1721 - Show All - Breeding Methods table#303
timparsons merged 3 commits intodevelopfrom
bug/BI-1721

Conversation

@timparsons
Copy link
Member

@timparsons timparsons commented Feb 28, 2023

Description

Story: https://breedinginsight.atlassian.net/browse/BI-1721

Original bug was that the "Show All" button wasn't working for the Breeding Methods table. When investigating how to fix this, there was a lot of duplicate and confusing code around pagination, so I refactored that to utilize PaginationController and the mechanism for a parent component to update page information as needed, and for the ExpandableTable component to now be primarily responsible for handling paging events from the PaginationControls.

Dependencies

bi-api - bug/BI-1721

Testing

  1. On a given table, click the 'Show All' button, and ensure that all records are now visible
  2. Click the 'Show All' button again, and ensure that the table is paginated again
  3. Ensure pagination works

Testing applies to the all tables:

  • Admin Users
  • Admin Programs
  • Program Users
  • Program Locations
  • Program Breeding Methods
  • Enabling System Breeding Methods
  • All Germplasm
  • Germplasm Lists
  • Germplasm List Details
  • Ontology (active)
  • Ontology (archived)
  • Experiments
  • Import Experiment
  • Import Ontology
  • Import Germplasm

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: https://github.com/Breeding-Insight/taf/actions/runs/4297630354

@github-actions github-actions bot added the bug Something isn't working label Feb 28, 2023
@timparsons timparsons requested review from a team, davedrp and dmeidlin and removed request for a team February 28, 2023 21:54
Copy link
Contributor

@dmeidlin dmeidlin left a comment

Choose a reason for hiding this comment

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

Both "Germplasm" and "Experiments&Observations" are stuck in a recursive loop that keeps making requests to the back end.

Comment on lines 44 to 45
constructor(paginationResult?: any) {
if (paginationResult){
Copy link
Contributor

Choose a reason for hiding this comment

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

Starting TSv2.2 the object type is used for generic objects. We should probably start using object or object | null in place of any when appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

This makes sense. For this specific instance, I tried changing it to object, then had other compilation errors. If you're ok with it, I'd rather not change this now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, that's fine.

@timparsons
Copy link
Member Author

Both "Germplasm" and "Experiments&Observations" are stuck in a recursive loop that keeps making requests to the back end.

@dmeidlin Good catch! This has been fixed

@timparsons timparsons merged commit 78e090c into develop Mar 16, 2023
@timparsons timparsons deleted the bug/BI-1721 branch March 16, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants