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

Trib 244: bug fix users may not cosign themselves #163

Merged
merged 11 commits into from
Mar 19, 2024
Merged

Trib 244: bug fix users may not cosign themselves #163

merged 11 commits into from
Mar 19, 2024

Conversation

mrsbluerose
Copy link
Collaborator

  • updates cosign service saveCosign() return type to Optional
  • updates Connect API call to cosign service saveCosign() to Optional
  • adds Connect API return for bad request 403 forbidden

TESTING
Cosign service and Connect API

  • updates variable names for clarity (test, mock, expected)
  • updates return values to the new Optional change
  • simplifies assertions with recursive comparison method
  • corrects testing values for save cosign happy path to be different ids
  • adds save cosign sad path for when IDs are the same

QUESTIONS:
This ticket is a stab in the dark. Please let me know if you want changes to the return values for the ConnectAPI save cosign endpoint.

- updates cosign service saveCosign() return type to Optional<CosignDTO>
- updates call to cosign service saveCosignI() to Optional<CosignDTO>
- adds return for bad request 403 forbidden
- updates variable names for clarity (test, mock, expected)
- updates return values to the new Optional<CosignDTO> change
- simplifies assertions with recursive comparison method
- updates variable names for clarity (test, mock, expected)
- updates return values to the new Optional<CosignDTO> change
- adds test for user trying to cosign themselves saveCosignSadPathUserCosignsThemselves()
- adds test saveCosignFailsWhenIdsEqual()
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.89%. Comparing base (9c4e45b) to head (4f0ac89).

Additional details and impacted files
@@                    Coverage Diff                     @@
##             feature/connect-page     #163      +/-   ##
==========================================================
+ Coverage                   79.79%   79.89%   +0.09%     
- Complexity                    529      531       +2     
==========================================================
  Files                          77       77              
  Lines                        1638     1646       +8     
  Branches                      266      267       +1     
==========================================================
+ Hits                         1307     1315       +8     
  Misses                        295      295              
  Partials                       36       36              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@haxwell
Copy link
Contributor

haxwell commented Mar 11, 2024

Looks good so far.. I will run it and test it tomorrow.


if(opt.isEmpty()) {
log.error("Users may not cosign themselves. ");
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

According to a quick Googling, 400 BAD REQUEST, perhaps including the string "Users may not cosign themselves.", is most appropriate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In oder to return a message, I changed the return type from ResponseEntity to ResponseEntity. The endpoint now returns either a CosignDTO or a GenericMessageDTO with a string. Is this what you were envisioning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, I had a couple merge conflicts to fix, and it may make for some strange formatting commits in the cosign service imports. I didn't realize you were working on it and I didn't pull before push. oops.

mrsbluerose and others added 6 commits March 19, 2024 10:28
- updates the saveCosign endpoint to return Response Entity
- updates the return for when the user is trying to cosign themselves to return a 400 BAD_REQUEST and a GenericResponseDTO message.
- updates saveCosignSadPathUserCosignsThemselves to check for the new return values "BAD_REQUEST" and a string message.
@mrsbluerose mrsbluerose requested a review from haxwell March 19, 2024 16:58
@haxwell haxwell merged commit 015fde1 into savvato-software:feature/connect-page Mar 19, 2024
3 checks passed
@mrsbluerose mrsbluerose deleted the TRIB-244 branch March 19, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants