Sending valid error messages from backend during sign in/ signup to the forntend #201

piotrkosinski137 · 2020-10-23T13:45:21Z

🚀 Description

#199
Sorry for formatting on generated mapstruct files

📄 Motivation and Context
Fixes Issue #199

codecov · 2020-10-23T13:47:09Z

Codecov Report

Merging #201 into master will decrease coverage by 0.04%.
The diff coverage is 60.60%.

@@             Coverage Diff              @@
##             master     #201      +/-   ##
============================================
- Coverage     67.48%   67.44%   -0.05%     
- Complexity      170      171       +1     
============================================
  Files            27       31       +4     
  Lines           732      728       -4     
  Branches         31       29       -2     
============================================
- Hits            494      491       -3     
+ Misses          230      229       -1     
  Partials          8        8

Impacted Files	Coverage Δ	Complexity Δ
...n/java/com/myhome/controllers/ApiErrorHandler.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...java/com/myhome/security/AppUserDetailFetcher.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...ava/com/myhome/security/AppUserDetailsService.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...a/com/myhome/security/AuthenticationException.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...om/myhome/security/MyHomeAuthenticationFilter.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...in/java/com/myhome/controllers/UserController.java	`100.00% <100.00%> (ø)`	`4.00 <0.00> (-3.00)`
...yhome/services/springdatajpa/UserSDJpaService.java	`100.00% <100.00%> (+8.00%)`	`11.00 <6.00> (+2.00)`
...atajpa/exceptions/EmailAlreadyExistsException.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
...pringdatajpa/exceptions/UserNotFoundException.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdeefa5...d22fb4f. Read the comment docs.

jmprathab · 2020-10-29T02:25:19Z

@piotrkosinski1992 Can you please resolve the conflicts?

mslowiak

There are some changes needed described in comments
Rebase is needed to resolve conflicts
Please name PR and commit messages as described in CONTRIBUTTING.MD

mslowiak · 2020-10-31T19:58:33Z

api/src/main/resources/api.yaml

@@ -30,13 +30,6 @@ paths:
      responses:
        '201':
          description: "if user created"
-          content:


mslowiak · 2020-10-31T19:59:45Z

service/src/main/generated/com/myhome/controllers/dto/mapper/CommunityMapperImpl.java

+import javax.annotation.Generated;
+import org.springframework.stereotype.Component;
+
+@Generated(


Why we have those mappers in service/src/main/generated?
They were under service/build/generated and not tracked by git

mslowiak · 2020-10-31T20:01:15Z

service/src/main/java/com/myhome/controllers/ApiErrorHandler.java

+import org.springframework.web.bind.annotation.ControllerAdvice;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+
+@ControllerAdvice


How about RestControllerAdvice?
https://stackoverflow.com/a/43124517/5785871

mslowiak · 2020-10-31T20:06:07Z

service/src/test/java/com/myhome/services/unit/UserSDJpaServiceTest.java

-    assertEquals(response, createdUserDto);
-    assertEquals(0, createdUserDto.getCommunityIds().size());
-    verify(userRepository).findByEmail(request.getEmail());
+    verify(userRepository).save(User.builder()


mslowiak · 2020-10-31T20:06:26Z

service/src/test/java/com/myhome/services/unit/UserSDJpaServiceTest.java


    // when
-    Optional<UserDto> createdUserDto = userService.createUser(request);
+    Exception exception = Assertions.assertThrows(


Let's use static import for assertThrows

mslowiak · 2020-10-31T20:06:39Z

service/src/test/java/com/myhome/services/unit/UserSDJpaServiceTest.java

-    Optional<UserDto> createdUserDto = userService.createUser(request);
+    Exception exception = Assertions.assertThrows(
+        EmailAlreadyExistsException.class,
+        () -> userService.createUser(request));


Let's move ); to the new line

mslowiak · 2020-10-31T20:07:01Z

service/src/test/java/com/myhome/services/unit/UserSDJpaServiceTest.java

@@ -179,11 +188,34 @@ void getUserDetailsNotFound() {
        .willReturn(Optional.empty());

    // when
-    Optional<UserDto> createdUserDto = userService.getUserDetails(USER_ID);
+    Exception exception = Assertions.assertThrows(


Let's use static import

mslowiak · 2020-10-31T20:08:14Z

service/src/main/java/com/myhome/services/springdatajpa/UserSDJpaService.java

-    } else {
-      return Optional.empty();
+  @Override public void createUser(UserDto dto) {
+    verifyEmailUniqueness(dto);


Let's wrap it in optional and use filter/orElse rather than putting that in void methods

mslowiak · 2020-10-31T20:09:26Z

service/src/main/java/com/myhome/services/springdatajpa/UserSDJpaService.java

-  private UserDto createUserInRepository(UserDto request) {
-    User user = userMapper.userDtoToUser(request);
-    User savedUser = userRepository.save(user);
+  private void saveUser(UserDto request) {


Let's return the UserDto from this method and also from the service.
Just ignore it in UserController.

That would:

be easier to tests

will not have magic logic inside void methods

will be more functional approach

mslowiak · 2020-10-31T20:12:07Z

service/src/main/java/com/myhome/controllers/ApiErrorHandler.java

+@ControllerAdvice
+public class ApiErrorHandler {
+
+  @ExceptionHandler(value = { Exception.class })


Let's pass here all the exceptions that we throw in application and create handleException method per each HTTP code that we want to return - we will do not have to manage the http codes in exception descriptions.

Non handled exceptions should be handled as INTERNAL_SERVER_ERROR (500 code)

mslowiak · 2021-02-18T16:20:20Z

Closing - no feedback from contributor

Error handling with some refactoring

ace4b5d

piotrkosinski137 mentioned this pull request Oct 23, 2020

Sending valid error messages from backend during sign in/ signup to the forntend #199

Open

2 tasks

piotrkosinski137 changed the title ~~Issue 199 Sending valid error messages from backend during sign in/ signup to the forntend~~ Sending valid error messages from backend during sign in/ signup to the forntend Oct 23, 2020

piotrkosinski137 added 2 commits October 23, 2020 16:06

Bumping coverage

9e95bdf

Bumping coverage

d22fb4f

jmprathab linked an issue Oct 29, 2020 that may be closed by this pull request

Sending valid error messages from backend during sign in/ signup to the forntend #199

Open

2 tasks

mslowiak requested changes Oct 31, 2020

View reviewed changes

mslowiak assigned piotrkosinski137 Oct 31, 2020

mslowiak added the awaiting-review-changes Awaiting for PR owner to make changes label Oct 31, 2020

piotrkosinski137 removed their assignment Nov 10, 2020

mslowiak closed this Feb 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sending valid error messages from backend during sign in/ signup to the forntend #201

Sending valid error messages from backend during sign in/ signup to the forntend #201

piotrkosinski137 commented Oct 23, 2020

codecov bot commented Oct 23, 2020 •

edited

Loading

jmprathab commented Oct 29, 2020

mslowiak left a comment

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak Oct 31, 2020

mslowiak commented Feb 18, 2021

Sending valid error messages from backend during sign in/ signup to the forntend #201

Sending valid error messages from backend during sign in/ signup to the forntend #201

Conversation

piotrkosinski137 commented Oct 23, 2020

🚀 Description

codecov bot commented Oct 23, 2020 • edited Loading

Codecov Report

jmprathab commented Oct 29, 2020

mslowiak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mslowiak commented Feb 18, 2021

codecov bot commented Oct 23, 2020 •

edited

Loading