Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/faker #2

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Feature/faker #2

wants to merge 14 commits into from

Conversation

christopheGaon
Copy link

@christopheGaon christopheGaon commented Jul 29, 2022

Have a faker Adding nir support for parameter

  • based on nir from Person Provider in Fr_fr from Faker
  • add Params gender, dateBirth,and departement for nir generation
  • using phpspec and NirValidator to test the generator
  • qual and test passed

Have a faker Adding nnp provider

  • Add provider to generate nnp valid
  • using phpspec and NnpValidator to test the generator
  • qual and test passed

bidule added 3 commits July 29, 2022 14:05
using hoa/regex to use same regex as NirValidator
using phpspec and NirValidator to test the generator
qual and test passed

todo :
generate InvalidNir
generate ValidNir from parameters
@christopheGaon christopheGaon marked this pull request as ready for review August 2, 2022 14:25
composer.json Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
.php-cs-fixer.dist.php Outdated Show resolved Hide resolved
src/Faker/NirProvider.php Outdated Show resolved Hide resolved
src/Faker/NirProvider.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
src/Faker/NirProvider.php Outdated Show resolved Hide resolved
christopheGaon and others added 4 commits August 3, 2022 12:14
review clean comments and fix

Co-authored-by: Dylan Broussard <[email protected]>
fix cs fixer

Co-authored-by: Dylan Broussard <[email protected]>
NirProvider.php
Provider for Faker to create nir from Params (gender, datebirth,departement)

NnpProvider.php
Provider for Faker to create nnp

TDD for thoses Providers
NirProviderSpec.php
NnpProviderSpec.php

revert on qual overrride
add NnpProvider.php suppress warning for psalm
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
src/Faker/NirProvider.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NnpProviderSpec.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
{
return [
'beNirFromParams' => function (string $value, \DateTime $dateNaissance, string $departement) {
$bool = preg_match(NirValidator::NIR_REGEX, $value, $matches);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Est-ce qu'on ne pourrait pas mettre un nom plus explicite que $bool ? Je ne comprends pas vraiment à quoi correspond la valeur

tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
src/Faker/NnpProvider.php Outdated Show resolved Hide resolved
*/
public function nnp(string $gender = null): string
{
// Gender
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Du coup même remarque sur le fait de stocker chaque valeur dans une variable et concaténer à la fin

Copy link
Author

@christopheGaon christopheGaon Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

donc tu preferes que je stocke les parts dans des var ? car la je renvois direct la concatenation

src/Faker/NirProvider.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@roukmoute roukmoute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il faudrait aussi revoir les commentaires, car certains sont en anglais et d'autres en français.
Il faudrait ne choisir qu'une seule langue.

composer.json Outdated Show resolved Hide resolved
pmd-ruleset.xml Outdated Show resolved Hide resolved
src/Faker/NirProvider.php Outdated Show resolved Hide resolved
$nir = $this->numberBetween(1, 2);
}

if ($date === null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il serait préférable de faire la condition comme au dessus, c'est à dire de vérifier si c'est bien un DatetimeInterface.

src/Faker/NirProvider.php Outdated Show resolved Hide resolved
tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
}

/**
* Valide un NIR généré à partir du validateur du package
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il faudrait revoir cette phrase car elle ne veut pas dire grand chose, le concept de package est très lié à Java, pas au monde PHP.

tests/Spec/Cnamts/Nir/Faker/NirProviderSpec.php Outdated Show resolved Hide resolved
return [
'beNirFromParams' => function (string $value, \DateTime $dateNaissance, string $departement) {
$bool = preg_match(NirValidator::NIR_REGEX, $value, $matches);
$verifYear = $matches['moisNaissance'] === $dateNaissance->format('m');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est du franglais pour le coup

tests/Spec/Cnamts/Nir/Faker/NnpProviderSpec.php Outdated Show resolved Hide resolved
christopheGaon and others added 3 commits August 5, 2022 10:11
first pass on cs fixer

Co-authored-by: Dylan Broussard <[email protected]>
Co-authored-by: Mathias STRASSER <[email protected]>
change NirProvider to AssureProvider as it override nir methode
refacto code
update qual setup
simplify test on NnpProviderSpec.php
bidule and others added 3 commits August 8, 2022 11:25
class AssureProvider extends Person
{
/* @phpstan-ignore-next-line */
public function nir($gender = null, $formatted = false, \DateTime $birthday = null, string $departmentCode = null): string

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi ne pas typer gender et formatted ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants