Improve migrations and validation

django-verdant/rca/forms.py

@@ -0,0 +1,44 @@
+from  wagtail.wagtailadmin.forms import WagtailAdminPageForm
+
+
+class StaffPageForm(WagtailAdminPageForm):
+    def clean(self):
+        cleaned_data = super(StaffPageForm, self).clean()
+
+        for form in self.formsets.get('roles', []):
+            self.clean_staff_page_role_form(form, cleaned_data)
+
+    def clean_staff_page_role_form(self, form, staff_page_cleaned_data):
+        if not form.is_valid():
+            return
+
+        area = form.cleaned_data.get('area')
+        location = form.cleaned_data.get('location')
+        programme = form.cleaned_data.get('programme')
+        school = form.cleaned_data.get('school')
+        staff_type = staff_page_cleaned_data.get('staff_type')
+
+        # Will display all errors at the same time, so need to create a dict
+        errors = {x: [] for x in ('area', 'location', 'programme', 'school')}
+
+        # Staff location must be only for technical staff
+        if staff_type != 'technical' and location:
+            errors['location'].append('Location can be assigned only to technical staff')
+
+        # School and programme must be only for academic staff
+        if staff_type != 'academic':
+            if school:
+                errors['school'].append('School can be assigned only to academic staff.')
+
+            if programme:
+                errors['programme'].append('Programme can be only assigned to academic staff.')
+
+        # Area cannot be filled in when staff is non-academic/administrative
+        if staff_type not in ('academic', 'administrative') and area:
+            errors['area'].append('Area can be only assigned to academic or administrative staff.')
+
+        # If there are any errors in our dict, raise them.
+        if any(errors.itervalues()):
+            for field_name, field_errors in errors.items():
+                for field_error in field_errors:
+                    form.add_error(field_name, field_error)

django-verdant/rca/migrations/0087_staffpage_migrate_area_data_to_roles.py

@@ -1,33 +1,54 @@
 # -*- coding: utf-8 -*-
 # Generated by Django 1.9.13 on 2017-07-06 21:26
 from __future__ import unicode_literals
+import json
 
 from django.db import migrations
-from django.db.models import Count
 
 
-class Migration(migrations.Migration):
-    def move_area_data(apps, schema_editor):
-        """
-        Move StaffPage.area to StaffPageRole.area if the user is
-        academic/administrative and if StaffPageRole.area is empty.
-        """
-        StaffPage = apps.get_model('rca', 'StaffPage')
-
-        # Move area data to roles
-        records = StaffPage.objects \
-                            .prefetch_related('roles') \
-                            .annotate(roles_count=Count('roles__id')) \
-                            .filter(staff_type__in=('administrative', 'academic')) \
-                            .filter(area__isnull=False) \
-                            .filter(roles_count__gt=0)
-
-        for staff in records:
-            first_role = staff.roles.filter(area__isnull=True).first()
-            if first_role is not None:
-                first_role.area = staff.area
-                first_role.save()
+def move_area_data(apps, schema_editor):
+    """
+    Move StaffPage.area to StaffPageRole.area if the user is
+    academic/administrative and if StaffPageRole.area is empty.
+    """
+    StaffPage = apps.get_model('rca', 'StaffPage')
+
+    # Move area data to roles
+    records = StaffPage.objects \
+                        .prefetch_related('roles') \
+                        .filter(staff_type__in=('administrative', 'academic')) \
+                        .filter(area__isnull=False)
+
+    print('Found {} records to update.'.format(records.count()))
+
+    for staff in records:
+        first_role = staff.roles.filter(area__isnull=True).first()
+
+        # Update revisions
+        if staff.has_unpublished_changes:
+            # Can't use get_latest_revision() as I am unable to call
+            # model methods in migrations.
+            revision = staff.revisions.order_by('-created_at', '-id').first()
+
+            if revision:
+                revision_json = json.loads(revision.content_json)
+
+                if revision_json['roles']:
+                    print('Updating draft of {} (#{}).'.format(staff, staff.pk))
 
+                    revision_json['roles'][0]['area'] = staff.area_id
+
+                    revision.content_json = json.dumps(revision_json)
+                    revision.save()
+
+        if first_role is not None:
+            print('Updating live version of {} (#{}).'.format(staff, staff.pk))
+
+            first_role.area = staff.area
+            first_role.save()
+
+
+class Migration(migrations.Migration):
     dependencies = [
         ('rca', '0086_schoolpageresearchlinks_allow_to_add_external_links'),
     ]

django-verdant/rca/migrations/0088_staffpage_move_area_and_location_field_to_roles.py

@@ -16,10 +16,6 @@ class Migration(migrations.Migration):
             model_name='staffpage',
             name='area',
         ),
-        migrations.RemoveField(
-            model_name='staffpage',
-            name='staff_location',
-        ),
         migrations.AddField(
             model_name='staffpagerole',
             name='location',

django-verdant/rca/migrations/0089_staffpage_move_location_data.py

@@ -0,0 +1,59 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.13 on 2017-07-06 21:26
+from __future__ import unicode_literals
+import json
+
+from django.db import migrations
+
+
+def move_location_data(apps, schema_editor):
+    """
+    Move StaffPage.location to StaffPageRole.location if the user is
+    technical.
+    """
+    StaffPage = apps.get_model('rca', 'StaffPage')
+
+    # Move location data to roles
+    records = StaffPage.objects \
+                        .prefetch_related('roles') \
+                        .exclude(staff_location='') \
+                        .filter(staff_type='technical') \
+
+    print('Found {} records of technical staff with location data '
+            'to migrate.'.format(records.count()))
+
+    for staff in records:
+        # Update revisions
+        if staff.has_unpublished_changes:
+            # Can't use get_latest_revision() as I am unable to call
+            # model methods in migrations.
+            revision = staff.revisions.order_by('-created_at', '-id').first()
+
+            if revision:
+                revision_json = json.loads(revision.content_json)
+
+                if revision_json['roles']:
+                    print('Updating draft of {} (#{}).'.format(staff, staff.pk))
+
+                    revision_json['roles'][0]['location'] = staff.staff_location
+
+                    revision.content_json = json.dumps(revision_json)
+                    revision.save()
+
+        first_role = staff.roles.first()
+
+        if first_role is not None:
+            print('Updating live version of {} (#{}).'.format(staff, staff.pk))
+
+            first_role.location = staff.staff_location
+            first_role.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('rca', '0088_staffpage_move_area_and_location_field_to_roles'),
+    ]
+
+    operations = [
+        migrations.RunPython(move_location_data)
+    ]

django-verdant/rca/migrations/0090_staffpage_remove_location.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.13 on 2017-07-06 21:28
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('rca', '0089_staffpage_move_location_data'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='staffpage',
+            name='staff_location',
+        ),
+    ]

django-verdant/rca/models.py

@@ -66,6 +66,7 @@
 from rca_signage.constants import SCREEN_CHOICES
 from reachout_choices import REACHOUT_PROJECT_CHOICES, REACHOUT_PARTICIPANTS_CHOICES, REACHOUT_THEMES_CHOICES, REACHOUT_PARTNERSHIPS_CHOICES
 
+from .forms import StaffPageForm
 from .help_text import help_text
 
 
@@ -3043,31 +3044,6 @@ class StaffPageRole(Orderable):
         FieldPanel('email'),
     ]
 
-    def clean(self):
-        if hasattr(self, 'page'):
-            # Will display all errors at the same time, so need to create a dict
-            errors = {x: [] for x in ('area', 'locatgion', 'programme', 'school')}
-
-            # Staff location must be only for technical staff
-            if self.page.staff_type != 'technical' and self.location:
-                errors['location'].append('Location can be assigned only to technical staff')
-
-            # School and programme must be only for academic staff
-            if self.page.staff_type != 'academic':
-                if self.school:
-                    errors['school'].append('School can be assigned only to academic staff.')
-
-                if self.programme:
-                    errors['programme'].append('Programme can be only assigned to academic staff.')
-
-            # Area cannot be filled in when staff is non-academic/administrative
-            if self.page.staff_type not in ('academic', 'administrative') and self.area:
-                errors['area'].append('Area can be only assigned to academic or administrative staff.')
-
-            # If there are any errors in our dict, raise them.
-            if any(errors.itervalues()):
-                raise ValidationError(errors)
-
 
 class StaffPageCollaborations(Orderable):
     page = ParentalKey('rca.StaffPage', related_name='collaborations')
@@ -3142,6 +3118,8 @@ class StaffPage(Page, SocialFields):
     feed_image = models.ForeignKey('rca.RcaImage', null=True, blank=True, on_delete=models.SET_NULL, related_name='+', help_text=help_text('rca.StaffPage', 'feed_image', default="The image displayed in content feeds, such as the news carousel. Should be 16:9 ratio."))
     ad_username = models.CharField(max_length=255, blank=True, verbose_name='AD username', help_text=help_text('rca.StaffPage', 'ad_username'))
 
+    base_form_class = StaffPageForm
+
     search_fields = Page.search_fields + [
         index.RelatedFields('roles', [
             index.RelatedFields('area', [