From 0224ee16a7134863528be8d383d253e7392f31e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kordowski?= Date: Sat, 27 Jan 2024 12:11:02 +0100 Subject: [PATCH 1/4] Do not change object order when coping --- ordered_model/models.py | 3 ++- tests/tests.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/ordered_model/models.py b/ordered_model/models.py index 29e8183f..25a1a337 100644 --- a/ordered_model/models.py +++ b/ordered_model/models.py @@ -211,7 +211,8 @@ def next(self): def save(self, *args, **kwargs): order_field_name = self.order_field_name - wrt_changed = self._wrt_map() != self._original_wrt_map + adding = self.pk is None or self._state.adding + wrt_changed = not adding and self._wrt_map() != self._original_wrt_map if wrt_changed and getattr(self, order_field_name) is not None: # do delete-like upshuffle using original_wrt values! diff --git a/tests/tests.py b/tests/tests.py index 06cf95fd..f1cfe474 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -173,6 +173,23 @@ def setUp(self): with assertNumQueries(self, 1): Answer.objects.get(id=self.q1_a1.id) + def test_copy(self): + q3 = Question.objects.create() + old_q1_a1_pk = self.q1_a1.pk + self.q1_a1.pk = None + self.q1_a1.question = q3 + self.q1_a1.save() + self.assertSequenceEqual( + Answer.objects.values_list("pk", "order"), + [ + (old_q1_a1_pk, 0), + (self.q1_a2.pk, 1), + (self.q2_a1.pk, 0), + (self.q2_a2.pk, 1), + (self.q1_a1.pk, 0), + ], + ) + def test_saved_order(self): self.assertSequenceEqual( Answer.objects.values_list("pk", "order"), @@ -300,6 +317,28 @@ def test_bottom(self): ) +class OrderWithRespectToCustomPKTest(TestCase): + def setUp(self): + group = CustomPKGroup.objects.create(name="g1") + self.item = CustomPKGroupItem.objects.create(name="g1 i1", group=group) + CustomPKGroupItem.objects.create(name="g1 i2", group=group) + + def test_copy(self): + group = CustomPKGroup.objects.create(name="g2") + self.item.name = "g2 i1" + self.item._state.adding = True + self.item.group = group + self.item.save() + self.assertSequenceEqual( + CustomPKGroupItem.objects.order_by("name").values_list("name", "order"), + [ + ("g1 i1", 0), + ("g1 i2", 1), + ("g2 i1", 0), + ], + ) + + class OrderWithRespectToReorderTests(TestCase): def setUp(self): q1 = Question.objects.create() From 2a84901c85d1ff281a1d377f562a39599393ec0d Mon Sep 17 00:00:00 2001 From: Chris Shucksmith Date: Mon, 29 Jan 2024 10:57:31 +0000 Subject: [PATCH 2/4] add CHANGES and comments --- CHANGES.md | 1 + ordered_model/models.py | 2 ++ tests/tests.py | 3 +++ 3 files changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index edef9814..50cad247 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,7 @@ Unreleased ---------- - Fix `post_delete` signal triggered upshuffles to do a potentially expensive full reordering of the owrt group (#307) - Support passing custom `--batch_size` to `reorder_model` management command (#303) +- Support Django model instance copy protocol (ie. clearing pk and saving) (#313) 3.7.4 - 2023-03-17 diff --git a/ordered_model/models.py b/ordered_model/models.py index 25a1a337..d0575971 100644 --- a/ordered_model/models.py +++ b/ordered_model/models.py @@ -211,6 +211,8 @@ def next(self): def save(self, *args, **kwargs): order_field_name = self.order_field_name + # support model-instance copy protocol, by detaching from _original_wrt_map + # https://docs.djangoproject.com/en/4.2/topics/db/queries/#copying-model-instances adding = self.pk is None or self._state.adding wrt_changed = not adding and self._wrt_map() != self._original_wrt_map diff --git a/tests/tests.py b/tests/tests.py index f1cfe474..dc6405d4 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -176,9 +176,11 @@ def setUp(self): def test_copy(self): q3 = Question.objects.create() old_q1_a1_pk = self.q1_a1.pk + # make a copy by clearing pk self.q1_a1.pk = None self.q1_a1.question = q3 self.q1_a1.save() + self.assertNotEqual(old_q1_a1_pk, self.q1_a1.pk) self.assertSequenceEqual( Answer.objects.values_list("pk", "order"), [ @@ -326,6 +328,7 @@ def setUp(self): def test_copy(self): group = CustomPKGroup.objects.create(name="g2") self.item.name = "g2 i1" + # make a copy by setting adding flag self.item._state.adding = True self.item.group = group self.item.save() From 99f4a056d01a24f3fcee8c0c2a3ceea9ac97873b Mon Sep 17 00:00:00 2001 From: Chris Shucksmith Date: Mon, 29 Jan 2024 11:01:39 +0000 Subject: [PATCH 3/4] fix: upstream black change to formatting rules --- ordered_model/admin.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ordered_model/admin.py b/ordered_model/admin.py index 2536339e..481ac27a 100644 --- a/ordered_model/admin.py +++ b/ordered_model/admin.py @@ -91,9 +91,11 @@ def move_view(self, request, object_id, direction): redir_path = "%s%s%s" % ( mangled, "/" if not mangled.endswith("/") else "", - ("?" + iri_to_uri(request.META.get("QUERY_STRING", ""))) - if request.META.get("QUERY_STRING", "") - else "", + ( + ("?" + iri_to_uri(request.META.get("QUERY_STRING", ""))) + if request.META.get("QUERY_STRING", "") + else "" + ), ) return HttpResponseRedirect(redir_path) @@ -196,9 +198,11 @@ def move_view(self, request, admin_id, object_id, direction): redir_path = "%s%s%s" % ( mangled, "/" if not mangled.endswith("/") else "", - ("?" + iri_to_uri(request.META.get("QUERY_STRING", ""))) - if request.META.get("QUERY_STRING", "") - else "", + ( + ("?" + iri_to_uri(request.META.get("QUERY_STRING", ""))) + if request.META.get("QUERY_STRING", "") + else "" + ), ) return HttpResponseRedirect(redir_path) From 9fb8dc600c1bcad360e9bc8240382e32ea22598f Mon Sep 17 00:00:00 2001 From: Chris Shucksmith Date: Mon, 29 Jan 2024 11:33:35 +0000 Subject: [PATCH 4/4] problem: bad case --- tests/tests.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/tests.py b/tests/tests.py index dc6405d4..08f32e2b 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -188,10 +188,29 @@ def test_copy(self): (self.q1_a2.pk, 1), (self.q2_a1.pk, 0), (self.q2_a2.pk, 1), - (self.q1_a1.pk, 0), + (self.q1_a1.pk, 0), # now q3 ], ) + # copy q2_a2 -> q2_a3 + self.q2_a2_pk = self.q2_a2.pk + self.q2_a3 = self.q2_a2 + self.q2_a3.pk = None + self.q2_a3.save() + self.assertNotEqual(self.q2_a3.pk, self.q2_a2_pk) + self.assertSequenceEqual( + Answer.objects.values_list("pk", "order"), + [ + (old_q1_a1_pk, 0), + (self.q1_a2.pk, 1), + (self.q2_a1.pk, 0), + (self.q2_a2_pk, 1), + (self.q2_a3.pk, 2), + (self.q1_a1.pk, 0), # now q3 + ], + ) + + def test_saved_order(self): self.assertSequenceEqual( Answer.objects.values_list("pk", "order"),