Merge branch '136-household-reposition-date' into 'master'

Fix bug when repositionning at same date in Household

See merge request Chill-Projet/chill-bundles!584

.changes/unreleased/Fixed-20230829-181332.yaml

@@ -0,0 +1,6 @@
+kind: Fixed
+body: '[household] when moving a person to a sharing position to a not-sharing position
+  on the same household on the same date, remove the previous household membership on the same household. This fix duplicate member.'
+time: 2023-08-29T18:13:32.799479781+02:00
+custom:
+  Issue: "136"

.changes/unreleased/Fixed-20230829-181837.yaml

@@ -0,0 +1,6 @@
+kind: Fixed
+body: |
+  Add missing translation for comment field placeholder in repositionning household editor.
+time: 2023-08-29T18:18:37.691526331+02:00
+custom:
+  Issue: ""

.changes/unreleased/UX-20230829-181733.yaml

@@ -0,0 +1,6 @@
+kind: UX
+body: |
+  Uniformize badge-person in household banner (background, size)
+time: 2023-08-29T18:17:33.190396543+02:00
+custom:
+  Issue: ""

src/Bundle/ChillMainBundle/Tests/Notification/Email/NotificationMailerTest.php

@@ -37,7 +37,6 @@ class NotificationMailerTest extends TestCase
      */
     public function testPostPersistComment(): void
     {
-        $this->expectNotToPerformAssertions();
         $user1 = (new User())->setEmail('user1@foo.com');
         $user2 = (new User())->setEmail('user2@foo.com');
         $user3 = (new User())->setEmail('user3@foo.com');
@@ -67,7 +66,7 @@ class NotificationMailerTest extends TestCase
             }
 
             return false;
-        }));
+        }))->shouldBeCalledTimes(2);
 
         $objectManager = $this->prophesize(EntityManagerInterface::class);
 
@@ -77,7 +76,6 @@ class NotificationMailerTest extends TestCase
 
     public function testPostPersistCommentDestWithNullEmail(): void
     {
-        $this->expectNotToPerformAssertions();
         $user1 = (new User())->setEmail('user1@foo.com');
         $user2 = (new User())->setEmail('user2@foo.com');
         $user3 = (new User())->setEmail(null);
@@ -107,7 +105,7 @@ class NotificationMailerTest extends TestCase
             }
 
             return false;
-        }));
+        }))->shouldBeCalledTimes(1);
 
         $objectManager = $this->prophesize(EntityManagerInterface::class);
 

src/Bundle/ChillPersonBundle/Household/MembersEditor.php

@@ -55,6 +55,14 @@ class MembersEditor
         $this->eventDispatcher = $eventDispatcher;
     }
 
+    /**
+     * Add a person to the household
+     *
+     * The person is added to the household associated with this editor's instance.
+     *
+     * If the person is also a member of another household, or the same household at the same position, the person
+     * is not associated any more with the previous household.
+     */
     public function addMovement(DateTimeImmutable $date, Person $person, ?Position $position, ?bool $holder = false, ?string $comment = null): self
     {
         if (null === $this->household) {
@@ -69,68 +77,66 @@ class MembersEditor
             ->setComment($comment);
         $this->household->addMember($membership);
 
-        if (null !== $position) {
+        if ($membership->getShareHousehold()) {
-            if ($position->getShareHousehold()) {
+            // launch event only if moving to a "share household" position,
-                // launch event only if moving to a "share household" position,
+            // and if the destination household is different than the previous one
-                // and if the destination household is different than the previous one
+            $event = new PersonAddressMoveEvent($person);
-                $event = new PersonAddressMoveEvent($person);
+            $event->setNextMembership($membership);
-                $event->setNextMembership($membership);
 
-                $counter = 0;
+            $counter = 0;
 
-                foreach ($person->getHouseholdParticipationsShareHousehold() as $participation) {
+            foreach ($person->getHouseholdParticipationsShareHousehold() as $participation) {
-                    if ($participation === $membership) {
+                if ($participation === $membership) {
-                        continue;
+                    continue;
-                    }
-
-                    if ($participation->getStartDate() > $membership->getStartDate()) {
-                        continue;
-                    }
-
-                    ++$counter;
-
-                    if ($participation->getEndDate() === null || $participation->getEndDate() > $date) {
-                        $participation->setEndDate($date);
-                        $this->membershipsAffected[] = $participation;
-                        $this->oldMembershipsHashes[] = spl_object_hash($participation);
-
-                        if ($participation->getHousehold() !== $this->household) {
-                            $event->setPreviousMembership($participation);
-                            $this->events[] = $event;
-                        }
-                    }
                 }
 
-                // send also the event if there was no participation before
+                if ($participation->getStartDate() > $membership->getStartDate()) {
-                if (0 === $counter) {
+                    continue;
-                    $this->events[] = $event;
                 }
 
-                foreach ($person->getHouseholdParticipationsNotShareHousehold() as $participation) {
+                ++$counter;
-                    if ($participation->getHousehold() === $this->household
+
-                        && $participation->getEndDate() === null || $participation->getEndDate() > $membership->getStartDate()
+                if ($participation->getEndDate() === null || $participation->getEndDate() > $date) {
-                        && $participation->getStartDate() <= $membership->getStartDate()
+                    $participation->setEndDate($date);
-                    ) {
+                    $this->membershipsAffected[] = $participation;
-                        $participation->setEndDate($membership->getStartDate());
+                    $this->oldMembershipsHashes[] = spl_object_hash($participation);
+
+                    if ($participation->getHousehold() !== $this->household) {
+                        $event->setPreviousMembership($participation);
+                        $this->events[] = $event;
                     }
                 }
-            } else {
+            }
-                // if a members is moved to the same household than the one he belongs to,
+
-                // we should make it leave the household
+            // send also the event if there was no participation before
-                if ($person->getCurrentHousehold($date) === $this->household) {
+            if (0 === $counter) {
-                    $this->leaveMovement($date, $person);
+                $this->events[] = $event;
+            }
+
+            foreach ($person->getHouseholdParticipationsNotShareHousehold() as $participation) {
+                if ($participation->getHousehold() === $this->household
+                    && $participation->getEndDate() === null || $participation->getEndDate() > $membership->getStartDate()
+                    && $participation->getStartDate() <= $membership->getStartDate()
+                ) {
+                    $participation->setEndDate($membership->getStartDate());
+                }
+            }
+        } else {
+            // if there are multiple belongings not sharing household, close the others
+            foreach ($person->getHouseholdParticipations() as $participation) {
+                if ($participation === $membership) {
+                    continue;
                 }
 
-                // if there are multiple belongings not sharing household, close the others
+                if ($participation->getHousehold() === $this->household
-                foreach ($person->getHouseholdParticipationsNotShareHousehold() as $participation) {
+                    && ($participation->getEndDate() === null || $participation->getEndDate() > $membership->getStartDate())
-                    if ($participation === $membership) {
+                    && $participation->getStartDate() <= $membership->getStartDate()
-                        continue;
+                ) {
-                    }
+                    if ($participation->getShareHousehold()) {
-
+                        // if a members is moved to the same household than the one he belongs to,
-                    if ($participation->getHousehold() === $this->household
+                        // we should make it leave the household
-                        && ($participation->getEndDate() === null || $participation->getEndDate() > $membership->getStartDate())
+                        $this->leaveMovement($date, $person);
-                        && $participation->getStartDate() <= $membership->getStartDate()
+                    } else {
-                    ) {
                         $participation->setEndDate($membership->getStartDate());
                     }
                 }
@@ -158,6 +164,15 @@ class MembersEditor
         return null !== $this->household;
     }
 
+    /**
+     * Makes a person leave the household.
+     *
+     * Makes a person leave the household **associated with this editor**.
+     *
+     * @param DateTimeImmutable $date
+     * @param Person $person
+     * @return $this
+     */
     public function leaveMovement(
         DateTimeImmutable $date,
         Person $person
@@ -167,8 +182,9 @@ class MembersEditor
 
         $criteria->where(
             $expr->andX(
-                $expr->lt('startDate', $date),
+                $expr->lte('startDate', $date),
-                $expr->isNull('endDate')
+                $expr->isNull('endDate'),
+                $expr->eq('shareHousehold', true)
             )
         );
 

src/Bundle/ChillPersonBundle/Resources/public/chill/chillperson.scss

@@ -207,10 +207,11 @@ div.banner {
             span.badge-member {
                 flex-shrink: 0; flex-grow: 0; flex-basis: auto;
                 color: $white;
+                background-color: transparentize($white, 0.85);
                 border: 1px solid transparentize($white, 0.75);
                 border-bottom: 3px solid transparentize( shade-color( $chill-green, 20%), 0.3);
                 border-radius: 8px;
-                padding: 0.2em 0.7em;
+                padding: 0.0em 0.5em;
                 margin-bottom: 0.2em;
                 margin-right: 0.3em;
 

src/Bundle/ChillPersonBundle/Resources/public/vuejs/HouseholdMembersEditor/components/PersonComment.vue

@@ -1,7 +1,7 @@
 <template>
   <ckeditor
       name="content"
-      v-bind:placeholder="$t('comment.content')"
+      v-bind:placeholder="$t('household_members_editor.positioning.comment_placeholder')"
       :editor="editor"
       v-model="content"
       tag-name="textarea">

src/Bundle/ChillPersonBundle/Resources/public/vuejs/HouseholdMembersEditor/js/i18n.js

@@ -53,6 +53,7 @@ const appMessages = {
          persons_to_positionnate: 'Usagers à positionner',
          holder: "Titulaire",
          comment: "Commentaire",
+         comment_placeholder: "Associer un commentaire",
        },
       app: {
        next: 'Suivant',

src/Bundle/ChillPersonBundle/Tests/Household/MembersEditorTest.php

@@ -74,6 +74,55 @@ final class MembersEditorTest extends TestCase
         $this->assertContains(null, $endDates);
     }
 
+    /**
+     * We test that a leave move is possible when member startdate is same as current date
+     *
+     */
+    public function testLeaveMovementInSameHouseholdFromShareHouseholdToNotShareHouseholdOnSameDate()
+    {
+        $person = new Person();
+        $household = new Household();
+
+        $factory = $this->buildMembersEditorFactory();
+
+        $positionSharing = (new Position())->setShareHousehold(true);
+        $positionNotSharing = (new Position())->setShareHousehold(false);
+
+        // create add move
+        $editor = $factory->createEditor($household);
+        $editor->addMovement(new DateTimeImmutable('today'), $person, $positionSharing);
+        $editor->postMove();
+
+        self::assertContains($person, $household->getCurrentPersons());
+        self::assertSame($household, $person->getCurrentHousehold());
+        self::assertCount(1, $household->getMembers());
+
+        // create leave move
+        $eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
+        $eventDispatcher
+            ->dispatch(Argument::type(PersonAddressMoveEvent::class))
+            ->shouldBeCalled();
+        $factory = $this->buildMembersEditorFactory(
+            $eventDispatcher->reveal(),
+            null
+        );
+        $editor = $factory->createEditor($household);
+        $editor->addMovement(new DateTimeImmutable('today'), $person, $positionNotSharing);
+        $editor->postMove();
+
+        $participations = $household->getMembers();
+        self::assertCount(2, $participations);
+
+        $sharing = $participations->filter(fn (HouseholdMember $hm) => $hm->getShareHousehold());
+        self::assertCount(1, $sharing);
+
+        $notSharing = $participations->filter(fn (HouseholdMember $hm) => !$hm->getShareHousehold());
+        self::assertCount(1, $notSharing);
+
+        self::assertNotNull($sharing[0]->getEndDate());
+        self::assertEquals(new DateTimeImmutable('today'), $sharing[0]->getEndDate());
+    }
+
     /**
      * We test here a move for a person:.
      *
@@ -98,8 +147,17 @@ final class MembersEditorTest extends TestCase
         $this->assertContains($person, $household->getCurrentPersons());
 
         // we do the move to the position not sharing household
+        $eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
+        $eventDispatcher
+            ->dispatch(Argument::type(PersonAddressMoveEvent::class))
+            ->shouldNotBeCalled();
+        $factory = $this->buildMembersEditorFactory(
+            $eventDispatcher->reveal(),
+            null
+        );
         $editor = $factory->createEditor($household2 = new Household());
         $editor->addMovement(new DateTimeImmutable('yesterday'), $person, $positionNotSharing);
+        $editor->postMove();
 
         $sharings = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => $m->getShareHousehold());
         $notSharing = $household2->getCurrentMembers()->filter(static fn (HouseholdMember $m) => !$m->getShareHousehold());
@@ -118,7 +176,7 @@ final class MembersEditorTest extends TestCase
      * * which was in a position "sharing household"
      * * which move to the same household, in a position "not sharing household"
      */
-    public function testMoveFromSharingHouseholdToNotSharingHousehouldInSamehousehold()
+    public function testMoveFromSharingHouseholdToNotSharingHousehouldInSamehouseholdOnDifferentDate()
     {
         $person = new Person();
         $household = new Household();
@@ -134,8 +192,17 @@ final class MembersEditorTest extends TestCase
         $this->assertContains($person, $household->getCurrentPersons());
 
         // we do the move to the position not sharing household
+        $eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
+        $eventDispatcher
+            ->dispatch(Argument::type(PersonAddressMoveEvent::class))
+            ->shouldBeCalled();
+        $factory = $this->buildMembersEditorFactory(
+            $eventDispatcher->reveal(),
+            null
+        );
         $editor = $factory->createEditor($household);
         $editor->addMovement(new DateTimeImmutable('yesterday'), $person, $positionNotSharing);
+        $editor->postMove();
 
         $sharings = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => $m->getShareHousehold());
         $notSharing = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => !$m->getShareHousehold());
@@ -148,6 +215,84 @@ final class MembersEditorTest extends TestCase
         $this->assertContains($person, $notSharing->map($getPerson));
     }
 
+    /**
+     * We test here a move for a person:.
+     *
+     * * which was in a position "not sharing household"
+     * * which move to the same household, in a position "sharing household"
+     */
+    public function testMoveFromNotSharingHouseholdToSharingHousehouldInSamehousehold()
+    {
+        $person = new Person();
+        $household = new Household();
+        $positionSharing = (new Position())->setShareHousehold(true);
+        $positionNotSharing = (new Position())->setShareHousehold(false);
+        $factory = $this->buildMembersEditorFactory();
+        $editor = $factory->createEditor($household);
+
+        // we add the member to the household, at the position "not sharing"
+        $editor->addMovement(new DateTimeImmutable('1 month ago'), $person, $positionNotSharing);
+
+        // double check that the person is in the household
+        $this->assertContains($person, $household->getCurrentPersons());
+
+        // we do the move to the position sharing household
+        $editor = $factory->createEditor($household);
+        $editor->addMovement(new DateTimeImmutable('yesterday'), $person, $positionSharing);
+
+        self::assertCount(2, $household->getMembers());
+
+        $sharings = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => $m->getShareHousehold());
+        $notSharing = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => !$m->getShareHousehold());
+
+        $this->assertCount(0, $notSharing);
+        $this->assertCount(1, $sharings);
+
+        $getPerson = static fn (HouseholdMember $m) => $m->getPerson();
+
+        $this->assertContains($person, $sharings->map($getPerson));
+        $this->assertNotContains($person, $notSharing->map($getPerson));
+    }
+
+    /**
+     * We test here a move for a person:.
+     *
+     * * which was in a position "not sharing household"
+     * * which move to the same household, in a position "sharing household"
+     */
+    public function testMoveFromNotSharingHouseholdToSharingHousehouldInSamehouseholdOnSameDate()
+    {
+        $person = new Person();
+        $household = new Household();
+        $positionSharing = (new Position())->setShareHousehold(true);
+        $positionNotSharing = (new Position())->setShareHousehold(false);
+        $factory = $this->buildMembersEditorFactory();
+        $editor = $factory->createEditor($household);
+
+        // we add the member to the household, at the position "not sharing"
+        $editor->addMovement(new DateTimeImmutable('today'), $person, $positionNotSharing);
+
+        // double check that the person is in the household
+        $this->assertContains($person, $household->getCurrentPersons());
+
+        // we do the move to the position sharing household
+        $editor = $factory->createEditor($household);
+        $editor->addMovement(new DateTimeImmutable('today'), $person, $positionSharing);
+
+        self::assertCount(2, $household->getMembers());
+
+        $sharings = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => $m->getShareHousehold());
+        $notSharing = $household->getCurrentMembers()->filter(static fn (HouseholdMember $m) => !$m->getShareHousehold());
+
+        $this->assertCount(0, $notSharing);
+        $this->assertCount(1, $sharings);
+
+        $getPerson = static fn (HouseholdMember $m) => $m->getPerson();
+
+        $this->assertContains($person, $sharings->map($getPerson));
+        $this->assertNotContains($person, $notSharing->map($getPerson));
+    }
+
     public function testMovePersonWithoutSharedHousehold()
     {
         $person = new Person();
@@ -235,13 +380,26 @@ final class MembersEditorTest extends TestCase
         $this->assertEquals($date, $membership1->getEndDate());
     }
 
-    public function testPostMoveToAPositionNotSharingHousehold()
+    public function testPostMoveToAPositionNotSharingHouseholdOnSameDay()
     {
         $person = new Person();
-        $position = (new Position())
+        $positionNotSharing = (new Position())
             ->setShareHousehold(false);
+        $positionSharing = (new Position())->setShareHousehold(true);
         $household1 = new Household();
         $household2 = new Household();
+
+        $factory = $this->buildMembersEditorFactory();
+        $editor = $factory->createEditor($household1);
+        $editor->addMovement(new DateTimeImmutable('today'), $person, $positionSharing);
+        $editor->postMove();
+
+        self::assertContains($person, $household1->getCurrentPersons());
+        self::assertContains($person, $household1->getCurrentMembers()
+            ->filter(fn (HouseholdMember $m) => $m->getShareHousehold())
+            ->map(fn (HouseholdMember $m) => $m->getPerson()));
+        self::assertSame($household1, $person->getCurrentHousehold());
+
         $eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
         $eventDispatcher
             ->dispatch(Argument::type(PersonAddressMoveEvent::class))
@@ -250,11 +408,78 @@ final class MembersEditorTest extends TestCase
             $eventDispatcher->reveal(),
             null
         );
-        $editor = $factory->createEditor($household1);
+        $editor = $factory->createEditor($household2);
-
-        $editor->addMovement(new DateTimeImmutable('now'), $person, $position);
 
+        $editor->addMovement(new DateTimeImmutable('today'), $person, $positionNotSharing);
         $editor->postMove();
+
+        // $household1 still contains $person
+        self::assertContains($person, $household1->getCurrentPersons());
+        self::assertContains($person, $household1->getCurrentMembers()
+            ->filter(fn (HouseholdMember $m) => $m->getShareHousehold())
+            ->map(fn (HouseholdMember $m) => $m->getPerson()));
+        self::assertSame($household1, $person->getCurrentHousehold());
+
+        // $household2 contains $person at non-sharing position
+        self::assertContains($person, $household2->getCurrentMembers()
+            ->filter(fn (HouseholdMember $m) => !$m->getShareHousehold())
+            ->map(fn (HouseholdMember $m) => $m->getPerson()));
+        self::assertContains(
+            $household2,
+            $person->getHouseholdParticipationsNotShareHousehold()
+                ->map(fn (HouseholdMember $hm) => $hm->getHousehold())
+        );
+    }
+
+    public function testPostMoveToAPositionNotSharingHouseholdOnDifferentDays()
+    {
+        $person = new Person();
+        $positionNotSharing = (new Position())
+            ->setShareHousehold(false);
+        $positionSharing = (new Position())->setShareHousehold(true);
+        $household1 = new Household();
+        $household2 = new Household();
+
+        $factory = $this->buildMembersEditorFactory();
+        $editor = $factory->createEditor($household1);
+        $editor->addMovement(new DateTimeImmutable('1 year ago'), $person, $positionSharing);
+        $editor->postMove();
+
+        self::assertContains($person, $household1->getCurrentPersons());
+        self::assertContains($person, $household1->getCurrentMembers()
+            ->filter(fn (HouseholdMember $m) => $m->getShareHousehold())
+            ->map(fn (HouseholdMember $m) => $m->getPerson()));
+        self::assertSame($household1, $person->getCurrentHousehold());
+
+        $eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
+        $eventDispatcher
+            ->dispatch(Argument::type(PersonAddressMoveEvent::class))
+            ->shouldNotBeCalled();
+        $factory = $this->buildMembersEditorFactory(
+            $eventDispatcher->reveal(),
+            null
+        );
+        $editor = $factory->createEditor($household2);
+
+        $editor->addMovement(new DateTimeImmutable('yesterday'), $person, $positionNotSharing);
+        $editor->postMove();
+
+        // $household1 still contains $person
+        self::assertContains($person, $household1->getCurrentPersons());
+        self::assertContains($person, $household1->getCurrentMembers()
+            ->filter(fn (HouseholdMember $m) => $m->getShareHousehold())
+            ->map(fn (HouseholdMember $m) => $m->getPerson()));
+        self::assertSame($household1, $person->getCurrentHousehold());
+
+        // $household2 contains $person at non-sharing position
+        self::assertContains($person, $household2->getCurrentMembers()
+            ->filter(fn (HouseholdMember $m) => !$m->getShareHousehold())
+            ->map(fn (HouseholdMember $m) => $m->getPerson()));
+        self::assertContains(
+            $household2,
+            $person->getHouseholdParticipationsNotShareHousehold()
+                ->map(fn (HouseholdMember $hm) => $hm->getHousehold())
+        );
     }
 
     public function testPostMoveToAPositionSharingHouseholdAndSameHousehold()