handle hidden users in teams (#1841)

* allow to add disabled users to team
* allow to include disabled users in user-select
* include disabled users when editing team members ONLY if they are already part of team

src/API/TeamController.php

@@ -305,10 +305,6 @@ final class TeamController extends BaseApiController
             throw new NotFoundException('User not found');
         }
 
-        if (!$user->isEnabled()) {
-            throw new BadRequestHttpException('Cannot add disabled user to team');
-        }
-
         if ($user->isInTeam($team)) {
             throw new BadRequestHttpException('User is already member of the team');
         }

src/Form/TeamEditForm.php

@@ -23,6 +23,9 @@ class TeamEditForm extends AbstractType
      */
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
+        /** @var Team|null $team */
+        $team = $options['data'] ?? null;
+
         $builder
             ->add('name', TextType::class, [
                 'label' => 'label.name',
@@ -55,6 +58,8 @@ class TeamEditForm extends AbstractType
                     'title' => 'Team member',
                     'description' => 'Array of team member IDs',
                 ],
+                // make sure that disabled users show up in the result list
+                'include_users' => (null !== $team && $team->getUsers()->count() > 0 ? $team->getUsers()->toArray() : [])
             ])
         ;
     }

src/Form/Type/UserType.php

@@ -11,6 +11,7 @@ namespace App\Form\Type;
 
 use App\Entity\User;
 use App\Repository\Query\UserFormTypeQuery;
+use App\Repository\Query\VisibilityInterface;
 use App\Repository\UserRepository;
 use Symfony\Bridge\Doctrine\Form\Type\EntityType;
 use Symfony\Component\Form\AbstractType;
@@ -34,6 +35,13 @@ class UserType extends AbstractType
                 return $user->getDisplayName();
             },
             'choice_translation_domain' => false,
+            // whether disabled users should be included in the result list
+            'include_disabled' => false,
+            // an array of users, which will always be included in the result list
+            // why? if the base entity could include disabled users, which should not be hidden in/removed from the list
+            // eg. when editing a team that has disabled users, these users would be removed silently
+            // see https://github.com/kevinpapst/kimai2/pull/1841
+            'include_users' => [],
             'documentation' => [
                 'type' => 'integer',
                 'description' => 'User ID',
@@ -45,6 +53,14 @@ class UserType extends AbstractType
                 $query = new UserFormTypeQuery();
                 $query->setUser($options['user']);
 
+                if ($options['include_disabled'] === true) {
+                    $query->setVisibility(VisibilityInterface::SHOW_BOTH);
+                }
+
+                if (!empty($options['include_users'])) {
+                    $query->setUsersAlwaysIncluded($options['include_users']);
+                }
+
                 return $repo->getQueryBuilderForFormType($query);
             };
         });

src/Repository/Query/BaseFormTypeQuery.php

@@ -247,6 +247,17 @@ abstract class BaseFormTypeQuery
         return $this;
     }
 
+    /**
+     * @param Team[] $teams
+     * @return self
+     */
+    public function setTeams(array $teams): self
+    {
+        $this->teams = $teams;
+
+        return $this;
+    }
+
     /**
      * @return Team[]
      */

src/Repository/Query/UserFormTypeQuery.php

@@ -9,9 +9,40 @@
 
 namespace App\Repository\Query;
 
+use App\Entity\User;
+
 /**
- * Can be used for pre-filling form types with the: UserRepository
+ * Can be used to pre-fill form types with: UserRepository::getQueryBuilderForFormType()
  */
 final class UserFormTypeQuery extends BaseFormTypeQuery
 {
+    use VisibilityTrait;
+
+    /**
+     * @var User[]
+     */
+    private $includeUsers = [];
+
+    /**
+     * Sets a list of users which must be included in the result always.
+     *
+     * @param array $users
+     * @return UserFormTypeQuery
+     */
+    public function setUsersAlwaysIncluded(array $users): UserFormTypeQuery
+    {
+        $this->includeUsers = $users;
+
+        return $this;
+    }
+
+    /**
+     * Get the list of users which should always be included in the result.
+     *
+     * @return User[]
+     */
+    public function getUsersAlwaysIncluded(): array
+    {
+        return $this->includeUsers;
+    }
 }

src/Repository/UserRepository.php

@@ -141,8 +141,22 @@ class UserRepository extends EntityRepository implements UserLoaderInterface
     {
         $qb = $this->createQueryBuilder('u');
 
-        $qb->andWhere($qb->expr()->eq('u.enabled', ':enabled'));
-        $qb->setParameter('enabled', true, \PDO::PARAM_BOOL);
+        $or = $qb->expr()->orX();
+
+        if ($query->isShowVisible()) {
+            $or->add($qb->expr()->eq('u.enabled', ':enabled'));
+            $qb->setParameter('enabled', true, \PDO::PARAM_BOOL);
+        }
+
+        $includeAlways = $query->getUsersAlwaysIncluded();
+        if (!empty($includeAlways)) {
+            $or->add($qb->expr()->in('u', ':users'));
+            $qb->setParameter('users', $includeAlways);
+        }
+
+        if ($or->count() > 0) {
+            $qb->andWhere($or);
+        }
 
         $qb->orderBy('u.username', 'ASC');
 

tests/API/TeamControllerTest.php

@@ -249,12 +249,6 @@ class TeamControllerTest extends APIControllerBaseTest
         self::assertEquals(Response::HTTP_BAD_REQUEST, $client->getResponse()->getStatusCode());
         $json = json_decode($client->getResponse()->getContent(), true);
         self::assertEquals('User is already member of the team', $json['message']);
-
-        // cannot add disabled user
-        $this->request($client, '/api/teams/' . $result['id'] . '/members/3', 'POST');
-        self::assertEquals(Response::HTTP_BAD_REQUEST, $client->getResponse()->getStatusCode());
-        $json = json_decode($client->getResponse()->getContent(), true);
-        self::assertEquals('Cannot add disabled user to team', $json['message']);
     }
 
     public function testDeleteMemberAction()

tests/Repository/Query/BaseFormTypeQueryTest.php

@@ -44,12 +44,17 @@ abstract class BaseFormTypeQueryTest extends TestCase
         self::assertEmpty($sut->getTeams());
 
         self::assertInstanceOf(BaseFormTypeQuery::class, $sut->addTeam(new Team()));
-        self::assertEquals(1, \count($sut->getTeams()));
+        self::assertCount(1, $sut->getTeams());
 
         $team = new Team();
         self::assertInstanceOf(BaseFormTypeQuery::class, $sut->addTeam($team));
-        self::assertEquals(1, \count($sut->getTeams()));
+        self::assertCount(1, $sut->getTeams());
         self::assertSame($team, $sut->getTeams()[0]);
+
+        self::assertInstanceOf(BaseFormTypeQuery::class, $sut->setTeams([]));
+        self::assertEmpty($sut->getTeams());
+        self::assertInstanceOf(BaseFormTypeQuery::class, $sut->setTeams([new Team(), new Team()]));
+        self::assertCount(2, $sut->getTeams());
     }
 
     protected function assertActivity(BaseFormTypeQuery $sut)

tests/Repository/Query/UserFormTypeQueryTest.php

@@ -9,6 +9,7 @@
 
 namespace App\Tests\Repository\Query;
 
+use App\Entity\User;
 use App\Repository\Query\UserFormTypeQuery;
 
 /**
@@ -23,4 +24,15 @@ class UserFormTypeQueryTest extends BaseFormTypeQueryTest
 
         $this->assertBaseQuery($sut);
     }
+
+    public function testUsersAreAlwaysIncluded()
+    {
+        $sut = new UserFormTypeQuery();
+
+        $users = [(new User())->setUsername('foo'), new User(), new User()];
+
+        self::assertEquals([], $sut->getUsersAlwaysIncluded());
+        self::assertInstanceOf(UserFormTypeQuery::class, $sut->setUsersAlwaysIncluded($users));
+        self::assertSame($users, $sut->getUsersAlwaysIncluded());
+    }
 }