Skip to content

[BI-1375] - Make GIDs clickable links#218

Merged
HMS17 merged 9 commits intodevelopfrom
feature/BI-1375
May 11, 2022
Merged

[BI-1375] - Make GIDs clickable links#218
HMS17 merged 9 commits intodevelopfrom
feature/BI-1375

Conversation

@HMS17
Copy link
Contributor

@HMS17 HMS17 commented Apr 13, 2022

Description

Story: BI-1375 - Make GIDs clickable links

Added GermplasmLink.vue component for germplasm GID links
Updated GermplasmDetails.vue to make parent GIDs clickable links (but not the current germplasm GID as it would be redundant)
Updated GermplasmTable.vue to make all GIDs clickable links
Changed germplasmUUID in GermplasmDetails.vue to a computed property in order to enable load of new germplasm information on clicking a parent GID

Dependencies

bi-api/feature/BI-1375

Testing

  • Open Germplasm Table
  • Check that each GID, Female Parent GID, and Male Parent GID in a row is a link and leads to the correct germplasm details page
  • Open Germplasm Details Page for a germplasm with male and female parent
  • Parent GIDs should be displayed and each should be a clickable link to their respective germplasm details page
  • Open Germplasm Details Page for a germplasm with female parent only
  • Female parent GID should be displayed and should be a clickable link to its respective germplasm details page
  • Open Germplasm Details Page for a germplasm with no parents
  • No parent GID links should be displayed

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 dmeidlin and removed request for a team April 15, 2022 18:36
@HMS17 HMS17 marked this pull request as ready for review April 15, 2022 18:36
<template>
<router-link v-if="this.germplasmUUID" v-bind:to="{name: 'germplasm-details', params: {programId: activeProgram.id, germplasmId: this.germplasmUUID}}">
{{ this.germplasmGID }}
</router-link>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering for this if it would be better to have the GID not be a link if UUID isn't provided, rather than not showing the GID at all?

I'm thinking that because from a user perspective it might be better to have a broken link rather than missing data in the case of a data error such as missing UUID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, changed to be text in the case of no UUID

v-bind:germplasmUUID="Pedigree.parsePedigreeString(germplasm.additionalInfo.pedigreeByUUID).femaleParent"
v-bind:germplasmGID="Pedigree.parsePedigreeString(germplasm.pedigree).femaleParent"
> </GermplasmLink>
<template v-if="Pedigree.parsePedigreeString(germplasm.pedigree).maleParent">
Copy link
Contributor

Choose a reason for hiding this comment

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

In the GermplasmLink above you have the v-if directly in the component, but here you have it in a template. Is it possible to move this into the GermplasmLink for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now I have the GermplasmLink as a one germplasm per link sort of thing rather than a more complicated component that includes multiple configurations of links. The v-if here is wrapped around the "/ " part of the pedigree since the slash and gid only show up if there is a male parent. I could add some sort of boolean to pass in but I would still need a boolean on the outside for the slash lest I make the Germplasm Link more complicated by adding the slash. So what I have now seems to be the simplest route?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah gotcha, I missed the /. No need to change anything, thanks for the explanation!

{{ props.row.data.additionalInfo.createdBy.userName }}
</b-table-column>
<b-table-column field="germplasmId" v-slot="props" :th-attrs="(column) => ({scope:'col'})">
<b-table-column field="externalReferences" v-slot="props" :th-attrs="(column) => ({scope:'col'})">
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this field be something like showDetails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on discussion with Tim, confusion resolved by removing the field as it is unneeded here.

@HMS17 HMS17 requested review from nickpalladino and removed request for dmeidlin May 3, 2022 15:06
@HMS17 HMS17 assigned nickpalladino and unassigned dmeidlin May 3, 2022
@HMS17 HMS17 requested a review from ctucker3 May 3, 2022 16:15
v-bind:germplasmUUID="Pedigree.parsePedigreeString(germplasm.additionalInfo.pedigreeByUUID).femaleParent"
v-bind:germplasmGID="Pedigree.parsePedigreeString(germplasm.pedigree).femaleParent"
> </GermplasmLink>
<template v-if="Pedigree.parsePedigreeString(germplasm.pedigree).maleParent">
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah gotcha, I missed the /. No need to change anything, thanks for the explanation!

@HMS17 HMS17 merged commit bf0a94d into develop May 11, 2022
@HMS17 HMS17 deleted the feature/BI-1375 branch May 11, 2022 16:15
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.

4 participants