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

Fix TypeError in CountryFilter.choices() #460

Merged
merged 2 commits into from
Apr 1, 2024
Merged

Fix TypeError in CountryFilter.choices() #460

merged 2 commits into from
Apr 1, 2024

Conversation

foutrelis
Copy link
Contributor

When no country is selected, self.used_parameters.get(self.field.name) will return None. This raises a TypeError when CountryFilter.choices() tries to check whether a specific country choice is currently selected.

Avoid this by checking that the parameter is not empty similarly to how Django's built-in admin.filters.RelatedFieldListFilter checks for this.

Fixes: 04662d1 ("Support Django 5.0")

When no country is selected, self.used_parameters.get(self.field.name)
will return None. This raises a TypeError when CountryFilter.choices()
tries to check whether a specific country choice is currently selected.

Avoid this by checking that the parameter is not empty similarly to how
Django's built-in admin.filters.RelatedFieldListFilter checks for this.

Fixes: 04662d1 ("Support Django 5.0")
@foutrelis foutrelis mentioned this pull request Apr 1, 2024
@foutrelis
Copy link
Contributor Author

The issue is reproducible with something simple like:

from django.db import models
from django_countries.fields import CountryField

class Person(models.Model):
    name = models.CharField(max_length=100)
    country = CountryField()
from django.contrib import admin
from django_countries.filters import CountryFilter

from .models import Person

@admin.register(Person)
class PersonAdmin(admin.ModelAdmin):
    list_filter = [("country", CountryFilter)]

@claudep
Copy link

claudep commented Apr 1, 2024

I guess a regression test is required here.

@foutrelis
Copy link
Contributor Author

I guess a regression test is required here.

Good suggestion, thanks. I extended the test for this method to also check without a selected country.

Add a regression test for the case where no country has been specified
in the filter parameters. There was already a test where a country was
given, but the issue in question only occurred when none was selected.
@SmileyChris SmileyChris merged commit 5d7600c into SmileyChris:main Apr 1, 2024
4 checks passed
@foutrelis foutrelis deleted the fix-type-error-in-country-filter branch April 1, 2024 20:48
@SmileyChris
Copy link
Owner

Thanks! I merged and released a new point version with your fix.

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.

None yet

3 participants