Merge pull request #491 from RockefellerArchiveCenter/issue-454

Removes overriding of `post` in form views

.travis.yml

@@ -17,6 +17,6 @@ script:
   - docker-compose exec $CONTAINER pre-commit run --all-files --show-diff-on-failure
   - docker-compose exec $CONTAINER coverage run --omit=aurora/bag_transfer/migrations manage.py test
 after_script:
-  - docker-compose exec $CONTAINER coverage report -m
+  - docker-compose exec $CONTAINER coverage report --omit=bag_transfer/migrations/* -m
 notifications:
   email: false

aurora/bag_transfer/accession/views.py

@@ -18,8 +18,8 @@
 from django.contrib import messages
 from django.db.models import CharField, F
 from django.db.models.functions import Concat
-from django.shortcuts import redirect, render, reverse
-from django.views.generic import DetailView, ListView
+from django.shortcuts import reverse
+from django.views.generic import CreateView, DetailView, ListView
 from django_datatables_view.base_datatable_view import BaseDatatableView
 
 
@@ -85,40 +85,38 @@ def get_page_title(self, context):
         return context["object"].title
 
 
-class AccessionCreateView(PageTitleMixin, AccessioningArchivistMixin, JSONResponseMixin, ListView):
+class AccessionCreateView(PageTitleMixin, AccessioningArchivistMixin, JSONResponseMixin, CreateView):
     template_name = "accession/create.html"
     page_title = "Create Accession Record"
     model = Accession
     form_class = AccessionForm
 
-    def post(self, request, *args, **kwargs):
-        """Saves accessions, and delivers data to DELIVERY_URL if configured."""
-        form = self.form_class(request.POST)
-        creators_formset = CreatorsFormSet(request.POST)
-        id_list = list(map(int, request.GET.get("transfers").split(",")))
+    def get_success_url(self):
+        return reverse("accession:detail", kwargs={"pk": self.object.pk})
+
+    def form_valid(self, form):
+        """Saves associated formsets and delivers accession data."""
+        creators_formset = CreatorsFormSet(self.request.POST)
+        id_list = list(map(int, self.request.GET.get("transfers").split(",")))
         transfers_list = Transfer.objects.filter(pk__in=id_list)
         rights_statements = (
             RightsStatement.objects.filter(transfer__in=id_list)
             .annotate(rights_group=F("rights_basis"))
-            .order_by("rights_group")
-        )
-        if form.is_valid() and creators_formset.is_valid():
+            .order_by("rights_group"))
+        if creators_formset.is_valid():
             form.process_status = Accession.CREATED
             accession = form.save()
             creators_formset.save()
             self.update_accession_rights(RightsStatement.merge_rights(rights_statements), accession)
             self.update_accession_transfers(transfers_list, accession)
-            messages.success(request, "
Accession created successfully!")
+            messages.success(self.request, "
Accession created successfully!")
             if settings.DELIVERY_URL:
                 try:
                     accession_data = AccessionSerializer(
-                        accession, context={"request": request}
-                    )
+                        accession, context={"request": self.request})
                     resp = requests.post(
                         settings.DELIVERY_URL,
-                        data=json.dumps(
-                            accession_data.data, indent=4, sort_keys=True, default=str
-                        ),
+                        data=json.dumps(accession_data.data, default=str),
                         headers={
                             "Content-Type": "application/json",
                             "apikey": settings.API_KEY,
@@ -127,27 +125,14 @@ def post(self, request, *args, **kwargs):
                     resp.raise_for_status()
                     accession.process_status = Accession.DELIVERED
                     accession.save()
-                    messages.success(request, "Accession data delivered.")
+                    messages.success(self.request, "Accession data delivered.")
                 except Exception as e:
-                    messages.error(
-                        request, "Error delivering accession data: {}".format(e)
-                    )
-            return redirect("accession:detail", accession.pk)
+                    messages.error(self.request, "Error delivering accession data: {}".format(e))
+            return super().form_valid(form)
         messages.error(
-            request,
-            "There was a problem with your submission. Please correct the error(s) below and try again.",
-        )
-        return render(
-            request,
-            self.template_name,
-            {
-                "page_title": "Create Accession Record",
-                "form": form,
-                "creators_formset": creators_formset,
-                "rights_statements": rights_statements,
-                "transfers": transfers_list,
-            },
-        )
+            self.request,
+            "There was a problem with your submission. Please correct the error(s) below and try again.")
+        return super().form_invalid()
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)

aurora/bag_transfer/migrations/0035_auto_20210719_1528.py

@@ -7,7 +7,7 @@
 class Migration(migrations.Migration):
 
     dependencies = [
-        ('bag_transfer', '0034_auto_20210706_1512'),
+        ('bag_transfer', '0035_auto_20210719_1454'),
     ]
 
     operations = [

aurora/bag_transfer/orgs/urls.py

@@ -1,6 +1,7 @@
 from bag_transfer.orgs.views import (BagItProfileAPIAdminView,
+                                     BagItProfileCreateView,
                                      BagItProfileDetailView,
-                                     BagItProfileManageView,
+                                     BagItProfileUpdateView,
                                      OrganizationCreateView,
                                      OrganizationDetailView,
                                      OrganizationEditView,
@@ -16,7 +17,7 @@
     url(r"^(?P<pk>\d+)/edit/$", OrganizationEditView.as_view(), name="edit"),
     url(
         r"^(?P<pk>\d+)/bagit_profiles/add/$",
-        BagItProfileManageView.as_view(),
+        BagItProfileCreateView.as_view(),
         name="bagit-profiles-add",
     ),
     url(
@@ -26,7 +27,7 @@
     ),
     url(
         r"^(?P<pk>\d+)/bagit_profile/edit$",
-        BagItProfileManageView.as_view(),
+        BagItProfileUpdateView.as_view(),
         name="bagit-profiles-edit",
     ),
     url(

aurora/bag_transfer/orgs/views.py

@@ -16,7 +16,7 @@
 from django.contrib import messages
 from django.contrib.messages.views import SuccessMessageMixin
 from django.http import Http404
-from django.shortcuts import get_object_or_404, redirect, render
+from django.shortcuts import get_object_or_404
 from django.urls import reverse
 from django.views.generic import (CreateView, DetailView, ListView,
                                   TemplateView, UpdateView)
@@ -74,9 +74,10 @@ class OrganizationListView(PageTitleMixin, ArchivistMixin, ListView):
     model = Organization
 
 
-class BagItProfileManageView(PageTitleMixin, TemplateView):
+class BagItProfileManageView(PageTitleMixin):
     template_name = "bagit_profiles/manage.html"
     model = BagItProfile
+    form_class = BagItProfileForm
 
     def get_page_title(self, context):
         organization = get_object_or_404(Organization, pk=self.kwargs.get("pk"))
@@ -106,97 +107,70 @@ def get_context_data(self, **kwargs):
         context["organization"] = organization
         return context
 
-    def post(self, request, *args, **kwargs):
+    def get_success_url(self):
+        return reverse("orgs:detail", kwargs={"pk": self.kwargs.get("pk")})
+
+    def form_valid(self, form):
+        """Saves associated formsets."""
         organization = get_object_or_404(Organization, pk=self.kwargs.get("pk"))
-        instance = organization.bagit_profile
-        form = BagItProfileForm(request.POST, instance=instance)
-        if form.is_valid():
-            bagit_profile = instance if instance else form.save()
-            bag_info_formset = BagItProfileBagInfoFormset(
-                request.POST, instance=bagit_profile, prefix="bag_info")
-            manifests_allowed_formset = ManifestsAllowedFormset(
-                request.POST, instance=bagit_profile, prefix="manifests_allowed")
-            manifests_formset = ManifestsRequiredFormset(
-                request.POST, instance=bagit_profile, prefix="manifests")
-            serialization_formset = AcceptSerializationFormset(
-                request.POST, instance=bagit_profile, prefix="serialization")
-            version_formset = AcceptBagItVersionFormset(
-                request.POST, instance=bagit_profile, prefix="version")
-            tag_manifests_formset = TagManifestsRequiredFormset(
-                request.POST, instance=bagit_profile, prefix="tag_manifests")
-            tag_files_formset = TagFilesRequiredFormset(
-                request.POST, instance=bagit_profile, prefix="tag_files")
-            forms_to_save = [
-                bag_info_formset,
-                manifests_allowed_formset,
-                manifests_formset,
-                serialization_formset,
-                version_formset,
-                tag_manifests_formset,
-                tag_files_formset,
-            ]
-            for formset in forms_to_save:
-                if not formset.is_valid():
-                    messages.error(
-                        request,
-                        "There was a problem with your submission. Please correct the error(s) below and try again.",
-                    )
-                    return render(
-                        request,
-                        self.template_name,
-                        {
-                            "organization": organization,
-                            "form": form,
-                            "bag_info_formset": bag_info_formset,
-                            "manifests_allowed_formset": manifests_allowed_formset,
-                            "manifests_formset": manifests_formset,
-                            "serialization_formset": serialization_formset,
-                            "version_formset": version_formset,
-                            "tag_manifests_formset": tag_manifests_formset,
-                            "tag_files_formset": tag_files_formset,
-                        },
-                    )
-            for formset in forms_to_save:
+        bagit_profile = form.save()
+        bag_info_formset = BagItProfileBagInfoFormset(
+            self.request.POST, instance=bagit_profile, prefix="bag_info")
+        manifests_allowed_formset = ManifestsAllowedFormset(
+            self.request.POST, instance=bagit_profile, prefix="manifests_allowed")
+        manifests_formset = ManifestsRequiredFormset(
+            self.request.POST, instance=bagit_profile, prefix="manifests")
+        serialization_formset = AcceptSerializationFormset(
+            self.request.POST, instance=bagit_profile, prefix="serialization")
+        version_formset = AcceptBagItVersionFormset(
+            self.request.POST, instance=bagit_profile, prefix="version")
+        tag_manifests_formset = TagManifestsRequiredFormset(
+            self.request.POST, instance=bagit_profile, prefix="tag_manifests")
+        tag_files_formset = TagFilesRequiredFormset(
+            self.request.POST, instance=bagit_profile, prefix="tag_files")
+        forms_to_save = [
+            bag_info_formset,
+            manifests_allowed_formset,
+            manifests_formset,
+            serialization_formset,
+            version_formset,
+            tag_manifests_formset,
+            tag_files_formset,
+        ]
+        for formset in forms_to_save:
+            if not formset.is_valid():
+                messages.error(
+                    self.request,
+                    "There was a problem with your submission. Please correct the error(s) below and try again.")
+                return super().form_invalid(form)
+            else:
                 formset.save()
-            bagit_profile.version = bagit_profile.version + Decimal(1)
-            bagit_profile.bagit_profile_identifier = request.build_absolute_uri(
-                reverse(
-                    "bagitprofile-detail",
-                    kwargs={"pk": bagit_profile.id, "format": "json"},
-                )
-            )
-            bagit_profile.save_to_org(organization)
-            messages.success(
-                request,
-                "BagIt Profile for {} saved".format(organization.name),
+        bagit_profile.version = bagit_profile.version + Decimal(1)
+        bagit_profile.bagit_profile_identifier = self.request.build_absolute_uri(
+            reverse(
+                "bagitprofile-detail",
+                kwargs={"pk": bagit_profile.id, "format": "json"},
             )
-            return redirect("orgs:detail", organization.pk)
-        messages.error(
-            request,
-            "There was a problem with your submission. Please correct the error(s) below and try again.",
-        )
-        return render(
-            request,
-            self.template_name,
-            {
-                "form": BagItProfileForm(request.POST, instance=instance),
-                "organization": Organization.objects.get(pk=self.kwargs.get("pk")),
-                "bag_info_formset": BagItProfileBagInfoFormset(
-                    request.POST, prefix="bag_info"),
-                "manifests_allowed_formset": ManifestsAllowedFormset(
-                    request.POST, prefix="manifests_allowed"),
-                "manifests_formset": ManifestsRequiredFormset(
-                    request.POST, prefix="manifests"),
-                "serialization_formset": AcceptSerializationFormset(
-                    request.POST, prefix="serialization"),
-                "version_formset": AcceptBagItVersionFormset(
-                    request.POST, prefix="version"),
-                "tag_manifests_formset": TagManifestsRequiredFormset(
-                    request.POST, prefix="tag_manifests"),
-                "tag_files_formset": TagFilesRequiredFormset(
-                    request.POST, prefix="tag_files"),
-            },
         )
+        bagit_profile.save_to_org(organization)
+        messages.success(
+            self.request,
+            "BagIt Profile for {} saved".format(organization.name))
+        return super().form_valid(form)
+
+    def form_invalid(self, form):
+        messages.error(
+            self.request,
+            "There was a problem with your submission. Please correct the error(s) below and try again.")
+        return super().form_invalid(form)
+
+
+class BagItProfileCreateView(BagItProfileManageView, CreateView):
+    pass
+
+
+class BagItProfileUpdateView(BagItProfileManageView, UpdateView):
+    pass
 
 
 class BagItProfileDetailView(PageTitleMixin, OrgReadViewMixin, DetailView):

aurora/bag_transfer/rights/urls.py

@@ -1,13 +1,13 @@
-from bag_transfer.rights.views import (RightsAPIAdminView, RightsDetailView,
-                                       RightsManageView)
+from bag_transfer.rights.views import (RightsAPIAdminView, RightsCreateView,
+                                       RightsDetailView, RightsUpdateView)
 from django.conf.urls import url
 
 app_name = 'rights'
 
 urlpatterns = [
-    url(r"^add/$", RightsManageView.as_view(), name="add"),
+    url(r"^add/$", RightsCreateView.as_view(), name="add"),
     url(r"^(?P<pk>\d+)/$", RightsDetailView.as_view(), name="detail"),
-    url(r"^(?P<pk>\d+)/edit$", RightsManageView.as_view(), name="edit"),
+    url(r"^(?P<pk>\d+)/edit$", RightsUpdateView.as_view(), name="edit"),
     url(
         r"^(?P<pk>\d+)/(?P<action>(delete))/$", RightsAPIAdminView.as_view(), name="api"
     ),