Improve person accompanyingPeriod validation - close #410

This commit is contained in:
Marc Ducobu 2015-03-06 20:36:21 +01:00
parent c80326d0ed
commit 5634a19174
5 changed files with 110 additions and 158 deletions

View File

@ -4,7 +4,7 @@
* Chill is a software for social workers * Chill is a software for social workers
* *
* Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS,
* <http://www.champs-libres.coop> * <http://www.champs-libres.coop>, <info@champs-libres.coop>
* *
* This program is free software: you can redistribute it and/or modify * This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as * it under the terms of the GNU Affero General Public License as
@ -40,7 +40,6 @@ class AccompanyingPeriodController extends Controller
return $this->render('ChillPersonBundle:AccompanyingPeriod:list.html.twig', return $this->render('ChillPersonBundle:AccompanyingPeriod:list.html.twig',
array('accompanying_periods' => $person->getAccompanyingPeriodsOrdered(), array('accompanying_periods' => $person->getAccompanyingPeriodsOrdered(),
'person' => $person)); 'person' => $person));
} }
public function createAction($person_id) { public function createAction($person_id) {
@ -51,40 +50,34 @@ class AccompanyingPeriodController extends Controller
} }
$accompanyingPeriod = new AccompanyingPeriod(new \DateTime()); $accompanyingPeriod = new AccompanyingPeriod(new \DateTime());
$accompanyingPeriod->setPerson($person) $accompanyingPeriod->setDateClosing(new \DateTime());
->setDateOpening(new \DateTime())
->setDateClosing(new \DateTime());
$person->addAccompanyingPeriod(
$accompanyingPeriod);
$form = $this->createForm(new AccompanyingPeriodType(), $form = $this->createForm(new AccompanyingPeriodType(),
$accompanyingPeriod, array('period_action' => 'update')); $accompanyingPeriod, array('period_action' => 'update'));
$request = $this->getRequest(); $request = $this->getRequest();
if ($request->getMethod() === 'POST') { if ($request->getMethod() === 'POST') {
$form->handleRequest($request); $form->handleRequest($request);
$errors = $this->_validatePerson($person); $errors = $this->_validatePerson($person);
$flashBag = $this->get('session')->getFlashBag(); $flashBag = $this->get('session')->getFlashBag();
if ($form->isValid(array('Default', 'closed')) if ($form->isValid(array('Default', 'closed'))
&& count($errors) === 0) { && count($errors) === 0) {
$em = $this->getDoctrine()->getManager(); $em = $this->getDoctrine()->getManager();
$em->persist($accompanyingPeriod); $em->persist($accompanyingPeriod);
$em->flush(); $em->flush();
$flashBag->add('success', $flashBag->add('success',
$this->get('translator')->trans( $this->get('translator')->trans(
'Period created!')); 'Period created!'));
return $this->redirect($this->generateUrl('chill_person_accompanying_period_list', return $this->redirect($this->generateUrl('chill_person_accompanying_period_list',
array('person_id' => $person->getId()))); array('person_id' => $person->getId())));
} else { } else {
$flashBag->add('danger', $this->get('translator') $flashBag->add('danger', $this->get('translator')
->trans('Error! Period not created!')); ->trans('Error! Period not created!'));
@ -94,8 +87,6 @@ class AccompanyingPeriodController extends Controller
} }
} }
return $this->render('ChillPersonBundle:AccompanyingPeriod:form.html.twig', return $this->render('ChillPersonBundle:AccompanyingPeriod:form.html.twig',
array( array(
'form' => $form->createView(), 'form' => $form->createView(),
@ -117,18 +108,13 @@ class AccompanyingPeriodController extends Controller
} }
$person = $accompanyingPeriod->getPerson(); $person = $accompanyingPeriod->getPerson();
$form = $this->createForm(new AccompanyingPeriodType(),
$form = $this->createForm(new AccompanyingPeriodType(), $accompanyingPeriod, array('period_action' => 'update'));
$accompanyingPeriod, array('period_action' => 'update'));
$request = $this->getRequest(); $request = $this->getRequest();
if ($request->getMethod() === 'POST') { if ($request->getMethod() === 'POST') {
$form->handleRequest($request); $form->handleRequest($request);
$errors = $this->_validatePerson($person); $errors = $this->_validatePerson($person);
$flashBag = $this->get('session')->getFlashBag(); $flashBag = $this->get('session')->getFlashBag();
if ($form->isValid(array('Default', 'closed')) if ($form->isValid(array('Default', 'closed'))
@ -136,13 +122,12 @@ class AccompanyingPeriodController extends Controller
$em->flush(); $em->flush();
$flashBag->add('success', $flashBag->add('success',
$this->get('translator')->trans( $this->get('translator')->trans(
'Period updating done')); 'Period updating done'));
return $this->redirect($this->generateUrl('chill_person_accompanying_period_list', return $this->redirect($this->generateUrl('chill_person_accompanying_period_list',
array('person_id' => $person->getId()))); array('person_id' => $person->getId())));
} else { } else {
$flashBag->add('danger', $this->get('translator') $flashBag->add('danger', $this->get('translator')
->trans('Error when updating the period')); ->trans('Error when updating the period'));
@ -152,8 +137,6 @@ class AccompanyingPeriodController extends Controller
} }
} }
return $this->render('ChillPersonBundle:AccompanyingPeriod:form.html.twig', return $this->render('ChillPersonBundle:AccompanyingPeriod:form.html.twig',
array( array(
'form' => $form->createView(), 'form' => $form->createView(),
@ -171,9 +154,9 @@ class AccompanyingPeriodController extends Controller
if ($person->isOpen() === false) { if ($person->isOpen() === false) {
$this->get('session')->getFlashBag() $this->get('session')->getFlashBag()
->add('danger', $this->get('translator') ->add('danger', $this->get('translator')
->trans('Beware period is closed', ->trans('Beware period is closed',
array('%name%' => $person->__toString()))); array('%name%' => $person->__toString())));
return $this->redirect( return $this->redirect(
$this->generateUrl('chill_person_accompanying_period_list', array( $this->generateUrl('chill_person_accompanying_period_list', array(
@ -182,7 +165,6 @@ class AccompanyingPeriodController extends Controller
} }
$current = $person->getCurrentAccompanyingPeriod(); $current = $person->getCurrentAccompanyingPeriod();
$form = $this->createForm(new AccompanyingPeriodType(), $current, array( $form = $this->createForm(new AccompanyingPeriodType(), $current, array(
'period_action' => 'close' 'period_action' => 'close'
)); ));
@ -198,9 +180,9 @@ class AccompanyingPeriodController extends Controller
if (count($errors) === 0) { if (count($errors) === 0) {
$this->get('session')->getFlashBag() $this->get('session')->getFlashBag()
->add('success', $this->get('translator') ->add('success', $this->get('translator')
->trans('Period closed!', ->trans('Period closed!',
array('%name%' => $person->__toString()))); array('%name%' => $person->__toString())));
$this->getDoctrine()->getManager()->flush(); $this->getDoctrine()->getManager()->flush();
@ -247,14 +229,14 @@ class AccompanyingPeriodController extends Controller
* @return \Symfony\Component\Validator\ConstraintViolationListInterface * @return \Symfony\Component\Validator\ConstraintViolationListInterface
*/ */
private function _validatePerson(Person $person) { private function _validatePerson(Person $person) {
$errors = $this->get('validator')->validate($person, $errors = $this->get('validator')->validate($person,
array('Default')); array('Default'));
$errors_accompanying_period = $this->get('validator')->validate($person, $errors_accompanying_period = $this->get('validator')->validate($person,
array('accompanying_period_consistent')); array('accompanying_period_consistent'));
foreach($errors_accompanying_period as $error ) { foreach($errors_accompanying_period as $error ) {
$errors->add($error); $errors->add($error);
} }
return $errors; return $errors;
} }
@ -270,11 +252,11 @@ class AccompanyingPeriodController extends Controller
$request = $this->getRequest(); $request = $this->getRequest();
//in case the person is already open //in case the person is already open
if ($person->isOpen() === true) { if ($person->isOpen()) {
$this->get('session')->getFlashBag() $this->get('session')->getFlashBag()
->add('danger', $this->get('translator') ->add('danger', $this->get('translator')
->trans('Error! Period %name% is not closed ; it can be open', ->trans('Error! Period %name% is not closed ; it can be open',
array('%name%' => $person->__toString()))); array('%name%' => $person->__toString())));
return $this->redirect( return $this->redirect(
$this->generateUrl('chill_person_accompanying_period_list', array( $this->generateUrl('chill_person_accompanying_period_list', array(
@ -304,21 +286,19 @@ class AccompanyingPeriodController extends Controller
$this->getDoctrine()->getManager()->flush(); $this->getDoctrine()->getManager()->flush();
return $this->redirect( return $this->redirect(
$this->generateUrl('chill_person_accompanying_period_list', array( $this->generateUrl('chill_person_accompanying_period_list', array(
'person_id' => $person->getId() 'person_id' => $person->getId()
)) )));
);
} else { } else {
$this->get('session')->getFlashBag() $this->get('session')->getFlashBag()
->add('danger', $this->get('translator') ->add('danger', $this->get('translator')
->trans('Period not opened')); ->trans('Period not opened'));
foreach ($errors as $error) { foreach ($errors as $error) {
$this->get('session')->getFlashBag() $this->get('session')->getFlashBag()
->add('info', $error->getMessage()); ->add('info', $error->getMessage());
} }
} }
} else { // if errors in forms } else { // if errors in forms
$this->get('session')->getFlashBag() $this->get('session')->getFlashBag()
->add('danger', $this->get('translator') ->add('danger', $this->get('translator')
@ -327,17 +307,14 @@ class AccompanyingPeriodController extends Controller
} }
return $this->render('ChillPersonBundle:AccompanyingPeriod:form.html.twig', return $this->render('ChillPersonBundle:AccompanyingPeriod:form.html.twig',
array( array('form' => $form->createView(),
'form' => $form->createView(), 'person' => $person,
'person' => $person, 'accompanying_period' => $accompanyingPeriod));
'accompanying_period' => $accompanyingPeriod
));
} }
private function _getPerson($id) { private function _getPerson($id) {
return $this->getDoctrine()->getManager() return $this->getDoctrine()->getManager()
->getRepository('ChillPersonBundle:Person') ->getRepository('ChillPersonBundle:Person')
->find($id); ->find($id);
} }
} }

View File

@ -4,7 +4,7 @@
* Chill is a software for social workers * Chill is a software for social workers
* *
* Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS,
* <http://www.champs-libres.coop> * <http://www.champs-libres.coop>, <info@champs-libres.coop>
* *
* This program is free software: you can redistribute it and/or modify * This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as * it under the terms of the GNU Affero General Public License as
@ -206,8 +206,6 @@ class AccompanyingPeriod
return $this; return $this;
} }
/// VALIDATION function /// VALIDATION function
public function isDateConsistent(ExecutionContextInterface $context) { public function isDateConsistent(ExecutionContextInterface $context) {
if ($this->isOpen()) { if ($this->isOpen()) {

View File

@ -5,8 +5,8 @@ namespace Chill\PersonBundle\Entity;
/* /*
* Chill is a software for social workers * Chill is a software for social workers
* *
* Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS, * Copyright (C) 2014-2015, Champs Libres Cooperative SCRLFS,
* <http://www.champs-libres.coop> * <http://www.champs-libres.coop>, <info@champs-libres.coop>
* *
* This program is free software: you can redistribute it and/or modify * This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as * it under the terms of the GNU Affero General Public License as
@ -567,93 +567,67 @@ class Person {
* *
* This method add violation errors. * This method add violation errors.
*/ */
public function isAccompanyingPeriodValid(ExecutionContextInterface $context) { public function isAccompanyingPeriodValid(ExecutionContextInterface $context)
$r = $this->checkAccompanyingPeriodIsNotCovering(); {
$r = $this->checkAccompanyingPeriodsAreNotCollapsing();
if ($r !== true) { if ($r !== true) {
if ($r['result'] === self::ERROR_OPENING_NOT_CLOSED_IS_BEFORE_NEW_LINE) { if ($r['result'] === self::ERROR_PERIODS_ARE_COLLAPSING) {
$context->addViolationAt('history', $context->addViolationAt('accompanyingPeriods',
'Accompanying period not closed is before the new line', 'Two accompanying periods have days in commun',
array() ); array());
return; }
}
$context->addViolationAt('history', 'Periods are collapsing', if ($r['result'] === self::ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD) {
array( $context->addViolationAt('accompanyingPeriods',
'%dateOpening%' => $r['dateOpening']->format('d-m-Y'), 'A period is opened and a period is added after it',
'%dateClosing%' => $r['dateClosing']->format('d-m-Y'), array());
'%date%' => $r['date']->format('d-m-Y') }
)
);
} }
} }
const ERROR_OPENING_IS_INSIDE_CLOSING = 1; const ERROR_PERIODS_ARE_COLLAPSING = 1; // when two different periods
const ERROR_OPENING_NOT_CLOSED_IS_BEFORE_NEW_LINE = 2; // have days in commun
const ERROR_OPENING_NOT_CLOSE_IS_INSIDE_CLOSED_ACCOMPANYING_PERIOD_LINE = 3; const ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD = 2; // where there exist
const ERROR_OPENING_IS_BEFORE_OTHER_LINE_AND_CLOSED_IS_AFTER_THIS_LINE = 4; // a period opened and another one after it
public function checkAccompanyingPeriodIsNotCovering() /**
{ * Function used for validation that check if the accompanying periods of
* the person are not collapsing (i.e. have not shared days) or having
* a period after an open period.
*
* @return true | array True if the accompanying periods are not collapsing,
* an array with data for displaying the error
*/
public function checkAccompanyingPeriodsAreNotCollapsing()
{
$periods = $this->getAccompanyingPeriodsOrdered(); $periods = $this->getAccompanyingPeriodsOrdered();
$periodsNbr = sizeof($periods);
foreach ($periods as $key => $period) { $i = 0;
//accompanying period is open : we must check the arent any period after
if ($period->isOpen()) { while($i < $periodsNbr - 1) {
foreach ($periods as $subKey => $against) { $periodI = $periods[$i];
//if we are checking the same, continue $periodAfterI = $periods[$i + 1];
if ($key === $subKey) {
continue; if($periodI->isOpen()) {
} return array(
'result' => self::ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD,
if ($period->getDateOpening() > $against->getDateOpening() 'dateOpening' => $periodAfterI->getDateOpening(),
&& $period->getDateOpening() < $against->getDateOpening()) { 'dateClosing' => $periodAfterI->getDateClosing(),
// the period date opening is inside another opening line 'date' => $periodI->getDateOpening()
return array( );
'result' => self::ERROR_OPENING_NOT_CLOSE_IS_INSIDE_CLOSED_ACCOMPANYING_PERIOD_LINE, } elseif ($periodI->getDateClosing() >= $periodAfterI->getDateOpening()) {
'dateOpening' => $against->getDateOpening(), return array(
'dateClosing' => $against->getDateClosing(), 'result' => self::ERROR_PERIODS_ARE_COLLAPSING,
'date' => $period->getDateOpening() 'dateOpening' => $periodI->getDateOpening(),
);
} 'dateClosing' => $periodI->getDateClosing(),
'date' => $periodAfterI->getDateOpening()
if ($period->getDateOpening() < $against->getDateOpening() );
&& $period->getDateClosing() > $against->getDateClosing()) {
// the period date opening is inside another opening line
return array(
'result' => self::ERROR_OPENING_IS_BEFORE_OTHER_LINE_AND_CLOSED_IS_AFTER_THIS_LINE,
'dateOpening' => $against->getDateOpening(),
'dateClosing' => $against->getDateClosing(),
'date' => $period->getDateOpening()
);
}
//if we have an aopening later...
if ($period->getDateOpening() < $against->getDateClosing()) {
return array( 'result' => self::ERROR_OPENING_NOT_CLOSED_IS_BEFORE_NEW_LINE,
'dateOpening' => $against->getDateOpening(),
'dateClosing' => $against->getDateClosing(),
'date' => $period->getDateOpening()
);
}
}
} else {
//we must check there is not covering lines
foreach ($periods as $subKey => $against) {
//check if dateOpening is inside an `against` line
if ($period->getDateOpening() > $against->getDateOpening()
&& $period->getDateOpening() < $against->getDateClosing()) {
return array(
'result' => self::ERROR_OPENING_IS_INSIDE_CLOSING,
'dateOpening' => $against->getDateOpening(),
'dateClosing' => $against->getDateClosing(),
'date' => $period->getDateOpening()
);
}
}
} }
$i++;
} }
return true; return true;
} }
} }

View File

@ -29,8 +29,9 @@ use Chill\PersonBundle\Entity\AccompanyingPeriod\ClosingMotive;
/** /**
* Test the creation or deletion of accompanying periods * Test the creation or deletion of accompanying periods
* *
* @author Julien Fastré <julien.fastre@champs-libres.coop> * The person on which the test is done has a (current) period opened (and not
* closed) starting the 2015-01-05.
*/ */
class AccompanyingPeriodControllerTest extends WebTestCase class AccompanyingPeriodControllerTest extends WebTestCase
{ {
@ -56,14 +57,19 @@ class AccompanyingPeriodControllerTest extends WebTestCase
const CLOSING_INPUT = 'chill_personbundle_accompanyingperiod[date_closing]'; const CLOSING_INPUT = 'chill_personbundle_accompanyingperiod[date_closing]';
const CLOSING_MOTIVE_INPUT = 'chill_personbundle_accompanyingperiod[closingMotive]'; const CLOSING_MOTIVE_INPUT = 'chill_personbundle_accompanyingperiod[closingMotive]';
/**
* Setup before the first test of this class (see phpunit doc)
*/
public static function setUpBeforeClass() public static function setUpBeforeClass()
{ {
static::bootKernel(); static::bootKernel();
static::$em = static::$kernel->getContainer() static::$em = static::$kernel->getContainer()
->get('doctrine.orm.entity_manager'); ->get('doctrine.orm.entity_manager');
} }
/**
* Setup before each test method (see phpunit doc)
*/
public function setUp() public function setUp()
{ {
$this->client = static::createClient(array(), array( $this->client = static::createClient(array(), array(
@ -74,13 +80,15 @@ class AccompanyingPeriodControllerTest extends WebTestCase
$this->person = (new Person(new \DateTime('2015-01-05'))) $this->person = (new Person(new \DateTime('2015-01-05')))
->setFirstName('Roland') ->setFirstName('Roland')
->setLastName('Gallorime') ->setLastName('Gallorime')
->setGenre(Person::GENRE_MAN) ->setGenre(Person::GENRE_MAN);
;
static::$em->persist($this->person); static::$em->persist($this->person);
static::$em->flush(); static::$em->flush();
} }
/**
* TearDown after each test method (see phpunit doc)
*/
public function tearDown() public function tearDown()
{ {
static::$em->refresh($this->person); static::$em->refresh($this->person);
@ -207,7 +215,6 @@ class AccompanyingPeriodControllerTest extends WebTestCase
*/ */
public function testAddNewPeriodBeforeActual() public function testAddNewPeriodBeforeActual()
{ {
$crawler = $this->client->request('GET', '/en/person/' $crawler = $this->client->request('GET', '/en/person/'
.$this->person->getId().'/accompanying-period/create'); .$this->person->getId().'/accompanying-period/create');
@ -244,8 +251,6 @@ class AccompanyingPeriodControllerTest extends WebTestCase
*/ */
public function testCreatePeriodWithClosingAfterCurrentFails() public function testCreatePeriodWithClosingAfterCurrentFails()
{ {
$this->markTestSkipped('this feature is not yet implemented');
$crawler = $this->client->request('GET', '/en/person/' $crawler = $this->client->request('GET', '/en/person/'
.$this->person->getId().'/accompanying-period/create'); .$this->person->getId().'/accompanying-period/create');
@ -280,8 +285,6 @@ class AccompanyingPeriodControllerTest extends WebTestCase
*/ */
public function testCreatePeriodWithOpeningAndClosingAfterCurrentFails() public function testCreatePeriodWithOpeningAndClosingAfterCurrentFails()
{ {
$this->markTestSkipped('this feature is not yet implemented');
$crawler = $this->client->request('GET', '/en/person/' $crawler = $this->client->request('GET', '/en/person/'
.$this->person->getId().'/accompanying-period/create'); .$this->person->getId().'/accompanying-period/create');

View File

@ -126,9 +126,9 @@ class PersonTest extends \PHPUnit_Framework_TestCase
$period = $p->getCurrentAccompanyingPeriod()->setDateClosing($g); $period = $p->getCurrentAccompanyingPeriod()->setDateClosing($g);
$p->close($period); $p->close($period);
$r = $p->checkAccompanyingPeriodIsNotCovering(); $r = $p->checkAccompanyingPeriodsAreNotCollapsing();
$this->assertEquals($r['result'], Person::ERROR_OPENING_IS_INSIDE_CLOSING); $this->assertEquals($r['result'], Person::ERROR_PERIODS_ARE_COLLAPSING);
} }
/** /**
@ -147,8 +147,8 @@ class PersonTest extends \PHPUnit_Framework_TestCase
$f = new \DateTime("2013/1/1"); $f = new \DateTime("2013/1/1");
$p->open(new AccompanyingPeriod($f)); $p->open(new AccompanyingPeriod($f));
$r = $p->checkAccompanyingPeriodIsNotCovering(); $r = $p->checkAccompanyingPeriodsAreNotCollapsing();
$this->assertEquals($r['result'], Person::ERROR_OPENING_NOT_CLOSED_IS_BEFORE_NEW_LINE); $this->assertEquals($r['result'], Person::ERROR_ADDIND_PERIOD_AFTER_AN_OPEN_PERIOD);
} }
} }