django-localflavor icon indicating copy to clipboard operation
django-localflavor copied to clipboard

fix: Accept French Postal Services identifiers in forms

Open Chatewgne opened this issue 11 months ago • 3 comments

This PR is intended to fix the following issue : https://github.com/django/django-localflavor/issues/504

The form validation in FRSIRENENumberMixin is missing a special case from the SIRENE register.

The identifiers for Postal Services need a special validation process and are not currently validated.

Source (in french) : https://www.sirene.fr/static-resources/doc/lettres/lettre-16-novembre-2013.pdf

All Changes

  • [x] Add an entry to the docs/changelog.rst describing the change.

  • [x] Add an entry for your name in the docs/authors.rst file if it's not already there.

Chatewgne avatar Mar 27 '24 12:03 Chatewgne

Sorry for the delay for reviewing, could you please add/complement a test for this?

claudep avatar Apr 11 '24 18:04 claudep

Thanks for getting back to me @claudep, I have added the test cases :)

Chatewgne avatar Apr 22 '24 08:04 Chatewgne

The exception only applies to SIRET, not to SIREN, so this update isn't correct. I think we should remove the mixin and use stdnum to validate both of the fields directly in the form class. This would add few lines of common code to both classes but it makes things clearer.

benkonrath avatar May 16 '24 11:05 benkonrath

Any reason to turn this PR as a draft?

claudep avatar Aug 16 '24 15:08 claudep

The exception only applies to SIRET, not to SIREN, so this update isn't correct. I think we should remove the mixin and use stdnum to validate both of the fields directly in the form class. This would add few lines of common code to both classes but it makes things clearer.

I have removed the mixin as suggested and revised validation for both SIRET and SIREN.

Reminder : the SIREN is included in the SIRET (9 first characters)

Validation :

  • SIREN has 9 chars AND validates luhn algorithm
  • SIRET has 14 chars AND validates luhn algorithm OR starts with "356000000" and the sum of the 14 characters is a multiple of 5

Sources :

Chatewgne avatar Aug 23 '24 14:08 Chatewgne

Thanks!

benkonrath avatar Aug 23 '24 20:08 benkonrath