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

Return MSFList, not plain list, from form field #118

Merged
merged 1 commit into from
Feb 14, 2021

Conversation

aleh-rymasheuski
Copy link
Contributor

This pull request fixes MultiSelectFormField to return the proper type - MSFList instead of list.

Consider this model:

from django.db import models
from multiselectfield import MultiSelectField


class TestModel(models.Model):
    msf_field = MultiSelectField(choices=[("a", "Value a"), ("b", "Value b")])

    def save(self, *args, **kwargs):
        if self.id:
            # an edit
            old = self.__class__.objects.get(pk=self.pk)
            print(f"{type(self.msf_field)=} | {type(old.msf_field)=}")
            print(f"{repr(self.msf_field)=} | {repr(old.msf_field)=}")
            print(f"{str(self.msf_field)=} | {str(old.msf_field)=}")
            assert self.msf_field == old.msf_field, "unequal"
            assert str(self.msf_field) == str(old.msf_field), "string representation unequal"  # <- this fails

        super().save(*args, **kwargs)

Whenever a user edits an instance of this model through the admin (default ModelAdmin is sufficient for testing) without editing the multiselect field, the second assertion will fail.

While no sane person will actually perform such assertions in model save, audit-logging libraries (like https://github.com/jazzband/django-auditlog) may report false changes based on unequal string representation.

@coveralls
Copy link

coveralls commented Oct 30, 2020

Coverage Status

Coverage increased (+0.1%) to 91.579% when pulling 5a056b5 on alieh-rymasheuski:form-return-type into 9c43f19 on goinnn:master.

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.

3 participants