[pull] main from django:main

django/db/models/sql/query.py

@@ -534,7 +534,8 @@ def get_aggregation(self, using, aggregate_exprs):
             # Queries with distinct_fields need ordering and when a limit is
             # applied we must take the slice from the ordered query. Otherwise
             # no need for ordering.
-            inner_query.clear_ordering(force=False)
+            if inner_query.orderby_issubset_groupby:
+                inner_query.clear_ordering(force=False)
             if not inner_query.distinct:
                 # If the inner query uses default select and it has some
                 # aggregate annotations, then we must make sure the inner
@@ -2338,6 +2339,33 @@ def add_ordering(self, *ordering):
         else:
             self.default_ordering = False
 
+    @property
+    def orderby_issubset_groupby(self):
+        if self.extra_order_by:
+            # Raw SQL from extra(order_by=...) can't be reliably compared
+            # against resolved OrderBy/Col expressions. Treat as not a subset.
+            return False
+        if self.group_by in (None, True):
+            # There is either no aggregation at all (None), or the group by
+            # is generated automatically from model fields (True), in which
+            # case the order by is necessarily a subset of them.
+            return True
+        if not self.order_by:
+            # Although an empty set is always a subset, there's no point in
+            # clearing ordering when there isn't any. Avoid the clone() below.
+            return True
+        # Don't pollute the original query (might disrupt joins).
+        q = self.clone()
+        order_by_set = {
+            (
+                order_by.resolve_expression(q)
+                if hasattr(order_by, "resolve_expression")
+                else F(order_by).resolve_expression(q)
+            )
+            for order_by in q.order_by
+        }
+        return order_by_set.issubset(self.group_by)
+
     def clear_ordering(self, force=False, clear_default=True):
         """
         Remove any ordering settings if the current query allows it without

django/template/defaulttags.py

@@ -1296,6 +1296,10 @@ def querystring(context, *args, **kwargs):
     Keyword arguments are treated as an extra, final mapping. These mappings
     are processed sequentially, with later arguments taking precedence.
 
+    Passing `None` as a value removes the corresponding key from the result.
+    For iterable values, `None` entries are ignored, but if all values are
+    `None`, the key is removed.
+
     A query string prefixed with `?` is returned.
 
     Raise TemplateSyntaxError if a positional argument is not a mapping or if
@@ -1327,7 +1331,8 @@ def querystring(context, *args, **kwargs):
                 "querystring requires mappings for positional arguments (got "
                 "%r instead)." % d
             )
-        for key, value in d.items():
+        items = d.lists() if isinstance(d, QueryDict) else d.items()
+        for key, value in items:
             if not isinstance(key, str):
                 raise TemplateSyntaxError(
                     "querystring requires strings for mapping keys (got %r "
@@ -1336,7 +1341,8 @@ def querystring(context, *args, **kwargs):
             if value is None:
                 params.pop(key, None)
             elif isinstance(value, Iterable) and not isinstance(value, str):
-                params.setlist(key, value)
+                # Drop None values; if no values remain, the key is removed.
+                params.setlist(key, [v for v in value if v is not None])
             else:
                 params[key] = value
     query_string = params.urlencode() if params else ""

docs/releases/6.0.1.txt

@@ -9,4 +9,6 @@ Django 6.0.1 fixes several bugs in 6.0.
 Bugfixes
 ========
 
-* ...
+* Fixed a regression in Django 6.0 where :ttag:`querystring` mishandled
+  multi-value :class:`~django.http.QueryDict` keys, both by only preserving the
+  last value and by incorrectly handling ``None`` values (:ticket:`36783`).

tests/aggregation_regress/tests.py

@@ -171,6 +171,24 @@ def assertObjectAttrs(self, obj, **kwargs):
         for attr, value in kwargs.items():
             self.assertEqual(getattr(obj, attr), value)
 
+    def test_count_preserve_group_by(self):
+        # new release of the same book
+        Book.objects.create(
+            isbn="113235613",
+            name=self.b4.name,
+            pages=self.b4.pages,
+            rating=4.0,
+            price=Decimal("39.69"),
+            contact=self.a5,
+            publisher=self.p3,
+            pubdate=datetime.date(2018, 11, 3),
+        )
+        qs = Book.objects.values("contact__name", "publisher__name").annotate(
+            publications=Count("id")
+        )
+        self.assertEqual(qs.order_by("id").count(), len(qs.order_by("id")))
+        self.assertEqual(qs.extra(order_by=["id"]).count(), len(qs.order_by("id")))
+
     def test_annotation_with_value(self):
         values = (
             Book.objects.filter(

tests/migrations/test_loader.py

@@ -682,8 +682,11 @@ def test_loading_order_does_not_create_circular_dependency(self):
         with tempfile.NamedTemporaryFile(
             mode="w", encoding="utf-8", suffix=".py", dir=tests_dir, delete=False
         ) as test_settings:
+            self.addCleanup(os.remove, test_settings.name)
             for attr, value in settings._wrapped.__dict__.items():
-                if attr.isupper():
+                # Only write builtin values so that any settings that reference
+                # a value that needs an import are omitted.
+                if attr.isupper() and type(value).__module__ == "builtins":
                     test_settings.write(f"{attr} = {value!r}\n")
             # Provide overrides here, instead of via decorators.
             test_settings.write(f"DATABASES = {settings.DATABASES}\n")
@@ -693,8 +696,6 @@ def test_loading_order_does_not_create_circular_dependency(self):
                 "INSTALLED_APPS=[a for a in INSTALLED_APPS if a.startswith('django')]\n"
             )
             test_settings.write(f"INSTALLED_APPS += {INSTALLED_APPS}\n")
-        test_settings_name = test_settings.name
-        self.addCleanup(os.remove, test_settings_name)
 
         test_environ = os.environ.copy()
         test_environ["PYTHONPATH"] = str(tests_dir)
@@ -709,7 +710,7 @@ def test_loading_order_does_not_create_circular_dependency(self):
             "app1",
             "--skip-checks",
             "--settings",
-            Path(test_settings_name).stem,
+            Path(test_settings.name).stem,
         ]
         try:
             subprocess.run(

tests/template_tests/syntax_tests/test_querystring.py

@@ -58,6 +58,22 @@ def test_querystring_multiple(self):
         context = RequestContext(request)
         self.assertRenderEqual("querystring_multiple", context, expected="?x=y&a=b")
 
+    @setup({"querystring_multiple_lists": "{% querystring %}"})
+    def test_querystring_multiple_lists(self):
+        request = self.request_factory.get("/", {"x": ["y", "z"], "a": ["b", "c"]})
+        context = RequestContext(request)
+        expected = "?x=y&x=z&a=b&a=c"
+        self.assertRenderEqual("querystring_multiple_lists", context, expected=expected)
+
+    @setup({"querystring_lists_with_replacement": "{% querystring a=1 %}"})
+    def test_querystring_lists_with_replacement(self):
+        request = self.request_factory.get("/", {"x": ["y", "z"], "a": ["b", "c"]})
+        context = RequestContext(request)
+        expected = "?x=y&x=z&a=1"
+        self.assertRenderEqual(
+            "querystring_lists_with_replacement", context, expected=expected
+        )
+
     @setup({"querystring_empty_params": "{% querystring qd %}"})
     def test_querystring_empty_params(self):
         cases = [{}, QueryDict()]
@@ -111,6 +127,44 @@ def test_querystring_remove_from_dict(self):
         context = RequestContext(request, {"my_dict": {"test": None}})
         self.assertRenderEqual("querystring_remove_dict", context, expected="?a=1")
 
+    @setup({"querystring_remove_querydict": "{% querystring request my_query_dict %}"})
+    def test_querystring_remove_querydict(self):
+        request = self.request_factory.get("/", {"x": "1"})
+        my_qd = QueryDict(mutable=True)
+        my_qd["x"] = None
+        context = RequestContext(
+            request, {"request": request.GET, "my_query_dict": my_qd}
+        )
+        self.assertRenderEqual("querystring_remove_querydict", context, expected="?")
+
+    @setup(
+        {"querystring_remove_querydict_many": "{% querystring request my_query_dict %}"}
+    )
+    def test_querystring_remove_querydict_many(self):
+        request = self.request_factory.get(
+            "/", {"test": ["value1", "value2"], "a": [1, 2]}
+        )
+
+        qd_none = QueryDict(mutable=True)
+        qd_none["test"] = None
+
+        qd_list_none = QueryDict(mutable=True)
+        qd_list_none.setlist("test", [None, None])
+
+        qd_empty_list = QueryDict(mutable=True)
+        qd_empty_list.setlist("test", [])
+
+        for qd in (qd_none, qd_list_none, qd_empty_list):
+            with self.subTest(my_query_dict=qd):
+                context = RequestContext(
+                    request, {"request": request.GET, "my_query_dict": qd}
+                )
+                self.assertRenderEqual(
+                    "querystring_remove_querydict_many",
+                    context,
+                    expected="?a=1&a=2",
+                )
+
     @setup({"querystring_variable": "{% querystring a=a %}"})
     def test_querystring_variable(self):
         request = self.request_factory.get("/")