Implement Jaap his feedback

Author: vmcj

Diff for: webapp/config/packages/security.yaml

@@ -4,7 +4,7 @@ security:
     role_hierarchy:
         ROLE_JURY: [ROLE_CLARIFICATION_RW, ROLE_API, ROLE_API_READER, ROLE_API_SOURCE_READER]
         ROLE_ADMIN: [ROLE_JURY, ROLE_JUDGEHOST, ROLE_API_WRITER,
-                     ROLE_API_PROBLEM_CHANGE, ROLE_API_CONTEST_CHANGE]
+                     ROLE_API_PROBLEM_EDITOR, ROLE_API_CONTEST_EDITOR]
         ROLE_SUPER_ADMIN: [ROLE_ADMIN, ROLE_ALLOWED_TO_SWITCH]
 
     # https://symfony.com/doc/current/security.html#registering-the-user-hashing-passwords

Diff for: webapp/migrations/Version20240629154640.php

@@ -9,12 +9,17 @@
 
 final class Version20240629154640 extends AbstractMigration
 {
-    private const NEW_ROLES = ['api_problem_change' => 'API Problem Changer',
-                               'api_contest_change' => 'API Contest Changer'];
+    private const NEW_ROLES = ['api_problem_editor' => 'API Problem Editor',
+                               'api_contest_editor' => 'API Contest Editor'];
 
     public function getDescription(): string
     {
-        return 'Add new roles to the database.';
+        return "Add new roles to the database.
+                Problem editor can add/delete/edit anything related to problems; files, testcases.
+                Contest editor can add/delete/edit the time & connected problems, but not the files
+                or testcases of those problems.
+                They are a subset of the ADMIN role in the API but not a proper superset of the API_WRITER
+                as that also has access to push teams etc.";
     }
 
     public function up(Schema $schema): void

Diff for: webapp/src/Controller/API/ContestController.php

@@ -74,7 +74,7 @@ public function __construct(
      * Add a new contest.
      * @throws BadRequestHttpException
      */
-    #[IsGranted('ROLE_API_CONTEST_CHANGE')]
+    #[IsGranted('ROLE_API_CONTEST_EDITOR')]
     #[Rest\Post('')]
     #[OA\RequestBody(
         required: true,
@@ -200,7 +200,7 @@ public function bannerAction(Request $request, string $cid): Response
     /**
      * Delete the banner for the given contest.
      */
-    #[IsGranted('ROLE_API_CONTEST_CHANGE')]
+    #[IsGranted('ROLE_API_CONTEST_EDITOR')]
     #[Rest\Delete('/{cid}/banner', name: 'delete_contest_banner')]
     #[OA\Response(response: 204, description: 'Deleting banner succeeded')]
     #[OA\Parameter(ref: '#/components/parameters/cid')]
@@ -220,7 +220,7 @@ public function deleteBannerAction(Request $request, string $cid): Response
     /**
      * Set the banner for the given contest.
      */
-    #[IsGranted('ROLE_API_CONTEST_CHANGE')]
+    #[IsGranted('ROLE_API_CONTEST_EDITOR')]
     #[Rest\Post("/{cid}/banner", name: 'post_contest_banner')]
     #[Rest\Put("/{cid}/banner", name: 'put_contest_banner')]
     #[OA\RequestBody(
@@ -268,7 +268,7 @@ public function setBannerAction(Request $request, string $cid, ValidatorInterfac
     /**
      * Delete the problemset document for the given contest.
      */
-    #[IsGranted('ROLE_API_CONTEST_CHANGE')]
+    #[IsGranted('ROLE_API_CONTEST_EDITOR')]
     #[Rest\Delete('/{cid}/problemset', name: 'delete_contest_problemset')]
     #[OA\Response(response: 204, description: 'Deleting problemset document succeeded')]
     #[OA\Parameter(ref: '#/components/parameters/cid')]
@@ -288,7 +288,7 @@ public function deleteProblemsetAction(Request $request, string $cid): Response
     /**
      * Set the problemset document for the given contest.
      */
-    #[IsGranted('ROLE_API_CONTEST_CHANGE')]
+    #[IsGranted('ROLE_API_CONTEST_EDITOR')]
     #[Rest\Post("/{cid}/problemset", name: 'post_contest_problemset')]
     #[Rest\Put("/{cid}/problemset", name: 'put_contest_problemset')]
     #[OA\RequestBody(
@@ -384,7 +384,7 @@ public function problemsetAction(Request $request, string $cid): Response
      * Change the start time or unfreeze (thaw) time of the given contest.
      * @throws NonUniqueResultException
      */
-    #[IsGranted(new Expression("is_granted('ROLE_API_WRITER') or is_granted('ROLE_API_CONTEST_CHANGE')"))]
+    #[IsGranted(new Expression("is_granted('ROLE_API_WRITER') or is_granted('ROLE_API_CONTEST_EDITOR')"))]
     #[Rest\Patch('/{cid}')]
     #[OA\RequestBody(
         required: true,

Diff for: webapp/src/Controller/API/ProblemController.php

@@ -61,7 +61,7 @@ public function __construct(
      * @throws BadRequestHttpException
      * @throws NonUniqueResultException
      */
-    #[IsGranted('ROLE_API_PROBLEM_CHANGE')]
+    #[IsGranted('ROLE_API_PROBLEM_EDITOR')]
     #[Rest\Post('/add-data')]
     #[OA\RequestBody(
         required: true,
@@ -176,7 +176,7 @@ public function listAction(Request $request): Response
      * @return array{problem_id: string, messages: array<string, string[]>}
      * @throws NonUniqueResultException
      */
-    #[IsGranted('ROLE_API_PROBLEM_CHANGE')]
+    #[IsGranted('ROLE_API_PROBLEM_EDITOR')]
     #[Rest\Post('')]
     #[OA\RequestBody(
         required: true,
@@ -237,7 +237,7 @@ public function addProblemAction(Request $request): array
     /**
      * Unlink a problem from this contest.
      */
-    #[IsGranted('ROLE_API_PROBLEM_CHANGE')]
+    #[IsGranted('ROLE_API_PROBLEM_EDITOR')]
     #[Rest\Delete('/{id}')]
     #[OA\Response(response: 204, description: 'Problem unlinked from contest succeeded')]
     #[OA\Parameter(ref: '#/components/parameters/id')]
@@ -290,7 +290,7 @@ public function unlinkProblemAction(Request $request, string $id): Response
     /**
      * Link an existing problem to this contest.
      */
-    #[IsGranted('ROLE_API_PROBLEM_CHANGE')]
+    #[IsGranted('ROLE_API_PROBLEM_EDITOR')]
     #[Rest\Put('/{id}')]
     #[OA\Response(
         response: 200,

Diff for: webapp/src/DataFixtures/DefaultData/RoleFixture.php

@@ -29,8 +29,8 @@ public function load(ObjectManager $manager): void
             'api_writer'         => 'API writer',
             'api_source_reader'  => 'Source code reader',
             'clarification_rw'   => 'Clarification handler',
-            'api_problem_change' => 'API Problem Changer',
-            'api_contest_change' => 'API Contest Changer'
+            'api_problem_editor' => 'API Problem Editor',
+            'api_contest_editor' => 'API Contest Editor'
         ];
         foreach ($roles as $roleName => $description) {
             if (!($role = $manager->getRepository(Role::class)->findOneBy(['dj_role' => $roleName]))) {

Diff for: webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php

@@ -21,7 +21,7 @@
 class ContestControllerAdminTest extends ContestControllerTest
 {
     protected ?string $apiUser = 'admin';
-    protected static string $testedRole = 'api_contest_change';
+    protected static string $testedRole = 'api_contest_editor';
 
     private function parseSortYaml(string $yamlString): array
     {
@@ -326,7 +326,7 @@ public function provideChangeTimes(): Generator
 
         // Show that this works for both roles
         yield [['id' => 1, 'scoreboard_thaw_time' => '-14 seconds'], 200, 'Demo contest', [], true, true, ['admin']];
-        yield [['id' => 1, 'scoreboard_thaw_time' => '-13 seconds'], 200, 'Demo contest', [], true, true, ['api_contest_change']];
+        yield [['id' => 1, 'scoreboard_thaw_time' => '-13 seconds'], 200, 'Demo contest', [], true, true, ['api_contest_editor']];
     }
 
     /**

Diff for: webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php

@@ -11,7 +11,7 @@
 class ProblemControllerAdminTest extends ProblemControllerTest
 {
     protected ?string $apiUser = 'admin';
-    protected static string $testedRole = 'api_problem_change';
+    protected static string $testedRole = 'api_problem_editor';
 
     protected function setUp(): void
     {