Skip to content

BI-2045 (Failure to report error when uploading duplicated experiment name)#333

Merged
davedrp merged 7 commits intorelease/0.9from
bug/BI-2045
Mar 5, 2024
Merged

BI-2045 (Failure to report error when uploading duplicated experiment name)#333
davedrp merged 7 commits intorelease/0.9from
bug/BI-2045

Conversation

@davedrp
Copy link
Contributor

@davedrp davedrp commented Feb 19, 2024

Description

BI-2045 (Failure to report error when uploading duplicated experiment name)

Dependencies

Please include any dependencies to other code branches, testing configurations, scripts to be run, etc.

Testing

Successfully import an experiment.
Attempt to import the same experiment file (with no ObsUnitIDs).
EXPECTED RESULT

  • The banner message Error detected in file, XXX.xls. Experiment Title already exists. Import cannot proceed.

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>

@davedrp davedrp requested review from a team, dmeidlin and mlm483 and removed request for a team February 19, 2024 17:54
@github-actions github-actions bot added the bug Something isn't working label Feb 19, 2024
Copy link
Contributor

@mlm483 mlm483 left a comment

Choose a reason for hiding this comment

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

Tested, working.

@davedrp davedrp assigned mlm483 and unassigned dmeidlin Feb 20, 2024
@davedrp
Copy link
Contributor Author

davedrp commented Mar 4, 2024

Please Re-review. There were two things that have been fixed since this PR was approved.

  1. The unit test where failing.
  2. The logic did not match the flowchart in the source of truth. The User was getting an error message even if there was a new Env Name.

@davedrp davedrp requested review from dmeidlin and mlm483 March 4, 2024 17:34
@Test
@SneakyThrows
public void importNewExpMultiNewEnvNoObsSuccess() {
public void importNewExpMultiNewEnvError() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test asserts that both environments import successfully, so it's not clear to me why the test name needs to change to include "Error".

Copy link
Contributor

@dmeidlin dmeidlin left a comment

Choose a reason for hiding this comment

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

Just a couple of small points, otherwise looks good.

Copy link
Contributor

@mlm483 mlm483 left a comment

Choose a reason for hiding this comment

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

Tested, working. 🚀

@davedrp davedrp requested a review from dmeidlin March 4, 2024 20:34
@davedrp davedrp merged commit 1c88caa into release/0.9 Mar 5, 2024
@davedrp davedrp deleted the bug/BI-2045 branch March 5, 2024 14:43
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.

3 participants