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

Forwading ManyToMany model field #1257

Open sridhar562345 opened 3 years ago

sridhar562345 commented 3 years ago

models.py

class InvestmentGroup(models.Model):
    name = models.CharField(max_length=200, blank=True, null=True)
    persons = models.ManyToManyField('Person', blank=True, related_name='investment_groups')
    lead_investor = models.ForeignKey(
        'Person', blank=True, null=True, on_delete=models.CASCADE, related_name='lead_investment_groups'
    )

forms.py

class InvestmentGroupModelForm(forms.ModelForm):
    def __init__(self, *args, **kwargs):
        super(InvestmentGroupModelForm, self).__init__(*args, **kwargs)

    class Meta:
        model = models.InvestmentGroup
        fields = '__all__'
        widgets = {
            "lead_investor": autocomplete.ModelSelect2(
                url="lead-investor-autocomplete",
                forward=["name"]
            )
        }

AutoCompleteview

class LeadInvestorAutoComplete(autocomplete.Select2QuerySetView):
    def get_queryset(self):
        # Don't forget to filter out results depending on the visitor !
        if not self.request.user.is_authenticated:
            return models.Person.objects.none()

        qs = models.Person.objects.all()
        persons = self.forwarded.get('persons', None)
        print(persons) # Output: []
        if persons:
            qs = qs.filter(person__in=persons)

        return qs

I am getting empty values if I forward many to many field like this but works for other fields like name or foreign keys. Does anyone know how to forward ManyToMany field?

sridhar562345 commented 3 years ago

@huxley @bd808 @blueyed @xrmx sorry for mentioning you guys here, Is it possible to forward m2m field?

jpic commented 2 years ago

@sridhar562345 it should be supported: https://github.com/yourlabs/django-autocomplete-light/blob/master/src/dal/static/autocomplete_light/autocomplete_light.js#L312

@gagarski probably uses it

sridhar562345 commented 2 years ago

@jpic Maybe it is not working if use filter_horizontal for many to many field. I will check this when I am free.

gagarski commented 2 years ago

Hi there. It's been a while since I implemented this forwarding logic and tbh currently pretty far from Python and Django (so it wouldn't be honest to say that I use it 😊). But I'll try to remember as much as possible.

I mostly used DAL outside django admin, yet I definitely has ensured that it at least basically works in admin. I assume you are talking about admin site, since you are mentioning filter_horizontal.

As far as I remember, forwarding is mostly done on the JS side, so what matters is form field type, not a model field. If I recall correctly it worked fine with standard HTML multiple select field and DAL's Select2 with multi-select option (it actually uses hidden HTML multi-select as a backing field). Also, as I can see, it handles set-of-checkbox representation of multi-select. For all of that, I believe you can even find a Selenium tests in the test_project.

I definitely did not check it with filter_horizontal since it's first time I hear of it (as I said, I did not use Django admin site too much). If you want to debug it, you should probably look into the code mentioned by @jpic (getForwardStrategy and dealing with its result). Probably filter_horizontal control uses different kind of backing HTML control, so it should be handled separately.

I believe there is even a way to handle this control on client side without monkey-patching DAL code: https://django-autocomplete-light.readthedocs.io/en/master/tutorial.html?highlight=forward#customizing-forwarding-logic (Yet this way is poorly documented, shame on me). Here are the examples of registering JS handlers: https://github.com/yourlabs/django-autocomplete-light/blob/master/test_project/forward_different_fields/static/js_handlers.js

Yet I think adding this control to DAL code would be a nice contribution.

jpic commented 2 years ago

Hi @gagarski !

I hope you are doing well! Thank you so much for answering!

I'm wondering if I should add support for the new autocomplete-light script in the existing forward code, or if it's possible to make it much simpler.

For example, I was wondering if it would solve the same use cases if the autocomplete widget would just be POST'ing the whole form to the autocomplete endpoint, in which case you'd set something like "query-mode=POST" and get everything instead of having to configure field forwarding. Are there any use case that your code supports and that this idea wouldn't? For this particular use case it would work, because whatever hidden field filter_horizontal is using would be POST'ed ...

If you have had, since last time you have worked with this, any new idea about this please let us know!

@sridhar562345 Let's see how filter_horizontal support looks like in a pull request then!

Have fun!

:tophat:

gagarski commented 2 years ago

Hi @jpic! It's good to know that DAL is alive and being developed!

Regarding the forwarding feature: the whole journey towards complicating the things started with field renaming during forward. Let's start with textbook country + city case. While implementing a form with such chained fields you'll probably end up implementing something like CountryAutocompleteView and CityAutocompleteView. The latter will probably be country-aware and accept smth like country as a parameter. Now you want to have onother form which has src_country+src_city and dest_country+dest_city fields. For both src_city and dest_city you'll probably want the same "filter by country logic". Should you create another two AutocompleteViews? Or should you make CityAutocompleteView aware of src_city and dst_city parameters? Or should you somehow manage to name both fields as country? At the time I liked neither of these options and decided to make forward mechanism to be a layer to "translate from context into AutocompleteView API language". It currently supports quite specific set of cases (and the rest of them actually can be worked around more easily that renaming), but I could not think of other cases.

Forwarding the whole form and using django forms mechanism to parse it on view side sounds tempting actually. But it actually suffers from the same dest_country+dest_city issue. Either your view should be aware of all the forms that are using it, or you should do some translation using JS hook (if DAL would provide one), or stick with similar names everywhere. Either way it becomes DAL user's responsibility (yet it may be a good thing for DAL:)).

The example with countries and cities is actually synthetic and for me there was more complex reasons why I could not stick with naming strategy. Yet I admit that I may be only one person in the world who had this issue :D

Another issues with post-the-whole-form approach that comes to mind are that form data can contain files and might be quite big (you wouldn't want to post 10 MB files just to get autocomplete suggestions) and classic issues with django formsets (you might need to have a lot of formset-specific code).

Anyway, I think the decision is up to you.