django / django-localflavor

Country-specific Django helpers, formerly of contrib fame
https://django-localflavor.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
812 stars 289 forks source link

Override formfield method on Brazilian Model Fields #489

Open lucasgueiros opened 1 year ago

lucasgueiros commented 1 year ago

Hello. The Model Fields on Brazilian localflavor does not override the method formfield. By instance, the BRPostalCodeField is defined as:

class BRPostalCodeField(CharField):
    """
    A model field for the brazilian zip code

    .. versionadded:: 2.2
    """

    description = _("Postal Code")

    def __init__(self, *args, **kwargs):
        kwargs['max_length'] = 9
        super().__init__(*args, **kwargs)
        self.validators.append(validators.BRPostalCodeValidator())

But in other countries this method was overrided. As instance, this is the Canadian PostalCode:

class CAPostalCodeField(CharField):
    """
    A model field that stores the Canadian Postal code in the database.

    Forms represent it as a :class:`~localflavor.ca.forms.CAPostalCodeField` field.

    .. versionadded:: 4.0
    """

    description = _("Canadian Postal Code")

    def __init__(self, *args, **kwargs):
        kwargs['max_length'] = 7
        super().__init__(*args, **kwargs)

    def formfield(self, **kwargs):
        defaults = {'form_class': CAPostalCodeFormField}
        defaults.update(kwargs)
        return super().formfield(**defaults)

I made this change on Brazilian fields and it has passed all the tests. Do I need to write more tests or I can already submit a pull request?

claudep commented 1 year ago

It would be nice if you can provide a test that fails before your changes and passes afterwards (so that it would catch any future regression)!

lucasgueiros commented 1 year ago

Thanks, @claudep ! I wrote 3 tests and altered one more. They fail with the current code but pass with my commits. I also added max_length and min_length validatiosn to the forms.BRZipCodeFIeld, so I could write it a test case similar to BRCPFFIeld and BRCNPJField. Finnaly, I removed the formfield override I made to BRStateField because it doesn't changed anything on the field behaviour. In future, it would be possible to enhance this field and, maybe, this override would be valuable. I will send all change as a Pull Request.