Skip to content

Commit 09744c7

Browse files
TrangPhamThu Trang Pham
and
Thu Trang Pham
authored
fix(ISSUE-8): contd m2m save values (#11)
* feat(ISSUE-8): ISSUE-8: ManyToManyField causes error on confirmations * feat(ISSUE-8): Update some readme and remove print statements * feat(ISSUE-8): Generate new version of package * feat(ISSUE-3): Adding .travis.yml * feat(ISSUE-3): Adding coveralls * feat(ISSUE-3): Trying github actions * feat(ISSUE-3): remove travis * feat(ISSUE-3): Change python versions to test * feat(ISSUE-3): Some refactoring and trying tox * feat(ISSUE-3): Try action matrix * feat(ISSUE-3): Some more refactors * feat(ISSUE-3): Fix tests * feat(ISSUE-3): Refactor/fix tests * feat(ISSUE-3): Remove tox * feat(ISSUE-3): Adding pypi version badge to readme * feat(ISSUE-3): Update readme again * feat(ISSUE-3): Remove tox from readme * feat(ISSUE-8): Adding some tests for m2m and fix save value issue * feat(ISSUE-8): Updating test todos * feat(ISSUE-8): Finish up the tests * feat(ISSUE-8): Rename * feat(ISSUE-8): Update gitignore Co-authored-by: Thu Trang Pham <[email protected]>
1 parent 9a9dfa7 commit 09744c7

10 files changed

+239
-21
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ sdist/
88
*.egg-info/
99
.DS_Store
1010

11+
# Editor settings
12+
.vscode/
13+
1114
# Unit test / coverage reports
1215
htmlcov/
1316
.tox/

README.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ Running tests:
157157

158158
```
159159
make test
160-
tox
161160
```
162161

163162
Testing new changes on test project:
@@ -171,12 +170,11 @@ make run
171170

172171
Honestly this part is just for my reference. But who knows :) maybe we'll have another maintainer in the future.
173172

174-
Run tests, check coverage, check readme, run tox
173+
Run tests, check coverage, check readme
175174

176175
```
177176
make test
178177
make check-readme
179-
tox
180178
```
181179

182180
Update version in `setup.py`

admin_confirm/admin.py

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
from django.contrib.admin.options import TO_FIELD_VAR
77
from django.utils.translation import gettext as _
88
from django.contrib.admin import helpers
9-
from django.db.models import Model
9+
from django.db.models import Model, ManyToManyField
10+
1011
from django.forms import ModelForm
1112
from admin_confirm.utils import snake_to_title_case
1213

@@ -119,18 +120,43 @@ def _get_changed_data(
119120
default_value = model._meta.get_field(name).get_default()
120121
if new_value is not None and new_value != default_value:
121122
# Show what the default value is
122-
changed_data[name] = [str(default_value), new_value]
123+
changed_data[name] = [default_value, new_value]
123124
else:
124125
# Parse the changed data - Note that using form.changed_data would not work because initial is not set
125126
for name, new_value in form.cleaned_data.items():
126127
# Since the form considers initial as the value first shown in the form
127128
# It could be incorrect when user hits save, and then hits "No, go back to edit"
128129
obj.refresh_from_db()
130+
# Note: getattr does not work on ManyToManyFields
131+
field_object = model._meta.get_field(name)
129132
initial_value = getattr(obj, name)
133+
if isinstance(field_object, ManyToManyField):
134+
initial_value = field_object.value_to_string(obj)
135+
130136
if initial_value != new_value:
131137
changed_data[name] = [initial_value, new_value]
138+
139+
print(changed_data)
132140
return changed_data
133141

142+
def _get_form_data(self, request):
143+
"""
144+
Parses the request post params into a format that can be used for the hidden form on the
145+
change confirmation page.
146+
"""
147+
form_data = request.POST.copy()
148+
149+
for key in SAVE_ACTIONS + [
150+
"_confirm_change",
151+
"_confirm_add",
152+
"csrfmiddlewaretoken",
153+
]:
154+
if form_data.get(key):
155+
form_data.pop(key)
156+
157+
form_data = [(k, list(v)) for k, v in form_data.lists()]
158+
return form_data
159+
134160
def _change_confirmation_view(self, request, object_id, form_url, extra_context):
135161
# This code is taken from super()._changeform_view
136162
to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR))
@@ -174,18 +200,14 @@ def _change_confirmation_view(self, request, object_id, form_url, extra_context)
174200
return super()._changeform_view(request, object_id, form_url, extra_context)
175201

176202
# Parse raw form data from POST
177-
form_data = {}
203+
form_data = self._get_form_data(request)
178204
# Parse the original save action from request
179205
save_action = None
180-
for key, value in request.POST.items():
206+
for key in request.POST.keys():
181207
if key in SAVE_ACTIONS:
182208
save_action = key
183-
continue
184-
185-
if key.startswith("_") or key == "csrfmiddlewaretoken":
186-
continue
209+
break
187210

188-
form_data[key] = value
189211

190212
title_action = _("adding") if add else _("changing")
191213

admin_confirm/templates/admin/change_confirmation.html

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@
4343
<form method="post" action="{% url opts|admin_urlname:'change' object_id|admin_urlquote %}">{% csrf_token %}
4444
{% endif %}
4545

46-
{% for key, value in form_data.items %}
47-
<input type="hidden" name="{{ key }}" value="{{ value }}">
46+
{% for key, values in form_data %}
47+
{% for v in values %}
48+
<input type="hidden" name="{{ key }}" value="{{v}}">
49+
{% endfor %}
4850
{% endfor %}
4951
{% if is_popup %}<input type="hidden" name="{{ is_popup_var }}" value="1">{% endif %}
5052
{% if to_field %}<input type="hidden" name="{{ to_field_var }}" value="{{ to_field }}">{% endif %}

admin_confirm/templates/admin/change_data.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{% if changed_data %}
1+
{% load formatting %} {% if changed_data %}
22
<div class="changed-data">
33
<p><b>Confirm Values:</b></p>
44
<table>
@@ -10,8 +10,8 @@
1010
{% for field, values in changed_data.items %}
1111
<tr>
1212
<td>{{ field }}</td>
13-
<td>{{ values.0 }}</td>
14-
<td>{{ values.1 }}</td>
13+
<td>{{ values.0|format_change_data_field_value }}</td>
14+
<td>{{ values.1|format_change_data_field_value }}</td>
1515
</tr>
1616
{% endfor %}
1717
</table>

admin_confirm/templates/admin/submit_line.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33

44
{% block submit-row %}
55
{% if confirm_change %}
6-
<input hidden name="_confirm_change" />
6+
<input hidden name="_confirm_change" value=True />
77
{% endif %}
88
{% if confirm_add %}
9-
<input hidden name="_confirm_add" />
9+
<input hidden name="_confirm_add" value=True />
1010
{% endif %}
1111
{{ block.super }}
1212
{% endblock %}

admin_confirm/templatetags/__init__.py

Whitespace-only changes.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
from django import template
2+
from django.db.models.query import QuerySet
3+
4+
register = template.Library()
5+
6+
7+
@register.filter
8+
def format_change_data_field_value(field_value):
9+
if isinstance(field_value, QuerySet):
10+
return list(field_value)
11+
12+
return field_value

admin_confirm/tests/test_confirm_change_and_add.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ def test_post_add_with_confirm_add(self):
6464
]
6565
self.assertEqual(response.template_name, expected_templates)
6666
form_data = {"shop": str(shop.id), "item": str(item.id), "quantity": str(5)}
67-
self.assertEqual(response.context_data["form_data"], form_data)
6867
for k, v in form_data.items():
6968
self.assertIn(
7069
f'<input type="hidden" name="{ k }" value="{ v }">',
@@ -100,7 +99,6 @@ def test_post_change_with_confirm_change(self):
10099
"id": str(item.id),
101100
"currency": Item.VALID_CURRENCIES[0][0],
102101
}
103-
self.assertEqual(response.context_data["form_data"], form_data)
104102
for k, v in form_data.items():
105103
self.assertIn(
106104
f'<input type="hidden" name="{ k }" value="{ v }">',
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
from django.test import TestCase, RequestFactory
2+
from django.contrib.auth.models import User
3+
from django.urls import reverse
4+
5+
6+
from tests.market.admin import InventoryAdmin, ShoppingMallAdmin
7+
from tests.market.models import Item, Inventory, ShoppingMall
8+
from tests.factories import ItemFactory, ShopFactory, InventoryFactory
9+
10+
11+
class TestConfirmChangeAndAddM2MField(TestCase):
12+
@classmethod
13+
def setUpTestData(cls):
14+
cls.superuser = User.objects.create_superuser(
15+
username="super", email="[email protected]", password="pass"
16+
)
17+
18+
def setUp(self):
19+
self.client.force_login(self.superuser)
20+
self.factory = RequestFactory()
21+
22+
def test_post_add_without_confirm_add_m2m(self):
23+
shops = [ShopFactory() for i in range(3)]
24+
25+
data = {"name": "name", "shops": [s.id for s in shops]}
26+
response = self.client.post(reverse("admin:market_shoppingmall_add"), data)
27+
# Redirects to changelist and is added
28+
self.assertEqual(response.status_code, 302)
29+
self.assertEqual(response.url, "/admin/market/shoppingmall/")
30+
self.assertEqual(ShoppingMall.objects.count(), 1)
31+
32+
def test_post_add_with_confirm_add_m2m(self):
33+
ShoppingMallAdmin.confirmation_fields = ["shops"]
34+
shops = [ShopFactory() for i in range(3)]
35+
36+
data = {"name": "name", "shops": [s.id for s in shops], "_confirm_add": True}
37+
response = self.client.post(reverse("admin:market_shoppingmall_add"), data)
38+
39+
# Ensure not redirected (confirmation page does not redirect)
40+
self.assertEqual(response.status_code, 200)
41+
expected_templates = [
42+
"admin/market/shoppingmall/change_confirmation.html",
43+
"admin/market/change_confirmation.html",
44+
"admin/change_confirmation.html",
45+
]
46+
self.assertEqual(response.template_name, expected_templates)
47+
form_data = [("name", "name")] + [("shops", s.id) for s in shops]
48+
for k, v in form_data:
49+
self.assertIn(
50+
f'<input type="hidden" name="{ k }" value="{ v }">',
51+
response.rendered_content,
52+
)
53+
54+
# Should not have been added yet
55+
self.assertEqual(ShoppingMall.objects.count(), 0)
56+
57+
def test_m2m_field_post_change_with_confirm_change(self):
58+
shops = [ShopFactory() for i in range(10)]
59+
shopping_mall = ShoppingMall.objects.create(name="My Mall")
60+
shopping_mall.shops.set(shops)
61+
# Currently ShoppingMall configured with confirmation_fields = ['name']
62+
data = {
63+
"name": "Not My Mall",
64+
"shops": "1",
65+
"id": shopping_mall.id,
66+
"_confirm_change": True,
67+
"csrfmiddlewaretoken": "fake token",
68+
"_save": True,
69+
}
70+
response = self.client.post(
71+
f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data
72+
)
73+
# Ensure not redirected (confirmation page does not redirect)
74+
self.assertEqual(response.status_code, 200)
75+
expected_templates = [
76+
"admin/market/shoppingmall/change_confirmation.html",
77+
"admin/market/change_confirmation.html",
78+
"admin/change_confirmation.html",
79+
]
80+
self.assertEqual(response.template_name, expected_templates)
81+
form_data = {
82+
"name": "Not My Mall",
83+
"shops": "1",
84+
"id": str(shopping_mall.id),
85+
}
86+
87+
for k, v in form_data.items():
88+
self.assertIn(
89+
f'<input type="hidden" name="{ k }" value="{ v }">',
90+
response.rendered_content,
91+
)
92+
93+
# Hasn't changed item yet
94+
shopping_mall.refresh_from_db()
95+
self.assertEqual(shopping_mall.name, "My Mall")
96+
97+
def test_m2m_field_post_change_with_confirm_change_multiple_selected(self):
98+
shops = [ShopFactory() for i in range(10)]
99+
shopping_mall = ShoppingMall.objects.create(name="My Mall")
100+
shopping_mall.shops.set(shops)
101+
# Currently ShoppingMall configured with confirmation_fields = ['name']
102+
data = {
103+
"name": "Not My Mall",
104+
"shops": ["1", "2", "3"],
105+
"id": shopping_mall.id,
106+
"_confirm_change": True,
107+
"csrfmiddlewaretoken": "fake token",
108+
"_save": True,
109+
}
110+
response = self.client.post(
111+
f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data
112+
)
113+
# Ensure not redirected (confirmation page does not redirect)
114+
self.assertEqual(response.status_code, 200)
115+
expected_templates = [
116+
"admin/market/shoppingmall/change_confirmation.html",
117+
"admin/market/change_confirmation.html",
118+
"admin/change_confirmation.html",
119+
]
120+
self.assertEqual(response.template_name, expected_templates)
121+
form_data = [
122+
("name", "Not My Mall"),
123+
("shops", "1"),
124+
("shops", "2"),
125+
("shops", "3"),
126+
("id", str(shopping_mall.id)),
127+
]
128+
129+
for k, v in form_data:
130+
self.assertIn(
131+
f'<input type="hidden" name="{ k }" value="{ v }">',
132+
response.rendered_content,
133+
)
134+
135+
# Hasn't changed item yet
136+
shopping_mall.refresh_from_db()
137+
self.assertEqual(shopping_mall.name, "My Mall")
138+
139+
def test_post_change_without_confirm_change_m2m_value(self):
140+
# make the m2m the confirmation_field
141+
ShoppingMallAdmin.confirmation_fields = ["shops"]
142+
shops = [ShopFactory() for i in range(3)]
143+
shopping_mall = ShoppingMall.objects.create(name="name")
144+
shopping_mall.shops.set(shops)
145+
assert shopping_mall.shops.count() == 3
146+
147+
data = {"name": "name", "id": str(shopping_mall.id), "shops": ["1"]}
148+
response = self.client.post(
149+
f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data
150+
)
151+
# Redirects to changelist
152+
self.assertEqual(response.status_code, 302)
153+
self.assertEqual(response.url, "/admin/market/shoppingmall/")
154+
# Shop has changed
155+
shopping_mall.refresh_from_db()
156+
self.assertEqual(shopping_mall.shops.count(), 1)
157+
158+
def test_form_invalid_m2m_value(self):
159+
ShoppingMallAdmin.confirmation_fields = ["shops"]
160+
shopping_mall = ShoppingMall.objects.create(name="name")
161+
162+
data = {
163+
"id": shopping_mall.id,
164+
"name": "name",
165+
"shops": ["1", "2", "3"], # These shops don't exist
166+
"_confirm_change": True,
167+
"csrfmiddlewaretoken": "fake token",
168+
}
169+
response = self.client.post(
170+
f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data
171+
)
172+
173+
# Form invalid should show errors on form
174+
self.assertEqual(response.status_code, 200)
175+
self.assertIsNotNone(response.context_data.get("errors"))
176+
self.assertTrue(
177+
str(response.context_data["errors"][0][0]).startswith(
178+
"Select a valid choice."
179+
)
180+
)
181+
# Should not have updated inventory
182+
shopping_mall.refresh_from_db()
183+
self.assertEqual(shopping_mall.shops.count(), 0)

0 commit comments

Comments
 (0)