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

change_list_filter_confirm peoduces wrong url parameters #1007

Closed
danaki opened this issue Nov 27, 2022 · 10 comments
Closed

change_list_filter_confirm peoduces wrong url parameters #1007

danaki opened this issue Nov 27, 2022 · 10 comments
Assignees

Comments

@danaki
Copy link

danaki commented Nov 27, 2022

class TariffAdmin(admin.ModelAdmin):
    change_list_template = "admin/change_list_filter_confirm.html"
    list_filter = (
        'merchant',
        'contract__legal'
    )

Selecting merchant and contract__legal in this example multiple times (and after that pressing Apply) produces an url like: http://127.0.0.1:8000/admin/backend/tariff/?contract__legal__legal_id__exact=1&contract__legal__legal_id__exact=1&merchant__merchant_id__exact=2&contract__legal__legal_id__exact=1&merchant__merchant_id__exact=3

While select inputs allow only single choise.

@sehmaschine
Copy link
Owner

sehmaschine commented Nov 28, 2022 via email

@danaki
Copy link
Author

danaki commented Nov 28, 2022

@sehmaschine
Copy link
Owner

sehmaschine commented Nov 28, 2022

I don't mean the template ... I'm talking about GrappelliAutocompleteFilter. Does the issue still exist when you remove this filter?

@danaki
Copy link
Author

danaki commented Nov 28, 2022

Well, GrappelliAutocompleteFilter is commented, I will update ticket text

@danaki
Copy link
Author

danaki commented Nov 28, 2022

Yes, the issue persists on this minimal example.

class TariffAdmin(admin.ModelAdmin):
    change_list_template = "admin/change_list_filter_confirm.html"
    list_filter = (
        'merchant',
    )

where

class Tariff(models.Model):
    tariff_id = models.BigAutoField(primary_key=True)
    merchant = models.ForeignKey(Merchant, models.DO_NOTHING, blank=True, null=True)

Applying filter multiple times produces url like this:
http://127.0.0.1:8000/admin/backend/tariff/?merchant__merchant_id__exact=2&merchant__merchant_id__exact=4&merchant__merchant_id__exact=7

@sehmaschine
Copy link
Owner

I see, thank for clarifying. We'll take a look asap. If you're in a hurry, feel free to submit a PR.

@sehmaschine sehmaschine self-assigned this Nov 29, 2022
@danaki
Copy link
Author

danaki commented Dec 28, 2022

By digging into source I found out that in filter.html

<select class="grp-filter-choice" data-field-name="{{ field_name }}">

field_name appears to be empty and admin_list_filter() registered by @register.simple_tag is not called. Django's version appears to be called instead. I'm running Django 3.2 and followed all the steps of installation in grappeli documentation. Does that mean that tag search order is different since some versions of django? How do I force it to use grapelli's version?

@danaki
Copy link
Author

danaki commented Dec 29, 2022

The issue appears to be more complex and I'm still curious why on Earth django doesnt look up grappeli's template tags, but it also appears that Grapelli's filters do not properly handle related fields.

By default it is not a problem, the page immidiately refresh once you choose an option from select box. But if you have filters that require confirmation ("Apply" button) and your filters configured to look in relations like so:

    list_filter = (
        'contract__legal'
    )

then it incorrectly "glues" together selected items to be used for window.location later.

Here's the patch to fix this problem.

diff --git a/grappelli/static/grappelli/js/grappelli.js b/grappelli/static/grappelli/js/grappelli.js
index 26ba70aa..7ab46ff2 100644
--- a/grappelli/static/grappelli/js/grappelli.js
+++ b/grappelli/static/grappelli/js/grappelli.js
@@ -126,7 +126,8 @@ var inputTypes = [
                     // Split query param to get the fieldName
                     var fieldName = param.split('=')[0];
                     if (fieldName.search('__') != -1) {
-                        fieldName = param.split('__')[0];
+                        var lastIndex = fieldName.lastIndexOf('__');
+                        fieldName = fieldName.slice(0, lastIndex);
                     }
                     // Check if fieldName already exists in searchStringDict and add it resp. its values
                     var fieldNameIndex = windowQueryDict.findIndex(el => el.fieldName === fieldName);
diff --git a/grappelli/templatetags/grp_tags.py b/grappelli/templatetags/grp_tags.py
index d8feea50..29d9ee52 100644
--- a/grappelli/templatetags/grp_tags.py
+++ b/grappelli/templatetags/grp_tags.py
@@ -212,7 +212,11 @@ def admin_list_filter(cl, spec):
     field_name = getattr(spec, "field", None)
     parameter_name = getattr(spec, "parameter_name", None)
     if field_name is not None:
-        field_name = spec.field.name
+        target_field = getattr(field_name, "target_field", None)
+        if target_field is not None:
+            field_name = '%s__%s' % (spec.field_path, spec.field.target_field.name)
+        else:
+            field_name = spec.field.name
     elif parameter_name is not None:
         field_name = spec.parameter_name
     try:

without it keys like "merchant__merchant_id__exact" reduced to just "merchant" while "merchant__merchant_id" would be more correct. If you wish I can create a PR later.

@sehmaschine
Copy link
Owner

I think the only change needed in order to resolve this is to move grp_tags to the end here:

{% load i18n admin_urls static admin_list grp_tags %}

At least with my setup, this seems to work.

Can you please check that?

@danaki
Copy link
Author

danaki commented Jan 4, 2023

Ok, this might fix the lookup of admin_list_filter, gonna try it tonight.

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

No branches or pull requests

2 participants