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-1910 - Allow Breeder to Upload a Genotype Sample File #343

Merged
merged 12 commits into from
Nov 29, 2023

Conversation

timparsons
Copy link
Member

@timparsons timparsons commented Oct 30, 2023

Description

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

Created a new UI component named "Sample Management"

  • Defined a new import type: Genotype Sample
  • Defined a new import template
  • Created a list view of all sample submissions
  • Created a detailed view of an individual submission
  • Implemented ability to manual update the status of a submission
  • Implemented the ability to automatically submit to a vendor and check the status

Dependencies

Testing

Testing with germplasm

  1. Create germplasm records in your program
  2. On the Import Data page, go to the "Genotype Samples" tab, and download the template
  3. Populate the template, and make sure to populate the GID column
  4. Upload the sample submission file, and ensure the preview is correct, and confirming succeeds
  5. Navigate to the submission's detail page, and ensure the GID column is populated

Testing with ObsUnitIDs

  1. Create germplasm records in your program
  2. Create an experiment
  3. On the Import Data page, go to the "Genotype Samples" tab, and download the template
  4. Populate the template, and make sure to populate the ObsUnitID column
  5. Upload the sample submission file, and ensure the preview is correct, and confirming succeeds
  6. Navigate to the submission's detail page, and ensure the ObsUnitID and GID columns are populated

Manual Submission

  1. When on the submission detail page, and the submission has not been submitted, click on "Manage Submission" and you should see an option for "Manual Update Submission"
  2. Clicking on "Manual Update Submission" should open a modal window with a dropdown allowing you to change the submissions status
  3. Change the status to "SUBMITTED" and click save, ensure save succeeds and the metadata on the detail page is updated

Automated Submission

  1. Set the BRAPI_VENDOR_SUBMISSION_ENABLED property to true for both bi-web and bi-api and restart
  2. When on the submission detail page, and the submission has not been submitted, click on "Manage Submission" and you should see an option for "Submit to DArT"
  3. Clicking on "Submit to DArT" should open a modal window asking you to confirm this action
  4. Click "Submit"
  5. Ensure the submission was successful and that the metadata on the detail page is updated to show the vendor submission ID and status
  6. Click on "Manage Submission" and you should see an option for "Check Vendor Status".
  7. Click "Check Vendor Status", and a modal should appear with a loading indicator
  8. Ensure there are no errors, and that the "Last Status Check" is updated to the current time

General testing

  1. Ensure that all successful uploads display in the Sample Management table, and show correct status information and metadata
  2. Ensure when viewing a submission detail that the metadata is correct, the table containing the sample information is correct, and the tab containing plate layouts is correct

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/6765876438

Copy link
Member

@nickpalladino nickpalladino left a comment

Choose a reason for hiding this comment

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

Did you figure out that issue from the demo where the table didn't display after a certain sequence of menu item selections?

z-index: 1;
pointer-events: none;
}
.th-wrap {
Copy link
Member

Choose a reason for hiding this comment

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

Does this affect the display of anything with existing tables?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I just restructured the scss to be nested

private loading = true;
private germplasm?: Germplasm;

fetchGermplasm() {
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this, doesn't seem to be actually loading anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing anymore. I'll remove that!

Copy link
Member

Choose a reason for hiding this comment

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

sounds good

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

</div>
<div class="column is-narrow">
<div class="has-text-dark has-text-centered is-size-7">
<button class="button is-outlined is-primary" :id="'download-sample-template'" v-on:click="generateTemplate()">Download the Sample Import Template</button>
Copy link
Member

Choose a reason for hiding this comment

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

What was the reason the file was generated rather than hosted on box like the other imports?

Copy link
Member Author

Choose a reason for hiding this comment

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

Speed of development primarily. Doing this allowed me to quickly make changes to the template as I was receiving feedback from others. I can make a file and put it on Box like the others. It may be worth talking about a static template vs generated template looking to the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the template to be served from Box

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good

@timparsons
Copy link
Member Author

Did you figure out that issue from the demo where the table didn't display after a certain sequence of menu item selections?

@nickpalladino I did! It was a data loading workflow issue. Once I fixed that workflow, it is now working as expected

@davedrp
Copy link
Contributor

davedrp commented Nov 21, 2023

Testing. I found a bug.

  • I imported a sample submission file with a row having the value of lowercase 'c'.
  • It imported without error and saved the row with a value of lowercase 'c'.
  • BUT, when retrieving the data from the submission's detail page, I got the error message "Error while trying to load submission details"
    • the error seems to occur in SubmissionDetails.vue in the section of code with plateLayout[sample.row.charCodeAt(0) - 'A'.charCodeAt(0)]

@timparsons
Copy link
Member Author

Testing. I found a bug.

  • I imported a sample submission file with a row having the value of lowercase 'c'.
  • It imported without error and saved the row with a value of lowercase 'c'.
  • BUT, when retrieving the data from the submission's detail page, I got the error message "Error while trying to load submission details"
    • the error seems to occur in SubmissionDetails.vue in the section of code with plateLayout[sample.row.charCodeAt(0) - 'A'.charCodeAt(0)]

Good catch @davedrp!! I've fixed this by forcing the row to be uppercase when saving

@davedrp
Copy link
Contributor

davedrp commented Nov 29, 2023

developer test PASSED

Copy link
Contributor

@davedrp davedrp left a comment

Choose a reason for hiding this comment

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

I have left some recommendations in the comments, but they are not essential.

private activeTab = 'details';
private submissionLoading: boolean = true;
private submissionDetailsLoading: boolean = true;
private downloadModalActive: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

downloadModalActive is never used

import StatusEnum = VendorOrderStatusResponseResult.StatusEnum;
import {Plate} from "@/breeding-insight/brapi/model/geno/plate";
import * as XLSX from "xlsx";
import {WorkBook, WorkSheet} from "xlsx";
Copy link
Contributor

Choose a reason for hiding this comment

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

WorkSheet is never used.

private downloadModalActive: boolean = false;
private paginationController: PaginationController = new PaginationController();
private submission?: SampleSubmission;
private germNameOptions?: Array<String> = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Contents of collection 'germNameOptions' are updated, but never queried


import * as api from '@/util/api';
import { BiResponse, Response } from '@/breeding-insight/model/BiResponse';
import { BreedingMethod } from '@/breeding-insight/model/BreedingMethod';
Copy link
Contributor

Choose a reason for hiding this comment

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

'BreedingMethod' imported but not used

import {Result, ResultGenerator} from "@/breeding-insight/model/Result";
import {SampleSubmission} from "@/breeding-insight/model/SampleSubmission";
import {VendorOrderSubmission} from "@/breeding-insight/brapi/model/geno/vendorOrderSubmission";
import {BrAPIDAOUtil} from "@/breeding-insight/dao/BrAPIDAOUtil";
Copy link
Contributor

Choose a reason for hiding this comment

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

'BrAPIDAOUtil' imported but not used

@timparsons timparsons merged commit 4323a59 into develop Nov 29, 2023
1 check passed
@timparsons timparsons deleted the feature/BI-1910 branch November 29, 2023 19:50
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