ソースを参照

Remove form from the basket line formset and bypass its validation if related model instance does not exist any more.

master
Alexander Gaevsky 6年前
コミット
cca43aa4d1

+ 26
- 9
src/oscar/apps/basket/forms.py ファイルの表示

@@ -1,6 +1,7 @@
1 1
 from django import forms
2 2
 from django.conf import settings
3 3
 from django.db.models import Sum
4
+from django.forms.utils import ErrorDict
4 5
 from django.utils.translation import gettext_lazy as _
5 6
 
6 7
 from oscar.core.loading import get_model
@@ -19,15 +20,31 @@ class BasketLineForm(forms.ModelForm):
19 20
         super().__init__(*args, **kwargs)
20 21
         self.instance.strategy = strategy
21 22
 
22
-        max_allowed_quantity = None
23
-        num_available = getattr(self.instance.purchase_info.availability, 'num_available', None)
24
-        basket_max_allowed_quantity = self.instance.basket.max_allowed_quantity()[0]
25
-        if all([num_available, basket_max_allowed_quantity]):
26
-            max_allowed_quantity = min(num_available, basket_max_allowed_quantity)
27
-        else:
28
-            max_allowed_quantity = num_available or basket_max_allowed_quantity
29
-        if max_allowed_quantity:
30
-            self.fields['quantity'].widget.attrs['max'] = max_allowed_quantity
23
+        # Evaluate max allowed quantity check only if line still exists, in
24
+        # order to avoid check run against missing instance -
25
+        # https://github.com/django-oscar/django-oscar/issues/2873.
26
+        if self.instance.id:
27
+            max_allowed_quantity = None
28
+            num_available = getattr(self.instance.purchase_info.availability, 'num_available', None)
29
+            basket_max_allowed_quantity = self.instance.basket.max_allowed_quantity()[0]
30
+            if all([num_available, basket_max_allowed_quantity]):
31
+                max_allowed_quantity = min(num_available, basket_max_allowed_quantity)
32
+            else:
33
+                max_allowed_quantity = num_available or basket_max_allowed_quantity
34
+            if max_allowed_quantity:
35
+                self.fields['quantity'].widget.attrs['max'] = max_allowed_quantity
36
+
37
+    def full_clean(self):
38
+        if not self.instance.id:
39
+            self.cleaned_data = {}
40
+            self._errors = ErrorDict()
41
+            return
42
+        return super().full_clean()
43
+
44
+    def has_changed(self):
45
+        if not self.instance.id:
46
+            return False
47
+        return super().has_changed()
31 48
 
32 49
     def clean_quantity(self):
33 50
         qty = self.cleaned_data['quantity']

+ 16
- 0
src/oscar/apps/basket/formsets.py ファイルの表示

@@ -1,4 +1,5 @@
1 1
 from django.forms.models import BaseModelFormSet, modelformset_factory
2
+from django.utils.functional import cached_property
2 3
 
3 4
 from oscar.core.loading import get_classes, get_model
4 5
 
@@ -23,9 +24,24 @@ class BaseBasketLineFormSet(BaseModelFormSet):
23 24
         """
24 25
         if super()._should_delete_form(form):
25 26
             return True
27
+
28
+        # If related form instance already removed, let's remove this form
29
+        # as well.
30
+        if not form.instance.id:
31
+            return True
26 32
         if self.can_delete and 'quantity' in form.cleaned_data:
27 33
             return form.cleaned_data['quantity'] == 0
28 34
 
35
+    @cached_property
36
+    def forms_with_instances(self):
37
+        return [f for f in self.forms if f.instance.id]
38
+
39
+    def __iter__(self):
40
+        """
41
+        Skip forms with removed lines when iterating through the formset.
42
+        """
43
+        return iter(self.forms_with_instances)
44
+
29 45
 
30 46
 BasketLineFormSet = modelformset_factory(
31 47
     Line, form=BasketLineForm, formset=BaseBasketLineFormSet, extra=0,

+ 1
- 1
src/oscar/apps/basket/views.py ファイルの表示

@@ -152,7 +152,7 @@ class BasketView(ModelFormSetView):
152 152
 
153 153
         for form in formset:
154 154
             if (hasattr(form, 'cleaned_data')
155
-                    and form.cleaned_data['save_for_later']):
155
+                    and getattr(form.cleaned_data, 'save_for_later', False)):
156 156
                 line = form.instance
157 157
                 if self.request.user.is_authenticated:
158 158
                     self.move_line_to_saved_basket(line)

+ 78
- 0
tests/functional/test_basket.py ファイルの表示

@@ -200,3 +200,81 @@ class SavedBasketTests(WebTestCase):
200 200
         # we can't add more than stock level into basket
201 201
         self.assertEqual(Basket.open.get(id=basket.id).lines.get(product=product).quantity, 1)
202 202
         self.assertRedirects(response, reverse('basket:summary'))
203
+
204
+
205
+class BasketFormSetTests(WebTestCase):
206
+    csrf_checks = False
207
+
208
+    def test_formset_with_removed_line(self):
209
+        products = [create_product() for i in range(3)]
210
+        basket = factories.create_basket(empty=True)
211
+        basket.owner = self.user
212
+        basket.save()
213
+
214
+        add_product(basket, product=products[0])
215
+        add_product(basket, product=products[1])
216
+        add_product(basket, product=products[2])
217
+        response = self.get(reverse('basket:summary'))
218
+        formset = response.context['formset']
219
+        self.assertEqual(len(formset.forms), 3)
220
+
221
+        basket.lines.filter(product=products[0]).delete()
222
+
223
+        management_form = formset.management_form
224
+        data = {
225
+            formset.add_prefix('INITIAL_FORMS'): management_form.initial['INITIAL_FORMS'],
226
+            formset.add_prefix('MAX_NUM_FORMS'): management_form.initial['MAX_NUM_FORMS'],
227
+            formset.add_prefix('TOTAL_FORMS'): management_form.initial['TOTAL_FORMS'],
228
+            'form-0-quantity': 1,
229
+            'form-0-id': formset.forms[0].instance.id,
230
+            'form-1-quantity': 2,
231
+            'form-1-id': formset.forms[1].instance.id,
232
+            'form-2-quantity': 2,
233
+            'form-2-id': formset.forms[2].instance.id,
234
+        }
235
+        response = self.post(reverse('basket:summary'), params=data)
236
+        self.assertEqual(response.status_code, 302)
237
+        formset = response.follow().context['formset']
238
+        self.assertEqual(len(formset.forms), 2)
239
+        self.assertEqual(len(formset.forms_with_instances), 2)
240
+        self.assertEqual(basket.lines.all()[0].quantity, 2)
241
+        self.assertEqual(basket.lines.all()[1].quantity, 2)
242
+
243
+    def test_invalid_formset_with_removed_line(self):
244
+        products = [create_product() for i in range(3)]
245
+        basket = factories.create_basket(empty=True)
246
+        basket.owner = self.user
247
+        basket.save()
248
+
249
+        add_product(basket, product=products[0])
250
+        add_product(basket, product=products[1])
251
+        add_product(basket, product=products[2])
252
+        response = self.get(reverse('basket:summary'))
253
+        formset = response.context['formset']
254
+        self.assertEqual(len(formset.forms), 3)
255
+
256
+        basket.lines.filter(product=products[0]).delete()
257
+
258
+        stockrecord = products[1].stockrecords.first()
259
+        stockrecord.num_in_stock = 0
260
+        stockrecord.save()
261
+
262
+        management_form = formset.management_form
263
+        data = {
264
+            formset.add_prefix('INITIAL_FORMS'): management_form.initial['INITIAL_FORMS'],
265
+            formset.add_prefix('MAX_NUM_FORMS'): management_form.initial['MAX_NUM_FORMS'],
266
+            formset.add_prefix('TOTAL_FORMS'): management_form.initial['TOTAL_FORMS'],
267
+            'form-0-quantity': 1,
268
+            'form-0-id': formset.forms[0].instance.id,
269
+            'form-1-quantity': 2,
270
+            'form-1-id': formset.forms[1].instance.id,
271
+            'form-2-quantity': 2,
272
+            'form-2-id': formset.forms[2].instance.id,
273
+        }
274
+        response = self.post(reverse('basket:summary'), params=data)
275
+        self.assertEqual(response.status_code, 200)
276
+        formset = response.context['formset']
277
+        self.assertEqual(len(formset.forms), 3)
278
+        self.assertEqual(len(formset.forms_with_instances), 2)
279
+        self.assertEqual(basket.lines.all()[0].quantity, 1)
280
+        self.assertEqual(basket.lines.all()[1].quantity, 1)

読み込み中…
キャンセル
保存