DMC/scipro
7
0
Fork 0
Code Issues43 Pull Requests9 Actions Projects1 Releases Wiki Activity

2934 Fix wrong decisions show up for reviewers

Browse Source
This commit is contained in:
Andreas Svanberg 2023-09-16 12:57:15 +02:00
parent cf810ba724
commit 59ab4e428b
6 changed files with 32 additions and 25 deletions
core/src
main/java/se/su/dsv/scipro/reviewing
MyReviewService.javaReviewerDecisionReminderWorker.javaReviewingServiceImpl.java
test/java/se/su/dsv/scipro/reviewing
ReviewingServiceImplTest.java
view/src
main/java/se/su/dsv/scipro/reviewer
ApprovalReviewerPanel.java
test/java/se/su/dsv/scipro/reviewer
ApprovalReviewerPanelTest.java

6
core/src/main/java/se/su/dsv/scipro/reviewing/MyReviewService.java

@ -8,9 +8,9 @@ import java.util.Collection;
import java.util.List;
public interface MyReviewService {
List<Decision> findAll(Filter filter, final Page<Sort> pageable);
List<ReviewerApproval> findAll(Filter filter);
long count(Filter filter);
List<Decision> findAllDecisions(Filter filter, final Page<Sort> pageable);
List<ReviewerApproval> findAllApprovals(Filter filter);
long countDecisions(Filter filter);
long countUndecidedRoughDraft(User reviewer);
Decision findDecision(Long id);

4
core/src/main/java/se/su/dsv/scipro/reviewing/ReviewerDecisionReminderWorker.java

@ -56,8 +56,8 @@ public class ReviewerDecisionReminderWorker extends AbstractWorker {
@Override
protected void doWork() {
List<ReviewerApproval> reviewerRoughDraftApprovals = myReviewService.findAll(roughDraftApprovalFilter);
List<ReviewerApproval> reviewerFinalSeminarApprovals = myReviewService.findAll(finalSeminarApprovalFilter);
List<ReviewerApproval> reviewerRoughDraftApprovals = myReviewService.findAllApprovals(roughDraftApprovalFilter);
List<ReviewerApproval> reviewerFinalSeminarApprovals = myReviewService.findAllApprovals(finalSeminarApprovalFilter);
Date today = new Date();

28
core/src/main/java/se/su/dsv/scipro/reviewing/ReviewingServiceImpl.java

@ -36,8 +36,8 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
}
@Override
public List<Decision> findAll(final Filter filter, final Page<Sort> pageable) {
BooleanBuilder predicate = fromFilter(filter);
public List<Decision> findAllDecisions(final Filter filter, final Page<Sort> pageable) {
BooleanBuilder predicate = fromFilterForDecision(filter);
LiteralExpression<?> sortProperty = switch (pageable.sort().sortBy()) {
case REQUESTED -> QDecision.decision.requested;
case DEADLINE -> QDecision.decision.deadline;
@ -60,18 +60,26 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
}
@Override
public List<ReviewerApproval> findAll(final Filter filter) {
return findAll(fromFilter2(filter));
public List<ReviewerApproval> findAllApprovals(final Filter filter) {
return findAll(fromFilterForApprovals(filter));
}
@Override
public long count(final Filter filter) {
return count(fromFilter(filter));
public long countDecisions(final Filter filter) {
return from(QDecision.decision)
.join(QDecision.decision.reviewerApproval, QReviewerApproval.reviewerApproval)
.where(fromFilterForDecision(filter))
.select(QDecision.decision.count())
.fetchFirst();
}
@Override
public long countUndecidedRoughDraft(final User reviewer) {
return count(forReviewer(reviewer).and(isUndecided()).and(forStep(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL)));
return from(QDecision.decision)
.join(QDecision.decision.reviewerApproval, QReviewerApproval.reviewerApproval)
.where(forReviewer(reviewer).and(isUndecided()).and(forStep(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL)))
.select(QDecision.decision.count())
.fetchFirst();
}
@Override
@ -126,7 +134,7 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
return QReviewerApproval.reviewerApproval.decisions.any().status.in(statuses);
}
private BooleanBuilder fromFilter(Filter filter) {
private BooleanBuilder fromFilterForDecision(Filter filter) {
BooleanBuilder bb = new BooleanBuilder();
bb.and(forReviewer(filter.getUser()));
bb.and(forStep(filter.getStep()));
@ -137,7 +145,7 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
return bb;
}
private BooleanBuilder fromFilter2(Filter filter) {
private BooleanBuilder fromFilterForApprovals(Filter filter) {
BooleanBuilder bb = new BooleanBuilder();
bb.and(forStep(filter.getStep()));
bb.and(forStatus(filter.getStatuses()));
@ -147,7 +155,7 @@ public class ReviewingServiceImpl extends AbstractServiceImpl<ReviewerApproval,
private static Predicate isUndecided() {
return allOf(
QReviewerApproval.reviewerApproval.decisions.any().status.eq(Status.UNDECIDED),
QDecision.decision.status.eq(Status.UNDECIDED),
QReviewerApproval.reviewerApproval.project.projectStatus.eq(ProjectStatus.ACTIVE)
);
}

10
core/src/test/java/se/su/dsv/scipro/reviewing/ReviewingServiceImplTest.java

@ -58,7 +58,7 @@ public class ReviewingServiceImplTest extends ReviewingModuleTest {
@Test
public void count() {
finalSeminarApprovalService.requestApproval(project, createFileUpload(), "test");
assertEquals(1, reviewingService.count(getFilter(ReviewerApproval.Step.FINAL_SEMINAR_APPROVAL)));
assertEquals(1, reviewingService.countDecisions(getFilter(ReviewerApproval.Step.FINAL_SEMINAR_APPROVAL)));
}
private MyReviewService.Filter getFilter(ReviewerApproval.Step step) {
@ -71,13 +71,13 @@ public class ReviewingServiceImplTest extends ReviewingModuleTest {
@Test
public void find_undecided() {
finalSeminarApprovalService.requestApproval(project, createFileUpload(), "test");
assertThat(reviewingService.findAll(getFilter(ReviewerApproval.Step.FINAL_SEMINAR_APPROVAL), ALL), hasItem(where(Decision::getReviewerApproval, instanceOf(FinalSeminarApproval.class))));
assertThat(reviewingService.findAllDecisions(getFilter(ReviewerApproval.Step.FINAL_SEMINAR_APPROVAL), ALL), hasItem(where(Decision::getReviewerApproval, instanceOf(FinalSeminarApproval.class))));
}
@Test
public void find_undecided_rough_draft_approvals() {
roughDraftApprovalService.requestApproval(project, createFileUpload(), "test");
assertThat(reviewingService.findAll(getFilter(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL), ALL), hasItem(where(Decision::getReviewerApproval, instanceOf(RoughDraftApproval.class))));
assertThat(reviewingService.findAllDecisions(getFilter(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL), ALL), hasItem(where(Decision::getReviewerApproval, instanceOf(RoughDraftApproval.class))));
}
@Test
@ -108,7 +108,7 @@ public class ReviewingServiceImplTest extends ReviewingModuleTest {
roughDraftApprovalService.requestApproval(project, createFileUpload(), "my request");
assertTrue(myRequest.isRight());
List<Decision> requests = reviewingService.findAll(getFilter(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL), ALL);
List<Decision> requests = reviewingService.findAllDecisions(getFilter(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL), ALL);
assertEquals(1, requests.size());
}
@ -125,7 +125,7 @@ public class ReviewingServiceImplTest extends ReviewingModuleTest {
var sortByRequested = new Page.Sort<>(MyReviewService.Sort.TITLE, Page.Direction.DESCENDING);
TestPage page = new TestPage(0, Long.MAX_VALUE, sortByRequested);
List<Decision> requests =
reviewingService.findAll(getFilter(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL), page);
reviewingService.findAllDecisions(getFilter(ReviewerApproval.Step.ROUGH_DRAFT_APPROVAL), page);
assertEquals(2, requests.size());
assertEquals(request2.right(), requests.get(0).getReviewerApproval());

4
view/src/main/java/se/su/dsv/scipro/reviewer/ApprovalReviewerPanel.java

@ -96,12 +96,12 @@ public abstract class ApprovalReviewerPanel extends Panel {
@Override
public Iterator<Decision> iterator(final long first, final long count) {
return myReviewService.findAll(filter.getObject(), new WicketPage<>(first, count, getSort())).iterator();
return myReviewService.findAllDecisions(filter.getObject(), new WicketPage<>(first, count, getSort())).iterator();
}
@Override
public long size() {
return myReviewService.count(filter.getObject());
return myReviewService.countDecisions(filter.getObject());
}
@Override

5
view/src/test/java/se/su/dsv/scipro/reviewer/ApprovalReviewerPanelTest.java

@ -9,7 +9,6 @@ import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.reviewing.MyReviewService;
import se.su.dsv.scipro.reviewing.RoughDraftApproval;
import se.su.dsv.scipro.system.DegreeType;
import se.su.dsv.scipro.system.Page;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.User;
import se.su.dsv.scipro.test.DomainObjects;
@ -32,8 +31,8 @@ public class ApprovalReviewerPanelTest extends SciProTest {
project = createProject();
RoughDraftApproval approval = createApproval();
when(projectService.findOne(project.getId())).thenReturn(project);
when(myReviewService.count(any(MyReviewService.Filter.class))).thenReturn(1L);
when(myReviewService.findAll(any(MyReviewService.Filter.class), any())).thenReturn(Collections.singletonList(approval.getCurrentDecision()));
when(myReviewService.countDecisions(any(MyReviewService.Filter.class))).thenReturn(1L);
when(myReviewService.findAllDecisions(any(MyReviewService.Filter.class), any())).thenReturn(Collections.singletonList(approval.getCurrentDecision()));
when(myReviewService.findDecision(any())).thenReturn(approval.getCurrentDecision());
when(roughDraftApprovalService.findBy(project)).thenReturn(Optional.of(approval));
setLoggedInAs(project.getReviewer());