Skip to content

[BI-1339] Germplasm Details page#217

Merged
HMS17 merged 12 commits intodevelopfrom
feature/BI-1339
Apr 14, 2022
Merged

[BI-1339] Germplasm Details page#217
HMS17 merged 12 commits intodevelopfrom
feature/BI-1339

Conversation

@HMS17
Copy link
Contributor

@HMS17 HMS17 commented Apr 5, 2022

Description

Story: BI-1339 - Germplasm Details page

Updates to enable display of individual germplasm details

  • Added GermplasmDetails.vue to display individual germplasm information
  • Updated GermplasmTable.vue to include a column with links to germplasm details for each germplasm and to retrieve the germplasm UUID for use in said link
  • Added reference source to .env.development to aid in retrieval of germplasm UUID from external references on front end
  • Added api call to new /programs/${programId}/brapi/v2/germplasm/${germplasmId} endpoint to retrieve individual germplasm information
  • Updated router to include navigation to germplasm details page

Dependencies

bi-api/feature/BI-1339

Testing

  1. Import germplasm list with entries with pedigrees, partial pedigrees, and no pedigrees
  2. Go to germplasm table - each row should have show details link
  3. Click on show details link
  4. Should navigate to germplasm details page
  5. Germplasm info displayed should correspond to selected 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: <link to TAF run>

@HMS17 HMS17 requested review from a team, ctucker3 and timparsons and removed request for a team April 7, 2022 19:00
@HMS17 HMS17 marked this pull request as ready for review April 7, 2022 19:01
config.method = 'get';
config.programId = programId;
config.germplasmId = germplasmId;
config.params = {}; //todo check if needed
Copy link
Contributor

Choose a reason for hiding this comment

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

Still a TODO?

reject(error);
})
} else {
reject();
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to have a rejection reason in case we ever need to debug this.

<router-link v-bind:to="{name: 'germplasm-all', params: {programId: activeProgram.id}}">
All Germplasm
</router-link>
<p></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a class of mb-4 instead of the empty <p></p> on the router-link if you're going for spacing?

}).catch((error) => {
// Display error that germplasm cannot be loaded
this.$emit('show-error-notification', 'Error while trying to load germplasm');
throw error;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think throwing this error here will make it so you never get to the finally to be able to turn off the germplasmLoading. Have you tried that out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested it and yeah it gets to finally.

return dateTime.format("DD/MM/YYYY");
}
return "";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be useful on Germplasm model.

getGermplasmUUID(references: ExternalReferences){
let val = references.find(ref => ref.referenceSource === process.env.VUE_APP_BI_REFERENCE_SOURCE);
return val ? val.referenceID : "";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be useful on Germplasm model.

@HMS17 HMS17 requested a review from ctucker3 April 13, 2022 15:51
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.

Need to run the code still, but one suggested change

.env.development Outdated
VUE_APP_LOG_LEVEL=${WEB_LOG_LEVEL}

# The reference source
VUE_APP_BI_REFERENCE_SOURCE=breeding-insight.org
Copy link
Member

Choose a reason for hiding this comment

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

can this be changed to breedinginsight.org

@HMS17 HMS17 requested a review from timparsons April 13, 2022 22:19
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.

Testing passed

@HMS17 HMS17 merged commit 23c03f7 into develop Apr 14, 2022
@HMS17 HMS17 deleted the feature/BI-1339 branch April 14, 2022 22:48
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