Addition of Kudos #2504
Addition of Kudos #2504
Conversation
…alter design of kudos cards, organize pending/approved kudos on Manage Kudos page
Co-authored-by: Jesse Hanner <jahanner7@gmail.com>
…ing/check-ins into feature-2016/refactor-kudos
It isn't used by the front end, and performs no function in the back end
| @Override | ||
| public List<KudosResponseDTO> findByValues(@Nullable UUID recipientId, @Nullable UUID senderId, @Nullable Boolean isPending) { | ||
| if (isPending != null) { | ||
| return findByPending(Boolean.TRUE.equals(isPending)); | ||
| } else if (recipientId != null) { | ||
| return findAllToMember(recipientId); | ||
| } else if (senderId != null) { | ||
| return findAllFromMember(senderId); | ||
| } else { | ||
| if (!currentUserServices.isAdmin()) { | ||
| throw new PermissionException(NOT_AUTHORIZED_MSG); | ||
| } | ||
|
|
||
| return kudosRepository.findAll() | ||
| .stream() | ||
| .map(this::constructKudosResponseDTO) | ||
| .toList(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Not 100% sure does what is expected...
So if isPending is set to true or false, then findByPending is called, and recipientId and senderId are effectively ignored...
I'm not sure if there's a requirement for different behavior, but I just wanted to mention the point
There was a problem hiding this comment.
Yeah this is non-ideal. Not broken from a frontend perspective, but not ideal. I'll likely open an issue for this.
web-ui/src/pages/KudosHomePage.jsx
Outdated
|
|
||
| const loadKudos = useCallback(async () => { | ||
| setKudosLoading(true); | ||
| const res = await getAllKudos(csrf, true); |
There was a problem hiding this comment.
Is this right? We want to be showing isPending=true kudos on the home screen?
| } | ||
|
|
||
| @Put | ||
| @Secured(RoleType.Constants.ADMIN_ROLE) |
There was a problem hiding this comment.
Does this warrant a new permission, or is ADMIN_ROLE sufficient? I can see why having non-admins who can approve Kudos may be useful
There was a problem hiding this comment.
This warrants an issue as well. Pulling into develop as is for now.
No description provided.