fix(api): seperate and fix user add and update endpoint logic

Author: LuukBlankenstijn

webapp/src/Controller/API/UserController.php

@@ -291,12 +291,12 @@ public function singleAction(Request $request, string $id): Response
      * Add a new user.
      */
     #[IsGranted('ROLE_API_WRITER')]
-    #[Rest\Post]
+    #[Rest\Put("")]
     #[OA\RequestBody(
         required: true,
         content: [
             new OA\MediaType(
-                mediaType: 'multipart/form-data',
+                mediaType: 'application/json',
                 schema: new OA\Schema(ref: new Model(type: AddUser::class))
             ),
         ]
@@ -311,19 +311,19 @@ public function addAction(
         AddUser $addUser,
         Request $request
     ): Response {
-        return $this->addOrUpdateUser($addUser, $request);
+        return $this->addUser($addUser, $request);
     }
 
     /**
-     * Update an existing User or create one with the given ID
+     * Update an existing User
      */
     #[IsGranted('ROLE_API_WRITER')]
-    #[Rest\Put('/{id}')]
+    #[Rest\Patch('/{id}')]
     #[OA\RequestBody(
         required: true,
         content: [
             new OA\MediaType(
-                mediaType: 'multipart/form-data',
+                mediaType: 'application/json',
                 schema: new OA\Schema(ref: new Model(type: UpdateUser::class))
             ),
         ]
@@ -336,28 +336,19 @@ public function addAction(
     public function updateAction(
         #[MapRequestPayload(validationFailedStatusCode: Response::HTTP_BAD_REQUEST)]
         UpdateUser $updateUser,
+        string $id,
         Request $request
     ): Response {
-        return $this->addOrUpdateUser($updateUser, $request);
+        return $this->updateUser($updateUser, $id, $request);
     }
 
-    protected function addOrUpdateUser(AddUser $addUser, Request $request): Response
+    protected function addUser(AddUser $addUser, Request $request): Response
     {
-        if ($addUser instanceof UpdateUser && !$addUser->id) {
-            throw new BadRequestHttpException('`id` field is required');
-        }
-
         if ($this->em->getRepository(User::class)->findOneBy(['username' => $addUser->username])) {
             throw new BadRequestHttpException(sprintf("User %s already exists", $addUser->username));
         }
 
         $user = new User();
-        if ($addUser instanceof UpdateUser) {
-            $existingUser = $this->em->getRepository(User::class)->findOneBy(['externalid' => $addUser->id]);
-            if ($existingUser) {
-                $user = $existingUser;
-            }
-        }
         $user
             ->setUsername($addUser->username)
             ->setName($addUser->name)
@@ -366,7 +357,8 @@ protected function addOrUpdateUser(AddUser $addUser, Request $request): Response
             ->setPlainPassword($addUser->password)
             ->setEnabled($addUser->enabled ?? true);
 
-        if ($addUser instanceof UpdateUser) {
+        // if not set let it autogenerate
+        if ($addUser->id) {
             $user->setExternalid($addUser->id);
         }
 
@@ -415,6 +407,76 @@ protected function addOrUpdateUser(AddUser $addUser, Request $request): Response
         return $this->renderCreateData($request, $user, 'user', $user->getUserid());
     }
 
+
+    protected function updateUser(UpdateUser $updateUser, string $id, Request $request): Response
+    {
+        $user = $this->em->getRepository(User::class)->findOneBy(['externalid' => $id]);
+        if (!$user) {
+            throw new BadRequestHttpException(sprintf("User does not exist", $id));
+        }
+        if ($updateUser->username !== null) {
+            $user->setUsername($updateUser->username);
+        }
+        if ($updateUser->name !== null) {
+            $user->setName($updateUser->name);
+        }
+        if ($updateUser->email !== null) {
+            $user->setEmail($updateUser->email);
+        }
+        if ($updateUser->ip !== null) {
+            $user->setIpAddress($updateUser->ip);
+        }
+        if ($updateUser->password !== null) {
+            $user->setPlainPassword($updateUser->password);
+        }
+        if ($updateUser->enabled !== null) {
+            $user->setEnabled($updateUser->enabled);
+        }
+        if ($updateUser->teamId) {
+            /** @var Team|null $team */
+            $team = $this->em->createQueryBuilder()
+                ->from(Team::class, 't')
+                ->select('t')
+                ->andWhere('t.externalid = :team')
+                ->setParameter('team', $updateUser->teamId)
+                ->getQuery()
+                ->getOneOrNullResult();
+
+            if ($team === null) {
+                throw new BadRequestHttpException(sprintf("Team %s not found", $updateUser->teamId));
+            }
+            $user->setTeam($team);
+        }
+
+        $roles = $updateUser->roles;
+        // For the file import we change a CDS user to the roles needed for ICPC CDS.
+        if ($user->getUsername() === 'cds') {
+            $roles = ['cds'];
+        }
+        if (in_array('cds', $roles)) {
+            $roles = ['api_source_reader', 'api_writer', 'api_reader', ...array_diff($roles, ['cds'])];
+        }
+        foreach ($roles as $djRole) {
+            if ($djRole === '') {
+                continue;
+            }
+            if ($djRole === 'judge') {
+                $djRole = 'jury';
+            }
+            $role = $this->em->getRepository(Role::class)->findOneBy(['dj_role' => $djRole]);
+            if ($role === null) {
+                throw new BadRequestHttpException(sprintf("Role %s not found", $djRole));
+            }
+            $user->addUserRole($role);
+        }
+
+        $this->em->persist($user);
+        $this->em->flush();
+        $this->dj->auditlog('user', $user->getUserid(), 'updated');
+
+        return $this->renderCreateData($request, $user, 'user', $user->getUserid());
+    }
+
     protected function getQueryBuilder(Request $request): QueryBuilder
     {
         $queryBuilder = $this->em->createQueryBuilder()

webapp/src/DataTransferObject/AddUser.php

@@ -8,10 +8,9 @@
 #[OA\Schema(required: ['username', 'name', 'roles'])]
 class AddUser
 {
-    /**
-     * @param array<string> $roles
-     */
     public function __construct(
+        #[OA\Property(nullable: true)]
+        public readonly ?string $id,
         public readonly string $username,
         public readonly string $name,
         #[OA\Property(format: 'email', nullable: true)]
@@ -25,6 +24,6 @@ public function __construct(
         #[OA\Property(nullable: true)]
         public readonly ?string $teamId,
         #[Serializer\Type('array<string>')]
-        public readonly array $roles,
+        public readonly array $roles = [],
     ) {}
 }

webapp/src/DataTransferObject/UpdateUser.php

@@ -2,22 +2,30 @@
 
 namespace App\DataTransferObject;
 
+use JMS\Serializer\Annotation as Serializer;
 use OpenApi\Attributes as OA;
 
-#[OA\Schema(required: ['id', 'username', 'name', 'roles'])]
-class UpdateUser extends AddUser
+#[OA\Schema(required: ['roles'])]
+class UpdateUser
 {
     public function __construct(
-        public readonly string $id,
-        string $username,
-        string $name,
-        ?string $email,
-        ?string $ip,
-        ?string $password,
-        ?bool $enabled,
-        ?string $teamId,
-        array $roles
-    ) {
-        parent::__construct($username, $name, $email, $ip, $password, $enabled, $teamId, $roles);
-    }
+        public readonly ?string $id = null,
+        #[OA\Property(nullable: true)]
+        public readonly ?string $username = null,
+        #[OA\Property(nullable: true)]
+        public readonly ?string $name = null,
+        #[OA\Property(format: 'email', nullable: true)]
+        public readonly ?string $email = null,
+        #[OA\Property(nullable: true)]
+        public readonly ?string $ip = null,
+        #[OA\Property(format: 'password', nullable: true)]
+        public readonly ?string $password = null,
+        #[OA\Property(nullable: true)]
+        public readonly ?bool $enabled = null,
+        #[OA\Property(nullable: true)]
+        public readonly ?string $teamId = null,
+        #[OA\Property(nullable: true)]
+        #[Serializer\Type('array<string>')]
+        public readonly array $roles = []
+    ) {}
 }