From cf2408be1b2ec83684962300aa2db13efba34491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 5 Oct 2018 22:54:30 +0200 Subject: [PATCH 1/2] improvement on import - person may have a null gender --- CHANGELOG.md | 9 + Command/ImportPeopleFromCSVCommand.php | 292 ++++++++++++++++-- DependencyInjection/ChillPersonExtension.php | 1 + Export/Aggregator/GenderAggregator.php | 4 + Export/Filter/GenderFilter.php | 25 +- Resources/config/doctrine/Person.orm.yml | 1 + Resources/config/services/command.yml | 10 + .../migrations/Version20181005140249.php | 26 ++ 8 files changed, 338 insertions(+), 30 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 Resources/config/services/command.yml create mode 100644 Resources/migrations/Version20181005140249.php diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..173f90ee6 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,9 @@ + +Master branch +============= + +- Improve import of person to allow multiple centers by file ; +- Launch an event on person import ; +- Allow person to have a `null` gender ; +- Allow filters and aggregator to handle null gender ; + diff --git a/Command/ImportPeopleFromCSVCommand.php b/Command/ImportPeopleFromCSVCommand.php index 736d595be..0237cfad1 100644 --- a/Command/ImportPeopleFromCSVCommand.php +++ b/Command/ImportPeopleFromCSVCommand.php @@ -29,6 +29,13 @@ use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Console\Question\ChoiceQuestion; use Symfony\Component\Console\Helper\Table; use Chill\PersonBundle\Entity\Person; +use Chill\MainBundle\Entity\Address; +use Chill\MainBundle\Entity\PostalCode; +use Chill\MainBundle\Entity\Center; +use Chill\CustomFieldsBundle\Service\CustomFieldProvider; +use Symfony\Component\Console\Question\ConfirmationQuestion; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\Event; /** * @@ -67,6 +74,12 @@ class ImportPeopleFromCSVCommand extends ContainerAwareCommand */ protected $em; + /** + * + * @var EventDispatcherInterface + */ + protected $eventDispatcher; + /** * the line currently read * @@ -80,6 +93,12 @@ class ImportPeopleFromCSVCommand extends ContainerAwareCommand */ protected $customFieldMapping = array(); + /** + * + * @var CustomFieldProvider + */ + protected $customFieldProvider; + /** * Contains an array of information searched in the file. * @@ -96,7 +115,11 @@ class ImportPeopleFromCSVCommand extends ContainerAwareCommand ['opening_date', 'The column header for opening date', 'opening_date'], ['closing_date', 'The column header for closing date', 'closing_date'], ['memo', 'The column header for memo', 'memo'], - ['phonenumber', 'The column header for phonenumber', 'phonenumber'] + ['phonenumber', 'The column header for phonenumber', 'phonenumber'], + ['street1', 'The column header for street 1', 'street1'], + ['postalcode', 'The column header for postal code', 'postalcode'], + ['locality', 'The column header for locality', 'locality'], + ['center', 'The column header for center', 'center'] ); /** @@ -106,11 +129,26 @@ class ImportPeopleFromCSVCommand extends ContainerAwareCommand */ protected static $defaultDateInterpreter = "%d/%m/%Y|%e/%m/%y|%d/%m/%Y|%e/%m/%Y"; - + public function __construct( + \Psr\Log\LoggerInterface $logger, + \Chill\MainBundle\Templating\TranslatableStringHelper $helper, + \Doctrine\ORM\EntityManagerInterface $em, + CustomFieldProvider $customFieldProvider, + EventDispatcherInterface $eventDispatcher + ) { + $this->logger = $logger; + $this->helper = $helper; + $this->em = $em; + $this->customFieldProvider = $customFieldProvider; + $this->eventDispatcher = $eventDispatcher; + + parent::__construct('chill:person:import'); + } + protected function configure() { - $this->setName('chill:person:import') + $this ->addArgument('csv_file', InputArgument::REQUIRED, "The CSV file to import") ->setDescription("Import people from a csv file") ->setHelp(<<addArgument('locale', InputArgument::REQUIRED, "The locale to use in displaying translatable strings from entities") - ->addArgument('center', InputArgument::REQUIRED, - "The id of the center") + ->addOption( + 'force-center', + null, + InputOption::VALUE_REQUIRED, + "The id of the center" + ) ->addOption( 'force', null, @@ -161,7 +207,7 @@ EOF ->addOption( 'dump-choice-matching', null, - InputOption::VALUE_OPTIONAL, + InputOption::VALUE_REQUIRED, "The path of the file to dump the matching between label in CSV and answers" ) ->addOption( @@ -245,9 +291,7 @@ EOF $cfMappingsOptions = $this->input->getOption('custom-field'); /* @var $em \Doctrine\Common\Persistence\ObjectManager */ - $em = $this->getContainer()->get('doctrine.orm.entity_manager'); - /* @var $this->helper \Chill\MainBundle\Templating\TranslatableStringHelper */ - $this->helper = $this->getContainer()->get('chill.main.helper.translatable_string'); + $em = $this->em; foreach($cfMappingsOptions as $cfMappingStringOption) { list($rowNumber, $cfSlug) = preg_split('|=|', $cfMappingStringOption); @@ -317,7 +361,7 @@ EOF protected function dumpAnswerMatching() { - if ($this->input->hasOption('dump-choice-matching')) { + if ($this->input->hasOption('dump-choice-matching') && !empty($this->input->getOption('dump-choice-matching'))) { $this->logger->debug("Dump the matching between answer and choices"); $str = json_encode($this->cacheAnswersMapping, JSON_PRETTY_PRINT); @@ -330,7 +374,6 @@ EOF protected function execute(InputInterface $input, OutputInterface $output) { - $this->logger = new ConsoleLogger($output); $this->input = $input; $this->output = $output; @@ -356,13 +399,30 @@ EOF $this->logger->debug("Processing line ".$this->line); if ($line === 1 ) { $this->logger->debug('Processing line 1, headers'); - + $rawHeaders = $row; $headers = $this->processingHeaders($row); } else { $person = $this->createPerson($row, $headers); - $this->processingCustomFields($person, $row); - if ($this->input->getOption('force') === TRUE) { + if (count($this->customFieldMapping) > 0) { + $this->processingCustomFields($person, $row); + } + + $event = new Event(); + $event->person = $person; + $event->rawHeaders = $rawHeaders; + $event->row = $row; + $event->headers = $headers; + $event->skipPerson = false; + $event->force = $this->input->getOption('force'); + $event->input = $this->input; + $event->output = $this->output; + $event->helperSet = $this->getHelperSet(); + + $this->eventDispatcher->dispatch('chill_person.person_import', $event); + + if ($this->input->getOption('force') === TRUE + && $event->skipPerson === false) { $this->em->persist($person); } @@ -451,13 +511,13 @@ EOF $openingDate = $this->processDate($openingDateString, $this->input->getOption('opening_date_format')); $person = $openingDate instanceof \DateTime ? new Person($openingDate) : new Person(); - - // currently, import only men - $person->setGender(Person::MALE_GENDER); - // add the center - $center = $this->em->getRepository('ChillMainBundle:Center') - ->find($this->input->getArgument('center')); + $center = $this->getCenter($row, $headers); + + if ($center === null) { + throw new \Exception("center not found"); + } + $person->setCenter($center); foreach($headers as $column => $info) { @@ -486,12 +546,198 @@ EOF case 'phonenumber': $person->setPhonenumber($value); break; + + // we just keep the column number for those data + case 'postalcode': + $postalCodeValue = $value; + break; + case 'street1': + $street1Value = $value; + break; + case 'locality': + $localityValue = $value; + break; } } + // handle address + if (\in_array('postalcode', $headers)) { + + $address = new Address(); + $postalCode = $this->guessPostalCode($postalCodeValue, $localityValue ?? ''); + + if ($postalCode === null) { + throw new \Exception("The locality is not found"); + } + + $address->setPostcode($postalCode); + + if (\in_array('street1', $headers)) { + $address->setStreetAddress1($street1Value); + } + $address->setValidFrom(new \DateTime('today')); + + $person->addAddress($address); + } + return $person; } + protected function getCenter($row, $headers) + { + if ($this->input->hasOption('force-center') && !empty($this->input->getOption('force-center'))) { + return $this->em->getRepository('ChillMainBundle:Center') + ->find($this->input->getOption('force-center')); + } else { + $columnCenter = \array_search('center', $headers); + $centerName = \trim($row[$columnCenter]); + + try { + return $this->em->createQuery('SELECT c FROM ChillMainBundle:Center c ' + . 'WHERE c.name = :center_name') + ->setParameter('center_name', $centerName) + ->getSingleResult() + ; + } catch (\Doctrine\ORM\NonUniqueResultException $e) { + return $this->guessCenter($centerName); + } catch (\Doctrine\ORM\NoResultException $e) { + return $this->guessCenter($centerName); + } + } + } + + protected function guessCenter($centerName) + { + if (!\array_key_exists('_center_picked', $this->cacheAnswersMapping)) { + $this->cacheAnswersMapping['_center_picked'] = []; + } + + if (\array_key_exists($centerName, $this->cacheAnswersMapping['_center_picked'])) { + $id = $this->cacheAnswersMapping['_center_picked'][$centerName]; + + return $this->em->getRepository(Center::class) + ->find($id); + } + + $centers = $this->em->createQuery("SELECT c FROM ChillMainBundle:Center c " + . "ORDER BY SIMILARITY(c.name, :center_name) DESC") + ->setParameter('center_name', $centerName) + ->setMaxResults(10) + ->getResult() + ; + + if (count($centers) > 1) { + if (\strtolower($centers[0]->getName()) === \strtolower($centerName)) { + return $centers[0]; + } + } + + $centersByName = []; + $names = \array_map(function(Center $c) use (&$centersByName) { + $n = $c->getName(); + $centersByName[$n] = $c; + return $n; + + }, $centers); + $names[] = "none of them"; + + $helper = $this->getHelper('question'); + $question = new ChoiceQuestion(sprintf("Which center match the name \"%s\" ? (default to \"%s\")", $centerName, $names[0]), + $names, + 0); + + $answer = $helper->ask($this->input, $this->output, $question); + + if ($answer === 'none of them') { + $questionCreate = new ConfirmationQuestion("Would you like to create it ?", false); + $create = $helper->ask($this->input, $this->output, $questionCreate); + + if ($create) { + $center = (new Center()) + ->setName($centerName) + ; + + if ($this->input->getOption('force') === TRUE) { + $this->em->persist($center); + $this->em->flush(); + } + + return $center; + } + } + + $center = $centersByName[$answer]; + + $this->cacheAnswersMapping['_center_picked'][$centerName] = $center->getId(); + + return $center; + } + + protected function guessPostalCode($postalCode, $locality) + { + if (!\array_key_exists('_postal_code_picked', $this->cacheAnswersMapping)) { + $this->cacheAnswersMapping['_postal_code_picked'] = []; + } + + if (\array_key_exists($postalCode, $this->cacheAnswersMapping['_postal_code_picked'])) { + if (\array_key_exists($locality, $this->cacheAnswersMapping['_postal_code_picked'][$postalCode])) { + $id = $this->cacheAnswersMapping['_postal_code_picked'][$postalCode][$locality]; + + return $this->em->getRepository(PostalCode::class)->find($id); + } + } + + $postalCodes = $this->em->createQuery("SELECT pc FROM ".PostalCode::class." pc " + . "WHERE pc.code = :postal_code " + . "ORDER BY SIMILARITY(pc.name, :locality) DESC " + ) + ->setMaxResults(10) + ->setParameter('postal_code', $postalCode) + ->setParameter('locality', $locality) + ->getResult() + ; + + if (count($postalCodes) >= 1) { + if ($postalCodes[0]->getCode() === $postalCode + && $postalCodes[0]->getName() === $locality) { + return $postalCodes[0]; + } + } + + if (count($postalCodes) === 0) { + return null; + } + + $postalCodeByName = []; + $names = \array_map(function(PostalCode $pc) use (&$postalCodeByName) { + $n = $pc->getName(); + $postalCodeByName[$n] = $pc; + + return $n; + }, $postalCodes); + $names[] = 'none of them'; + + $helper = $this->getHelper('question'); + $question = new ChoiceQuestion(sprintf("Which postal code match the " + . "name \"%s\" with postal code \"%s\" ? (default to \"%s\")", + $locality, $postalCode, $names[0]), + $names, + 0); + + $answer = $helper->ask($this->input, $this->output, $question); + + if ($answer === 'none of them') { + return null; + } + + $pc = $postalCodeByName[$answer]; + + $this->cacheAnswersMapping['_postal_code_picked'][$postalCode][$locality] = + $pc->getId(); + + return $pc; + } + protected function processBirthdate(Person $person, $value) { @@ -556,7 +802,7 @@ EOF /* @var $factory \Symfony\Component\Form\FormFactory */ $factory = $this->getContainer()->get('form.factory'); /* @var $cfProvider \Chill\CustomFieldsBundle\Service\CustomFieldProvider */ - $cfProvider = $this->getContainer()->get('chill.custom_field.provider'); + $cfProvider = $this->customFieldProvider; $cfData = array(); /* @var $$customField \Chill\CustomFieldsBundle\Entity\CustomField */ @@ -722,7 +968,7 @@ EOF $value)); } } - var_dump($this->cacheAnswersMapping[$cf->getSlug()][$value]); + $form->submit(array($cf->getSlug() => $this->cacheAnswersMapping[$cf->getSlug()][$value])); $value = $form->getData()[$cf->getSlug()]; diff --git a/DependencyInjection/ChillPersonExtension.php b/DependencyInjection/ChillPersonExtension.php index b1fb57099..f74ed63aa 100644 --- a/DependencyInjection/ChillPersonExtension.php +++ b/DependencyInjection/ChillPersonExtension.php @@ -62,6 +62,7 @@ class ChillPersonExtension extends Extension implements PrependExtensionInterfac $loader->load('services/controller.yml'); $loader->load('services/search.yml'); $loader->load('services/menu.yml'); + $loader->load('services/command.yml'); } private function handlePersonFieldsParameters(ContainerBuilder $container, $config) diff --git a/Export/Aggregator/GenderAggregator.php b/Export/Aggregator/GenderAggregator.php index 58e0df994..6f4314a76 100644 --- a/Export/Aggregator/GenderAggregator.php +++ b/Export/Aggregator/GenderAggregator.php @@ -83,6 +83,10 @@ class GenderAggregator implements AggregatorInterface return $this->translator->trans('woman'); case Person::MALE_GENDER : return $this->translator->trans('man'); + case Person::BOTH_GENDER: + return $this->translator->trans('both'); + case null: + return $this->translator->trans('Not given'); case '_header' : return $this->translator->trans('Gender'); default: diff --git a/Export/Filter/GenderFilter.php b/Export/Filter/GenderFilter.php index dc8697de0..9039e4315 100644 --- a/Export/Filter/GenderFilter.php +++ b/Export/Filter/GenderFilter.php @@ -51,17 +51,19 @@ class GenderFilter implements FilterInterface, $builder->add('accepted_genders', ChoiceType::class, array( 'choices' => array( 'Woman' => Person::FEMALE_GENDER, - 'Man' => Person::MALE_GENDER + 'Man' => Person::MALE_GENDER, + 'Both' => Person::BOTH_GENDER, + 'Not given' => 'null' ), 'choices_as_values' => true, - 'multiple' => false, - 'expanded' => false + 'multiple' => true, + 'expanded' => true )); } public function validateForm($data, ExecutionContextInterface $context) { - if ($data['accepted_genders'] === null) { + if (!is_array($data['accepted_genders']) || count($data['accepted_genders']) === 0 ) { $context->buildViolation("You should select an option") ->addViolation(); } @@ -70,8 +72,13 @@ class GenderFilter implements FilterInterface, public function alterQuery(QueryBuilder $qb, $data) { $where = $qb->getDQLPart('where'); - $clause = $qb->expr()->in('person.gender', ':person_gender'); - + $isIn = $qb->expr()->in('person.gender', ':person_gender'); + + if (!\in_array('null', $data['accepted_genders'])) { + $clause = $isIn; + } else { + $clause = $qb->expr()->orX($isIn, $qb->expr()->isNull('person.gender')); + } if ($where instanceof Expr\Andx) { $where->add($clause); @@ -80,7 +87,11 @@ class GenderFilter implements FilterInterface, } $qb->add('where', $where); - $qb->setParameter('person_gender', $data['accepted_genders']); + $qb->setParameter('person_gender', \array_filter( + $data['accepted_genders'], + function($el) { + return $el !== 'null'; + })); } /** diff --git a/Resources/config/doctrine/Person.orm.yml b/Resources/config/doctrine/Person.orm.yml index 19d4abb07..df911068b 100644 --- a/Resources/config/doctrine/Person.orm.yml +++ b/Resources/config/doctrine/Person.orm.yml @@ -28,6 +28,7 @@ Chill\PersonBundle\Entity\Person: gender: type: string length: 9 + nullable: true memo: type: text default: '' diff --git a/Resources/config/services/command.yml b/Resources/config/services/command.yml new file mode 100644 index 000000000..c99e488a4 --- /dev/null +++ b/Resources/config/services/command.yml @@ -0,0 +1,10 @@ +services: + Chill\PersonBundle\Command\ImportPeopleFromCSVCommand: + arguments: + $logger: '@logger' + $helper: '@Chill\MainBundle\Templating\TranslatableStringHelper' + $em: '@Doctrine\ORM\EntityManagerInterface' + $customFieldProvider: '@Chill\CustomFieldsBundle\Service\CustomFieldProvider' + $eventDispatcher: '@Symfony\Component\EventDispatcher\EventDispatcherInterface' + tags: + - { name: console.command } diff --git a/Resources/migrations/Version20181005140249.php b/Resources/migrations/Version20181005140249.php new file mode 100644 index 000000000..6ca116295 --- /dev/null +++ b/Resources/migrations/Version20181005140249.php @@ -0,0 +1,26 @@ +abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.'); + + $this->addSql('ALTER TABLE chill_person_person ALTER gender DROP NOT NULL'); + } + + public function down(Schema $schema) : void + { + $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.'); + + $this->addSql('ALTER TABLE chill_person_person ALTER gender SET NOT NULL'); + } +} From 2a9aff304aa96a482e9327277540835440e534ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 8 Oct 2018 22:07:47 +0200 Subject: [PATCH 2/2] remove inexistant css file (person.css) --- CHANGELOG.md | 1 + Resources/views/layout.html.twig | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 173f90ee6..20c235d81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,4 +6,5 @@ Master branch - Launch an event on person import ; - Allow person to have a `null` gender ; - Allow filters and aggregator to handle null gender ; +- remove inexistant `person.css` file diff --git a/Resources/views/layout.html.twig b/Resources/views/layout.html.twig index 0d6bd372e..19ff40fd9 100644 --- a/Resources/views/layout.html.twig +++ b/Resources/views/layout.html.twig @@ -24,13 +24,6 @@ {% extends "ChillMainBundle::layoutWithVerticalMenu.html.twig" %} -{% block css %} - {% stylesheets output="css/person.css" filter="cssrewrite" - "bundles/chillperson/css/person.css" - %} - - {% endstylesheets %} -{% endblock %} {% block top_banner %}