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

Using a ManyToManyWidget does not allow to ignore an empty value #1788

Open
nemesifier opened this issue Apr 11, 2024 · 1 comment
Open

Using a ManyToManyWidget does not allow to ignore an empty value #1788

nemesifier opened this issue Apr 11, 2024 · 1 comment
Labels

Comments

@nemesifier
Copy link

nemesifier commented Apr 11, 2024

Describe the bug

    def clean(self, value, row=None, **kwargs):
        if not value:
            return self.model.objects.none()

Try this in the python shell:

>>> self.model.objects.none() == self.model.objects.none()
False

This means that even modifying a field to add Model.objects.none() to the empty values doesn't work to ignore the empty queryset, so the cleaned value is always saved.

This can cause issues when the model being imported is referenced by an optional OneToOne relationship from another model and this model has an optional m2m, in this case the m2m is saved even when it shouldn't and it raises an error.

To Reproduce
...

Versions (please complete the following information):

  • Django Import Export: 3.30
  • Python 3.8.10
  • Django 4.2

Expected behavior

I think we could return an empty list instead of an empty queryset in this case, that way we can add '[]' to empty_values in Field to solve it. Alternatively we could just return an empty string and modify the rest of the code to ignore falsy or empty values.

Screenshots

Screenshot from 2024-04-11 11-22-07

@nemesifier nemesifier added the bug label Apr 11, 2024
@matthewhegarty
Copy link
Contributor

Thanks for raising. I would suggest to look at release 4. We will be changing the interface in a few places to standardize it, so would be a good opportunity to put a fix in. I think that returning an empty list would work ok, so no need to modify Field.

If you felt like creating a PR, that would be appreciated. Contributing guidelines are here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants