Skip to content

BI-2887: Germplasm download error with blank source or breeding method#520

Merged
nickpalladino merged 3 commits into
developfrom
bug/BI-2887
May 29, 2026
Merged

BI-2887: Germplasm download error with blank source or breeding method#520
nickpalladino merged 3 commits into
developfrom
bug/BI-2887

Conversation

@humsika
Copy link
Copy Markdown
Contributor

@humsika humsika commented May 28, 2026

Description

Story: BI-2887

This PR fixes a backend-only regression in germplasm export/download behavior introduced after BI-2789 made Source and Breeding Method optional for records without parent information.

Previously, germplasm downloads could fail when a record had:

  • a blank Breeding Method
  • a blank Source
  • both fields blank

This change makes the export processing null-safe so downloads succeed and blank optional values are exported as blank cells.

Dependencies

bi-api: bug/BI-2887
bi-web: develop

Testing

  • Verified download succeeds in all cases
  • Verified blank optional fields are exported as blank cells
  • Verified export column order is unchanged
  • Verified germplasm-list export preserves list order and entry numbers

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 and/or integration tests to cover this change or tests are not applicable
  • I have commented my code, particularly in hard-to-understand areas
  • I have either updated the source of truth or arranged for update with product owner if needed https://breedinginsight.atlassian.net/wiki/spaces/BI/pages/1559953409/Source+of+Truth

@github-actions github-actions Bot added the bug Something isn't working label May 28, 2026
@humsika humsika requested a review from nickpalladino May 28, 2026 04:51
Copy link
Copy Markdown
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.

Looks good, just had a request on value set in test for clarity. Also updated the acceptance criteria slightly in the card so we don't have to deal with the External UID case when having a blank source and will do that in a future card.

testGermplasm.setAccessionNumber("1");

JsonObject additionalInfo = new JsonObject();
additionalInfo.addProperty(GERMPLASM_BREEDING_METHOD, false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For clarity I think this should be set to a breeding method like "Autopolyploid"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I intentionally excluded the Breeding Method field here because this test is meant to cover the case where that field is not set in the data being downloaded.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like this would be adding the breeding method property and setting the value to false. There is an assertTrue(processedData.get(0).containsKey("Breeding Method")); at the end.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the clarification. That makes sense, I’ll make the changes accordingly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made changes as per the instructions.

@nickpalladino nickpalladino merged commit 1a4143c into develop May 29, 2026
1 check passed
@nickpalladino nickpalladino deleted the bug/BI-2887 branch May 29, 2026 14:21
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.

2 participants