yourlabs / django-autocomplete-light

A fresh approach to autocomplete implementations, specially for Django. Status: v4 alpha, v3 stable, v2 & v1 deprecated.
https://django-autocomplete-light.readthedocs.io
MIT License
1.79k stars 467 forks source link

3.3.0-rc6 ModelChoiceField form for ManyToManyField breaks if referenced model has __str__defined #1009

Open dlparker opened 6 years ago

dlparker commented 6 years ago

Using Django 2.0.4 and 3.3.0-rc6, python3.6.

The form shown below works with the models shown below if I return the pk from the __str__ method on the TagHelper model. If I return the "tag" field, it blows up during rendering. I have provided some captured rendering trace.

Looks to me like the optgoups method of the WidgetMixin is converting the supplied model instances to string when collecting the list of selected choices. This ends up at the filter_choices_to_render method of the QuerySetSelectMixin, and that method thinks that the list is full of primary keys.

Form:

class PhotoTagForm(forms.ModelForm):

    tags = forms.ModelChoiceField(
        queryset=TagHelper.objects.all().order_by('id'),
        widget=autocomplete.ModelSelect2Multiple(url='photos-tag-autocomplete',
                                         forward=['photo',]),
    )
    class Meta:
        model = PhotoTags
        fields = ('__all__')`

Models:


class TagHelper(models.Model):
    tag = models.CharField(max_length=500, unique=True)

    def __str__(self):
        # uncomment this and everything works dandy
        #return str(self.pk)
        # the following triggers the problem
        return self.tag

class PhotoTags(models.Model):
    photo = models.OneToOneField(Photo, on_delete=models.CASCADE)
    tags = models.ManyToManyField(TagHelper)

Debug trace of render failure:

Top:

screen shot 2018-04-26 at 3 35 07 pm

Relevant details:

screen shot 2018-04-26 at 3 32 48 pm
jpic commented 6 years ago

Seems like the error is in the autocomplete view, it responds with tag names instead of pks. It should respond as such:

{"results": [{"id": "1", "text": "test 1", "selected_text": "test 1"}, {"id": "2", "text": "test 2", "selected_text": "test 2"}, {"id": "3", "text": "test 3", "selected_text": "test 3"}, {"id": "4", "text": "test 4", "selected_text": "test 4"}, {"id": "5", "text": "test 5", "selected_text": "test 5"}, {"id": "6", "text": "test 6", "selected_text": "test 6"}, {"id": "7", "text": "test 7", "selected_text": "test 7"}, {"id": "8", "text": "test 8", "selected_text": "test 8"}, {"id": "9", "text": "test 9", "selected_text": "test 9"}, {"id": "10", "text": "test 10", "selected_text": "test 10"}], "pagination": {"more": true}}

When the user selects an option, what's in "id" should end in "selected_choices" (which is what the select html tag options values are)

dlparker commented 6 years ago

Dang, that was a surprise. You are right. If I had realized that the autocomplete view gets called during the rendering pass, I would have looked there.

Hmm. Not seeing it though.

The error is rendering the page on GET, not POST, by the way, so there is no call to autocomplete from the browser. If there are no TagHelper items related to the PhotoTag instance, the page loads fine. Also, the "create_field" option is working correctly. So I guess I don't follow your logic yet.

And here is the Autocomplete view, so you can check it out:


class TagAutocomplete(autocomplete.Select2QuerySetView):
    def get_queryset(self):
        qs = TagHelper.objects.all().order_by('id')

        photo_id = self.forwarded.get('photo', None)
        if photo_id:
            photo = Photo.objects.get(pk=int(photo_id))
            qs = qs.exclude(tag__in=photo.get_tags())
        if self.q:
            qs = qs.filter(tag__istartswith=self.q)
        return qs
jpic commented 6 years ago

That's the only thing i could guess given that the autocomplete view was missing. If it's not coming from the view then it's coming from the widget. Are you able to reproduce your issue in the test project ? (I tried the many to many example in the test project and it's working fine for me)

dlparker commented 6 years ago

Don't know what that is, so I'll find out and try it. Thanks for the suggestion.

jpic commented 6 years ago

There is a demo project with a many to many usage example: http://django-autocomplete-light.readthedocs.io/en/master/install.html#install-the-demo-project

It's in the test_project directory of this directory. It should help if you can isolate your issue in a new app of this test_project.

Otherwise, that would be the place where it casts options to string: https://github.com/yourlabs/django-autocomplete-light/blob/master/src/dal/widgets.py#L136-L137

Pretty weird indeed, patch welcome ;)

jpic commented 6 years ago

Do not exclude that sometimes, this method is executed with models as value, and sometimes with pks as value, depending on wether it's processing from GET or POST ... I'm not sure about this but i think i recall i might have had such a case to deal with, but can't reproduce, perhaps i was mis using django forms ? Anyway, it seems like it's working fine in the many to many example of the test_project, comparing both might help you find a lead.

dlparker commented 6 years ago

Running the test project blows up for me on the migrate step. I will try again tomorrow with a different machine, just incase the problem is something in my environment. I am running on Mac OSX, and although I use it for lots of django projects, weird things can happen. Tomorrow's attempt will be on a linux box, just to change as many things as possible. Since we all know that's how you run an experiment :)

jpic commented 6 years ago

Did you try removing the db file and running migrate to produce a new one ? I thought it would work on MacOSX. Thanks a heap for your report though, seems like a critical issue that needs to be fixed, we really need to figure out the conditions that make this happen.

mammique commented 4 years ago

Had the same problem, resolved by moving the widget from the Form's fields:

    tags = forms.ModelChoiceField(
        queryset=TagHelper.objects.all().order_by('id'),
        widget=autocomplete.ModelSelect2Multiple(url='photos-tag-autocomplete',
                                         forward=['photo',]),
    )

To the Meta class:

    class Meta:
        model = PhotoTags
        fields = ('__all__')`
        widgets = {
            'tags': autocomplete.ModelSelect2Multiple(
               'photos-tag-autocomplete', forward=['photo',]
            )
        }