fix: Accessibility issues

bag_transfer/accession/forms.py

@@ -46,6 +46,12 @@ class Meta:
             "creators": forms.widgets.MultipleHiddenInput(),
         }
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.fields["resource"].error_messages["required"] = (
+            "No ArchivesSpace resource target specified."
+        )
+
 
 CreatorsFormSet = forms.modelformset_factory(
     RecordCreators,

bag_transfer/accession/views.py

@@ -86,7 +86,7 @@ class AccessionDetailView(PageTitleMixin, OrgReadViewMixin, DetailView):
     model = Accession
 
     def get_page_title(self, context):
-        return context["object"].title
+        return "Accession: {}".format(context["object"].title)
 
 
 class AccessionCreateView(PageTitleMixin, AccessioningArchivistMixin, JSONResponseMixin, CreateView):
@@ -136,6 +136,7 @@ def form_valid(self, form):
         messages.error(
             self.request,
             "There was a problem with your submission. Please correct the error(s) below and try again.")
+        self._invalid_creators_formset = creators_formset
         return super().form_invalid(form)
 
     def get_context_data(self, **kwargs):
@@ -155,26 +156,29 @@ def get_context_data(self, **kwargs):
             organization,
             record_type,
             ", ".join([creator.name for creator in creators_list]))
-        context["form"] = AccessionForm(
-            initial={
-                "title": title,
-                "start_date": sorted(dates.get("start", []))[0],
-                "end_date": sorted(dates.get("end", []))[-1],
-                "description": " ".join(set(descriptions_list)),
-                "extent_files": extent_files,
-                "extent_size": extent_size,
-                "access_restrictions": " ".join(
-                    set(notes.get("other", []) + notes.get("license", []) + notes.get("statute", []))
-                ),
-                "use_restrictions": " ".join(set(notes.get("copyright", []))),
-                "acquisition_type": organization.acquisition_type,
-                "appraisal_note": " ".join(set(notes.get("appraisal", []))),
-                "organization": organization,
-                "language": language,
-                "creators": creators_list,
-            }
-        )
-        context["creators_formset"] = CreatorsFormSet(queryset=RecordCreators.objects.filter(name__in=creators_list))
+        form = context.get("form")
+        if form is None or not form.is_bound:
+            form = AccessionForm(
+                initial={
+                    "title": title,
+                    "start_date": sorted(dates.get("start", []))[0],
+                    "end_date": sorted(dates.get("end", []))[-1],
+                    "description": " ".join(set(descriptions_list)),
+                    "extent_files": extent_files,
+                    "extent_size": extent_size,
+                    "access_restrictions": " ".join(
+                        set(notes.get("other", []) + notes.get("license", []) + notes.get("statute", []))
+                    ),
+                    "use_restrictions": " ".join(set(notes.get("copyright", []))),
+                    "acquisition_type": organization.acquisition_type,
+                    "appraisal_note": " ".join(set(notes.get("appraisal", []))),
+                    "organization": organization,
+                    "language": language,
+                    "creators": creators_list,
+                }
+            )
+        context["form"] = form
+        context["creators_formset"] = getattr(self, "_invalid_creators_formset", None) or CreatorsFormSet(queryset=RecordCreators.objects.filter(name__in=creators_list))
         context["transfers"] = transfers_list
         context["rights_statements"] = rights_statements
         return context
@@ -330,7 +334,7 @@ def button(self, accession):
         button = "Accession not delivered"
         if self.request.user.can_accession():
             button = (
-                '<button class="btn btn--sm btn--blue deliver">Deliver Accession</button>'
+                '<button class="btn btn--sm btn--blue deliver" aria-label="Deliver accession {0}">Deliver Accession</button>'.format(accession.title)
                 if (accession.process_status < Accession.DELIVERED)
                 else '<p>' + accession.get_process_status_display() + "</p>"
             )

bag_transfer/appraise/views.py

@@ -108,14 +108,16 @@ def appraise_buttons(self, transfer):
                 btn_class = "btn btn--xs btn--light-blue"
                 note_class = "edit-note"
                 note_text = "Edit"
+                note_label = "Edit note for {}".format(transfer.bag_or_failed_name)
             else:
                 btn_class = "btn-info"
                 note_class = ""
                 note_text = "Add"
-            buttons = '<button class="btn btn--xs btn--blue appraisal-accept">Accept</button>\
-                       <button class="btn btn--xs btn--orange appraisal-reject">Reject</button>\
-                       <button class="btn btn--xs btn--light-blue appraisal-note {} {}" data-target="#modal-appraisal-note">{} Note</button>'.format(
-                btn_class, note_class, note_text
+                note_label = "Add note for {}".format(transfer.bag_or_failed_name)
+            buttons = '<button class="btn btn--xs btn--blue appraisal-accept" aria-label="Accept transfer {0}">Accept</button>\
+                    <button class="btn btn--xs btn--orange appraisal-reject" aria-label="Reject transfer {0}">Reject</button>\
+                    <button class="btn btn--xs btn--light-blue appraisal-note {1} {2}" data-target="#modal-appraisal-note" aria-label="{3}">{4} Note</button>'.format(
+                transfer.bag_or_failed_name, btn_class, note_class, note_label, note_text
             )
         return buttons
 

bag_transfer/bagit_profiles/form.py

@@ -21,10 +21,7 @@ class Meta:
             "version": forms.widgets.HiddenInput(),
             "bagit_profile_identifier": forms.widgets.HiddenInput(),
             "external_description": forms.widgets.Textarea(
-                attrs={
-                    "rows": 3,
-                    "aria-describedby": "id_external_description-help"
-                }
+                attrs={"rows": 3}
             ),
             "allow_fetch": forms.widgets.CheckboxInput(attrs={"class": "checkbox checkbox--blue"}),
             "serialization": forms.widgets.RadioSelect(attrs={"class": "radiobutton radiobutton--blue"}),
@@ -82,10 +79,13 @@ class BagItProfileBagInfoValuesForm(forms.ModelForm):
     class Meta:
         model = BagItProfileBagInfoValues
         fields = ("name",)
-        widgets = {
-            "name": forms.widgets.TextInput(
-                attrs={"aria-labelledby": "values-label", })
-        }
+
+    # Read the row's position to set unique aria-label for controlled value input
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        index = self.prefix.rsplit("-", 1)[-1] if self.prefix else ""
+        if index.isdigit():
+            self.fields["name"].widget.attrs["aria-label"] = "Controlled value %s" % (int(index) + 1)
 
 
 BagItProfileBagInfoValuesFormset = forms.inlineformset_factory(

bag_transfer/bagit_profiles/views.py

@@ -1,6 +1,7 @@
 from decimal import Decimal
 
 from django.contrib import messages
+from django.db import transaction
 from django.http import Http404
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
@@ -21,39 +22,54 @@ class BagItProfileManageView(PageTitleMixin):
     model = BagItProfile
     form_class = BagItProfileForm
 
+    def get_organization(self):
+        if self.object:
+            return self.object.organization
+        else:
+            return get_object_or_404(Organization, pk=self.request.GET.get("org"))
+
     def get_page_title(self, context):
-        return "Edit BagIt Profile" if self.object else "Create BagIt Profile"
+        org = self.get_organization()
+        if self.object:
+            return "Edit BagIt Profile: {}".format(org)
+        else:
+            return "Create BagIt Profile: {}".format(org)
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
-        if self.object:
-            form = BagItProfileForm(instance=self.object)
-            organization = self.object.organization
-        else:
-            source_organization = self.request.user.organization
-            organization = get_object_or_404(Organization, pk=self.request.GET.get("org"))
-            form = BagItProfileForm(
-                initial={
-                    "source_organization": source_organization,
-                    "contact_email": "archive@rockarch.org",
-                    "organization": organization})
+        organization = self.get_organization()
+
+        # Reuse the bound form that was built for this submission so errors/values survive
+        # a failed nested formset save
+        form = context.get("form")
+        if form is None or not form.is_bound:
+            if self.object:
+                form = BagItProfileForm(instance=self.object)
+            else:
+                source_organization = self.request.user.organization
+                form = BagItProfileForm(
+                    initial={
+                        "source_organization": source_organization,
+                        "contact_email": "archive@rockarch.org",
+                        "organization": organization})
         context["form"] = form
-        context["bag_info_formset"] = BagItProfileBagInfoFormset(instance=self.object, prefix="bag_info")
+        context["bag_info_formset"] = getattr(self, "_invalid_bag_info_formset", None) or BagItProfileBagInfoFormset(instance=self.object, prefix="bag_info")
         context["organization"] = organization
         return context
 
     def get_success_url(self):
-        return reverse("bagit-profiles:detail", kwargs={"pk": self.object.pk})
+        return reverse("orgs:detail", kwargs={"pk": self.object.organization.pk})
 
     def form_valid(self, form):
         """Saves associated formsets."""
-        bagit_profile = form.save()
+        # Only save once all are confirmed valid
+        bagit_profile = form.save(commit=False)
         bag_info_formset = BagItProfileBagInfoFormset(
             self.request.POST, instance=bagit_profile, prefix="bag_info")
         if not bag_info_formset.is_valid():
             error_messages = []
-            for form in bag_info_formset:
-                for field, errors in form.errors.items():
+            for bag_info_form in bag_info_formset:
+                for field, errors in bag_info_form.errors.items():
                     for error in errors:
                         error_messages.append(f"{field}: {error}")
 
@@ -67,16 +83,23 @@ def form_valid(self, form):
                 f"There was a problem with your submission: {detailed_errors} "
                 "Please correct the error(s) and try again."
             )
+            self._invalid_bag_info_formset = bag_info_formset
             return super().form_invalid(form)
-        else:
+        with transaction.atomic():
+            bagit_profile.save()
+            form.save_m2m()
             bag_info_formset.save()
-        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"},
+
+            self.object = bagit_profile
+            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"},
+                )
             )
-        )
+            bagit_profile.save()
+
         messages.success(self.request, "BagIt Profile saved")
         return super().form_valid(form)
 
@@ -102,9 +125,11 @@ class BagItProfileUpdateView(BagItProfileManageView, UpdateView):
 
 class BagItProfileDetailView(PageTitleMixin, OrgReadViewMixin, DetailView):
     template_name = "bagit_profiles/detail.html"
-    page_title = "BagIt Profile"
     model = BagItProfile
 
+    def get_page_title(self, context):
+        return "BagIt Profile: {}".format(context["object"].organization)
+
 
 class BagItProfileAPIAdminView(ManagingArchivistMixin, JSONResponseMixin, TemplateView):
 

bag_transfer/orgs/forms.py

@@ -0,0 +1,38 @@
+from django import forms
+
+from bag_transfer.models import Organization
+
+EMPTY_LABEL = "--- Select an acquisition type ---"
+
+
+class OrganizationForm(forms.ModelForm):
+    class Meta:
+        model = Organization
+        fields = ["name", "acquisition_type"]
+        labels = {
+            "name": "Organization Name",
+        }
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.fields["acquisition_type"].choices = [
+            ("", EMPTY_LABEL)
+        ] + list(Organization.ACQUISITION_TYPE_CHOICES)
+
+
+class OrganizationUpdateForm(forms.ModelForm):
+    class Meta:
+        model = Organization
+        fields = ["is_active", "name", "acquisition_type"]
+        labels = {
+            "name": "Organization Name",
+        }
+        widgets = {
+            "is_active": forms.widgets.CheckboxInput(attrs={"class": "checkbox checkbox--blue"}),
+        }
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.fields["acquisition_type"].choices = [
+            ("", EMPTY_LABEL)
+        ] + list(Organization.ACQUISITION_TYPE_CHOICES)

bag_transfer/orgs/views.py

@@ -7,13 +7,14 @@
                                             OrgReadViewMixin)
 from bag_transfer.mixins.viewmixins import PageTitleMixin
 from bag_transfer.models import Organization, Transfer
+from bag_transfer.orgs.forms import OrganizationForm, OrganizationUpdateForm
 
 
 class OrganizationCreateView(PageTitleMixin, ManagingArchivistMixin, SuccessMessageMixin, CreateView):
     template_name = "orgs/create.html"
     page_title = "Add Organization"
     model = Organization
-    fields = ["name", "acquisition_type"]
+    form_class = OrganizationForm
     success_message = "New Organization Saved!"
 
     def get_success_url(self):
@@ -47,7 +48,7 @@ def get_page_title(self, context):
 class OrganizationEditView(PageTitleMixin, ManagingArchivistMixin, SuccessMessageMixin, UpdateView):
     template_name = "orgs/update.html"
     model = Organization
-    fields = ["is_active", "name", "acquisition_type"]
+    form_class = OrganizationUpdateForm
     success_message = "Organization Saved!"
 
     def get_page_title(self, context):