Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ public enum Permission {
CAN_UPDATE_ALL_CHECKINS("Update all check-ins, including completed check-ins", "Check-ins"),
CAN_EDIT_SKILL_CATEGORIES("Edit skill categories", "Skill Categories"),
CAN_CREATE_REVIEW_ASSIGNMENTS("Create review assignments", "Reviews"),
CAN_VIEW_REVIEW_ASSIGNMENTS("View review assignments", "Reviews"),;
CAN_VIEW_REVIEW_ASSIGNMENTS("View review assignments", "Reviews"),
CAN_UPDATE_REVIEW_ASSIGNMENTS("Update review assignments", "Reviews"),
CAN_DELETE_REVIEW_ASSIGNMENTS("Delete review assignments", "Reviews");

private final String description;
private final String category;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.objectcomputing.checkins.services.reviews;

import com.objectcomputing.checkins.exceptions.NotFoundException;
import com.objectcomputing.checkins.services.agenda_item.AgendaItem;
import com.objectcomputing.checkins.services.permissions.Permission;
import com.objectcomputing.checkins.services.permissions.RequiredPermission;
import io.micronaut.core.annotation.Nullable;
Expand Down Expand Up @@ -93,4 +92,36 @@ public Mono<HttpResponse<Set<ReviewAssignment>>> findAssignmentsByPeriodId(@NotN
.subscribeOn(Schedulers.fromExecutor(ioExecutorService));
}

/**
* Update an existing {@link ReviewAssignment}.
*
* @param reviewAssignment the updated {@link ReviewAssignment}
* @return a streamable response containing the stored {@link ReviewAssignment}
*/
@RequiredPermission(Permission.CAN_UPDATE_REVIEW_ASSIGNMENTS)
@Put
public Mono<HttpResponse<ReviewAssignment>> update(@Body @Valid ReviewAssignment reviewAssignment, HttpRequest<ReviewAssignment> request) {

return Mono.fromCallable(() -> reviewAssignmentServices.update(reviewAssignment))
.publishOn(Schedulers.fromExecutor(eventLoopGroup))
.map(updatedReviewAssignment -> (HttpResponse<ReviewAssignment>) HttpResponse
.ok()
.headers(headers -> headers.location(URI.create(String.format("%s/%s", request.getPath(), updatedReviewAssignment.getId()))))
.body(updatedReviewAssignment))
.subscribeOn(Schedulers.fromExecutor(ioExecutorService));
}

/**
* Delete a {@link ReviewAssignment}.
*
* @param id the id of the review assignment to be deleted to delete
*/
@RequiredPermission(Permission.CAN_DELETE_REVIEW_ASSIGNMENTS)
@Delete("/{id}")
public HttpResponse<?> deleteReviewAssignment(@NotNull UUID id) {
reviewAssignmentServices.delete(id);
return HttpResponse
.ok();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ public interface ReviewAssignmentServices {
ReviewAssignment save(ReviewAssignment reviewAssignment);
ReviewAssignment findById(UUID id);

ReviewAssignment update(ReviewAssignment reviewAssignment);

void delete(UUID id);
Set<ReviewAssignment> findAllByReviewPeriodIdAndReviewerId(UUID reviewPeriodId, @Nullable UUID reviewerId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

import com.objectcomputing.checkins.exceptions.AlreadyExistsException;
import com.objectcomputing.checkins.exceptions.BadArgException;
import com.objectcomputing.checkins.exceptions.PermissionException;
import com.objectcomputing.checkins.services.memberprofile.MemberProfileRepository;
import io.micronaut.core.annotation.Nullable;
import jakarta.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.validation.constraints.NotNull;
import java.util.HashSet;
Expand All @@ -18,6 +21,8 @@ public class ReviewAssignmentServicesImpl implements ReviewAssignmentServices {

MemberProfileRepository memberProfileRepository;

private static final Logger LOG = LoggerFactory.getLogger(ReviewAssignmentServicesImpl.class);

public ReviewAssignmentServicesImpl(ReviewAssignmentRepository reviewAssignmentRepository, MemberProfileRepository memberProfileRepository) {
this.reviewAssignmentRepository = reviewAssignmentRepository;
this.memberProfileRepository = memberProfileRepository;
Expand Down Expand Up @@ -66,8 +71,26 @@ public Set<ReviewAssignment> findAllByReviewPeriodIdAndReviewerId(UUID reviewPer
return reviewAssignments;
}

@Override
public ReviewAssignment update(ReviewAssignment reviewAssignment) {
LOG.warn(String.format("Updating entity %s", reviewAssignment));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is warn the appropriate method here? That seems to indicate something is wrong.

if (reviewAssignment.getId() != null && reviewAssignmentRepository.findById(reviewAssignment.getId()).isPresent()) {
return reviewAssignmentRepository.update(reviewAssignment);
} else {
throw new BadArgException(String.format("ReviewAssignment %s does not exist, cannot update", reviewAssignment.getId()));
}
}

@Override
public void delete(UUID id) {
if (id != null && reviewAssignmentRepository.findById(id).isPresent()) {
reviewAssignmentRepository.deleteById(id);
} else {
throw new BadArgException(String.format("ReviewAssignment %s does not exist, cannot delete", id));
}
}

public Set<ReviewAssignment> defaultReviewAssignments(UUID reviewPeriodId) {
private Set<ReviewAssignment> defaultReviewAssignments(UUID reviewPeriodId) {
Set<ReviewAssignment> reviewAssignments = new HashSet<>();

memberProfileRepository.findAll().forEach(memberProfile -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ public interface PermissionFixture extends RolePermissionFixture {
Permission.CAN_VIEW_ALL_CHECKINS,
Permission.CAN_UPDATE_ALL_CHECKINS,
Permission.CAN_CREATE_REVIEW_ASSIGNMENTS,
Permission.CAN_VIEW_REVIEW_ASSIGNMENTS
Permission.CAN_VIEW_REVIEW_ASSIGNMENTS,
Permission.CAN_UPDATE_REVIEW_ASSIGNMENTS,
Permission.CAN_DELETE_REVIEW_ASSIGNMENTS
);


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,68 @@ public void testGETFindAssignmentsByPeriodIdWithReviewer() {
}



@Test
public void testPUTUpdateReviewAssignmentWithoutPermissions() {
ReviewAssignment reviewAssignment = createADefaultReviewAssignment();

final HttpRequest<ReviewAssignment> request = HttpRequest.PUT("/", reviewAssignment)
.basicAuth(MEMBER_ROLE, MEMBER_ROLE);
HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class,
() -> client.toBlocking().exchange(request, Map.class));

assertNotNull(responseException.getResponse());
assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus());
}

@Test
public void testPUTUpdateNonexistentReviewAssignment() {
ReviewAssignment reviewAssignment = new ReviewAssignment();
reviewAssignment.setId(UUID.randomUUID());
reviewAssignment.setRevieweeId(UUID.randomUUID());
reviewAssignment.setReviewerId(UUID.randomUUID());

final HttpRequest<ReviewAssignment> request = HttpRequest.
PUT("/", reviewAssignment).basicAuth(ADMIN_ROLE, ADMIN_ROLE);
HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class,
() -> client.toBlocking().exchange(request, Map.class));

assertNotNull(responseException.getResponse());
assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus());
}

@Test
public void testPUTUpdateNullReviewAssignment() {
final HttpRequest<String> request = HttpRequest.PUT("", "").basicAuth(ADMIN_ROLE, ADMIN_ROLE);
HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class,
() -> client.toBlocking().exchange(request, Map.class));

assertNotNull(responseException.getResponse());
assertEquals(HttpStatus.BAD_REQUEST, responseException.getStatus());
}

@Test
void deleteReviewAssignment() {
ReviewAssignment reviewAssignment = createADefaultReviewAssignment();

final HttpRequest<Object> request = HttpRequest.
DELETE(String.format("/%s", reviewAssignment.getId())).basicAuth(ADMIN_ROLE, ADMIN_ROLE);

final HttpResponse<ReviewAssignment> response = client.toBlocking().exchange(request, ReviewAssignment.class);

assertEquals(HttpStatus.OK, response.getStatus());
}

@Test
void deleteReviewAssignmentWithoutPermissions() {
ReviewAssignment reviewAssignment = createADefaultReviewAssignment();

final HttpRequest<Object> request = HttpRequest.
DELETE(String.format("/%s", reviewAssignment.getId())).basicAuth(MEMBER_ROLE, MEMBER_ROLE);
HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class,
() -> client.toBlocking().exchange(request, Map.class));

assertNotNull(responseException.getResponse());
assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus());
}
}