Conversation
|
Thanks for your PR! I'll try to review it in the next couple of days. |
benkonrath
left a comment
There was a problem hiding this comment.
Thanks for the PR and sorry for the delay in getting to this.
There are a couple of changes that I think should be made to improve this a little.
Let me know if you have any comments about the things I mentioned. Or you'd prefer me to just implement the changes (since there was a big delay in the review).
The string freeze for the 3.1 release is planned in two days (20 May). If you have time for the updates in the next two days then we can try to get this in. Otherwise, it can be included in the next release.
| } | ||
|
|
||
|
|
||
| class ByPostalCodeField(BaseKwargsUpdatedField, forms.CharField): |
There was a problem hiding this comment.
-
The country code should be in capital letters for consistency:
ByPostalCodeField->BYPostalCodeField -
I think it's better if we use
forms.RegexFieldinstead offorms.CharFieldsince the postal code is exactly 6 digits. That way we don't need to use max/min 6 and the duplicate (and possibly confusing) error messages.
| return value.upper() | ||
|
|
||
|
|
||
| class BYRegionField(BaseKwargsUpdatedField, forms.TypedChoiceField): |
There was a problem hiding this comment.
Instead of making ChoiceField for the regions, I think it's better to make a forms.Select widget that can be used in forms. The BY_REGIONS_CHOICES can be set directly on the model instead of using this custom form field.
Here's an example of a forms.Select widget:
django-localflavor/localflavor/md/forms.py
Lines 35 to 43 in 456dd7e
| if value is None: | ||
| return value |
There was a problem hiding this comment.
I don't think it's a good idea to use None as the default for empty value. 'empty_value': None should be remove from the initial_options and this code should be updated like this:
| if value is None: | |
| return value | |
| if value in self.empty_values: | |
| return self.empty_value |
This will let users set None if they prefer this. Otherwise an empty string will be used.
|
|
||
| initial_options = { | ||
| 'regex': r'[A-Z]{2}\d{7}', | ||
| 'max_length': 9, |
There was a problem hiding this comment.
max_length isn't needed as the regex will take care of checking the length. The same applies to other places with max_length is set with a regex that enforces the length (below).
| defaults = {'choices_form_class': BYRegionFormField} | ||
| defaults.update(kwargs) |
There was a problem hiding this comment.
I think it's better to set choices to BY_REGIONS_CHOICES instead of using BYRegionFormField for the choices_form_class.
Here's an example:
django-localflavor/localflavor/md/models.py
Lines 37 to 54 in 456dd7e
| '': '', | ||
| } | ||
|
|
||
| def _gen_pass_id(self, is_valid): |
There was a problem hiding this comment.
I think it's better to have a few valid and invalid examples like this so that tests will never get a false positive or negative. That said, the chances are pretty low that there will be problems so feel free to keep things like this if this is what you prefer.
|
Moved this to the 4.0 milestone as I didn't have time to make the changes in the last couple of days. |
4.0 (2023-04-22) ------------------ New flavors: - Nepal LocalFlavor: Support for Nepal added (`gh-451 <https://github.com/django/django-localflavor/pull/451>`_). - Belarus localflavor (`gh-422 <https://github.com/django/django-localflavor/pull/422>`_, `gh-442 <https://github.com/django/django-localflavor/pull/442>`_). - Ghana localflavor (`gh-460 <https://github.com/django/django-localflavor/pull/460>`_). New fields for existing flavors: - Added `fr.forms.FRRNAField` models field (`gh-443 <https://github.com/django/django-localflavor/pull/443>`_). - Added permanent account number(PAN) field in Indian flavor. (`gh-457 <https://github.com/django/django-localflavor/pull/457>`_). - Added the Canadian Models fields. (`gh-465 <https://github.com/django/django-localflavor/pull/465>`_). Modifications to existing flavors: - Properly validate IBANs using BBAN to ensure invalid IBANs cannot be entered, updated IBAN_SEPA_COUNTRIES and IBAN_COUNTRY_CODE_LENGTH to latest data (`gh-486 <https://github.com/django/django-localflavor/pull/486>`_). - Fix typo in Marijampolė county name in LTCountySelect (`gh-480 <https://github.com/django/django-localflavor/pull/480>`_). - Add support for new Finnish identity codes (`gh-478 <https://github.com/django/django-localflavor/pull/478>`_). - CIF spanish starting with 'U' bug resolved (`gh-469 <https://github.com/django/django-localflavor/pull/469>`_). - Fix error code for BRPostalCodeValidator (`gh-448 <https://github.com/django/django-localflavor/pull/448>`_). - Fix spelling of the India state of Chhattisgarh (`gh-444 <https://github.com/django/django-localflavor/pull/444>`_). - Fix CURP regex for MX flavor (`gh-449 <https://github.com/django/django-localflavor/pull/449>`_). - Change text based fields that inherited from `django.forms.Field` to inherit from `django.forms.CharField`. The following fields have been updated (`gh-446 <https://github.com/django/django-localflavor/pull/446>`_): - `at.forms.ATSocialSecurityNumberField` - `br.forms.BRStateChoiceField` - `ca.forms.CAProvinceField` - `ca.forms.CASocialInsuranceNumberField` - `ch.forms.CHIdentityCardNumberField` - `cu.forms.CUProvinceField` - `cu.forms.CURegionField` - `cz.forms.CZBirthNumberField` - `cz.forms.CZICNumberField` - `de.forms.DEIdentityCardNumberField` - `ee.forms.EEBusinessRegistryCode` - `ee.forms.EEPersonalIdentificationCode` - `fi.forms.FISocialSecurityNumber` - `gr.forms.GRTaxNumberCodeField` - `hr.forms.HRJMBAGField` - `hr.forms.HRJMBGField` - `hr.forms.HRLicensePlateField` - `hr.forms.HRPostalCodeField` - `id_.forms.IDLicensePlateField` - `id_.forms.IDNationalIdentityNumberField` - `id_.forms.IDPostCodeField` - `il.forms.ILIDNumberField` - `in_.forms.INAadhaarNumberField` - `in_.forms.INStateField` - `ir.forms.IRIDNumberField` - `it.forms.ITVatNumberField` - `lt.forms.LTPostalCodeField` - `lv.forms.LVPersonalCodeField` - `lv.forms.LVPostalCodeField` - `no.forms.NOSocialSecurityNumber` - `nz.forms.NZBankAccountNumberField` - `pt.forms.PTCitizenCardNumberField` - `pt.forms.PTSocialSecurityNumberField` - `ro.forms.ROCountyField` - `tr.forms.TRIdentificationNumberField` - `us.forms.USStateField` - Removed inconvenient word VACA from CURP_INCONVENIENT_WORDS for MX flavor Other changes: - Use 'return value' when value is in the empty_values list (`gh-461 <https://github.com/django/django-localflavor/pull/461>`_). - Dropped support for Django 2.2, 3.0 and 3.1. - Dropped support for Python 3.5. - Added support for Python 3.10 and 3.11.
Hi! I've added the localflavor for Belarus!
Released: