jjsm01/kimai2
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

improve team member handling (#3097)

Browse Source
This commit is contained in:
Kevin Papst
2022-01-25 13:44:49 +01:00
committed by GitHub
parent 8694a3bfe4
commit 1e269a6f82
8 changed files with 68 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/API/TeamController.php
View File

@@ -15,7 +15,6 @@ use App\Entity\Activity;
use App\Entity\Customer;
use App\Entity\Project;
use App\Entity\Team;
use App\Entity\TeamMember;
use App\Entity\User;
use App\Form\API\TeamApiEditForm;
use App\Repository\ActivityRepository;
@@ -201,7 +200,7 @@ final class TeamController extends BaseApiController
* Update an existing team
*
* @SWG\Patch(
* description="Update an existing team, you can pass all or just a subset of all attributes (passing users will replace all existing ones)",
* description="Update an existing team, you can pass all or just a subset of all attributes (passing members will replace all existing ones)",
* @SWG\Response(
* response=200,
* description="Returns the updated team",
@@ -235,11 +234,12 @@ final class TeamController extends BaseApiController
throw new NotFoundException();
}
// cache the current memberlist
/** @var TeamMember[] $originalMembers */
$originalMembers = [];
foreach ($team->getMembers() as $member) {
$originalMembers[] = $member;
if ($request->request->has('members')) {
foreach ($team->getMembers() as $member) {
$team->removeMember($member);
$this->repository->removeTeamMember($member);
}
$this->repository->saveTeam($team);
}
$form = $this->createForm(TeamApiEditForm::class, $team);
@@ -254,14 +254,6 @@ final class TeamController extends BaseApiController
return $this->viewHandler->handle($view);
}
// and now remove the ones, which are not in the list any longer
foreach ($originalMembers as $member) {
if (!$team->hasMember($member)) {
$member->getUser()->removeMembership($member);
$this->repository->removeTeamMember($member);
}
}
$this->repository->saveTeam($team);
$view = new View($team, Response::HTTP_OK);

28
src/Controller/ProfileController.php
View File

@@ -9,7 +9,6 @@
namespace App\Controller;
use App\Entity\TeamMember;
use App\Entity\User;
use App\Entity\UserPreference;
use App\Event\PrepareUserEvent;
@@ -19,7 +18,9 @@ use App\Form\UserPasswordType;
use App\Form\UserPreferencesForm;
use App\Form\UserRolesType;
use App\Form\UserTeamsType;
use App\Repository\TeamRepository;
use App\Repository\TimesheetRepository;
use App\Repository\UserRepository;
use App\Timesheet\TimesheetStatisticService;
use App\User\UserService;
use Doctrine\Common\Collections\ArrayCollection;
@@ -78,15 +79,13 @@ final class ProfileController extends AbstractController
* @Route(path="/{username}/edit", name="user_profile_edit", methods={"GET", "POST"})
* @Security("is_granted('edit', profile)")
*/
public function editAction(User $profile, Request $request): Response
public function editAction(User $profile, Request $request, UserRepository $userRepository): Response
{
$form = $this->createEditForm($profile);
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
$entityManager = $this->getDoctrine()->getManager();
$entityManager->persist($profile);
$entityManager->flush();
$userRepository->saveUser($profile);
$this->flashSuccess('action.update.success');
@@ -152,7 +151,7 @@ final class ProfileController extends AbstractController
* @Route(path="/{username}/roles", name="user_profile_roles", methods={"GET", "POST"})
* @Security("is_granted('roles', profile)")
*/
public function rolesAction(User $profile, Request $request): Response
public function rolesAction(User $profile, Request $request, UserRepository $userRepository): Response
{
$isSuperAdmin = $profile->isSuperAdmin();
@@ -166,9 +165,7 @@ final class ProfileController extends AbstractController
$profile->setSuperAdmin(true);
}
$entityManager = $this->getDoctrine()->getManager();
$entityManager->persist($profile);
$entityManager->flush();
$userRepository->saveUser($profile);
$this->flashSuccess('action.update.success');
@@ -186,7 +183,7 @@ final class ProfileController extends AbstractController
* @Route(path="/{username}/teams", name="user_profile_teams", methods={"GET", "POST"})
* @Security("is_granted('teams', profile)")
*/
public function teamsAction(User $profile, Request $request, UserService $service): Response
public function teamsAction(User $profile, Request $request, UserRepository $userRepository, TeamRepository $teamRepository): Response
{
$originalMembers = new ArrayCollection();
foreach ($profile->getMemberships() as $member) {
@@ -197,18 +194,15 @@ final class ProfileController extends AbstractController
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
$entityManager = $this->getDoctrine()->getManager();
/** @var TeamMember $member */
foreach ($originalMembers as $member) {
if (!$profile->hasMembership($member)) {
$member->getTeam()->removeMember($member);
$entityManager->remove($profile);
$member->setTeam(null);
$member->setUser(null);
$teamRepository->removeTeamMember($member);
}
}
$entityManager->persist($profile);
$entityManager->flush();
$userRepository->saveUser($profile);
$this->flashSuccess('action.update.success');

46
src/Entity/Team.php
View File

@@ -135,22 +135,11 @@ class Team
}
/**
* Indexed by ID to use it within collection type forms.
*
* @return TeamMember[]
* @return Collection<TeamMember>
*/
public function getMembers(): iterable
public function getMembers(): Collection
{
$all = [];
foreach ($this->members as $member) {
if ($member->getId() === null) {
$all[] = $member;
} else {
$all[$member->getId()] = $member;
}
}
return $all;
return $this->members;
}
public function addMember(TeamMember $member): void
@@ -167,12 +156,12 @@ class Team
throw new \InvalidArgumentException('Cannot set foreign team membership');
}
// when using the API an invalid user id does not trigger the validation first, but after calling this method :-(
// when using the API an invalid User ID triggers the validation too late
if ($member->getUser() === null) {
return;
}
if (null !== ($existing = $this->findMember($member))) {
if (null !== $this->findMemberByUser($member->getUser())) {
return;
}
@@ -185,22 +174,11 @@ class Team
return $this->members->contains($member);
}
private function findMember(TeamMember $member): ?TeamMember
{
foreach ($this->members as $oldMember) {
if ($oldMember->getUser() === $member->getUser() && $oldMember->getTeam() === $member->getTeam()) {
return $oldMember;
}
}
return null;
}
private function findMemberByUser(User $user): ?TeamMember
{
foreach ($this->members as $oldMember) {
if ($oldMember->getUser() === $user) {
return $oldMember;
foreach ($this->members as $member) {
if ($member->getUser() === $user) {
return $member;
}
}
@@ -209,12 +187,14 @@ class Team
public function removeMember(TeamMember $member): void
{
if (null === ($existingMember = $this->findMember($member))) {
if (!$this->members->contains($member)) {
return;
}
$this->members->removeElement($existingMember);
$existingMember->getUser()->removeMembership($existingMember);
$this->members->removeElement($member);
$member->getUser()->removeMembership($member);
$member->setTeam(null);
$member->setUser(null);
}
/**

2
src/Entity/TeamMember.php
View File

@@ -107,7 +107,7 @@ class TeamMember
public function __clone()
{
if ($this->id !== null) {
$id = null;
$this->id = null;
}
}
}

60
src/Entity/User.php
View File

@@ -579,12 +579,12 @@ class User implements UserInterface, EquatableInterface, \Serializable
throw new \InvalidArgumentException('Cannot set foreign user membership');
}
// when using the API an invalid user id does not trigger the validation first, but after calling this method :-(
// when using the API an invalid Team ID triggers the validation too late
if ($member->getTeam() === null) {
return;
}
if (null !== ($existing = $this->findMember($member))) {
if (null !== $this->findMemberByTeam($member->getTeam())) {
return;
}
@@ -592,33 +592,35 @@ class User implements UserInterface, EquatableInterface, \Serializable
$member->getTeam()->addMember($member);
}
private function findMemberByTeam(Team $team): ?TeamMember
{
foreach ($this->memberships as $member) {
if ($member->getTeam() === $team) {
return $member;
}
}
return null;
}
public function removeMembership(TeamMember $member): void
{
if (null === ($member = $this->findMember($member))) {
if (!$this->memberships->contains($member)) {
return;
}
$this->memberships->removeElement($member);
$member->getUser()->removeMembership($member);
$member->getTeam()->removeMember($member);
$member->setUser(null);
$member->setTeam(null);
}
/**
* Indexed by ID to use it within collection type forms.
*
* @return TeamMember[]
* @return Collection<TeamMember>
*/
public function getMemberships(): iterable
public function getMemberships(): Collection
{
$all = [];
foreach ($this->memberships as $member) {
if ($member->getId() === null) {
$all[] = $member;
} else {
$all[$member->getId()] = $member;
}
}
return $all;
return $this->memberships;
}
public function hasMembership(TeamMember $member): bool
@@ -626,17 +628,6 @@ class User implements UserInterface, EquatableInterface, \Serializable
return $this->memberships->contains($member);
}
private function findMember(TeamMember $member): ?TeamMember
{
foreach ($this->memberships as $oldMember) {
if ($oldMember->getUser() === $member->getUser() && $oldMember->getTeam() === $member->getTeam()) {
return $oldMember;
}
}
return null;
}
/**
* Checks if the user is member of any team.
*
@@ -750,10 +741,8 @@ class User implements UserInterface, EquatableInterface, \Serializable
public function isTeamleadOf(Team $team): bool
{
foreach ($this->memberships as $membership) {
if ($membership->getTeam() === $team) {
return $membership->isTeamlead();
}
if (null !== ($member = $this->findMemberByTeam($team))) {
return $member->isTeamlead();
}
return false;
@@ -830,10 +819,7 @@ class User implements UserInterface, EquatableInterface, \Serializable
return $this->auth === null || $this->auth === self::AUTH_INTERNAL;
}
/**
* {@inheritdoc}
*/
public function addRole($role)
public function addRole(string $role)
{
$role = strtoupper($role);
if ($role === static::DEFAULT_ROLE) {

7
tests/API/TeamControllerTest.php
View File

@@ -170,6 +170,13 @@ class TeamControllerTest extends APIControllerBaseTest
$this->assertNotEmpty($result['id']);
self::assertCount(3, $result['users']);
$this->request($client, '/api/teams/' . $updateId);
$result = json_decode($client->getResponse()->getContent(), true);
$this->assertIsArray($result);
self::assertApiResponseTypeStructure('TeamEntity', $result);
self::assertCount(3, $result['users']);
self::assertFalse($result['members'][1]['teamlead']);
self::assertEquals(1, $result['members'][1]['user']['id']);
self::assertEquals('clara_customer', $result['members'][1]['user']['username']);

3
tests/Entity/TeamTest.php
View File

@@ -92,6 +92,9 @@ class TeamTest extends TestCase
self::assertCount(0, $sut->getMembers());
self::assertFalse($sut->isTeamlead($user));
$member = new TeamMember();
$member->setUser($user);
$sut->addMember($member);
$member->setTeamlead(true);
self::assertTrue($sut->isTeamlead($user));

3
tests/Entity/UserTest.php
View File

@@ -449,6 +449,9 @@ class UserTest extends TestCase
self::assertFalse($sut->isTeamleadOf($team));
$member = new TeamMember();
$member->setTeam($team);
$sut->addMembership($member);
self::assertCount(1, $sut->getMemberships());
self::assertFalse($sut->isTeamleadOf($team));