Bug/bi 1852 by davedrp · Pull Request #294 · Breeding-Insight/bi-api

davedrp · 2023-09-26T19:25:52Z

Description

Story: BI-1852 - Duplicate breeding method can be created

Definition: A Breeding Method is a 'duplicate' if: there already exist a breeding method with the same name or abbreviation

Dependencies

bi-web: bug/BI-1852

Testing

Set up

Select a program for which the current user is a breeder
Go to the Program Administration page
Select the Breeding Methods tab

TEST 1 (create new breeding method)

Select the Create Breeding Method button
Create a new breeding method with the same name or abbreviation as an existing breeding method.

EXPECTED RESULT

An Error banner with the text:
-- A breeding method with the name '<name>' is already defined in the system.
or
-- A breeding method with the abbreviation '<abbreviation>' is already defined in the system.
No new breeding method was created.

Select the Create Breeding Method button
Create a new breeding method with a unique name and abbreviation.

EXPECTED RESULT

A Success banner is displayed
New breeding method was created and displayed.

TEST 2 (edit existing breeding method)

Edit an existing program breeding method.
Modify the name or abbreviation fields to duplicate an existing name or abbreviation .
Select the Save button

EXPECTED RESULT

An Error banner with the text:
-- A breeding method with the name '<name>' is already defined in the system.
or
-- A breeding method with the abbreviation '<abbreviation>' is already defined in the system.
Changes were not saved.

Edit an existing program breeding method.
Modify the name or abbreviation fields be new unique (non-duplicate) values.
Select the Save button

EXPECTED RESULT

A Success banner is displayed
Modified data is listed.

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>

Add unit test

timparsons · 2023-09-27T18:52:30Z

src/main/java/org/breedinginsight/services/impl/BreedingMethodServiceImpl.java

-                && StringUtils.isNotBlank(method.getAbbreviation())
-                && StringUtils.isNotBlank(method.getCategory())
-                && StringUtils.isNotBlank(method.getGeneticDiversity());
+     boolean isMissingRequiredFields(ProgramBreedingMethodEntity method) {


Please add the private modifier to this method

Explicitly declared the method 'protected'. Added comment 'protected' instead of 'private' so it is accessible to unit test.

timparsons · 2023-09-27T18:52:35Z

src/main/java/org/breedinginsight/services/impl/BreedingMethodServiceImpl.java

+                || StringUtils.isBlank(method.getCategory())
+                || StringUtils.isBlank(method.getGeneticDiversity());
+    }
+    boolean isDuplicateMethodNameFoundOnList(ProgramBreedingMethodEntity method, List<ProgramBreedingMethodEntity> programBreedingMethodEntityList) {


Please add the private modifier to this method

timparsons · 2023-09-27T18:54:24Z

src/main/java/org/breedinginsight/services/impl/BreedingMethodServiceImpl.java

+        else if(testMethod.getName()==null && method.getName()==null ){
+            isDup = true;
+        }


Isn't this not needed because a validation error would have already been thrown for the name not being supplied?

Left it as is to be extra-safe.

timparsons · 2023-09-27T18:54:34Z

src/main/java/org/breedinginsight/services/impl/BreedingMethodServiceImpl.java

+        else if(testMethod.getAbbreviation()==null && method.getAbbreviation()==null ){
+            isDup = true;
+        }


Isn't this not needed because a validation error would have already been thrown for the abbreviation not being supplied?

mlm483

Functionality is good, requested changes around specificity of error messages and some more pedantic code quality things.

src/test/java/org/breedinginsight/services/validators/TraitValidatorUnitTest.java

src/test/java/org/breedinginsight/services/impl/BreedingMethodServiceImplTest.java

src/main/java/org/breedinginsight/api/v1/controller/BreedingMethodController.java

src/main/java/org/breedinginsight/services/impl/BreedingMethodServiceImpl.java

… the linter happy

fixed type-o in test titles Improved error messages

davedrp · 2023-10-03T15:18:25Z

the branch did not get merged to develop

davedrp added 3 commits September 25, 2023 16:51

[BI-1852] check for duplicate breeding methods

7a28a60

[BI-1852] respond with 400 errer with exception's text

24e99f0

Add unit test

[BI-1852]fixed some existing 'Problems' in BreedingMethodController.java

3212e8a

davedrp requested review from a team, mlm483 and timparsons and removed request for a team September 26, 2023 19:25

github-actions bot added the bug Something isn't working label Sep 26, 2023

[BI-1852] fixed error message

177eeb3

davedrp marked this pull request as ready for review September 26, 2023 20:18

[BI-1852] Improved error messages. fixed bug.

cc4ddf6

timparsons requested changes Sep 27, 2023

View reviewed changes

mlm483 requested changes Sep 28, 2023

View reviewed changes

davedrp added 2 commits September 29, 2023 13:53

[BI-1852]changed returned type 'HttpResponse' to 'HttpResponse<?>' to…

45d54d7

… the linter happy

[BI-1852] explisitly declaired 'protected' methods

070e182

fixed type-o in test titles Improved error messages

davedrp requested review from mlm483 and timparsons September 29, 2023 19:18

davedrp assigned timparsons Sep 29, 2023

timparsons approved these changes Sep 29, 2023

View reviewed changes

mlm483 approved these changes Sep 29, 2023

View reviewed changes

Merge branch 'develop' into bug/BI-1852

ed84630

davedrp closed this Oct 3, 2023

davedrp reopened this Oct 3, 2023

davedrp merged commit 7f16bf3 into develop Oct 3, 2023

davedrp deleted the bug/BI-1852 branch October 3, 2023 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug/bi 1852#294

Bug/bi 1852#294
davedrp merged 8 commits intodevelopfrom
bug/BI-1852

davedrp commented Sep 26, 2023 •

edited

Loading

Uh oh!

timparsons Sep 27, 2023

Uh oh!

davedrp Sep 29, 2023

Uh oh!

timparsons Sep 27, 2023

Uh oh!

timparsons Sep 27, 2023

Uh oh!

davedrp Sep 29, 2023

Uh oh!

timparsons Sep 27, 2023

Uh oh!

mlm483 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davedrp commented Oct 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

davedrp commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Testing

Checklist:

Uh oh!

timparsons Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

davedrp Sep 29, 2023

Choose a reason for hiding this comment

Uh oh!

timparsons Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

timparsons Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

davedrp Sep 29, 2023

Choose a reason for hiding this comment

Uh oh!

timparsons Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

mlm483 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davedrp commented Oct 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davedrp commented Sep 26, 2023 •

edited

Loading