Move TCNo and INN from calculator to localized providers #108

krsriq · 2020-12-05T10:52:15Z

What is the reason for this PR?

Inn.php verifies the Russian tax identification number (INN number), TCNo.php the Turkish Identity Number, but both are currently placed in the core Calculator.

A new feature
Fixed an issue (resolve Move Russian Inn and Turkish TCNo from Calculator to localized providers #106)

Author's checklist

Follow the Contribution Guide
New features and changes are documented

Summary of changes

Moved Calculator\Inn methods to Provider\ru_RU\Company, Calculator\TCNo methods to Provider\tr_TR\Person
Marked Inn and TcNo classes and methods deprecated
Called the new Company/Person methods from Inn and TcNo for BC
Moved the test methods from Calculator to their respective Company and Person test classes

Review checklist

All checks have passed
Changes are approved by maintainer

src/Faker/Calculator/Inn.php

bram-pkg · 2020-12-06T12:54:09Z

src/Faker/Calculator/TCNo.php

    }

    /**
     * Checks whether a TCNo has a valid checksum
     *
+     * @deprecated Use {@link \Faker\Provider\tr_TR\Person::tcNoIsValid()} instead


Please be consistent in where you place the @deprecated (after @return)

Looks better now, thanks.

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

src/Faker/Calculator/Inn.php

src/Faker/Calculator/TCNo.php

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

bram-pkg

One small change requested, otherwise it looks good!

src/Faker/Provider/ru_RU/Company.php

src/Faker/Provider/tr_TR/Person.php

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

bram-pkg · 2020-12-06T21:54:31Z

Let's enable single_quotes and then rebase.

* Improve consistency of comments and phpdoc * Add StyleCI changes

* Enable whitespace_after_comma_in_array fixer * Apply fixes from StyleCI

* Enable modernize_types_casting fixer * Apply fixes from StyleCI

krsriq · 2020-12-08T11:59:39Z

Let's enable single_quotes and then rebase.

@bramceulemans - rebased a while ago.

src/Faker/Calculator/Inn.php

bram-pkg · 2020-12-13T20:16:06Z

Looks good to me now.
/cc @GrahamCampbell

test/Faker/Provider/tr_TR/PersonTest.php

GrahamCampbell · 2020-12-13T22:30:38Z

test/Faker/Provider/tr_TR/PersonTest.php

+
+    /**
+     * @dataProvider tcNoChecksumProvider
+     * @param $tcNo


Added param type now.

test/Faker/Provider/tr_TR/PersonTest.php

Signed-off-by: Daniel Schmelz <daniel@schmelz.org>

krsriq · 2020-12-14T16:26:21Z

@GrahamCampbell added @dataProvider @param types here now (and fixed them in the other class that was missing this with #147).

pimjansen · 2020-12-21T18:16:33Z

src/Faker/Calculator/Inn.php

-        }
-
-        return (string) (($sum % 11) % 10);
+        return \Faker\Provider\ru_RU\Company::inn10Checksum($inn);


Im not happy with this where we have a generic method call a locale specific?

pimjansen · 2020-12-21T18:16:41Z

src/Faker/Calculator/Inn.php

     */
    public static function isValid($inn)
    {
-        return strlen($inn) === 10 && self::checksum($inn) === $inn[9];
+        return \Faker\Provider\ru_RU\Company::inn10IsValid($inn);


@pimjansen the problem here is that INN and TCNo are locale specific but somehow ended up in the base Calculator.

We keep them in Calculator (and call the locale specific providers) and mark them deprecated for BC , but they should be removed from the Calculator asap IMO - especially considering Faker 2.0, where it makes no sense to have this in the core.

pimjansen · 2020-12-21T18:17:00Z

src/Faker/Calculator/TCNo.php

-        $eleventhDigit = ($evenSum + $oddSum + $tenthDigit) % 10;
-
-        return $tenthDigit . $eleventhDigit;
+        return \Faker\Provider\tr_TR\Person::tcNoChecksum($identityPrefix);


pimjansen · 2020-12-21T18:17:06Z

src/Faker/Calculator/TCNo.php

     */
    public static function isValid($tcNo)
    {
-        return self::checksum(substr($tcNo, 0, -2)) === substr($tcNo, -2, 2);
+        return \Faker\Provider\tr_TR\Person::tcNoIsValid($tcNo);


# Conflicts: # src/Faker/Calculator/Inn.php # src/Faker/Calculator/TCNo.php # src/Faker/Provider/ru_RU/Company.php # test/Faker/Calculator/InnTest.php # test/Faker/Calculator/TCNoTest.php

pimjansen · 2020-12-21T19:03:28Z

@krsriq lgtm after a green build

krsriq · 2020-12-21T19:30:10Z

@pimjansen done!

* move tcno and inn from calculator to localized providers * change inn method name * cleanup * Update src/Faker/Calculator/Inn.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * cleanup * fix bc change * Update src/Faker/Calculator/Inn.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Calculator/TCNo.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Calculator/Inn.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Calculator/TCNo.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Calculator/TCNo.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Provider/ru_RU/Company.php * Update src/Faker/Provider/ru_RU/Company.php * Update src/Faker/Provider/ru_RU/Company.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Provider/ru_RU/Company.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Provider/tr_TR/Person.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Provider/ru_RU/Company.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Update src/Faker/Provider/ru_RU/Company.php Co-authored-by: Bram Ceulemans <bramceulemans@me.com> * Improve consistency of comments and phpdoc (FakerPHP#110) * Improve consistency of comments and phpdoc * Add StyleCI changes * Enable whitespace_after_comma_in_array fixer (FakerPHP#112) * Enable whitespace_after_comma_in_array fixer * Apply fixes from StyleCI * Minor cleanup (FakerPHP#113) * enable StyleCI single_quote (FakerPHP#116) * Apply fixes from StyleCI * Enable modernize_types_casting fixer (FakerPHP#117) * Enable modernize_types_casting fixer * Apply fixes from StyleCI * move tcno and inn from calculator to localized providers * fix bc change * fix merge issue * fix newline * re-add docblock empty line * add type to data provider params Signed-off-by: Daniel Schmelz <daniel@schmelz.org> * run make cs * run make cs Co-authored-by: Bram Ceulemans <bramceulemans@me.com> Co-authored-by: Graham Campbell <GrahamCampbell@users.noreply.github.com>

move tcno and inn from calculator to localized providers

5b70290

krsriq changed the title ~~move tcno and inn from calculator to localized providers~~ Move TCNo and INN from calculator to localized providers Dec 5, 2020

pimjansen added the enhancement New feature or request label Dec 5, 2020

krsriq added 2 commits December 6, 2020 07:32

change inn method name

055c744

cleanup

08e6219

krsriq mentioned this pull request Dec 6, 2020

Move Russian Inn and Turkish TCNo from Calculator to localized providers #106

Closed

bram-pkg requested changes Dec 6, 2020

View reviewed changes

krsriq and others added 3 commits December 6, 2020 15:16

Update src/Faker/Calculator/Inn.php

c3991da

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

cleanup

4ca7ed9

fix bc change

10ce218

bram-pkg requested changes Dec 6, 2020

View reviewed changes

krsriq and others added 5 commits December 6, 2020 17:36

Update src/Faker/Calculator/Inn.php

a93dc7b

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Calculator/TCNo.php

7c90c54

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Calculator/Inn.php

6aa2d83

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Calculator/TCNo.php

b3dda42

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Calculator/TCNo.php

0076381

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

bram-pkg requested changes Dec 6, 2020

View reviewed changes

src/Faker/Provider/ru_RU/Company.php Outdated Show resolved Hide resolved

Update src/Faker/Provider/ru_RU/Company.php

3c7e4ea

bram-pkg requested changes Dec 6, 2020

View reviewed changes

src/Faker/Provider/ru_RU/Company.php Outdated Show resolved Hide resolved

Update src/Faker/Provider/ru_RU/Company.php

9b02807

bram-pkg requested changes Dec 6, 2020

View reviewed changes

krsriq and others added 4 commits December 6, 2020 22:17

Update src/Faker/Provider/ru_RU/Company.php

4055644

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Provider/ru_RU/Company.php

002a5f6

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Provider/tr_TR/Person.php

2c07246

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

Update src/Faker/Provider/ru_RU/Company.php

5592321

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

krsriq mentioned this pull request Dec 6, 2020

Change ru_RU inn doc FakerPHP/fakerphp.github.io#26

Merged

Update src/Faker/Provider/ru_RU/Company.php

e5c155b

Co-authored-by: Bram Ceulemans <bramceulemans@me.com>

krsriq mentioned this pull request Dec 6, 2020

How about we enable single_quote in StyleCI? #114

Closed

bram-pkg approved these changes Dec 6, 2020

View reviewed changes

Bram Ceulemans and others added 11 commits December 6, 2020 23:22

Improve consistency of comments and phpdoc (FakerPHP#110)

0796afa

* Improve consistency of comments and phpdoc * Add StyleCI changes

Enable whitespace_after_comma_in_array fixer (FakerPHP#112)

53d4d1a

* Enable whitespace_after_comma_in_array fixer * Apply fixes from StyleCI

Minor cleanup (FakerPHP#113)

f436e90

enable StyleCI single_quote (FakerPHP#116)

d32cae9

Apply fixes from StyleCI

2975b73

Enable modernize_types_casting fixer (FakerPHP#117)

dfebfb7

* Enable modernize_types_casting fixer * Apply fixes from StyleCI

move tcno and inn from calculator to localized providers

3304773

fix bc change

7fdc21f

fix merge issue

2d203ec

Merge branch 'main' into move_tcno_and_inn

dd609ac

fix newline

eb2a2e4

GrahamCampbell requested changes Dec 8, 2020

View reviewed changes

src/Faker/Calculator/Inn.php Outdated Show resolved Hide resolved

GrahamCampbell requested a review from Nyholm December 8, 2020 17:18

re-add docblock empty line

4803391

GrahamCampbell requested changes Dec 13, 2020

View reviewed changes

add type to data provider params

e639d96

Signed-off-by: Daniel Schmelz <daniel@schmelz.org>

krsriq mentioned this pull request Dec 14, 2020

Fix docblock dataProvider param type #147

Merged

3 tasks

pimjansen reviewed Dec 21, 2020

View reviewed changes

krsriq added 2 commits December 21, 2020 19:36

Merge remote-tracking branch 'upstream/main' into move_tcno_and_inn

1fe7013

# Conflicts: # src/Faker/Calculator/Inn.php # src/Faker/Calculator/TCNo.php # src/Faker/Provider/ru_RU/Company.php # test/Faker/Calculator/InnTest.php # test/Faker/Calculator/TCNoTest.php

run make cs

3e2de67

krsriq added 2 commits December 21, 2020 20:20

Merge branch 'main' into move_tcno_and_inn

5129c33

run make cs

0582827

pimjansen merged commit 830891d into FakerPHP:main Dec 21, 2020

krsriq deleted the move_tcno_and_inn branch January 23, 2022 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move TCNo and INN from calculator to localized providers #108

Move TCNo and INN from calculator to localized providers #108

krsriq commented Dec 5, 2020

bram-pkg Dec 6, 2020

krsriq Dec 6, 2020

bram-pkg left a comment

bram-pkg commented Dec 6, 2020 •

edited

Loading

krsriq commented Dec 8, 2020

bram-pkg commented Dec 13, 2020

GrahamCampbell Dec 13, 2020

krsriq Dec 14, 2020

krsriq commented Dec 14, 2020

pimjansen Dec 21, 2020

pimjansen Dec 21, 2020

krsriq Dec 21, 2020

pimjansen Dec 21, 2020

pimjansen Dec 21, 2020

pimjansen commented Dec 21, 2020

krsriq commented Dec 21, 2020

Move TCNo and INN from calculator to localized providers #108

Move TCNo and INN from calculator to localized providers #108

Conversation

krsriq commented Dec 5, 2020

What is the reason for this PR?

Author's checklist

Summary of changes

Review checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bram-pkg left a comment

Choose a reason for hiding this comment

bram-pkg commented Dec 6, 2020 • edited Loading

krsriq commented Dec 8, 2020

bram-pkg commented Dec 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krsriq commented Dec 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pimjansen commented Dec 21, 2020

krsriq commented Dec 21, 2020

bram-pkg commented Dec 6, 2020 •

edited

Loading