Skip to content

Commit

Permalink
Adding endpoint for reporting all review requests to a teacher at once.
Browse files Browse the repository at this point in the history
  • Loading branch information
krulis-martin committed Jul 27, 2024
1 parent a5218c6 commit 4fa3550
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ public function actionPending(string $id)
}

$this->sendSuccessResponse([
'solutions' => $this->assignmentSolutionViewFactory->getUserSolutionsData($solutions),
'solutions' => $this->assignmentSolutionViewFactory->getSolutionsData($solutions),
'assignments' => $this->assignmentsViewFactory->getAssignments(array_values($assignments)),
]);
}
Expand Down
53 changes: 52 additions & 1 deletion app/V1Module/presenters/AssignmentSolutionsPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
use App\Model\Repository\ReviewComments;
use App\Model\View\AssignmentSolutionSubmissionViewFactory;
use App\Model\View\AssignmentSolutionViewFactory;
use App\Model\View\AssignmentViewFactory;
use App\Model\View\GroupViewFactory;
use App\Model\View\SolutionFilesViewFactory;
use App\Exceptions\ForbiddenRequestException;
use App\Security\ACL\IAssignmentSolutionPermissions;
use App\Security\ACL\IUserPermissions;

/**
* Endpoints for manipulation of assignment solutions
Expand Down Expand Up @@ -62,6 +64,12 @@ class AssignmentSolutionsPresenter extends BasePresenter
*/
public $assignmentSolutionAcl;

/**
* @var IUserPermissions
* @inject
*/
public $userAcl;

/**
* @var SubmissionFailures
* @inject
Expand All @@ -74,6 +82,12 @@ class AssignmentSolutionsPresenter extends BasePresenter
*/
public $evaluationLoadingHelper;

/**
* @var AssignmentViewFactory
* @inject
*/
public $assignmentsViewFactory;

/**
* @var AssignmentSolutionViewFactory
* @inject
Expand Down Expand Up @@ -514,7 +528,10 @@ public function actionSetFlag(string $id, string $flag)

$this->sendSuccessResponse([
"solutions" => array_values($resSolutions),
"stats" => $this->groupViewFactory->getStudentsStats($groupOfSolution, $solution->getSolution()->getAuthor()),
"stats" => $this->groupViewFactory->getStudentsStats(
$groupOfSolution,
$solution->getSolution()->getAuthor()
),
]);
}

Expand Down Expand Up @@ -624,4 +641,38 @@ public function actionEvaluationScoreConfig(string $submissionId)
$scoreConfig = $evaluation !== null ? $evaluation->getScoreConfig() : null;
$this->sendSuccessResponse($scoreConfig);
}

public function checkReviewRequests(string $id)
{
$user = $this->users->findOrThrow($id);
if (!$this->userAcl->canListReviewRequests($user)) {
throw new ForbiddenRequestException("You are not allowed to list all review request of your groups");
}
}

/**
* Return all solutions with reviewRequest flag that given user might need to review
* (is admin/supervisor in corresponding groups).
* Along with that it returns all assignment entities of the corresponding solutions.
* @GET
* @param string $id of the user whose solutions with requested reviews are listed
*/
public function actionReviewRequests(string $id)
{
$user = $this->users->findOrThrow($id);
$solutions = $this->assignmentSolutions->findReviewRequestSolutionsOfTeacher($user);

$assignments = [];
foreach ($solutions as $solution) {
$assignment = $solution->getAssignment();
if ($assignment) {
$assignments[$assignment->getId()] = $assignment;
}
}

$this->sendSuccessResponse([
'solutions' => $this->assignmentSolutionViewFactory->getSolutionsData($solutions),
'assignments' => $this->assignmentsViewFactory->getAssignments(array_values($assignments)),
]);
}
}
4 changes: 2 additions & 2 deletions app/V1Module/presenters/AssignmentsPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ function (AssignmentSolution $solution) {
);

$this->sendSuccessResponse(
$this->assignmentSolutionViewFactory->getUserSolutionsData($solutions, $bestSolutions)
$this->assignmentSolutionViewFactory->getSolutionsData($solutions, $bestSolutions)
);
}

Expand Down Expand Up @@ -683,7 +683,7 @@ function (AssignmentSolution $solution) {
);

$this->sendSuccessResponse(
$this->assignmentSolutionViewFactory->getUserSolutionsData($solutions, $bestSolutions)
$this->assignmentSolutionViewFactory->getSolutionsData($solutions, $bestSolutions)
);
}

Expand Down
2 changes: 1 addition & 1 deletion app/V1Module/presenters/GroupsPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ function (AssignmentSolution $solution) {
);

$this->sendSuccessResponse(
$this->solutionsViewFactory->getUserSolutionsData($solutions, $bestSolutions)
$this->solutionsViewFactory->getSolutionsData($solutions, $bestSolutions)
);
}

Expand Down
1 change: 1 addition & 0 deletions app/V1Module/router/RouterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ private static function createUsersRoutes(string $prefix): RouteList
$router[] = new GetRoute("$prefix/<id>/calendar-tokens", "UserCalendars:userCalendars");
$router[] = new PostRoute("$prefix/<id>/calendar-tokens", "UserCalendars:createCalendar");
$router[] = new GetRoute("$prefix/<id>/pending-reviews", "AssignmentSolutionReviews:pending");
$router[] = new GetRoute("$prefix/<id>/review-requests", "AssignmentSolutions:reviewRequests");
return $router;
}

Expand Down
2 changes: 2 additions & 0 deletions app/V1Module/security/ACL/IUserPermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,6 @@ public function canViewCalendars(User $user): bool;
public function canEditCalendars(User $user): bool;

public function canListPendingReviews(User $user): bool;

public function canListReviewRequests(User $user): bool;
}
2 changes: 2 additions & 0 deletions app/config/permissions.neon
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ permissions:
- viewList
- viewAll
- listPendingReviews
- listReviewRequests

- allow: true
role: scope-plagiarism
Expand Down Expand Up @@ -426,6 +427,7 @@ permissions:
resource: user
actions:
- listPendingReviews
- listReviewRequests
conditions: user.isSameUser

- allow: true
Expand Down
25 changes: 21 additions & 4 deletions app/model/repository/AssignmentSolutions.php
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ public function getSolutionStats(Assignment $assignment, User $user): array
* @param User|null $user if not null, only soliutions of this user will be returned
* @return AssignmentSolution[]
*/
public function getReviewRequestSolutions(Group $group, ?User $user = null): array
public function findReviewRequestSolutions(Group $group, ?User $user = null): array
{
$qb = $this->createQueryBuilder('s')->innerJoin("s.assignment", "a");
$qb->where($qb->expr()->eq("a.group", ":group"))
Expand All @@ -348,15 +348,15 @@ public function getReviewRequestSolutions(Group $group, ?User $user = null): arr
}

/**
* As getReviewRequestSolutions(), but returns the result in associative array structure.
* As findReviewRequestSolutions(), but returns the result in associative array structure.
* @param Group $group from which the solutions are loaded
* @param User|null $user if not null, only soliutions of this user will be returned
* @return array[] solutions in array hierarchy [userId][assignmentId] -> Solution
*/
public function getReviewRequestSolutionsIndexed(Group $group, ?User $user = null): array
public function findReviewRequestSolutionsIndexed(Group $group, ?User $user = null): array
{
$res = [];
foreach ($this->getReviewRequestSolutions($group, $user) as $solution) {
foreach ($this->findReviewRequestSolutions($group, $user) as $solution) {
$uid = $solution->getSolution()->getAuthor()?->getId();
$aid = $solution->getAssignment()?->getId();
if ($uid && $aid) {
Expand All @@ -366,4 +366,21 @@ public function getReviewRequestSolutionsIndexed(Group $group, ?User $user = nul
}
return $res;
}

/**
* Return a list of solutions with review request flag set for given teacher.
* @param User $user who is responsible for the pending reviews (admin/supervisor)
* @return AssignmentSolution[]
*/
public function findReviewRequestSolutionsOfTeacher(User $user): array
{
$qb = $this->createQueryBuilder('s');
$qb->innerJoin("s.assignment", "a")->innerJoin("a.group", "g")->innerJoin("g.memberships", "gm");
$qb->where($qb->expr()->eq("gm.user", ":user"))
->andWhere($qb->expr()->in("gm.type", [ GroupMembership::TYPE_ADMIN, GroupMembership::TYPE_SUPERVISOR ]))
->andWhere($qb->expr()->isNull("s.reviewStartedAt"))
->andWhere($qb->expr()->eq("s.reviewRequest", 1))
->setParameter('user', $user->getId());
return $qb->getQuery()->getResult();
}
}
2 changes: 1 addition & 1 deletion app/model/view/AssignmentSolutionViewFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public function getSolutionData(AssignmentSolution $solution, $bestSolutionsHint
* Otherwise the view factory determines the `isBestSolution` value on its own.
* @return array
*/
public function getUserSolutionsData(array $solutions, $bestSolutionsHints = null)
public function getSolutionsData(array $solutions, $bestSolutionsHints = null)
{
$hint = null;
if ($bestSolutionsHints !== null) {
Expand Down
4 changes: 2 additions & 2 deletions app/model/view/GroupViewFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public function getStudentsStats(Group $group, User $student)
$group->getAssignments()->getValues(),
$student
);
$reviewRequestSolutions = $this->assignmentSolutions->getReviewRequestSolutionsIndexed($group, $student);
$reviewRequestSolutions = $this->assignmentSolutions->findReviewRequestSolutionsIndexed($group, $student);
return $this->getStudentStatsInternal($group, $student, $assignmentSolutions, $reviewRequestSolutions);
}

Expand All @@ -199,7 +199,7 @@ public function getAllStudentsStats(Group $group)
$assignmentSolutions = $this->assignmentSolutions->findBestSolutionsForAssignments(
$group->getAssignments()->getValues()
);
$reviewRequestSolutions = $this->assignmentSolutions->getReviewRequestSolutionsIndexed($group);
$reviewRequestSolutions = $this->assignmentSolutions->findReviewRequestSolutionsIndexed($group);
return array_map(
function ($student) use ($group, $assignmentSolutions, $reviewRequestSolutions) {
$solutions = array_key_exists(
Expand Down
24 changes: 24 additions & 0 deletions tests/Presenters/AssignmentSolutionsPresenter.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,30 @@ class TestAssignmentSolutionsPresenter extends Tester\TestCase
}
)->first();
}

public function testListReviewRequests()
{
PresenterTestHelper::loginDefaultAdmin($this->container);
$user = PresenterTestHelper::getUser($this->container, PresenterTestHelper::ADMIN_LOGIN);

$allSolutions = $this->presenter->assignmentSolutions->findBy([ 'reviewStartedAt' => null ]);
/** @var AssignmentSolution $solution */
$solution = array_pop($allSolutions);
$solution->setReviewRequest();
$this->presenter->assignmentSolutions->persist($solution);

$payload = PresenterTestHelper::performPresenterRequest(
$this->presenter,
'V1:AssignmentSolution',
'GET',
['action' => 'reviewRequests', 'id' => $user->getId() ],
);

Assert::count(1, $payload['solutions']);
Assert::count(1, $payload['assignments']);
Assert::same($solution->getId(), $payload['solutions'][0]['id']);
Assert::same($solution->getAssignment()->getId(), $payload['assignments'][0]['id']);
}
}

(new TestAssignmentSolutionsPresenter())->run();

0 comments on commit 4fa3550

Please sign in to comment.