FullUserDTO extinction; replaced all with BaseUserDTO
#288
Conversation
Daggerpov
requested review from
ShaneMander
and removed request for
ShaneMander
ShaneMander
left a comment
ShaneMander
left a comment
There was a problem hiding this comment.
Nice stuff, I've just left some comments on methods that can be removed since we no longer have a FullUserDTO
| List<UserDTO> users = userService.getUsersByTagId(tagId); | ||
| List<Map<String, Object>> enrichedUsers = new ArrayList<>(); | ||
|
|
||
| for (UserDTO user : users) { | ||
| Map<String, Object> enrichedUser = new HashMap<>(); | ||
| enrichedUser.put("user", user); | ||
| enrichedUser.put("friends", userService.getFriendsByUserId(user.getId())); | ||
| enrichedUser.put("friendTags", friendTagService.getFriendTagsByOwnerId(user.getId())); | ||
| enrichedUsers.add(enrichedUser); | ||
| } |
There was a problem hiding this comment.
Should probably be service method, no?
There was a problem hiding this comment.
Or replace it with a call to the new UserService::getUserWithFriendsAndTags
| UserDTO getUserById(UUID id); | ||
|
|
||
| FullUserDTO getFullUserById(UUID id); | ||
| BaseUserDTO getFullUserById(UUID id); |
There was a problem hiding this comment.
Can remove this because we already have IUserService::getBaseUserById
| UserDTO saveUserWithProfilePicture(UserDTO user, byte[] profilePicture); | ||
|
|
||
| FullUserDTO getFullUserByEmail(String email); | ||
| BaseUserDTO getFullUserByEmail(String email); |
| BaseUserDTO getFullUserByEmail(String email); | ||
|
|
||
| FullUserDTO getFullUserByUserEntity(User user); | ||
| BaseUserDTO getFullUserByUserEntity(User user); |
|
|
||
| // Helper | ||
| FullUserDTO getFullUserByUser(UserDTO user, Set<UUID> visitedUsers); | ||
| BaseUserDTO getFullUserByUser(UserDTO user, Set<UUID> visitedUsers); |
There was a problem hiding this comment.
This method can be removed and each usage of this method can be replaced with calls to UserMapper::toBaseDTO instead
| BaseUserDTO getFullUserByUser(UserDTO user, Set<UUID> visitedUsers); | ||
|
|
||
| List<FullUserDTO> convertUsersToFullUsers(List<UserDTO> users, Set<UUID> visitedUsers); | ||
| List<BaseUserDTO> convertUsersToFullUsers(List<UserDTO> users, Set<UUID> visitedUsers); |
There was a problem hiding this comment.
Can be removed and can replace old calls to this method with a new UserMapper::toBaseDTOList method
| boolean existsByEmail(String email); | ||
|
|
||
| FullUserDTO getFullUserByUsername(String username); | ||
| BaseUserDTO getFullUserByUsername(String username); |
There was a problem hiding this comment.
Can be removed since you've deleted the only usage
| private final IUserService userService; | ||
| private final IS3Service s3Service; | ||
| private final ILogger logger; | ||
| private final IFriendTagService friendTagService; |
There was a problem hiding this comment.
Can be removed when you replace the logic in UserController::getUsersByFriendTag with the call to UserService::getUserWithFriendsAndTags
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { |
There was a problem hiding this comment.
I might've missed it but where is this being used?
There was a problem hiding this comment.
I think I was getting warnings that classes are supposed to have either none of those two methods, equals() or hashCode() or both.
There was a problem hiding this comment.
java: Not generating hashCode: One of equals or hashCode exists. You should either write both of these or none of these (in the latter case, lombok generates them).
| @Deprecated(since = "Not being used on mobile currently.") | ||
| // full path: /api/v1/users/{id}?full=full | ||
| @GetMapping("{id}") | ||
| public ResponseEntity<AbstractUserDTO> getUser(@PathVariable UUID id, @RequestParam(value = "full", required = false) boolean full) { |
There was a problem hiding this comment.
I feel like this endpoint will become useful, especially if we want to enable functionality like clicking on someone's name from a user's friend list, for example, but it is very straightforward to add back when we need so I won't request a change here.
There was a problem hiding this comment.
Yea, it was just messy with the deprecated full stuff; if we need it, we'll do it again, just as you're saying 👍
#249
As
FullUserDTOisn't being used anywhere in the front-end