Skip to content

[BI-1473] - Append existing germplasm with optional details#211

Merged
nickpalladino merged 27 commits intodevelopfrom
feature/BI-1473
Apr 19, 2023
Merged

[BI-1473] - Append existing germplasm with optional details#211
nickpalladino merged 27 commits intodevelopfrom
feature/BI-1473

Conversation

@HMS17
Copy link
Contributor

@HMS17 HMS17 commented Sep 14, 2022

Description

Story: BI-1473 - Allow user to append existing germplasm with optional details

A new card was created to reduce the scope of this one: https://breedinginsight.atlassian.net/browse/BI-1735?atlOrigin=eyJpIjoiYzJmNjgzZjU1YjBlNGNlNzk4YTFiYzVjZGU2YzA1NDkiLCJwIjoiaiJ9

Dependencies

bi-web/BI-1473

Testing

see bi-web

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 tested that my code works with both the brapi-java-server and BreedBase
  • 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: <please include a link to TAF run>

@nickpalladino nickpalladino marked this pull request as ready for review March 28, 2023 20:27
public static String duplicateEntryNoMsg = "Entry numbers must be unique. Duplicated entry numbers found: %s";
public static String circularDependency = "Circular dependency in the pedigree tree";
public static String listNameAlreadyExists = "Import group name already exists";
public static String missingGID = "No germplasm of GID %s was found in the database";
Copy link
Contributor

Choose a reason for hiding this comment

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

missingGID and pedigreeAlreadyExists could be private.

Copy link
Member

Choose a reason for hiding this comment

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

There are some other pre-exisiting ones that could be as well. Would you mind if they're just all kept public?

Comment on lines +295 to +299
if (!StringUtils.isBlank(existingGermplasm.getPedigree())) {
ValidationError ve = new ValidationError("Pedigree", pedigreeAlreadyExists, HttpStatus.UNPROCESSABLE_ENTITY);
validationErrors.addError(i+2, ve ); // +2 instead of +1 to account for the column header row.
continue;
}
Copy link
Member

Choose a reason for hiding this comment

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

If a germplasm record has pedigree, and I include it in a second upload to append synonyms, it looks like this check would prevent that from happening. Is that correct?

Copy link
Member

Choose a reason for hiding this comment

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

Ya thanks, I updated the logic.

Error conditions:

  • has existing pedigree and file pedigree is different

Valid conditions:

  • no existing pedigree and file different pedigree (not blank though, will fail other validations)
  • existing pedigree and file pedigree same

ImportUpload upload,
Function<List<T>, ApiResponse> putMethod,
Consumer<ImportUpload> progressUpdateMethod) throws ApiException {

Copy link
Member

Choose a reason for hiding this comment

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

If this isn't implemented, you may want to have this method throw an UnsupportedOperationException for now

Copy link
Member

Choose a reason for hiding this comment

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

Added UnsupportedOperationException

@nickpalladino nickpalladino merged commit d3edda1 into develop Apr 19, 2023
@nickpalladino nickpalladino deleted the feature/BI-1473 branch April 19, 2023 20:46
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