-
Notifications
You must be signed in to change notification settings - Fork 21
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-105: Create endpoint for removing connection #146
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #146 +/- ##
=============================================
+ Coverage 75.50% 75.72% +0.21%
- Complexity 627 632 +5
=============================================
Files 92 93 +1
Lines 1915 1924 +9
Branches 267 266 -1
=============================================
+ Hits 1446 1457 +11
+ Misses 426 425 -1
+ Partials 43 42 -1 ☔ View full report in Codecov by Sentry. |
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(null); | ||
@Connect | ||
@PostMapping | ||
public boolean connect(@RequestBody @Valid ConnectRequest connectRequest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets return a ResponseEntity here as well, rather than just boolean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a different endpoint, but I'd be glad to refactor this in another issue! The endpoint in this PR is the one with the @DeleteMapping - this line seems to have changed due to reformatting.
@@ -102,7 +99,7 @@ public void connect(ConnectIncomingMessageDTO incoming) { | |||
|
|||
public ConnectOutgoingMessageDTO handleConnectionIntent(String connectionIntent, Long requestingUserId, Long toBeConnectedWithUserId) { | |||
if (connectionIntent == "") { | |||
List<Long> recipients = new ArrayList<>(Arrays.asList(toBeConnectedWithUserId)); | |||
List<Long> recipients = new ArrayList<>(Collections.singletonList(toBeConnectedWithUserId)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reason you used Collections.singletonList rather than Array.asList?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I was experimenting with the Collections.singletonList method and accidentally modified it here. I'll undo this change.
@@ -128,4 +125,16 @@ public ConnectOutgoingMessageDTO handleConnectionIntent(String connectionIntent, | |||
} | |||
return null; | |||
} | |||
|
|||
public boolean removeConnection(ConnectionRemovalRequest connectionRemovalRequest) { | |||
if (Objects.equals(connectionRemovalRequest.requestingUserId, connectionRemovalRequest.connectedWithUserId)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Object.equals() necessary here? Would == work as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the ConnectionRemovalRequest DTO uses the Long wrapper type for the user IDs, they're technically different objects, so I don't think == would work. But if I use the "long" primitive type, then == should also work fine. (Other DTO objects also use Long wrapper types though, so for consistency, using the wrapper type may be better.)
This PR addresses TRIB-105 and creates a DELETE endpoint for removing a connection. The following changes are made: