From c205bbddd3cca9fc3f37c184bebbf9d82aaad0be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 8 Mar 2021 10:26:12 +0100 Subject: [PATCH 1/5] writing test to reproduce bug --- Tests/Entity/PersonTest.php | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Tests/Entity/PersonTest.php b/Tests/Entity/PersonTest.php index 139c2200b..cffa0e731 100644 --- a/Tests/Entity/PersonTest.php +++ b/Tests/Entity/PersonTest.php @@ -24,6 +24,7 @@ namespace Chill\PersonBundle\Tests\Entity; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\Entity\AccompanyingPeriod; +use Chill\MainBundle\Entity\Address; /** * Unit tests for the person Entity @@ -151,4 +152,47 @@ class PersonTest extends \PHPUnit\Framework\TestCase $this->assertEquals($r['result'], Person::ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD); } + + public function testGetLastAddress() + { + $date0 = (\DateTime::createFromFormat('Y-m-d', '2021-02-05'))->settime(0,0); + $date1 = (\DateTime::createFromFormat('Y-m-d', '2021-03-05'))->settime(0,0); + $date2 = (\DateTime::createFromFormat('Y-m-d', '2021-04-05'))->settime(0,0); + $date3 = (\DateTime::createFromFormat('Y-m-d', '2021-05-05'))->settime(0,0); + $date4 = (\DateTime::createFromFormat('Y-m-d', '2021-06-05'))->settime(0,0); + $date5 = (\DateTime::createFromFormat('Y-m-d', '2021-07-05'))->settime(0,0); + $date6 = (\DateTime::createFromFormat('Y-m-d', '2021-08-05'))->settime(0,0); + $p = new Person($date1); + + $this->assertNull($p->getLastAddress($date1)); + + // add some address + $add1 = (new Address())->setValidFrom($date1); + // no address with date 2 + $add3 = (new Address())->setValidFrom($date3); + // no address with date 4 + $add5 = (new Address())->setValidFrom($date5); + + $p->addAddress($add1); + + // test that, if only one address, that does work: + $this->assertSame($add1, $p->getLastAddress($date1)); + $this->assertSame($add1, $p->getLastAddress($date2)); + $this->assertSame($add1, $p->getLastAddress()); + + // adress before the date should not work + $this->assertNull($p->getLastAddress($date0)); + + // add addresses + $p->addAddress($add3); + $p->addAddress($add5); + + // test retrieval of address + $this->assertSame($add1, $p->getLastAddress($date2)); + //$this->assertSame($add3, $p->getLastAddress($date3)); + dump($p->getLastAddress($date4), $add3); + $this->assertSame($add3, $p->getLastAddress($date4)); + //$this->assertSame($add5, $p->getLastAddress($date5)); + $this->assertSame($add5, $p->getLastAddress($date6)); + } } From 03601b97070e8ea1b65a01a9889d0119f20560c7 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Mon, 22 Mar 2021 12:32:29 +0100 Subject: [PATCH 2/5] Update Person::getLastAddress() based on feedback. --- Entity/AccompanyingPeriod.php | 109 ++++++++------------ Entity/Person.php | 52 +++++----- Tests/Entity/PersonTest.php | 183 +++++++++++++++++----------------- 3 files changed, 165 insertions(+), 179 deletions(-) diff --git a/Entity/AccompanyingPeriod.php b/Entity/AccompanyingPeriod.php index debc0a18d..2992ee8e0 100644 --- a/Entity/AccompanyingPeriod.php +++ b/Entity/AccompanyingPeriod.php @@ -3,7 +3,7 @@ /* * Chill is a software for social workers * - * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, + * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * , * * This program is free software: you can redistribute it and/or modify @@ -25,6 +25,7 @@ namespace Chill\PersonBundle\Entity; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Context\ExecutionContextInterface; use Chill\MainBundle\Entity\User; +use DateTimeImmutable; /** * AccompanyingPeriod @@ -34,111 +35,89 @@ class AccompanyingPeriod /** @var integer */ private $id; - /** @var \DateTime */ - private $openingDate; + private DateTimeImmutable $openingDate; - /** @var \DateTime */ - private $closingDate; + private ?DateTimeImmutable $closingDate = null; /** @var string */ private $remark = ''; /** @var \Chill\PersonBundle\Entity\Person */ private $person; - + /** @var AccompanyingPeriod\ClosingMotive */ private $closingMotive = null; - + /** * The user making the accompanying * * @var User */ private $user; - + /** - * - * @param \DateTime $dateOpening + * + * @param \DateTimeImmutable $dateOpening * @uses AccompanyingPeriod::setClosingDate() */ - public function __construct(\DateTime $dateOpening) { + public function __construct(\DateTimeImmutable $dateOpening) { $this->setOpeningDate($dateOpening); } /** * Get id * - * @return integer + * @return integer */ public function getId() { return $this->id; } - /** - * Set openingDate - * - * @param \DateTime $dateOpening - * @return AccompanyingPeriod - */ - public function setOpeningDate($openingDate) + public function setOpeningDate(DateTimeImmutable $openingDate): self { $this->openingDate = $openingDate; - + return $this; } - /** - * Get openingDate - * - * @return \DateTime - */ - public function getOpeningDate() + public function getOpeningDate(): DateTimeImmutable { return $this->openingDate; } /** * Set closingDate - * - * For closing a Person file, you should use Person::setClosed instead. * - * @param \DateTime $dateClosing - * @return AccompanyingPeriod - * + * For closing a Person file, you should use Person::setClosed instead. */ - public function setClosingDate($closingDate) + public function setClosingDate(DateTimeImmutable $closingDate): self { $this->closingDate = $closingDate; - + return $this; } - /** - * Get closingDate - * - * @return \DateTime - */ - public function getClosingDate() + public function getClosingDate(): ?DateTimeImmutable { return $this->closingDate; } - + /** - * + * * @return boolean */ - public function isOpen(): bool + public function isOpen(): bool { - if ($this->getOpeningDate() > new \DateTime('now')) { + if ($this->getOpeningDate() > new \DateTimeImmutable('now')) { return false; } if ($this->getClosingDate() === null) { return true; - } else { - return false; } + + return false; } /** @@ -152,16 +131,16 @@ class AccompanyingPeriod if ($remark === null) { $remark = ''; } - + $this->remark = $remark; - + return $this; } /** * Get remark * - * @return string + * @return string */ public function getRemark() { @@ -170,7 +149,7 @@ class AccompanyingPeriod /** * Set person. - * + * * For consistency, you should use Person::addAccompanyingPeriod instead. * * @param \Chill\PersonBundle\Entity\Person $person @@ -180,20 +159,20 @@ class AccompanyingPeriod public function setPerson(\Chill\PersonBundle\Entity\Person $person = null) { $this->person = $person; - + return $this; } /** * Get person * - * @return \Chill\PersonBundle\Entity\Person + * @return \Chill\PersonBundle\Entity\Person */ public function getPerson() { return $this->person; } - + public function getClosingMotive() { return $this->closingMotive; @@ -204,13 +183,13 @@ class AccompanyingPeriod $this->closingMotive = $closingMotive; return $this; } - + /** * If the period can be reopened. - * - * This function test if the period is closed and if the period is the last + * + * This function test if the period is closed and if the period is the last * for the associated person - * + * * @return boolean */ public function canBeReOpened() @@ -218,12 +197,12 @@ class AccompanyingPeriod if ($this->isOpen() === true) { return false; } - + $periods = $this->getPerson()->getAccompanyingPeriodsOrdered(); - + return end($periods) === $this; } - + public function reOpen() { $this->setClosingDate(null); @@ -235,29 +214,29 @@ class AccompanyingPeriod if ($this->isOpen()) { return; } - + if (! $this->isClosingAfterOpening()) { $context->buildViolation('The date of closing is before the date of opening') ->atPath('dateClosing') ->addViolation(); } } - + /** * Returns true if the closing date is after the opening date. - * + * * @return boolean */ public function isClosingAfterOpening() { $diff = $this->getOpeningDate()->diff($this->getClosingDate()); - + if ($diff->invert === 0) { return true; } else { return false; } } - + function getUser(): ?User { return $this->user; @@ -266,7 +245,7 @@ class AccompanyingPeriod function setUser(User $user): self { $this->user = $user; - + return $this; } diff --git a/Entity/Person.php b/Entity/Person.php index 9674d8b76..cc581d12d 100644 --- a/Entity/Person.php +++ b/Entity/Person.php @@ -28,7 +28,10 @@ use Chill\PersonBundle\Entity\MaritalStatus; use Doctrine\Common\Collections\ArrayCollection; use Chill\MainBundle\Entity\HasCenterInterface; use Chill\MainBundle\Entity\Address; +use DateTime; +use DateTimeImmutable; use Doctrine\Common\Collections\Criteria; +use Symfony\Component\VarDumper\VarDumper; /** * Person @@ -42,7 +45,7 @@ class Person implements HasCenterInterface { /** @var string The person's last name */ private $lastName; - + /** * * @var \Doctrine\Common\Collections\Collection @@ -119,20 +122,20 @@ class Person implements HasCenterInterface { * @var \Doctrine\Common\Collections\Collection */ private $addresses; - + /** * @var string */ private $fullnameCanonical; - - public function __construct(\DateTime $opening = null) { + + public function __construct(\DateTimeImmutable $opening = null) { $this->accompanyingPeriods = new ArrayCollection(); $this->spokenLanguages = new ArrayCollection(); $this->addresses = new ArrayCollection(); $this->altNames = new ArrayCollection(); if ($opening === null) { - $opening = new \DateTime(); + $opening = new \DateTimeImmutable(); } $this->open(new AccompanyingPeriod($opening)); @@ -331,7 +334,7 @@ class Person implements HasCenterInterface { { return $this->lastName; } - + public function getAltNames(): \Doctrine\Common\Collections\Collection { return $this->altNames; @@ -340,7 +343,7 @@ class Person implements HasCenterInterface { public function setAltNames(\Doctrine\Common\Collections\Collection $altNames) { $this->altNames = $altNames; - + return $this; } @@ -350,20 +353,20 @@ class Person implements HasCenterInterface { $this->altNames->add($altName); $altName->setPerson($this); } - + return $this; } - - public function removeAltName(PersonAltName $altName) + + public function removeAltName(PersonAltName $altName) { if ($this->altNames->contains($altName)) { $altName->setPerson(null); $this->altNames->removeElement($altName); } - + return $this; } - + /** * Set birthdate * @@ -745,27 +748,28 @@ class Person implements HasCenterInterface { * By default, the addresses are ordered by date, descending (the most * recent first) * - * @return \Chill\MainBundle\Entity\Address[] + * @return ArrayCollection */ - public function getAddresses() + public function getAddresses(): ArrayCollection { return $this->addresses; } - public function getLastAddress(\DateTime $date = null) + public function getLastAddress(?DateTimeImmutable $from = null): ?Address { - if ($date === null) { - $date = new \DateTime('now'); - } + $from ??= new DateTimeImmutable('now'); - $addresses = $this->getAddresses(); + $addressesIterator = $this->getAddresses() + ->filter(static fn (Address $address): bool => $address->getValidFrom() <= $from) + ->getIterator(); - if ($addresses == null) { + $addressesIterator->uasort( + static fn (Address $left, Address $right): int => $right->getValidFrom() <=> $left->getValidFrom() + ); - return null; - } - - return $addresses->first(); + return [] === ($addresses = iterator_to_array($addressesIterator)) ? + null : + current($addresses); } /** diff --git a/Tests/Entity/PersonTest.php b/Tests/Entity/PersonTest.php index cffa0e731..a8623aeaa 100644 --- a/Tests/Entity/PersonTest.php +++ b/Tests/Entity/PersonTest.php @@ -2,8 +2,8 @@ /* * Chill is a software for social workers - * - * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, + * + * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * , * * This program is free software: you can redistribute it and/or modify @@ -25,6 +25,10 @@ namespace Chill\PersonBundle\Tests\Entity; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\MainBundle\Entity\Address; +use DateInterval; +use DateTime; +use DateTimeImmutable; +use Generator; /** * Unit tests for the person Entity @@ -32,167 +36,166 @@ use Chill\MainBundle\Entity\Address; class PersonTest extends \PHPUnit\Framework\TestCase { /** - * Test the creation of an accompanying, its closure and the access to + * Test the creation of an accompanying, its closure and the access to * the current accompaniying period via the getCurrentAccompanyingPeriod * function. */ public function testGetCurrentAccompanyingPeriod() { - $d = new \DateTime('yesterday'); + $d = new \DateTimeImmutable('yesterday'); $p = new Person($d); - + $period = $p->getCurrentAccompanyingPeriod(); - + $this->assertInstanceOf('Chill\PersonBundle\Entity\AccompanyingPeriod', $period); $this->assertTrue($period->isOpen()); $this->assertEquals($d, $period->getOpeningDate()); - + //close and test - $period->setClosingDate(new \DateTime('tomorrow')); - + $period->setClosingDate(new \DateTimeImmutable('tomorrow')); + $shouldBeNull = $p->getCurrentAccompanyingPeriod(); $this->assertNull($shouldBeNull); } - + /** * Test if the getAccompanyingPeriodsOrdered function return a list of * periods ordered ascendency. */ public function testAccompanyingPeriodOrderWithUnorderedAccompanyingPeriod() - { - $d = new \DateTime("2013/2/1"); + { + $d = new \DateTimeImmutable("2013/2/1"); $p = new Person($d); - - $e = new \DateTime("2013/3/1"); + + $e = new \DateTimeImmutable("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - - $f = new \DateTime("2013/1/1"); + + $f = new \DateTimeImmutable("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - - $g = new \DateTime("2013/4/1"); + + $g = new \DateTimeImmutable("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - + $r = $p->getAccompanyingPeriodsOrdered(); - + $date = $r[0]->getOpeningDate()->format('Y-m-d'); - + $this->assertEquals($date, '2013-01-01'); } - + /** * Test if the getAccompanyingPeriodsOrdered function, for periods * starting at the same time order regarding to the closing date. */ public function testAccompanyingPeriodOrderSameDateOpening() { - $d = new \DateTime("2013/2/1"); + $d = new \DateTimeImmutable("2013/2/1"); $p = new Person($d); - - $g = new \DateTime("2013/4/1"); + + $g = new \DateTimeImmutable("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - - $f = new \DateTime("2013/2/1"); + + $f = new \DateTimeImmutable("2013/2/1"); $p->open(new AccompanyingPeriod($f)); - - $e = new \DateTime("2013/3/1"); + + $e = new \DateTimeImmutable("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); $r = $p->getAccompanyingPeriodsOrdered(); - + $date = $r[0]->getClosingDate()->format('Y-m-d'); - + $this->assertEquals($date, '2013-03-01'); } - + /** * Test if the function checkAccompanyingPeriodIsNotCovering returns * the good constant when two periods are collapsing : a period * is covering another one : start_1 < start_2 & end_2 < end_1 */ public function testDateCoveringWithCoveringAccompanyingPeriod() { - $d = new \DateTime("2013/2/1"); + $d = new \DateTimeImmutable("2013/2/1"); $p = new Person($d); - - $e = new \DateTime("2013/3/1"); + + $e = new \DateTimeImmutable("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - - $f = new \DateTime("2013/1/1"); + + $f = new \DateTimeImmutable("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - - $g = new \DateTime("2013/4/1"); + + $g = new \DateTimeImmutable("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - + $r = $p->checkAccompanyingPeriodsAreNotCollapsing(); - + $this->assertEquals($r['result'], Person::ERROR_PERIODS_ARE_COLLAPSING); } - + /** * Test if the function checkAccompanyingPeriodIsNotCovering returns * the good constant when two periods are collapsing : a period is open * before an existing period */ public function testNotOpenAFileReOpenedLater() { - $d = new \DateTime("2013/2/1"); + $d = new \DateTimeImmutable("2013/2/1"); $p = new Person($d); - - $e = new \DateTime("2013/3/1"); + + $e = new \DateTimeImmutable("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - - $f = new \DateTime("2013/1/1"); + + $f = new \DateTimeImmutable("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - + $r = $p->checkAccompanyingPeriodsAreNotCollapsing(); - + $this->assertEquals($r['result'], Person::ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD); } - - public function testGetLastAddress() + + public function dateProvider(): Generator { - $date0 = (\DateTime::createFromFormat('Y-m-d', '2021-02-05'))->settime(0,0); - $date1 = (\DateTime::createFromFormat('Y-m-d', '2021-03-05'))->settime(0,0); - $date2 = (\DateTime::createFromFormat('Y-m-d', '2021-04-05'))->settime(0,0); - $date3 = (\DateTime::createFromFormat('Y-m-d', '2021-05-05'))->settime(0,0); - $date4 = (\DateTime::createFromFormat('Y-m-d', '2021-06-05'))->settime(0,0); - $date5 = (\DateTime::createFromFormat('Y-m-d', '2021-07-05'))->settime(0,0); - $date6 = (\DateTime::createFromFormat('Y-m-d', '2021-08-05'))->settime(0,0); - $p = new Person($date1); - - $this->assertNull($p->getLastAddress($date1)); - - // add some address - $add1 = (new Address())->setValidFrom($date1); - // no address with date 2 - $add3 = (new Address())->setValidFrom($date3); - // no address with date 4 - $add5 = (new Address())->setValidFrom($date5); - - $p->addAddress($add1); - - // test that, if only one address, that does work: - $this->assertSame($add1, $p->getLastAddress($date1)); - $this->assertSame($add1, $p->getLastAddress($date2)); - $this->assertSame($add1, $p->getLastAddress()); - - // adress before the date should not work - $this->assertNull($p->getLastAddress($date0)); - - // add addresses - $p->addAddress($add3); - $p->addAddress($add5); - - // test retrieval of address - $this->assertSame($add1, $p->getLastAddress($date2)); - //$this->assertSame($add3, $p->getLastAddress($date3)); - dump($p->getLastAddress($date4), $add3); - $this->assertSame($add3, $p->getLastAddress($date4)); - //$this->assertSame($add5, $p->getLastAddress($date5)); - $this->assertSame($add5, $p->getLastAddress($date6)); + yield [(DateTimeImmutable::createFromFormat('Y-m-d', '2021-01-05'))->settime(0, 0)]; + yield [(DateTimeImmutable::createFromFormat('Y-m-d', '2021-02-05'))->settime(0, 0)]; + yield [(DateTimeImmutable::createFromFormat('Y-m-d', '2021-03-05'))->settime(0, 0)]; + } + + /** + * @dataProvider dateProvider + */ + public function testGetLastAddress(DateTimeImmutable $date) + { + $p = new Person($date); + + // Make sure that there is no last address. + $this->assertNull($p->getLastAddress()); + + // Take an arbitrary date before the $date in parameter. + $addressDate = $date->sub(new DateInterval('PT180M')); + + // 1. Smoke test: Test that the first address added is the last one. + $address1 = (new Address())->setValidFrom($addressDate); + $p->addAddress($address1); + + $this->assertCount(1, $p->getAddresses()); + $this->assertSame($address1, $p->getLastAddress()); + + // 2. Add an older address, which should not be the last address. + $address2 = (new Address())->setValidFrom($addressDate->sub(new DateInterval('PT30M'))); + $p->addAddress($address2); + + $this->assertCount(2, $p->getAddresses()); + $this->assertSame($address1, $p->getLastAddress()); + + // 3. Add a newer address, which should be the last address. + $address3 = (new Address())->setValidFrom($addressDate->add(new DateInterval('PT30M'))); + $p->addAddress($address3); + + $this->assertCount(3, $p->getAddresses()); + $this->assertSame($address3, $p->getLastAddress()); } } From 777fb258607046bd9ba897ebefb380c731566cfd Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 1 Apr 2021 12:04:35 +0200 Subject: [PATCH 3/5] tests: Add missing test based on review's feedback. --- Tests/Entity/PersonTest.php | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/Tests/Entity/PersonTest.php b/Tests/Entity/PersonTest.php index a8623aeaa..960ab169a 100644 --- a/Tests/Entity/PersonTest.php +++ b/Tests/Entity/PersonTest.php @@ -172,7 +172,7 @@ class PersonTest extends \PHPUnit\Framework\TestCase $p = new Person($date); // Make sure that there is no last address. - $this->assertNull($p->getLastAddress()); + $this::assertNull($p->getLastAddress()); // Take an arbitrary date before the $date in parameter. $addressDate = $date->sub(new DateInterval('PT180M')); @@ -181,21 +181,29 @@ class PersonTest extends \PHPUnit\Framework\TestCase $address1 = (new Address())->setValidFrom($addressDate); $p->addAddress($address1); - $this->assertCount(1, $p->getAddresses()); - $this->assertSame($address1, $p->getLastAddress()); + $this::assertCount(1, $p->getAddresses()); + $this::assertSame($address1, $p->getLastAddress()); // 2. Add an older address, which should not be the last address. - $address2 = (new Address())->setValidFrom($addressDate->sub(new DateInterval('PT30M'))); + $addressDate2 = $addressDate->sub(new DateInterval('PT30M')); + $address2 = (new Address())->setValidFrom($addressDate2); $p->addAddress($address2); - $this->assertCount(2, $p->getAddresses()); - $this->assertSame($address1, $p->getLastAddress()); + $this::assertCount(2, $p->getAddresses()); + $this::assertSame($address1, $p->getLastAddress()); // 3. Add a newer address, which should be the last address. - $address3 = (new Address())->setValidFrom($addressDate->add(new DateInterval('PT30M'))); + $addressDate3 = $addressDate->add(new DateInterval('PT30M')); + $address3 = (new Address())->setValidFrom($addressDate3); $p->addAddress($address3); - $this->assertCount(3, $p->getAddresses()); - $this->assertSame($address3, $p->getLastAddress()); + $this::assertCount(3, $p->getAddresses()); + $this::assertSame($address3, $p->getLastAddress()); + + // 4. Get the last address from a specific date. + $this::assertSame($address1, $p->getLastAddress($addressDate)); + $this::assertSame($address2, $p->getLastAddress($addressDate2)); + $this::assertSame($address3, $p->getLastAddress($addressDate3)); } + } From 48e2d2ceaba933b6efc906ce421378a76c4c1050 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Fri, 2 Apr 2021 10:44:21 +0200 Subject: [PATCH 4/5] Do not use DateTimeImmutable. --- Entity/AccompanyingPeriod.php | 39 +++++++++++++++------- Entity/Person.php | 14 ++++---- Tests/Entity/PersonTest.php | 61 +++++++++++++++++------------------ 3 files changed, 64 insertions(+), 50 deletions(-) diff --git a/Entity/AccompanyingPeriod.php b/Entity/AccompanyingPeriod.php index 2992ee8e0..36ce685e1 100644 --- a/Entity/AccompanyingPeriod.php +++ b/Entity/AccompanyingPeriod.php @@ -25,7 +25,6 @@ namespace Chill\PersonBundle\Entity; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Validator\Context\ExecutionContextInterface; use Chill\MainBundle\Entity\User; -use DateTimeImmutable; /** * AccompanyingPeriod @@ -35,9 +34,11 @@ class AccompanyingPeriod /** @var integer */ private $id; - private DateTimeImmutable $openingDate; + /** @var \DateTime */ + private $openingDate; - private ?DateTimeImmutable $closingDate = null; + /** @var \DateTime */ + private $closingDate; /** @var string */ private $remark = ''; @@ -56,11 +57,11 @@ class AccompanyingPeriod private $user; /** - * - * @param \DateTimeImmutable $dateOpening + * + * @param \DateTime $dateOpening * @uses AccompanyingPeriod::setClosingDate() */ - public function __construct(\DateTimeImmutable $dateOpening) { + public function __construct(\DateTime $dateOpening) { $this->setOpeningDate($dateOpening); } @@ -74,14 +75,25 @@ class AccompanyingPeriod return $this->id; } - public function setOpeningDate(DateTimeImmutable $openingDate): self + /** + * Set openingDate + * + * @param \DateTime $dateOpening + * @return AccompanyingPeriod + */ + public function setOpeningDate($openingDate) { $this->openingDate = $openingDate; return $this; } - public function getOpeningDate(): DateTimeImmutable + /** + * Get openingDate + * + * @return \DateTime + */ + public function getOpeningDate() { return $this->openingDate; } @@ -91,14 +103,19 @@ class AccompanyingPeriod * * For closing a Person file, you should use Person::setClosed instead. */ - public function setClosingDate(DateTimeImmutable $closingDate): self + public function setClosingDate($closingDate) { $this->closingDate = $closingDate; return $this; } - public function getClosingDate(): ?DateTimeImmutable + /** + * Get closingDate + * + * @return \DateTime + */ + public function getClosingDate() { return $this->closingDate; } @@ -109,7 +126,7 @@ class AccompanyingPeriod */ public function isOpen(): bool { - if ($this->getOpeningDate() > new \DateTimeImmutable('now')) { + if ($this->getOpeningDate() > new \DateTime('now')) { return false; } diff --git a/Entity/Person.php b/Entity/Person.php index cc581d12d..7a9313ecb 100644 --- a/Entity/Person.php +++ b/Entity/Person.php @@ -22,6 +22,7 @@ namespace Chill\PersonBundle\Entity; * along with this program. If not, see . */ +use ArrayIterator; use Symfony\Component\Validator\Context\ExecutionContextInterface; use Chill\MainBundle\Entity\Country; use Chill\PersonBundle\Entity\MaritalStatus; @@ -29,9 +30,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Chill\MainBundle\Entity\HasCenterInterface; use Chill\MainBundle\Entity\Address; use DateTime; -use DateTimeImmutable; use Doctrine\Common\Collections\Criteria; -use Symfony\Component\VarDumper\VarDumper; /** * Person @@ -128,14 +127,14 @@ class Person implements HasCenterInterface { */ private $fullnameCanonical; - public function __construct(\DateTimeImmutable $opening = null) { + public function __construct(\DateTime $opening = null) { $this->accompanyingPeriods = new ArrayCollection(); $this->spokenLanguages = new ArrayCollection(); $this->addresses = new ArrayCollection(); $this->altNames = new ArrayCollection(); if ($opening === null) { - $opening = new \DateTimeImmutable(); + $opening = new \DateTime(); } $this->open(new AccompanyingPeriod($opening)); @@ -747,18 +746,17 @@ class Person implements HasCenterInterface { /** * By default, the addresses are ordered by date, descending (the most * recent first) - * - * @return ArrayCollection */ public function getAddresses(): ArrayCollection { return $this->addresses; } - public function getLastAddress(?DateTimeImmutable $from = null): ?Address + public function getLastAddress(DateTime $from = null) { - $from ??= new DateTimeImmutable('now'); + $from ??= new DateTime('now'); + /** @var ArrayIterator $addressesIterator */ $addressesIterator = $this->getAddresses() ->filter(static fn (Address $address): bool => $address->getValidFrom() <= $from) ->getIterator(); diff --git a/Tests/Entity/PersonTest.php b/Tests/Entity/PersonTest.php index 960ab169a..a32823d20 100644 --- a/Tests/Entity/PersonTest.php +++ b/Tests/Entity/PersonTest.php @@ -27,7 +27,6 @@ use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\MainBundle\Entity\Address; use DateInterval; use DateTime; -use DateTimeImmutable; use Generator; /** @@ -42,7 +41,7 @@ class PersonTest extends \PHPUnit\Framework\TestCase */ public function testGetCurrentAccompanyingPeriod() { - $d = new \DateTimeImmutable('yesterday'); + $d = new \DateTime('yesterday'); $p = new Person($d); $period = $p->getCurrentAccompanyingPeriod(); @@ -52,7 +51,7 @@ class PersonTest extends \PHPUnit\Framework\TestCase $this->assertEquals($d, $period->getOpeningDate()); //close and test - $period->setClosingDate(new \DateTimeImmutable('tomorrow')); + $period->setClosingDate(new \DateTime('tomorrow')); $shouldBeNull = $p->getCurrentAccompanyingPeriod(); $this->assertNull($shouldBeNull); @@ -64,17 +63,17 @@ class PersonTest extends \PHPUnit\Framework\TestCase */ public function testAccompanyingPeriodOrderWithUnorderedAccompanyingPeriod() { - $d = new \DateTimeImmutable("2013/2/1"); + $d = new \DateTime("2013/2/1"); $p = new Person($d); - $e = new \DateTimeImmutable("2013/3/1"); + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - $f = new \DateTimeImmutable("2013/1/1"); + $f = new \DateTime("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - $g = new \DateTimeImmutable("2013/4/1"); + $g = new \DateTime("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); @@ -90,17 +89,17 @@ class PersonTest extends \PHPUnit\Framework\TestCase * starting at the same time order regarding to the closing date. */ public function testAccompanyingPeriodOrderSameDateOpening() { - $d = new \DateTimeImmutable("2013/2/1"); + $d = new \DateTime("2013/2/1"); $p = new Person($d); - $g = new \DateTimeImmutable("2013/4/1"); + $g = new \DateTime("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - $f = new \DateTimeImmutable("2013/2/1"); + $f = new \DateTime("2013/2/1"); $p->open(new AccompanyingPeriod($f)); - $e = new \DateTimeImmutable("2013/3/1"); + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); @@ -117,17 +116,17 @@ class PersonTest extends \PHPUnit\Framework\TestCase * is covering another one : start_1 < start_2 & end_2 < end_1 */ public function testDateCoveringWithCoveringAccompanyingPeriod() { - $d = new \DateTimeImmutable("2013/2/1"); + $d = new \DateTime("2013/2/1"); $p = new Person($d); - $e = new \DateTimeImmutable("2013/3/1"); + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - $f = new \DateTimeImmutable("2013/1/1"); + $f = new \DateTime("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - $g = new \DateTimeImmutable("2013/4/1"); + $g = new \DateTime("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); @@ -142,14 +141,14 @@ class PersonTest extends \PHPUnit\Framework\TestCase * before an existing period */ public function testNotOpenAFileReOpenedLater() { - $d = new \DateTimeImmutable("2013/2/1"); + $d = new \DateTime("2013/2/1"); $p = new Person($d); - $e = new \DateTimeImmutable("2013/3/1"); + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - $f = new \DateTimeImmutable("2013/1/1"); + $f = new \DateTime("2013/1/1"); $p->open(new AccompanyingPeriod($f)); $r = $p->checkAccompanyingPeriodsAreNotCollapsing(); @@ -159,15 +158,15 @@ class PersonTest extends \PHPUnit\Framework\TestCase public function dateProvider(): Generator { - yield [(DateTimeImmutable::createFromFormat('Y-m-d', '2021-01-05'))->settime(0, 0)]; - yield [(DateTimeImmutable::createFromFormat('Y-m-d', '2021-02-05'))->settime(0, 0)]; - yield [(DateTimeImmutable::createFromFormat('Y-m-d', '2021-03-05'))->settime(0, 0)]; + yield [(DateTime::createFromFormat('Y-m-d', '2021-01-05'))->settime(0, 0)]; + yield [(DateTime::createFromFormat('Y-m-d', '2021-02-05'))->settime(0, 0)]; + yield [(DateTime::createFromFormat('Y-m-d', '2021-03-05'))->settime(0, 0)]; } /** * @dataProvider dateProvider */ - public function testGetLastAddress(DateTimeImmutable $date) + public function testGetLastAddress(DateTime $date) { $p = new Person($date); @@ -175,35 +174,35 @@ class PersonTest extends \PHPUnit\Framework\TestCase $this::assertNull($p->getLastAddress()); // Take an arbitrary date before the $date in parameter. - $addressDate = $date->sub(new DateInterval('PT180M')); + $addressDate = clone $date; // 1. Smoke test: Test that the first address added is the last one. - $address1 = (new Address())->setValidFrom($addressDate); + $address1 = (new Address())->setValidFrom($addressDate->sub(new DateInterval('PT180M'))); $p->addAddress($address1); $this::assertCount(1, $p->getAddresses()); $this::assertSame($address1, $p->getLastAddress()); // 2. Add an older address, which should not be the last address. - $addressDate2 = $addressDate->sub(new DateInterval('PT30M')); - $address2 = (new Address())->setValidFrom($addressDate2); + $addressDate2 = clone $addressDate; + $address2 = (new Address())->setValidFrom($addressDate2->sub(new DateInterval('PT30M'))); $p->addAddress($address2); $this::assertCount(2, $p->getAddresses()); $this::assertSame($address1, $p->getLastAddress()); // 3. Add a newer address, which should be the last address. - $addressDate3 = $addressDate->add(new DateInterval('PT30M')); - $address3 = (new Address())->setValidFrom($addressDate3); + $addressDate3 = clone $addressDate; + $address3 = (new Address())->setValidFrom($addressDate3->add(new DateInterval('PT30M'))); $p->addAddress($address3); $this::assertCount(3, $p->getAddresses()); $this::assertSame($address3, $p->getLastAddress()); // 4. Get the last address from a specific date. - $this::assertSame($address1, $p->getLastAddress($addressDate)); - $this::assertSame($address2, $p->getLastAddress($addressDate2)); - $this::assertSame($address3, $p->getLastAddress($addressDate3)); + $this::assertEquals($address1, $p->getLastAddress($addressDate)); + $this::assertEquals($address2, $p->getLastAddress($addressDate2)); + $this::assertEquals($address3, $p->getLastAddress($addressDate3)); } } From 03243605dac1c0f5dfdf303badad99cf3e24b7fe Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Wed, 31 Mar 2021 14:57:25 +0200 Subject: [PATCH 5/5] Remove unrelated code style change. --- Entity/AccompanyingPeriod.php | 70 ++++++++++++++++++---------------- Entity/Person.php | 20 +++++----- Tests/Entity/PersonTest.php | 72 +++++++++++++++++------------------ 3 files changed, 83 insertions(+), 79 deletions(-) diff --git a/Entity/AccompanyingPeriod.php b/Entity/AccompanyingPeriod.php index 36ce685e1..debc0a18d 100644 --- a/Entity/AccompanyingPeriod.php +++ b/Entity/AccompanyingPeriod.php @@ -3,7 +3,7 @@ /* * Chill is a software for social workers * - * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, + * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * , * * This program is free software: you can redistribute it and/or modify @@ -45,17 +45,17 @@ class AccompanyingPeriod /** @var \Chill\PersonBundle\Entity\Person */ private $person; - + /** @var AccompanyingPeriod\ClosingMotive */ private $closingMotive = null; - + /** * The user making the accompanying * * @var User */ private $user; - + /** * * @param \DateTime $dateOpening @@ -68,7 +68,7 @@ class AccompanyingPeriod /** * Get id * - * @return integer + * @return integer */ public function getId() { @@ -84,7 +84,7 @@ class AccompanyingPeriod public function setOpeningDate($openingDate) { $this->openingDate = $openingDate; - + return $this; } @@ -100,13 +100,17 @@ class AccompanyingPeriod /** * Set closingDate - * + * * For closing a Person file, you should use Person::setClosed instead. + * + * @param \DateTime $dateClosing + * @return AccompanyingPeriod + * */ public function setClosingDate($closingDate) { $this->closingDate = $closingDate; - + return $this; } @@ -119,12 +123,12 @@ class AccompanyingPeriod { return $this->closingDate; } - + /** - * + * * @return boolean */ - public function isOpen(): bool + public function isOpen(): bool { if ($this->getOpeningDate() > new \DateTime('now')) { return false; @@ -132,9 +136,9 @@ class AccompanyingPeriod if ($this->getClosingDate() === null) { return true; + } else { + return false; } - - return false; } /** @@ -148,16 +152,16 @@ class AccompanyingPeriod if ($remark === null) { $remark = ''; } - + $this->remark = $remark; - + return $this; } /** * Get remark * - * @return string + * @return string */ public function getRemark() { @@ -166,7 +170,7 @@ class AccompanyingPeriod /** * Set person. - * + * * For consistency, you should use Person::addAccompanyingPeriod instead. * * @param \Chill\PersonBundle\Entity\Person $person @@ -176,20 +180,20 @@ class AccompanyingPeriod public function setPerson(\Chill\PersonBundle\Entity\Person $person = null) { $this->person = $person; - + return $this; } /** * Get person * - * @return \Chill\PersonBundle\Entity\Person + * @return \Chill\PersonBundle\Entity\Person */ public function getPerson() { return $this->person; } - + public function getClosingMotive() { return $this->closingMotive; @@ -200,13 +204,13 @@ class AccompanyingPeriod $this->closingMotive = $closingMotive; return $this; } - + /** * If the period can be reopened. - * - * This function test if the period is closed and if the period is the last + * + * This function test if the period is closed and if the period is the last * for the associated person - * + * * @return boolean */ public function canBeReOpened() @@ -214,12 +218,12 @@ class AccompanyingPeriod if ($this->isOpen() === true) { return false; } - + $periods = $this->getPerson()->getAccompanyingPeriodsOrdered(); - + return end($periods) === $this; } - + public function reOpen() { $this->setClosingDate(null); @@ -231,29 +235,29 @@ class AccompanyingPeriod if ($this->isOpen()) { return; } - + if (! $this->isClosingAfterOpening()) { $context->buildViolation('The date of closing is before the date of opening') ->atPath('dateClosing') ->addViolation(); } } - + /** * Returns true if the closing date is after the opening date. - * + * * @return boolean */ public function isClosingAfterOpening() { $diff = $this->getOpeningDate()->diff($this->getClosingDate()); - + if ($diff->invert === 0) { return true; } else { return false; } } - + function getUser(): ?User { return $this->user; @@ -262,7 +266,7 @@ class AccompanyingPeriod function setUser(User $user): self { $this->user = $user; - + return $this; } diff --git a/Entity/Person.php b/Entity/Person.php index 7a9313ecb..57e04fe73 100644 --- a/Entity/Person.php +++ b/Entity/Person.php @@ -44,7 +44,7 @@ class Person implements HasCenterInterface { /** @var string The person's last name */ private $lastName; - + /** * * @var \Doctrine\Common\Collections\Collection @@ -121,12 +121,12 @@ class Person implements HasCenterInterface { * @var \Doctrine\Common\Collections\Collection */ private $addresses; - + /** * @var string */ private $fullnameCanonical; - + public function __construct(\DateTime $opening = null) { $this->accompanyingPeriods = new ArrayCollection(); $this->spokenLanguages = new ArrayCollection(); @@ -333,7 +333,7 @@ class Person implements HasCenterInterface { { return $this->lastName; } - + public function getAltNames(): \Doctrine\Common\Collections\Collection { return $this->altNames; @@ -342,7 +342,7 @@ class Person implements HasCenterInterface { public function setAltNames(\Doctrine\Common\Collections\Collection $altNames) { $this->altNames = $altNames; - + return $this; } @@ -352,20 +352,20 @@ class Person implements HasCenterInterface { $this->altNames->add($altName); $altName->setPerson($this); } - + return $this; } - - public function removeAltName(PersonAltName $altName) + + public function removeAltName(PersonAltName $altName) { if ($this->altNames->contains($altName)) { $altName->setPerson(null); $this->altNames->removeElement($altName); } - + return $this; } - + /** * Set birthdate * diff --git a/Tests/Entity/PersonTest.php b/Tests/Entity/PersonTest.php index a32823d20..b1e6d6971 100644 --- a/Tests/Entity/PersonTest.php +++ b/Tests/Entity/PersonTest.php @@ -2,8 +2,8 @@ /* * Chill is a software for social workers - * - * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, + * + * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * , * * This program is free software: you can redistribute it and/or modify @@ -35,55 +35,55 @@ use Generator; class PersonTest extends \PHPUnit\Framework\TestCase { /** - * Test the creation of an accompanying, its closure and the access to + * Test the creation of an accompanying, its closure and the access to * the current accompaniying period via the getCurrentAccompanyingPeriod * function. */ public function testGetCurrentAccompanyingPeriod() { - $d = new \DateTime('yesterday'); + $d = new \DateTime('yesterday'); $p = new Person($d); - + $period = $p->getCurrentAccompanyingPeriod(); - + $this->assertInstanceOf('Chill\PersonBundle\Entity\AccompanyingPeriod', $period); $this->assertTrue($period->isOpen()); $this->assertEquals($d, $period->getOpeningDate()); - + //close and test $period->setClosingDate(new \DateTime('tomorrow')); - + $shouldBeNull = $p->getCurrentAccompanyingPeriod(); $this->assertNull($shouldBeNull); } - + /** * Test if the getAccompanyingPeriodsOrdered function return a list of * periods ordered ascendency. */ public function testAccompanyingPeriodOrderWithUnorderedAccompanyingPeriod() - { + { $d = new \DateTime("2013/2/1"); $p = new Person($d); - + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - + $f = new \DateTime("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - - $g = new \DateTime("2013/4/1"); + + $g = new \DateTime("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - + $r = $p->getAccompanyingPeriodsOrdered(); - + $date = $r[0]->getOpeningDate()->format('Y-m-d'); - + $this->assertEquals($date, '2013-01-01'); } - + /** * Test if the getAccompanyingPeriodsOrdered function, for periods * starting at the same time order regarding to the closing date. @@ -91,25 +91,25 @@ class PersonTest extends \PHPUnit\Framework\TestCase public function testAccompanyingPeriodOrderSameDateOpening() { $d = new \DateTime("2013/2/1"); $p = new Person($d); - - $g = new \DateTime("2013/4/1"); + + $g = new \DateTime("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - + $f = new \DateTime("2013/2/1"); $p->open(new AccompanyingPeriod($f)); - + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); $r = $p->getAccompanyingPeriodsOrdered(); - + $date = $r[0]->getClosingDate()->format('Y-m-d'); - + $this->assertEquals($date, '2013-03-01'); } - + /** * Test if the function checkAccompanyingPeriodIsNotCovering returns * the good constant when two periods are collapsing : a period @@ -118,23 +118,23 @@ class PersonTest extends \PHPUnit\Framework\TestCase public function testDateCoveringWithCoveringAccompanyingPeriod() { $d = new \DateTime("2013/2/1"); $p = new Person($d); - + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - + $f = new \DateTime("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - - $g = new \DateTime("2013/4/1"); + + $g = new \DateTime("2013/4/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($g); $p->close($period); - + $r = $p->checkAccompanyingPeriodsAreNotCollapsing(); - + $this->assertEquals($r['result'], Person::ERROR_PERIODS_ARE_COLLAPSING); } - + /** * Test if the function checkAccompanyingPeriodIsNotCovering returns * the good constant when two periods are collapsing : a period is open @@ -143,16 +143,16 @@ class PersonTest extends \PHPUnit\Framework\TestCase public function testNotOpenAFileReOpenedLater() { $d = new \DateTime("2013/2/1"); $p = new Person($d); - + $e = new \DateTime("2013/3/1"); $period = $p->getCurrentAccompanyingPeriod()->setClosingDate($e); $p->close($period); - + $f = new \DateTime("2013/1/1"); $p->open(new AccompanyingPeriod($f)); - + $r = $p->checkAccompanyingPeriodsAreNotCollapsing(); - + $this->assertEquals($r['result'], Person::ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD); }