From e5966fa2b111ff3d38b801bb1006867cfab7bb3b Mon Sep 17 00:00:00 2001
From: Martin Mahner <martin@mahner.org>
Date: Mon, 22 Apr 2024 04:56:56 +0200
Subject: [PATCH] Overall cleanup

---
 README.md                                     | 56 ++++++++-----------
 dynamic_raw_id/admin.py                       |  7 ++-
 dynamic_raw_id/filters.py                     |  2 +-
 .../dynamic_raw_id/js/dynamic_raw_id.js       | 15 +++--
 dynamic_raw_id/tests/test_admin_inlines.py    |  2 +-
 dynamic_raw_id/views.py                       | 21 ++++---
 dynamic_raw_id/widgets.py                     | 54 +++++-------------
 pyproject.toml                                |  2 +-
 8 files changed, 59 insertions(+), 100 deletions(-)

diff --git a/README.md b/README.md
index 060e42e..9bbe93b 100644
--- a/README.md
+++ b/README.md
@@ -8,7 +8,7 @@ string value on change and can be overridden via a template.
 
 See this example:
 
-<img src="https://d.pr/i/1kv7d.png" style="max-height: 500px;" alt="Screenshot of Django Admin"/>
+<img src="https://d.pr/i/1kv7d.png" style="max-height: 400px;" alt="Screenshot of Django Admin"/>
 
 ## Compatibility Matrix:
 
@@ -27,7 +27,7 @@ interface (`<select>`) for fields that are ForeignKey. This can result in long l
 times and unresponsive admin pages for models with thousands of instances, or with
 multiple ForeinKeys.
 
-The normal fix is to use Django's [ModelAdmin.raw_id_fields](https://docs.djangoproject.com/en/4.0/ref/contrib/admin/#django.contrib.admin.ModelAdmin.raw_id_fields),
+The normal fix is to use Django's [ModelAdmin.raw_id_fields][raw_id_docs],
 but by default it *only* shows the raw id of the related model instance, which is
 somewhat unhelpful.
 
@@ -100,68 +100,53 @@ implementation if all you want is your object's `__unicode__` value. To change
 the value displayed all you need to do is implement the correct template.
 
 django-dynamic-raw-id looks for this template
-structure `dynamic_raw_id/<app>/<model>.html``
+structure `dynamic_raw_id/<app>/<model>.html`
 and `dynamic_raw_id/<app>/multi_<model>.html` (for multi-value lookups).
 
 For instance, if I have a blog post with a `User` dynamic_raw_id field that I want
-display as `Firstname Lastname``, I would create the template
-``dynamic_raw_id/auth/user.html` with:
+display as `Firstname Lastname`, I would create the template
+`dynamic_raw_id/auth/user.html` with:
 
 ```html
 <span>{{ object.0.first_name }} {{ object.0.last_name }}</span>
 ```
 
-A custom admin URL prefix
-=========================
+### A custom admin URL prefix
 
 If you have your admin *and* the dynamic_raw_id scripts located on a different
-prefix than `/admin/dynamic_raw_id/` you need adjust the `DYNAMIC_RAW_ID_MOUNT_URL``
-JS variable.
+prefix than `/admin/dynamic_raw_id/` you need adjust the `DYNAMIC_RAW_ID_MOUNT_URL`
+Javascript variable.
 
 Example:
 
 ```python
-    # In case the app is setup at /foobar/dynamic_raw_id/
+# In case the app is setup at /foobar/dynamic_raw_id/
 path('foobar/dynamic_raw_id/', include('dynamic_raw_id.urls')),
 ```
 
 ```html
 
-<script>window.DYNAMIC_RAW_ID_MOUNT_URL = "{% url "
-admin:index
-" %}";</script>
+<script>window.DYNAMIC_RAW_ID_MOUNT_URL = "{% url 'admin:index' %}";</script>
 ```
 
-An ideal place is the admin `base_site.html` template. Full example:
+An ideal place is the admin `admin/base_site.html` template. Full example:
 
 ```html
-{% extends "admin/base.html" %}
-
-{% block title %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %}
+{% extends "admin/base_site.html" %}
 
 {% block extrahead %}
-{{ block.super }}
-<script>
-  window.DYNAMIC_RAW_ID_MOUNT_URL = "{% url "
-  admin:index
-  " %}";
-</script>
-{% endblock %}
-
-{% block branding %}
-<h1 id="site-name"><a href="{% url 'admin:index' %}">{{ site_header|default:_('Django
-  administration') }}</a></h1>
+  {{ block.super }}
+  <script>
+    window.DYNAMIC_RAW_ID_MOUNT_URL = "{% url 'admin:index' %}";
+  </script>
 {% endblock %}
-
-{% block nav-global %}{% endblock %}
 ```
 
-Testing and Local Development
-=============================
+# Testing and Local Development
 
 The testsuite uses Selenium to do frontend tests, we require Firefox and
-[geckodriver](https://github.com/mozilla/geckodriver) to be installed. You can
-install geckodriver on OS X with Homebrew:
+[geckodriver][geckodriver] to be installed. You can install geckodriver on OS X with
+Homebrew:
 
 ```bash
 $ brew install geckodriver
@@ -201,3 +186,6 @@ $ django-admin migrate
 $ django-admin createsuperuser
 $ django-admin runserver
 ```
+
+[raw_id_docs]: https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.raw_id_fields
+[geckodriver]: https://github.com/mozilla/geckodriver
diff --git a/dynamic_raw_id/admin.py b/dynamic_raw_id/admin.py
index 86bb2be..3978ca3 100644
--- a/dynamic_raw_id/admin.py
+++ b/dynamic_raw_id/admin.py
@@ -7,7 +7,7 @@
 from dynamic_raw_id.widgets import DynamicRawIDMultiIdWidget, DynamicRawIDWidget
 
 if TYPE_CHECKING:
-    from django.db.models.fields import Field as DbField
+    from django.db.models.fields import Field as DB_Field
     from django.forms.fields import Field as FormField
     from django.http import HttpRequest
 
@@ -17,19 +17,20 @@ class DynamicRawIDMixin(BaseModelAdmin):
 
     def formfield_for_foreignkey(
         self,
-        db_field: DbField,
+        db_field: DB_Field,
         request: HttpRequest | None = None,
         **kwargs: Any,
     ) -> FormField:
         if db_field.name in self.dynamic_raw_id_fields:
             rel = db_field.remote_field
             kwargs["widget"] = DynamicRawIDWidget(rel, self.admin_site)
+            kwargs["help_text"] = ""
             return db_field.formfield(**kwargs)
         return super().formfield_for_foreignkey(db_field, request, **kwargs)
 
     def formfield_for_manytomany(
         self,
-        db_field: DbField,
+        db_field: DB_Field,
         request: HttpRequest | None = None,
         **kwargs: Any,
     ) -> FormField:
diff --git a/dynamic_raw_id/filters.py b/dynamic_raw_id/filters.py
index 795ab15..b101f13 100644
--- a/dynamic_raw_id/filters.py
+++ b/dynamic_raw_id/filters.py
@@ -56,7 +56,7 @@ def __init__(  # noqa: PLR0913 Too Many arguments
         self.form = self.get_form(request, rel, model_admin.admin_site)
 
     def choices(self, changelist: Any) -> list:
-        """Filter choices are not available."""
+        """Filter choices do not exist, since we choose the popup value."""
         return []
 
     def expected_parameters(self) -> str:
diff --git a/dynamic_raw_id/static/dynamic_raw_id/js/dynamic_raw_id.js b/dynamic_raw_id/static/dynamic_raw_id/js/dynamic_raw_id.js
index acd317a..7f28cd0 100644
--- a/dynamic_raw_id/static/dynamic_raw_id/js/dynamic_raw_id.js
+++ b/dynamic_raw_id/static/dynamic_raw_id/js/dynamic_raw_id.js
@@ -7,14 +7,15 @@
  */
 if (!windowname_to_id) {
   function windowname_to_id(text) {
-    text = text.replace(/__dot__/g, '.');
-    text = text.replace(/__dash__/g, '-');
-    return text;
+    return text
+      .replace(/__dot__/g, '.')
+      .replace(/__dash__/g, '-')
+      .replace(/__\d+$/, '');
   }
 }
 
 function dismissRelatedLookupPopup(win, chosenId) {
-  const name = windowname_to_id(win.name).replace(/__\d+$/, '');
+  const name = windowname_to_id(win.name);
   const elem = document.getElementById(name);
   if (elem.className.indexOf('vManyToManyRawIdAdminField') !== -1 && elem.value) {
     elem.value += `,${chosenId}`;
@@ -83,14 +84,12 @@ function dismissRelatedLookupPopup(win, chosenId) {
     $('.dynamic_raw_id-clear-field').click(function() {
       const $this = $(this);
       $this
-        .parent()
-        .find('.vForeignKeyRawIdAdminField, .vManyToManyRawIdAdminField')
+        .closest('.vForeignKeyRawIdAdminField, .vManyToManyRawIdAdminField')
         .val('')
         .trigger('change');
 
       $this
-        .parent()
-        .find('.dynamic_raw_id_label')
+        .closest('.dynamic_raw_id_label')
         .html('&nbsp;');
     });
 
diff --git a/dynamic_raw_id/tests/test_admin_inlines.py b/dynamic_raw_id/tests/test_admin_inlines.py
index 1fbc151..cdb39d9 100644
--- a/dynamic_raw_id/tests/test_admin_inlines.py
+++ b/dynamic_raw_id/tests/test_admin_inlines.py
@@ -20,7 +20,7 @@ def test_widgets(selenium: WebDriver) -> None:
     Django Admin adds three 'inline' rows to the base model which we fill with
     three users.
 
-    Then click the 'Add another' link three times, and fill antoher three users.
+    Then click the 'Add another' link three times, and fill another three users.
     """
 
     # Create six test users first
diff --git a/dynamic_raw_id/views.py b/dynamic_raw_id/views.py
index c2a471a..40e85bd 100644
--- a/dynamic_raw_id/views.py
+++ b/dynamic_raw_id/views.py
@@ -16,7 +16,7 @@ def label_view(  # noqa: PLR0913 Too Many arguments
     request: HttpRequest,
     app_name: str,
     model_name: str,
-    template_name: str = "",
+    template_name: str,
     multi: bool = False,
     template_object_name: str = "object",
 ) -> HttpResponse:
@@ -38,9 +38,9 @@ def label_view(  # noqa: PLR0913 Too Many arguments
         model = apps.get_model(app_name, model_name)
     except LookupError:
         msg = f"Model {app_name}.{model_name} does not exist."
-        return HttpResponseBadRequest(settings.DEBUG and msg or "")
+        return HttpResponseBadRequest(msg)
 
-    # Check 'view' or 'change' permission depending on Django's version
+    # Check 'view' permission
     if not request.user.has_perm(f"{app_name}.view_{model_name}"):
         return HttpResponseForbidden()
 
@@ -48,24 +48,23 @@ def label_view(  # noqa: PLR0913 Too Many arguments
         if multi:
             model_template = f"dynamic_raw_id/{app_name}/multi_{model_name}.html"
             objs = model.objects.filter(pk__in=object_list)
-            objects = []
-            for obj in objs:
-                change_url = reverse(
-                    f"admin:{app_name}_{model_name}_change", args=[obj.pk]
-                )
-                objects.append((obj, change_url))
+            objects = [
+                (obj, reverse(f"admin:{app_name}_{model_name}_change", args=[obj.pk]))
+                for obj in objs
+            ]
             extra_context = {template_object_name: objects}
         else:
             model_template = f"dynamic_raw_id/{app_name}/{model_name}.html"
             obj = model.objects.get(pk=object_list[0])
             change_url = reverse(f"admin:{app_name}_{model_name}_change", args=[obj.pk])
             extra_context = {template_object_name: (obj, change_url)}
+
     # most likely, the pk wasn't convertable
     except ValueError:
         msg = "ValueError during lookup"
-        return HttpResponseBadRequest(settings.DEBUG and msg or "")
+        return HttpResponseBadRequest(msg)
     except model.DoesNotExist:
         msg = "Model instance does not exist"
-        return HttpResponseBadRequest(settings.DEBUG and msg or "")
+        return HttpResponseBadRequest(msg)
 
     return render(request, (model_template, template_name), extra_context)
diff --git a/dynamic_raw_id/widgets.py b/dynamic_raw_id/widgets.py
index ef3f6fc..d0ad8ef 100644
--- a/dynamic_raw_id/widgets.py
+++ b/dynamic_raw_id/widgets.py
@@ -1,11 +1,10 @@
 from __future__ import annotations
 
 from typing import TYPE_CHECKING, Any
+from urllib.parse import urlencode
 
 from django import forms
-from django.conf import settings
 from django.contrib.admin import widgets
-from django.core.exceptions import ImproperlyConfigured
 from django.urls import reverse
 from django.utils.encoding import force_str
 
@@ -14,49 +13,33 @@
     from django.template import Context
 
 
-class DynamicRawIDImproperlyConfigured(ImproperlyConfigured):
-    pass
-
-
 class DynamicRawIDWidget(widgets.ForeignKeyRawIdWidget):
     template_name: str = "dynamic_raw_id/admin/widgets/dynamic_raw_id_field.html"
 
     def get_context(self, name: str, value: Any, attrs: dict[str, Any]) -> Context:
         context = super().get_context(name, value, attrs)
-        model = self.rel.model
-        app_label = model._meta.app_label  # noqa: SLF001 Private member accessed
-        model_name = model._meta.object_name.lower()  # noqa: SLF001 Private member accessed
-        related_url = reverse(
-            f"admin:{app_label}_{model_name}_changelist",
-            current_app=self.admin_site.name,
-        )
+        app_name = self.rel.model._meta.app_label  # noqa: SLF001 Private member accessed
+        model_name = self.rel.model._meta.object_name.lower()  # noqa: SLF001 Private member accessed
 
-        params = self.url_parameters()
-        url = "?" + "&".join([f"{k}={v}" for k, v in params.items()]) if params else ""
-        if "class" not in attrs:
-            attrs["class"] = (
-                "vForeignKeyRawIdAdminField"  # The JavaScript looks for this hook.
-            )
-        app_name = model._meta.app_label.strip()  # noqa: SLF001 Private member accessed
-        model_name = model._meta.object_name.lower().strip()  # noqa: SLF001 Private member accessed
+        attrs.setdefault("class", "vForeignKeyRawIdAdminField")
 
         context.update(
-            {
-                "name": name,
-                "app_name": app_name,
-                "model_name": model_name,
-                "related_url": related_url,
-                "url": url,
-            }
+            name=name,
+            app_name=app_name,
+            model_name=model_name,
+            related_url=reverse(
+                f"admin:{app_name}_{model_name}_changelist",
+                current_app=self.admin_site.name,
+            ),
+            url=f"?{urlencode(self.url_parameters())}",
         )
         return context
 
     @property
     def media(self) -> forms.Media:
-        extra = "" if settings.DEBUG else ".min"
         return forms.Media(
             js=[
-                f"admin/js/vendor/jquery/jquery{extra}.js",
+                "admin/js/vendor/jquery/jquery.min.js",
                 "admin/js/jquery.init.js",
                 "admin/js/core.js",
                 "dynamic_raw_id/js/dynamic_raw_id.js",
@@ -65,17 +48,6 @@ def media(self) -> forms.Media:
 
 
 class DynamicRawIDMultiIdWidget(DynamicRawIDWidget):
-    def value_from_datadict(
-        self,
-        data: dict[str, Any],
-        files: Any | None,
-        name: str,
-    ) -> str | None:
-        value = data.get(name)
-        if value:
-            return value.split(",")
-        return None
-
     def render(
         self,
         name: str,
diff --git a/pyproject.toml b/pyproject.toml
index 0e205ac..70704ea 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -88,7 +88,7 @@ filterwarnings = [
 
 [tool.coverage]
 run.omit = [
-  "django_markup/tests/*",
+  "dynamic_raw_id/tests/*",
 ]
 report.exclude_lines = [
     "pragma: no cover",