Optimizing response of solution set-flag endpoint.

ReCodEx · Jul 26, 2024 · ff7fedb · ff7fedb
1 parent 32b2b0a
commit ff7fedb
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 49 deletions.
diff --git a/app/V1Module/presenters/AssignmentSolutionsPresenter.php b/app/V1Module/presenters/AssignmentSolutionsPresenter.php
@@ -21,6 +21,7 @@
 use App\Model\Repository\ReviewComments;
 use App\Model\View\AssignmentSolutionSubmissionViewFactory;
 use App\Model\View\AssignmentSolutionViewFactory;
+use App\Model\View\GroupViewFactory;
 use App\Model\View\SolutionFilesViewFactory;
 use App\Exceptions\ForbiddenRequestException;
 use App\Security\ACL\IAssignmentSolutionPermissions;
@@ -91,6 +92,12 @@ class AssignmentSolutionsPresenter extends BasePresenter
      */
     public $assignmentSolutionSubmissionViewFactory;
 
+    /**
+     * @var GroupViewFactory
+     * @inject
+     */
+    public $groupViewFactory;
+
     /**
      * @var PointsChangedEmailsSender
      * @inject
@@ -436,52 +443,74 @@ public function actionSetFlag(string $id, string $flag)
         $unique = $knownBoolFlags[$flag];
         $value = filter_var($req->getPost("value"), FILTER_VALIDATE_BOOLEAN);
         $oldValue = $solution->getFlag($flag);
-        if ($value !== $oldValue) {
-            // handle unique flags
-            $resetedSolution = null; // remeber original holder of a unique flag (for an email notification)
-            /* @phpstan-ignore-next-line */
-            if ($unique && $value) {
-                // flag has to be set to false for all other solutions of a user
-                $assignmentSolutions = $this->assignmentSolutions->findSolutions(
-                    $solution->getAssignment(),
-                    $solution->getSolution()->getAuthor()
-                );
-                foreach ($assignmentSolutions as $assignmentSolution) {
-                    if ($assignmentSolution->getFlag($flag)) {
-                        $resetedSolution = $assignmentSolution;
-                    }
-                    $assignmentSolution->setFlag($flag, false);
+        if ($value === $oldValue) {
+            $this->sendSuccessResponse(["solutions" => []]);
+            return; // nothing changed
+        }
+
+        // handle unique flags
+        $resetedSolution = null; // remeber original holder of a unique flag (for an email notification)
+        /* @phpstan-ignore-next-line */
+        if ($unique && $value) {
+            // flag has to be set to false for all other solutions of a user
+            $assignmentSolutions = $this->assignmentSolutions->findSolutions(
+                $solution->getAssignment(),
+                $solution->getSolution()->getAuthor()
+            );
+            foreach ($assignmentSolutions as $assignmentSolution) {
+                if ($assignmentSolution->getFlag($flag)) {
+                    $resetedSolution = $assignmentSolution;
                 }
+                $assignmentSolution->setFlag($flag, false);
             }
-            // handle given flag
-            $solution->setFlag($flag, $value);
-
-            // finally flush all changed to the database
-            $this->assignmentSolutions->flush();
-
-            // send notification email
-            $notificationMethod = $flag . 'FlagChanged';
-            if (method_exists($this->solutionFlagChangedEmailSender, $notificationMethod)) {
-                $this->solutionFlagChangedEmailSender->$notificationMethod(
-                    $this->getCurrentUser(),
-                    $solution,
-                    $value,
-                    $resetedSolution
-                );
-            }
+        }
+        // handle given flag
+        $solution->setFlag($flag, $value);
+
+        // finally flush all changed to the database
+        $this->assignmentSolutions->flush();
+
+        // send notification email
+        $notificationMethod = $flag . 'FlagChanged';
+        if (method_exists($this->solutionFlagChangedEmailSender, $notificationMethod)) {
+            $this->solutionFlagChangedEmailSender->$notificationMethod(
+                $this->getCurrentUser(),
+                $solution,
+                $value,
+                $resetedSolution
+            );
         }
 
-        // forward to student statistics of group
+        // assemble response (all entites and stats that may have changed)
+        $assignmentId = $solution->getAssignment()->getId();
         $groupOfSolution = $solution->getAssignment()->getGroup();
         if ($groupOfSolution === null) {
             throw new NotFoundException("Group for assignment '$id' was not found");
         }
 
-        $this->forward(
-            'Groups:studentsStats',
-            $groupOfSolution->getId(),
-            $solution->getSolution()->getAuthor()->getId()
-        );
+        $resSolutions = [ $id => $this->assignmentSolutionViewFactory->getSolutionData($solution) ];
+        if ($resetedSolution) {
+            $resSolutions[$resetedSolution->getId()] =
+                $this->assignmentSolutionViewFactory->getSolutionData($resetedSolution);
+        }
+
+        $resStats = $this->groupViewFactory->getStudentsStats($groupOfSolution, $solution->getSolution()->getAuthor());
+        $assignmentStats = array_filter($resStats['assignments'], function ($a) use ($assignmentId) {
+            $a['id'] === $assignmentId;
+        });
+        if ($assignmentStats) {
+            $bestId = reset($assignmentStats)['bestSolutionId'];
+            if ($bestId) {
+                $resSolutions[$bestId] = $this->assignmentSolutionViewFactory->getSolutionData(
+                    $this->assignmentSolutions->findOrThrow($bestId)
+                );
+            }
+        }
+
+        $this->sendSuccessResponse([
+            "solutions" => array_values($resSolutions),
+            "stats" => $resStats,
+        ]);
     }
 
     public function checkDownloadSolutionArchive(string $id)

diff --git a/tests/Presenters/AssignmentSolutionsPresenter.phpt b/tests/Presenters/AssignmentSolutionsPresenter.phpt
@@ -308,14 +308,17 @@ class TestAssignmentSolutionsPresenter extends Tester\TestCase
             ->andReturn(true)->once();
         $this->presenter->solutionFlagChangedEmailSender = $mockEmailsSender;
 
-        $request = new Nette\Application\Request(
+        $payload = PresenterTestHelper::performPresenterRequest(
+            $this->presenter,
             'V1:AssignmentSolutions',
             'POST',
             ['action' => 'setFlag', 'id' => $solution->getId(), 'flag' => 'accepted'],
             ['value' => true]
         );
-        $response = $this->presenter->run($request);
-        Assert::same(Nette\Application\Responses\ForwardResponse::class, get_class($response));
+
+        Assert::count(2, $payload);
+        Assert::count(1, $payload['solutions']);
+        Assert::same($solution->getId(), $payload['solutions'][0]['id']);
 
         // Check invariants
         $solution = $this->presenter->assignmentSolutions->get($solution->getId());
@@ -345,14 +348,17 @@ class TestAssignmentSolutionsPresenter extends Tester\TestCase
             ->andReturn(true)->once();
         $this->presenter->solutionFlagChangedEmailSender = $mockEmailsSender;
 
-        $request = new Nette\Application\Request(
+        $payload = PresenterTestHelper::performPresenterRequest(
+            $this->presenter,
             'V1:AssignmentSolutions',
             'POST',
             ['action' => 'setFlag', 'id' => $solution->getId(), 'flag' => 'accepted'],
             ['value' => false]
         );
-        $response = $this->presenter->run($request);
-        Assert::same(Nette\Application\Responses\ForwardResponse::class, get_class($response));
+
+        Assert::count(2, $payload);
+        Assert::count(1, $payload['solutions']);
+        Assert::same($solution->getId(), $payload['solutions'][0]['id']);
 
         // Check invariants
         $solution = $this->presenter->assignmentSolutions->get($solution->getId());
@@ -387,14 +393,20 @@ class TestAssignmentSolutionsPresenter extends Tester\TestCase
             ->andReturn(true)->once();
         $this->presenter->solutionFlagChangedEmailSender = $mockEmailsSender;
 
-        $request = new Nette\Application\Request(
+        $payload = PresenterTestHelper::performPresenterRequest(
+            $this->presenter,
             'V1:AssignmentSolutions',
             'POST',
             ['action' => 'setFlag', 'id' => $solution->getId(), 'flag' => 'reviewRequest'],
             ['value' => true]
         );
-        $response = $this->presenter->run($request);
-        Assert::same(Nette\Application\Responses\ForwardResponse::class, get_class($response));
+
+        Assert::count(2, $payload);
+        Assert::count(2, $payload['solutions']);
+        $ids = [$solution->getId(), $anotherSolution->getId()];
+        foreach ($payload['solutions'] as $s) {
+            Assert::true(in_array($s['id'], $ids));
+        }
 
         // Check invariants
         $this->presenter->assignmentSolutions->refresh($solution);
@@ -425,14 +437,17 @@ class TestAssignmentSolutionsPresenter extends Tester\TestCase
             ->andReturn(true)->once();
         $this->presenter->solutionFlagChangedEmailSender = $mockEmailsSender;
 
-        $request = new Nette\Application\Request(
+        $payload = PresenterTestHelper::performPresenterRequest(
+            $this->presenter,
             'V1:AssignmentSolutions',
             'POST',
             ['action' => 'setFlag', 'id' => $solution->getId(), 'flag' => 'reviewRequest'],
             ['value' => false]
         );
-        $response = $this->presenter->run($request);
-        Assert::same(Nette\Application\Responses\ForwardResponse::class, get_class($response));
+
+        Assert::count(2, $payload);
+        Assert::count(1, $payload['solutions']);
+        Assert::same($solution->getId(), $payload['solutions'][0]['id']);
 
         // Check invariants
         $this->presenter->assignmentSolutions->refresh($solution);