Merged ProductUpdateView and ProductCreateView

There's a lot of code duplication between the two. This is an attempt of
merging the two.

* The additional formsets are now easily overriden.
* is_valid logic is more concise
* is_stockrecord_submitted is now more robust
* Fix: Partner and Partner SKU were optional. They're required, only the
  entire form is optional.
* Fix: StockRecordForm errors are now displayed
* StockRecord is now deleted when clearing all fields

oscar/apps/dashboard/catalogue/app.py

@@ -10,8 +10,7 @@ class CatalogueApplication(Application):
 
     product_list_view = views.ProductListView
     product_create_redirect_view = views.ProductCreateRedirectView
-    product_create_view = views.ProductCreateView
-    product_update_view = views.ProductUpdateView
+    product_createupdate_view = views.ProductCreateUpdateView
     product_delete_view = views.ProductDeleteView
 
     category_list_view = views.CategoryListView
@@ -24,13 +23,13 @@ class CatalogueApplication(Application):
 
     def get_urls(self):
         urlpatterns = patterns('',
-            url(r'^products/(?P<pk>\d+)/$', self.product_update_view.as_view(),
+            url(r'^products/(?P<pk>\d+)/$', self.product_createupdate_view.as_view(),
                 name='catalogue-product'),
             url(r'^products/create/$',
                 self.product_create_redirect_view.as_view(),
                 name='catalogue-product-create'),
             url(r'^products/create/(?P<product_class_id>\d+)/$',
-                self.product_create_view.as_view(),
+                self.product_createupdate_view.as_view(),
                 name='catalogue-product-create'),
             url(r'^products/(?P<pk>\d+)/delete/$',
                 self.product_delete_view.as_view(),

oscar/apps/dashboard/catalogue/forms.py

@@ -66,11 +66,6 @@ class ProductSearchForm(forms.Form):
 
 
 class StockRecordForm(forms.ModelForm):
-    partner = forms.ModelChoiceField(queryset=Partner.objects.all(),
-                                    required=False,
-                                    label=_("Partner"))
-    partner_sku = forms.CharField(required=False,
-                                  label=_("Partner SKU"))
 
     def __init__(self, product_class, *args, **kwargs):
         self.product_class = product_class

oscar/apps/dashboard/catalogue/views.py

@@ -1,6 +1,7 @@
+from django.core.exceptions import ObjectDoesNotExist
 from django.views import generic
 from django.db.models import get_model
-from django.http import HttpResponseRedirect
+from django.http import HttpResponseRedirect, Http404
 from django.contrib import messages
 from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext_lazy as _
@@ -93,151 +94,69 @@ class ProductCreateRedirectView(generic.RedirectView):
                            kwargs={'product_class_id': product_class.id})
 
 
-class ProductCreateView(generic.CreateView):
+class ProductCreateUpdateView(generic.UpdateView):
     template_name = 'dashboard/catalogue/product_update.html'
     model = Product
     context_object_name = 'product'
-    form_class = ProductForm
-
-    def get_context_data(self, **kwargs):
-        ctx = super(ProductCreateView, self).get_context_data(**kwargs)
-        pclass = self.get_product_class()
-        if 'stockrecord_form' not in ctx:
-            ctx['stockrecord_form'] = StockRecordForm(pclass)
-        if 'category_formset' not in ctx:
-            ctx['category_formset'] = ProductCategoryFormSet()
-        if 'image_formset' not in ctx:
-            ctx['image_formset'] = ProductImageFormSet()
-        if 'recommended_formset' not in ctx:
-            ctx['recommended_formset'] = ProductRecommendationFormSet()
-        ctx['title'] = _('Create new %s product') % pclass.name
-        ctx['product_class'] = pclass
-        return ctx
-
-    def get_product_class(self):
-        return ProductClass.objects.get(id=self.kwargs['product_class_id'])
-
-    def get_form_kwargs(self):
-        kwargs = super(ProductCreateView, self).get_form_kwargs()
-        kwargs['product_class'] = self.get_product_class()
-        return kwargs
 
-    def is_stockrecord_submitted(self):
-        return len(self.request.POST.get('partner', '')) > 0
-
-    def get_stockrecord_form(self):
-        pclass = self.get_product_class()
-        if self.is_stockrecord_submitted():
-            return StockRecordForm(pclass, self.request.POST)
-
-        return StockRecordForm(pclass)
-
-    def form_invalid(self, form):
-        category_formset = ProductCategoryFormSet(self.request.POST)
-        image_formset = ProductImageFormSet(self.request.POST, self.request.FILES)
-        recommended_formset = ProductRecommendationFormSet(self.request.POST,
-                                            self.request.FILES)
-
-
-        messages.error(self.request,
-                       _("Your submitted data was not valid - please "
-                         "correct the below errors"))
-        ctx = self.get_context_data(form=form,
-                                    stockrecord_form=self.get_stockrecord_form(),
-                                    category_formset=category_formset,
-                                    image_formset=image_formset,
-                                    recommended_formset=recommended_formset)
-        return self.render_to_response(ctx)
-
-    def form_valid(self, form):
-        product = form.save()
-        category_formset = ProductCategoryFormSet(self.request.POST,
-                                                  instance=product)
-        image_formset = ProductImageFormSet(self.request.POST,
-                                            self.request.FILES,
-                                            instance=product)
-        recommended_formset = ProductRecommendationFormSet(self.request.POST,
-                                            self.request.FILES,
-                                            instance=product)
-
-        stockrecord_form = self.get_stockrecord_form()
-
-        if self.is_stockrecord_submitted():
-            is_valid = all([stockrecord_form.is_valid(),
-                            category_formset.is_valid(),
-                            image_formset.is_valid(),
-                            recommended_formset.is_valid()])
-        else:
-            is_valid = all([category_formset.is_valid(),
-                            image_formset.is_valid(),
-                            recommended_formset.is_valid()])
-        if is_valid:
-            if self.is_stockrecord_submitted():
-                # Save stock record
-                stockrecord = stockrecord_form.save(commit=False)
-                stockrecord.product = product
-                stockrecord.save()
-            # Save formsets
-            category_formset.save()
-            image_formset.save()
-            recommended_formset.save()
-
-            return HttpResponseRedirect(self.get_success_url(product))
-
-        messages.error(self.request,
-                       _("Your submitted data was not valid - please "
-                         "correct the below errors"))
-
-        # Delete product as its relations were not valid
-        product.delete()
-        ctx = self.get_context_data(form=form,
-                                    stockrecord_form=stockrecord_form,
-                                    category_formset=category_formset,
-                                    image_formset=image_formset,
-                                    recommended_formset=recommended_formset)
-        return self.render_to_response(ctx)
-
-    def get_url_with_querystring(self, url):
-        url_parts = [url]
-        if self.request.GET.urlencode():
-            url_parts += [self.request.GET.urlencode()]
-        return "?".join(url_parts)
-
-    def get_success_url(self, product):
-        messages.success(self.request, _("Created product '%s'") % product.title)
-        url = reverse('dashboard:catalogue-product-list')
-        if self.request.POST.get('action') == 'continue':
-            url = reverse('dashboard:catalogue-product',
-                           kwargs={"pk": product.id})
-        return self.get_url_with_querystring(url)
-
-
-class ProductUpdateView(generic.UpdateView):
-    template_name = 'dashboard/catalogue/product_update.html'
-    model = Product
-    context_object_name = 'product'
     form_class = ProductForm
+    category_formset = ProductCategoryFormSet
+    image_formset = ProductImageFormSet
+    recommendations_formset = ProductRecommendationFormSet
+    stockrecord_form = StockRecordForm
+
+    def get_object(self, queryset=None):
+        """
+        This parts allows generic.UpdateView to handle creating products as
+        well. The only distinction between an UpdateView and a CreateView
+        is that self.object is None. We emulate this behavior.
+        Additionally, self.product_class is set.
+        """
+        if 'pk' in self.kwargs:  # UpdateView
+            obj = super(ProductCreateUpdateView, self).get_object(queryset)
+            self.product_class = obj.product_class
+            return obj
+        else:  # CreateView
+            try:
+                product_class_id = self.kwargs.get('product_class_id', None)
+                self.product_class = ProductClass.objects.get(
+                    id=product_class_id)
+            except ObjectDoesNotExist:
+                raise Http404
+            else:
+                return None  # success
 
     def get_context_data(self, **kwargs):
-        ctx = super(ProductUpdateView, self).get_context_data(**kwargs)
+        ctx = super(ProductCreateUpdateView, self).get_context_data(**kwargs)
         if 'stockrecord_form' not in ctx:
             ctx['stockrecord_form'] = self.get_stockrecord_form()
         if 'category_formset' not in ctx:
-            ctx['category_formset'] = ProductCategoryFormSet(instance=self.object)
+            ctx['category_formset'] = self.category_formset(instance=self.object)
         if 'image_formset' not in ctx:
-            ctx['image_formset'] = ProductImageFormSet(instance=self.object)
+            ctx['image_formset'] = self.image_formset(instance=self.object)
         if 'recommended_formset' not in ctx:
-            ctx['recommended_formset'] = ProductRecommendationFormSet(instance=self.object)
-        ctx['title'] = ctx['product'].get_title()
+            ctx['recommended_formset'] = self.recommendations_formset(instance=self.object)
+        if self.object is None:
+            ctx['title'] = _('Create new %s product') % self.product_class.name
+        else:
+            ctx['title'] = ctx['product'].get_title()
         return ctx
 
     def get_form_kwargs(self):
-        kwargs = super(ProductUpdateView, self).get_form_kwargs()
-        kwargs['product_class'] = self.object.product_class
+        kwargs = super(ProductCreateUpdateView, self).get_form_kwargs()
+        kwargs['product_class'] = self.product_class
         return kwargs
 
     def is_stockrecord_submitted(self):
-        return len(self.request.POST.get('partner', '')) > 0
+        """
+        Check if there's POST data that matches StockRecordForm field names
+        """
+        fields = dict(self.stockrecord_form.base_fields.items() +
+                      self.stockrecord_form.declared_fields.items())
+        for name, field in fields.iteritems():
+            if len(self.request.POST.get(name, '')) > 0:
+                return True
+        return False
 
     def get_stockrecord_form(self):
         """
@@ -246,66 +165,85 @@ class ProductUpdateView(generic.UpdateView):
         stock record it will be passed into the form as
         ``instance``.
         """
-        stockrecord = None
-        if self.object.has_stockrecord:
+        try:
             stockrecord = self.object.stockrecord
-        if not self.is_stockrecord_submitted():
-            return StockRecordForm(self.object.product_class,
-                                   instance=stockrecord)
-        return StockRecordForm(
-            self.object.product_class,
-            self.request.POST,
+        except (AttributeError, StockRecord.DoesNotExist):
+            # either self.object is None, or no stockrecord
+            stockrecord = None
+        return self.stockrecord_form(
+            product_class=self.product_class,
+            data=self.request.POST if self.is_stockrecord_submitted() else None,
             instance=stockrecord)
 
+    def form_valid(self, form):
+        return self.process_all_forms(form)
+
     def form_invalid(self, form):
-        stockrecord_form = self.get_stockrecord_form()
+        return self.process_all_forms(form)
 
-        category_formset = ProductCategoryFormSet(self.request.POST,
-                                                  instance=self.object)
-        image_formset = ProductImageFormSet(self.request.POST,
-                                            self.request.FILES,
-                                            instance=self.object)
-        recommended_formset = ProductRecommendationFormSet(self.request.POST,
-                                            self.request.FILES,
-                                            instance=self.object)
-        ctx = self.get_context_data(form=form,
-                                    stockrecord_form=stockrecord_form,
-                                    category_formset=category_formset,
-                                    image_formset=image_formset,
-                                    recommended_formset=recommended_formset)
-        return self.render_to_response(ctx)
+    def process_all_forms(self, form):
+        """
+        Short-circuits the regular logic to have one place to have our
+        logic to check all forms
+        """
+        self.creating = self.object is None
+        # need to create the product here because the inline forms need it
+        # can't use commit=False because ProductForm does not support it
+        if self.creating and form.is_valid():
+            self.object = form.save()
 
-    def form_valid(self, form):
         stockrecord_form = self.get_stockrecord_form()
-        category_formset = ProductCategoryFormSet(self.request.POST,
-                                                  instance=self.object)
-        image_formset = ProductImageFormSet(self.request.POST,
-                                            self.request.FILES,
-                                            instance=self.object)
-        recommended_formset = ProductRecommendationFormSet(self.request.POST,
-                                            self.request.FILES,
-                                            instance=self.object)
+        category_formset = self.category_formset(self.request.POST,
+                                                 instance=self.object)
+        image_formset = self.image_formset(self.request.POST,
+                                           self.request.FILES,
+                                           instance=self.object)
+        recommended_formset = self.recommendations_formset(
+            self.request.POST, self.request.FILES, instance=self.object)
+
+        is_valid = all([
+            form.is_valid(),
+            category_formset.is_valid(),
+            image_formset.is_valid(),
+            recommended_formset.is_valid(),
+            not self.is_stockrecord_submitted() or stockrecord_form.is_valid(),
+            ])
+
+        if is_valid:
+            return self.forms_valid(form, stockrecord_form, category_formset,
+                                    image_formset, recommended_formset)
+        else:
+            # delete the temporary product again
+            if self.creating and form.is_valid():
+                self.object.delete()
+                self.object = None
+            return self.forms_invalid(form, stockrecord_form, category_formset,
+                                      image_formset, recommended_formset)
+
+    def forms_valid(self, form, stockrecord_form, category_formset,
+                    image_formset, recommended_formset):
         if self.is_stockrecord_submitted():
-            is_valid = all([stockrecord_form.is_valid(),
-                            category_formset.is_valid(),
-                            image_formset.is_valid(),
-                            recommended_formset.is_valid()])
+            # Save stock record
+            stockrecord = stockrecord_form.save(commit=False)
+            stockrecord.product = self.object
+            stockrecord.save()
         else:
-            is_valid = all([category_formset.is_valid(),
-                            image_formset.is_valid(),
-                            recommended_formset.is_valid()])
+            # delete it
+            if self.object.has_stockrecord:
+                self.object.stockrecord.delete()
 
-        if is_valid:
-            form.save()
-            if self.is_stockrecord_submitted():
-                stockrecord = stockrecord_form.save(commit=False)
-                stockrecord.product = self.object
-                stockrecord.save()
-            category_formset.save()
-            image_formset.save()
-            recommended_formset.save()
-            return HttpResponseRedirect(self.get_success_url())
+        # Save formsets
+        category_formset.save()
+        image_formset.save()
+        recommended_formset.save()
+
+        return HttpResponseRedirect(self.get_success_url())
 
+    def forms_invalid(self, form, stockrecord_form, category_formset,
+                      image_formset, recommended_formset):
+        messages.error(self.request,
+                       _("Your submitted data was not valid - please "
+                         "correct the below errors"))
         ctx = self.get_context_data(form=form,
                                     stockrecord_form=stockrecord_form,
                                     category_formset=category_formset,
@@ -320,12 +258,15 @@ class ProductUpdateView(generic.UpdateView):
         return "?".join(url_parts)
 
     def get_success_url(self):
-        messages.success(self.request, _("Updated product '%s'") %
-                         self.object.title)
+        if self.creating:
+            msg = _("Created product '%s'") % self.object.title
+        else:
+            msg = _("Updated product '%s'") % self.object.title
+        messages.success(self.request, msg)
         url = reverse('dashboard:catalogue-product-list')
         if self.request.POST.get('action') == 'continue':
             url = reverse('dashboard:catalogue-product',
-                           kwargs={"pk":self.object.id})
+                          kwargs={"pk": self.object.id})
         return self.get_url_with_querystring(url)
 
 
@@ -408,7 +349,7 @@ class CategoryCreateView(CategoryListMixin, generic.CreateView):
         return ctx
 
     def get_success_url(self):
-        messages.info(self.request, "Category created successfully")
+        messages.info(self.request, _("Category created successfully"))
         return super(CategoryCreateView, self).get_success_url()
 
 
@@ -419,11 +360,11 @@ class CategoryUpdateView(CategoryListMixin, generic.UpdateView):
 
     def get_context_data(self, **kwargs):
         ctx = super(CategoryUpdateView, self).get_context_data(**kwargs)
-        ctx['title'] = "Update category '%s'" % self.object.name
+        ctx['title'] = _("Update category '%s'") % self.object.name
         return ctx
 
     def get_success_url(self):
-        messages.info(self.request, "Category updated successfully")
+        messages.info(self.request, _("Category updated successfully"))
         return super(CategoryUpdateView, self).get_success_url()
 
 

oscar/templates/oscar/dashboard/catalogue/product_update.html

@@ -40,7 +40,7 @@
                     {% if product %}
                     <li class="active"><a href="#overview" data-toggle="tab">{% trans 'Overview' %}</a></li>
                     {% endif %}
-                    <li {% if not product %}class="active"{% endif %}><a href="#product_details" data-toggle="tab">{% trans 'Product details' %}</a></li>
+                    <li{% if not product %} class="active"{% endif %}><a href="#product_details" data-toggle="tab">{% trans 'Product details' %}</a></li>
                     <li><a href="#product_recommended" data-toggle="tab">{% trans 'Recommended Products' %}</a></li>
                     {% with variants=product.variants.all %}
                     {% if variants|length > 0 %}
@@ -326,29 +326,20 @@
                             {% for field in stockrecord_form %}
                                 {% if "partner" in field.id_for_label %}
                                     <td>
-                                        {% if field.is_hidden %}
-                                            {{ field }}
-                                        {% else %}
-                                            {{ field }}
-                                        {% endif %}
+                                        {{ field }}
+                                        {{ field.errors }}
                                     </td>
                                 {% endif %}
                                 {% if "price" in field.id_for_label %}
                                     <td>
-                                        {% if field.is_hidden %}
-                                            {{ field }}
-                                        {% else %}
-                                            {{ field }}
-                                        {% endif %}
+                                        {{ field }}
+                                        {{ field.errors }}
                                     </td>
                                 {% endif %}
                                 {% if "stock" in field.id_for_label %}
                                     <td>
-                                        {% if field.is_hidden %}
-                                            {{ field }}
-                                        {% else %}
-                                            {{ field }}
-                                        {% endif %}
+                                        {{ field }}
+                                        {{ field.errors }}
                                     </td>
                                 {% endif %}
                             {% endfor %}