Convert MultipleChoiceField to List of type String

[kimbriancanavan]

The django_filters.MultipleChoiceFilter is currently being converted to a String (due to MultipleChoiceFilter eventually inheriting from django.forms.fields.ChoiceField).

This PR changes that behavior so that it instead gets converted into a List of type String.

[coveralls]

Coverage increased (+0.01%) to 94.226% when pulling 634a3c0 on kimbriancanavan:master into 090ce6e on graphql-python:master.

[phalt]

Is this a breaking change?

[kimbriancanavan]

Is this a breaking change?

@phalt yes, it does change the way MultipleChoiceFilter is resolved in graphql (from simple string to list of strings), however resolving to string is a bug. It is possible that by fixing this bug, some existing queries in the wild will change from simply not working as intended to returning a proper error (because those queries would now be in violation of the schema)

e.g. Let's say you have a filter defined like so:

class ExampleFilter(django_filters.FilterSet):
    my_choice = django_filters.MultipleChoiceFilter(field_name='example', choices=(('A', 'A'), ('B', 'B')))

And you wanted to use this filter in a query. Since the MultipleChoiceField is resolved to string type, schema expects something like the following:

query {
    blabla (my_choice: "A") {
        [rest of query here]
    }
}

However this will not work, because the MultipleChoiceField expects a list or tuple:

class MultipleChoiceField(ChoiceField):
    hidden_widget = MultipleHiddenInput
    widget = SelectMultiple
    default_error_messages = {
        'invalid_choice': _('Select a valid choice. %(value)s is not one of the available choices.'),
        'invalid_list': _('Enter a list of values.'),
    }

    def to_python(self, value):
        if not value:
            return []
        elif not isinstance(value, (list, tuple)):
            raise ValidationError(self.error_messages['invalid_list'], code='invalid_list')
        return [str(val) for val in value]

If we make MultipleChoiceField resolve to list of strings then our original query can change to:

query {
    blabla (my_choice: ["A"]) {
        [rest of query here]
    }
}

or if we want to choose multiple options:

query {
    blabla (my_choice: ["A", "B"]) {
        [rest of query here]
    }
}

This will now work with the underlying MultipleChoiceField

[phalt]

I guess we should either document the change as part of release notes, or implement a deprecation warning so we don't break projects that are using the current behaviour without realizing it is wrong?

[kimbriancanavan]

I think I would prefer the documentation option. Any ideas how to proceed with that?

[phalt]

@kimbriancanavan looking at previous releases we have updated the documentation or written "migration paths" - code examples of before / after to help people update.

[zbyte64]

What is our versioning strategy with backwards incompatible changes?

[phalt]

@zbyte64 semver.org I guess. We've done major upgrades in the past with breaking changes, and we've written migration documents to handle it.

[ProjectCheshire]

I came across this bug just now!

[mvanlonden]

Since there's a breaking change here, let's hold off until we're ready for v3. I think it'd be best to do this once graphene hits v3 which should be by the end of the month.

[zbyte64]

In other places we are converting choices into enums, but here we are not. I also wonder if django_filters.MultipleChoiceFilter support enums.

[dmitrikuksik]

@phalt Stacked with that bug now. Will it be finally merged or appeared with a new graphene version?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jkimbo]

Thanks for the contribution @kimbriancanavan !

[topbloc-beiswenger]

Is there a workaround for this in the meantime, or do we have to wait for v3 to be released?