From 46434977fb4b3f186f0044417f878cb59e890f26 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Apr 2016 04:18:19 +0100 Subject: [PATCH 1/9] Created merge admin action for Person, Venue and Organisation models. Added template. --- RIGS/admin.py | 46 +++++++++++++++++-- .../templates/RIGS/admin_associate_merge.html | 21 +++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 RIGS/templates/RIGS/admin_associate_merge.html diff --git a/RIGS/admin.py b/RIGS/admin.py index 18dc554e..a72c4705 100644 --- a/RIGS/admin.py +++ b/RIGS/admin.py @@ -4,16 +4,20 @@ from django.contrib.auth.admin import UserAdmin from django.utils.translation import ugettext_lazy as _ import reversion +from django.contrib.admin import helpers +from django.template.response import TemplateResponse +from django.contrib import messages +from django.db import transaction +from django.core.exceptions import ObjectDoesNotExist + # Register your models here. -admin.site.register(models.Person, reversion.VersionAdmin) -admin.site.register(models.Organisation, reversion.VersionAdmin) admin.site.register(models.VatRate, reversion.VersionAdmin) -admin.site.register(models.Venue, reversion.VersionAdmin) admin.site.register(models.Event, reversion.VersionAdmin) admin.site.register(models.EventItem, reversion.VersionAdmin) admin.site.register(models.Invoice) admin.site.register(models.Payment) +@admin.register(models.Profile) class ProfileAdmin(UserAdmin): fieldsets = ( (None, {'fields': ('username', 'password')}), @@ -33,4 +37,38 @@ class ProfileAdmin(UserAdmin): form = forms.ProfileChangeForm add_form = forms.ProfileCreationForm -admin.site.register(models.Profile, ProfileAdmin) +@admin.register(models.Person, models.Organisation, models.Venue) +class AssociateAdmin(reversion.VersionAdmin): + list_display = ('id', 'name','number_of_events') + search_fields = ['id','name'] + + actions = ['merge'] + + def number_of_events(self,obj): + return obj.latest_events.count() + + def merge(self, request, queryset): + if request.POST.get('post'): # Has the user confirmed which is the master record? + try: + masterObjectPk = request.POST.get('master') + masterObject = queryset.get(pk = masterObjectPk) + except ObjectDoesNotExist: + self.message_user(request, "An error occured. Did you select a 'master' record?",level=messages.ERROR) + return + + with transaction.atomic(), reversion.create_revision(): + for obj in queryset.exclude(pk = masterObjectPk): + events = obj.event_set.all() + for event in events: + masterObject.event_set.add(event) + obj.delete() + + self.message_user(request, "Objects successfully merged.") + return + else: # Present the confirmation screen + context = { + 'title': _("Are you sure?"), + 'queryset': queryset, + 'action_checkbox_name': helpers.ACTION_CHECKBOX_NAME, + } + return TemplateResponse(request, 'RIGS/admin_associate_merge.html', context, current_app=self.admin_site.name) \ No newline at end of file diff --git a/RIGS/templates/RIGS/admin_associate_merge.html b/RIGS/templates/RIGS/admin_associate_merge.html new file mode 100644 index 00000000..e30b2e55 --- /dev/null +++ b/RIGS/templates/RIGS/admin_associate_merge.html @@ -0,0 +1,21 @@ +{% extends "admin/base_site.html" %} +{% load i18n l10n %} + +{% block content %} +
{% csrf_token %} +

The following objects will be merged. Please select the 'master' record which you would like to keep. Other records will have associated events moved to the 'master' copy, and then will be deleted.

+ + {% for item in queryset %} + {{item.pk}} | {{item}}
+ {% endfor %} + +
+ {% for obj in queryset %} + + {% endfor %} + + + +
+
+{% endblock %} \ No newline at end of file From 33ce4b622d3a8e034178f0bee8a10d0f72caea3d Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Apr 2016 04:18:53 +0100 Subject: [PATCH 2/9] Fixed bug with versioning interface when related objects are deleted --- RIGS/versioning.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/RIGS/versioning.py b/RIGS/versioning.py index 789fc5d7..c537de87 100644 --- a/RIGS/versioning.py +++ b/RIGS/versioning.py @@ -92,9 +92,16 @@ def model_compare(oldObj, newObj, excluded_keys=[]): if name in excluded_keys: continue # if we're excluding this field, skip over it - oldValue = getattr(oldObj, name, None) - newValue = getattr(newObj, name, None) + try: + oldValue = getattr(oldObj, name, None) + except ObjectDoesNotExist: + oldValue = None + try: + newValue = getattr(newObj, name, None) + except ObjectDoesNotExist: + newValue = None + try: bothBlank = (not oldValue) and (not newValue) if oldValue != newValue and not bothBlank: From ca6cddb3924bd185532a716ebb12aa9b79f951b7 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Apr 2016 11:50:34 +0100 Subject: [PATCH 3/9] Add comments display to versioning history (because why not). Maybe in future we could have a box people can type in before they save changes to an event... But that's a separate project --- RIGS/admin.py | 3 ++- RIGS/templates/RIGS/activity_feed_data.html | 3 +++ RIGS/templates/RIGS/activity_table.html | 2 ++ RIGS/templates/RIGS/version_history.html | 4 ++++ 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/RIGS/admin.py b/RIGS/admin.py index a72c4705..d8583e23 100644 --- a/RIGS/admin.py +++ b/RIGS/admin.py @@ -62,7 +62,8 @@ class AssociateAdmin(reversion.VersionAdmin): for event in events: masterObject.event_set.add(event) obj.delete() - + reversion.set_comment('Merging Objects') + self.message_user(request, "Objects successfully merged.") return else: # Present the confirmation screen diff --git a/RIGS/templates/RIGS/activity_feed_data.html b/RIGS/templates/RIGS/activity_feed_data.html index fc9af87e..e99baf8e 100644 --- a/RIGS/templates/RIGS/activity_feed_data.html +++ b/RIGS/templates/RIGS/activity_feed_data.html @@ -40,6 +40,9 @@ {% endif %} {% include 'RIGS/object_button.html' with object=version.new %} + {% if version.revision.comment %} + ({{ version.revision.comment }}) + {% endif %}

diff --git a/RIGS/templates/RIGS/activity_table.html b/RIGS/templates/RIGS/activity_table.html index bf625e44..1d491663 100644 --- a/RIGS/templates/RIGS/activity_table.html +++ b/RIGS/templates/RIGS/activity_table.html @@ -59,6 +59,7 @@ Version ID User Changes + Comment @@ -75,6 +76,7 @@ {% else %} {% include 'RIGS/version_changes.html' %} {% endif %} + {{ version.revision.comment }} {% endfor %} diff --git a/RIGS/templates/RIGS/version_history.html b/RIGS/templates/RIGS/version_history.html index 163a1ac5..924ea148 100644 --- a/RIGS/templates/RIGS/version_history.html +++ b/RIGS/templates/RIGS/version_history.html @@ -35,6 +35,7 @@ Version ID User Changes + Comment @@ -51,6 +52,9 @@ {% include 'RIGS/version_changes.html' %} {% endif %} + + {{ version.revision.comment }} + {% endif %} {% endfor %} From 03ca65602fd2e41a6556473820fa493c774e97c6 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Apr 2016 12:08:19 +0100 Subject: [PATCH 4/9] Allow sorting by number of events --- RIGS/admin.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RIGS/admin.py b/RIGS/admin.py index d8583e23..9bfcb3eb 100644 --- a/RIGS/admin.py +++ b/RIGS/admin.py @@ -9,6 +9,7 @@ from django.template.response import TemplateResponse from django.contrib import messages from django.db import transaction from django.core.exceptions import ObjectDoesNotExist +from django.db.models import Count # Register your models here. admin.site.register(models.VatRate, reversion.VersionAdmin) @@ -44,9 +45,14 @@ class AssociateAdmin(reversion.VersionAdmin): actions = ['merge'] + def get_queryset(self, request): + return super(AssociateAdmin, self).get_queryset(request).annotate(event_count = Count('event')) + def number_of_events(self,obj): return obj.latest_events.count() + number_of_events.admin_order_field = 'event_count' + def merge(self, request, queryset): if request.POST.get('post'): # Has the user confirmed which is the master record? try: From 99dfdcd253a094f3eb4c5613f600024a60ef72a6 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Apr 2016 12:53:04 +0100 Subject: [PATCH 5/9] Make confirmation more useful --- RIGS/admin.py | 35 ++++++++++++++++--- .../templates/RIGS/admin_associate_merge.html | 25 +++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/RIGS/admin.py b/RIGS/admin.py index 9bfcb3eb..0dfafd7a 100644 --- a/RIGS/admin.py +++ b/RIGS/admin.py @@ -10,6 +10,7 @@ from django.contrib import messages from django.db import transaction from django.core.exceptions import ObjectDoesNotExist from django.db.models import Count +from django.forms import ModelForm # Register your models here. admin.site.register(models.VatRate, reversion.VersionAdmin) @@ -38,19 +39,19 @@ class ProfileAdmin(UserAdmin): form = forms.ProfileChangeForm add_form = forms.ProfileCreationForm -@admin.register(models.Person, models.Organisation, models.Venue) class AssociateAdmin(reversion.VersionAdmin): list_display = ('id', 'name','number_of_events') search_fields = ['id','name'] - + list_display_links = ['id','name'] actions = ['merge'] + merge_fields = ['name'] + def get_queryset(self, request): return super(AssociateAdmin, self).get_queryset(request).annotate(event_count = Count('event')) def number_of_events(self,obj): return obj.latest_events.count() - number_of_events.admin_order_field = 'event_count' def merge(self, request, queryset): @@ -73,9 +74,35 @@ class AssociateAdmin(reversion.VersionAdmin): self.message_user(request, "Objects successfully merged.") return else: # Present the confirmation screen + + class TempForm(ModelForm): + class Meta: + model = queryset.model + fields = self.merge_fields + + forms = [] + for obj in queryset: + forms.append(TempForm(instance=obj)) + context = { 'title': _("Are you sure?"), 'queryset': queryset, 'action_checkbox_name': helpers.ACTION_CHECKBOX_NAME, + 'forms': forms } - return TemplateResponse(request, 'RIGS/admin_associate_merge.html', context, current_app=self.admin_site.name) \ No newline at end of file + return TemplateResponse(request, 'RIGS/admin_associate_merge.html', context, current_app=self.admin_site.name) + +@admin.register(models.Person) +class PersonAdmin(AssociateAdmin): + list_display = ('id', 'name','phone','email','number_of_events') + merge_fields = ['name','phone','email','address','notes'] + +@admin.register(models.Venue) +class VenueAdmin(AssociateAdmin): + list_display = ('id', 'name','phone','email','number_of_events') + merge_fields = ['name','phone','email','address','notes','three_phase_available'] + +@admin.register(models.Organisation) +class OrganisationAdmin(AssociateAdmin): + list_display = ('id', 'name','phone','email','number_of_events') + merge_fields = ['name','phone','email','address','notes','union_account'] \ No newline at end of file diff --git a/RIGS/templates/RIGS/admin_associate_merge.html b/RIGS/templates/RIGS/admin_associate_merge.html index e30b2e55..2128725c 100644 --- a/RIGS/templates/RIGS/admin_associate_merge.html +++ b/RIGS/templates/RIGS/admin_associate_merge.html @@ -5,13 +5,32 @@
{% csrf_token %}

The following objects will be merged. Please select the 'master' record which you would like to keep. Other records will have associated events moved to the 'master' copy, and then will be deleted.

- {% for item in queryset %} - {{item.pk}} | {{item}}
+ + {% for form in forms %} + {% if forloop.first %} + + + + {% for field in form %} + + {% endfor %} + + {% endif %} + + + + + {% for field in form %} + + {% endfor %} + {% endfor %} +
ID {{ field.label }}
{{form.instance.pk}} {{ field.value }}
+
{% for obj in queryset %} - + {% endfor %} From 44ccead0a46daea308a8657815170867f24bb43d Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 5 Apr 2016 15:32:13 +0100 Subject: [PATCH 6/9] Added merge tests I feel like it should be possible to abstract some of these so that they're not copied out for each model, but not sure how to go about it... --- RIGS/test_unit.py | 139 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 RIGS/test_unit.py diff --git a/RIGS/test_unit.py b/RIGS/test_unit.py new file mode 100644 index 00000000..b6fc8edd --- /dev/null +++ b/RIGS/test_unit.py @@ -0,0 +1,139 @@ +from django.core.urlresolvers import reverse +from django.test import TestCase +from datetime import date + +from RIGS import models +from django.core.exceptions import ObjectDoesNotExist + +class TestAdminMergeObjects(TestCase): + @classmethod + def setUpTestData(cls): + cls.profile = models.Profile.objects.create(username="testuser1", email="1@test.com", is_superuser=True, is_active=True, is_staff=True) + + cls.persons = { + 1: models.Person.objects.create(name="Person 1"), + 2: models.Person.objects.create(name="Person 2"), + 3: models.Person.objects.create(name="Person 3"), + } + + cls.organisations = { + 1: models.Organisation.objects.create(name="Organisation 1"), + 2: models.Organisation.objects.create(name="Organisation 2"), + 3: models.Organisation.objects.create(name="Organisation 3"), + } + + cls.venues = { + 1: models.Venue.objects.create(name="Venue 1"), + 2: models.Venue.objects.create(name="Venue 2"), + 3: models.Venue.objects.create(name="Venue 3"), + } + + cls.events = { + 1: models.Event.objects.create(name="TE E1", start_date=date.today(), person=cls.persons[1], organisation=cls.organisations[3], venue=cls.venues[2]), + 2: models.Event.objects.create(name="TE E2", start_date=date.today(), person=cls.persons[2], organisation=cls.organisations[2], venue=cls.venues[3]), + 3: models.Event.objects.create(name="TE E3", start_date=date.today(), person=cls.persons[3], organisation=cls.organisations[1], venue=cls.venues[1]), + 4: models.Event.objects.create(name="TE E4", start_date=date.today(), person=cls.persons[3], organisation=cls.organisations[3], venue=cls.venues[3]), + } + + def setUp(self): + self.profile.set_password('testuser') + self.profile.save() + self.assertTrue(self.client.login(username=self.profile.username, password='testuser')) + + def test_merge_confirmation(self): + change_url = reverse('admin:RIGS_venue_changelist') + data = { + 'action': 'merge', + '_selected_action': [unicode(val.pk) for key,val in self.venues.iteritems()] + + } + response = self.client.post(change_url, data, follow=True) + + self.assertContains(response, "The following objects will be merged") + for key,venue in self.venues.iteritems(): + self.assertContains(response, venue.name) + + def test_merge_no_master(self): + change_url = reverse('admin:RIGS_venue_changelist') + data = {'action': 'merge', + '_selected_action': [unicode(val.pk) for key,val in self.venues.iteritems()], + 'post':'yes', + } + response = self.client.post(change_url, data, follow=True) + + self.assertContains(response, "An error occured") + + def test_venue_merge(self): + change_url = reverse('admin:RIGS_venue_changelist') + + data = {'action': 'merge', + '_selected_action': [unicode(val.pk) for key,val in self.venues.iteritems()], + 'post':'yes', + 'master':self.venues[1].pk + } + + response = self.client.post(change_url, data, follow=True) + self.assertContains(response, "Objects successfully merged") + self.assertContains(response, self.venues[1].name) + + # Check the master copy still exists + self.assertTrue(models.Venue.objects.get(pk=self.venues[1].pk)) + + # Check the un-needed venues have been disposed of + self.assertRaises(ObjectDoesNotExist, models.Venue.objects.get, pk=self.venues[2].pk) + self.assertRaises(ObjectDoesNotExist, models.Venue.objects.get, pk=self.venues[3].pk) + + # Check the events have been moved to the master venue + for key,event in self.events.iteritems(): + updatedEvent = models.Event.objects.get(pk=event.pk) + self.assertEqual(updatedEvent.venue, self.venues[1]) + + def test_person_merge(self): + change_url = reverse('admin:RIGS_person_changelist') + + data = {'action': 'merge', + '_selected_action': [unicode(val.pk) for key,val in self.persons.iteritems()], + 'post':'yes', + 'master':self.persons[1].pk + } + + response = self.client.post(change_url, data, follow=True) + self.assertContains(response, "Objects successfully merged") + self.assertContains(response, self.persons[1].name) + + # Check the master copy still exists + self.assertTrue(models.Person.objects.get(pk=self.persons[1].pk)) + + # Check the un-needed people have been disposed of + self.assertRaises(ObjectDoesNotExist, models.Person.objects.get, pk=self.persons[2].pk) + self.assertRaises(ObjectDoesNotExist, models.Person.objects.get, pk=self.persons[3].pk) + + # Check the events have been moved to the master person + for key,event in self.events.iteritems(): + updatedEvent = models.Event.objects.get(pk=event.pk) + self.assertEqual(updatedEvent.person, self.persons[1]) + + def test_organisation_merge(self): + change_url = reverse('admin:RIGS_organisation_changelist') + + data = {'action': 'merge', + '_selected_action': [unicode(val.pk) for key,val in self.organisations.iteritems()], + 'post':'yes', + 'master':self.organisations[1].pk + } + + response = self.client.post(change_url, data, follow=True) + self.assertContains(response, "Objects successfully merged") + self.assertContains(response, self.organisations[1].name) + + # Check the master copy still exists + self.assertTrue(models.Organisation.objects.get(pk=self.organisations[1].pk)) + + # Check the un-needed organisations have been disposed of + self.assertRaises(ObjectDoesNotExist, models.Organisation.objects.get, pk=self.organisations[2].pk) + self.assertRaises(ObjectDoesNotExist, models.Organisation.objects.get, pk=self.organisations[3].pk) + + # Check the events have been moved to the master organisation + for key,event in self.events.iteritems(): + updatedEvent = models.Event.objects.get(pk=event.pk) + self.assertEqual(updatedEvent.organisation, self.organisations[1]) \ No newline at end of file From ebe08c3bf1dc7b3dd9411c0de691bee9a91468eb Mon Sep 17 00:00:00 2001 From: Tom Price Date: Wed, 6 Apr 2016 21:52:25 +0100 Subject: [PATCH 7/9] Update tests to check items that aren't selected aren't affected. PEP8 format the file --- RIGS/test_unit.py | 226 +++++++++++++++++++++++++--------------------- 1 file changed, 122 insertions(+), 104 deletions(-) diff --git a/RIGS/test_unit.py b/RIGS/test_unit.py index b6fc8edd..75fcca5a 100644 --- a/RIGS/test_unit.py +++ b/RIGS/test_unit.py @@ -5,135 +5,153 @@ from datetime import date from RIGS import models from django.core.exceptions import ObjectDoesNotExist + class TestAdminMergeObjects(TestCase): - @classmethod - def setUpTestData(cls): - cls.profile = models.Profile.objects.create(username="testuser1", email="1@test.com", is_superuser=True, is_active=True, is_staff=True) + @classmethod + def setUpTestData(cls): + cls.profile = models.Profile.objects.create(username="testuser1", email="1@test.com", is_superuser=True, + is_active=True, is_staff=True) - cls.persons = { - 1: models.Person.objects.create(name="Person 1"), - 2: models.Person.objects.create(name="Person 2"), - 3: models.Person.objects.create(name="Person 3"), - } + cls.persons = { + 1: models.Person.objects.create(name="Person 1"), + 2: models.Person.objects.create(name="Person 2"), + 3: models.Person.objects.create(name="Person 3"), + } - cls.organisations = { - 1: models.Organisation.objects.create(name="Organisation 1"), - 2: models.Organisation.objects.create(name="Organisation 2"), - 3: models.Organisation.objects.create(name="Organisation 3"), - } + cls.organisations = { + 1: models.Organisation.objects.create(name="Organisation 1"), + 2: models.Organisation.objects.create(name="Organisation 2"), + 3: models.Organisation.objects.create(name="Organisation 3"), + } - cls.venues = { - 1: models.Venue.objects.create(name="Venue 1"), - 2: models.Venue.objects.create(name="Venue 2"), - 3: models.Venue.objects.create(name="Venue 3"), - } + cls.venues = { + 1: models.Venue.objects.create(name="Venue 1"), + 2: models.Venue.objects.create(name="Venue 2"), + 3: models.Venue.objects.create(name="Venue 3"), + } - cls.events = { - 1: models.Event.objects.create(name="TE E1", start_date=date.today(), person=cls.persons[1], organisation=cls.organisations[3], venue=cls.venues[2]), - 2: models.Event.objects.create(name="TE E2", start_date=date.today(), person=cls.persons[2], organisation=cls.organisations[2], venue=cls.venues[3]), - 3: models.Event.objects.create(name="TE E3", start_date=date.today(), person=cls.persons[3], organisation=cls.organisations[1], venue=cls.venues[1]), - 4: models.Event.objects.create(name="TE E4", start_date=date.today(), person=cls.persons[3], organisation=cls.organisations[3], venue=cls.venues[3]), - } + cls.events = { + 1: models.Event.objects.create(name="TE E1", start_date=date.today(), person=cls.persons[1], + organisation=cls.organisations[3], venue=cls.venues[2]), + 2: models.Event.objects.create(name="TE E2", start_date=date.today(), person=cls.persons[2], + organisation=cls.organisations[2], venue=cls.venues[3]), + 3: models.Event.objects.create(name="TE E3", start_date=date.today(), person=cls.persons[3], + organisation=cls.organisations[1], venue=cls.venues[1]), + 4: models.Event.objects.create(name="TE E4", start_date=date.today(), person=cls.persons[3], + organisation=cls.organisations[3], venue=cls.venues[3]), + } - def setUp(self): - self.profile.set_password('testuser') - self.profile.save() - self.assertTrue(self.client.login(username=self.profile.username, password='testuser')) + def setUp(self): + self.profile.set_password('testuser') + self.profile.save() + self.assertTrue(self.client.login(username=self.profile.username, password='testuser')) - def test_merge_confirmation(self): - change_url = reverse('admin:RIGS_venue_changelist') - data = { - 'action': 'merge', - '_selected_action': [unicode(val.pk) for key,val in self.venues.iteritems()] + def test_merge_confirmation(self): + change_url = reverse('admin:RIGS_venue_changelist') + data = { + 'action': 'merge', + '_selected_action': [unicode(val.pk) for key, val in self.venues.iteritems()] - } - response = self.client.post(change_url, data, follow=True) + } + response = self.client.post(change_url, data, follow=True) - self.assertContains(response, "The following objects will be merged") - for key,venue in self.venues.iteritems(): - self.assertContains(response, venue.name) + self.assertContains(response, "The following objects will be merged") + for key, venue in self.venues.iteritems(): + self.assertContains(response, venue.name) - def test_merge_no_master(self): - change_url = reverse('admin:RIGS_venue_changelist') - data = {'action': 'merge', - '_selected_action': [unicode(val.pk) for key,val in self.venues.iteritems()], - 'post':'yes', - } - response = self.client.post(change_url, data, follow=True) + def test_merge_no_master(self): + change_url = reverse('admin:RIGS_venue_changelist') + data = {'action': 'merge', + '_selected_action': [unicode(val.pk) for key, val in self.venues.iteritems()], + 'post': 'yes', + } + response = self.client.post(change_url, data, follow=True) - self.assertContains(response, "An error occured") - - def test_venue_merge(self): - change_url = reverse('admin:RIGS_venue_changelist') + self.assertContains(response, "An error occured") - data = {'action': 'merge', - '_selected_action': [unicode(val.pk) for key,val in self.venues.iteritems()], - 'post':'yes', - 'master':self.venues[1].pk - } + def test_venue_merge(self): + change_url = reverse('admin:RIGS_venue_changelist') - response = self.client.post(change_url, data, follow=True) - self.assertContains(response, "Objects successfully merged") - self.assertContains(response, self.venues[1].name) + data = {'action': 'merge', + '_selected_action': [unicode(self.venues[1].pk), unicode(self.venues[2].pk)], + 'post': 'yes', + 'master': self.venues[1].pk + } - # Check the master copy still exists - self.assertTrue(models.Venue.objects.get(pk=self.venues[1].pk)) + response = self.client.post(change_url, data, follow=True) + self.assertContains(response, "Objects successfully merged") + self.assertContains(response, self.venues[1].name) - # Check the un-needed venues have been disposed of - self.assertRaises(ObjectDoesNotExist, models.Venue.objects.get, pk=self.venues[2].pk) - self.assertRaises(ObjectDoesNotExist, models.Venue.objects.get, pk=self.venues[3].pk) + # Check the master copy still exists + self.assertTrue(models.Venue.objects.get(pk=self.venues[1].pk)) - # Check the events have been moved to the master venue - for key,event in self.events.iteritems(): - updatedEvent = models.Event.objects.get(pk=event.pk) - self.assertEqual(updatedEvent.venue, self.venues[1]) + # Check the un-needed venue has been disposed of + self.assertRaises(ObjectDoesNotExist, models.Venue.objects.get, pk=self.venues[2].pk) - def test_person_merge(self): - change_url = reverse('admin:RIGS_person_changelist') + # Check the one we didn't delete is still there + self.assertEqual(models.Venue.objects.get(pk=self.venues[3].pk), self.venues[3]) - data = {'action': 'merge', - '_selected_action': [unicode(val.pk) for key,val in self.persons.iteritems()], - 'post':'yes', - 'master':self.persons[1].pk - } + # Check the events have been moved to the master venue + for key, event in self.events.iteritems(): + updatedEvent = models.Event.objects.get(pk=event.pk) + if event.venue == self.venues[3]: # The one we left in place + continue + self.assertEqual(updatedEvent.venue, self.venues[1]) - response = self.client.post(change_url, data, follow=True) - self.assertContains(response, "Objects successfully merged") - self.assertContains(response, self.persons[1].name) + def test_person_merge(self): + change_url = reverse('admin:RIGS_person_changelist') - # Check the master copy still exists - self.assertTrue(models.Person.objects.get(pk=self.persons[1].pk)) + data = {'action': 'merge', + '_selected_action': [unicode(self.persons[1].pk), unicode(self.persons[2].pk)], + 'post': 'yes', + 'master': self.persons[1].pk + } - # Check the un-needed people have been disposed of - self.assertRaises(ObjectDoesNotExist, models.Person.objects.get, pk=self.persons[2].pk) - self.assertRaises(ObjectDoesNotExist, models.Person.objects.get, pk=self.persons[3].pk) + response = self.client.post(change_url, data, follow=True) + self.assertContains(response, "Objects successfully merged") + self.assertContains(response, self.persons[1].name) - # Check the events have been moved to the master person - for key,event in self.events.iteritems(): - updatedEvent = models.Event.objects.get(pk=event.pk) - self.assertEqual(updatedEvent.person, self.persons[1]) + # Check the master copy still exists + self.assertTrue(models.Person.objects.get(pk=self.persons[1].pk)) - def test_organisation_merge(self): - change_url = reverse('admin:RIGS_organisation_changelist') + # Check the un-needed people have been disposed of + self.assertRaises(ObjectDoesNotExist, models.Person.objects.get, pk=self.persons[2].pk) - data = {'action': 'merge', - '_selected_action': [unicode(val.pk) for key,val in self.organisations.iteritems()], - 'post':'yes', - 'master':self.organisations[1].pk - } + # Check the one we didn't delete is still there + self.assertEqual(models.Person.objects.get(pk=self.persons[3].pk), self.persons[3]) - response = self.client.post(change_url, data, follow=True) - self.assertContains(response, "Objects successfully merged") - self.assertContains(response, self.organisations[1].name) + # Check the events have been moved to the master person + for key, event in self.events.iteritems(): + updatedEvent = models.Event.objects.get(pk=event.pk) + if event.person == self.persons[3]: # The one we left in place + continue + self.assertEqual(updatedEvent.person, self.persons[1]) - # Check the master copy still exists - self.assertTrue(models.Organisation.objects.get(pk=self.organisations[1].pk)) + def test_organisation_merge(self): + change_url = reverse('admin:RIGS_organisation_changelist') - # Check the un-needed organisations have been disposed of - self.assertRaises(ObjectDoesNotExist, models.Organisation.objects.get, pk=self.organisations[2].pk) - self.assertRaises(ObjectDoesNotExist, models.Organisation.objects.get, pk=self.organisations[3].pk) + data = {'action': 'merge', + '_selected_action': [unicode(self.organisations[1].pk), unicode(self.organisations[2].pk)], + 'post': 'yes', + 'master': self.organisations[1].pk + } - # Check the events have been moved to the master organisation - for key,event in self.events.iteritems(): - updatedEvent = models.Event.objects.get(pk=event.pk) - self.assertEqual(updatedEvent.organisation, self.organisations[1]) \ No newline at end of file + response = self.client.post(change_url, data, follow=True) + self.assertContains(response, "Objects successfully merged") + self.assertContains(response, self.organisations[1].name) + + # Check the master copy still exists + self.assertTrue(models.Organisation.objects.get(pk=self.organisations[1].pk)) + + # Check the un-needed organisations have been disposed of + self.assertRaises(ObjectDoesNotExist, models.Organisation.objects.get, pk=self.organisations[2].pk) + + # Check the one we didn't delete is still there + self.assertEqual(models.Organisation.objects.get(pk=self.organisations[3].pk), self.organisations[3]) + + # Check the events have been moved to the master organisation + for key, event in self.events.iteritems(): + updatedEvent = models.Event.objects.get(pk=event.pk) + if event.organisation == self.organisations[3]: # The one we left in place + continue + self.assertEqual(updatedEvent.organisation, self.organisations[1]) From 823db68a6a534e6f445f456c4893ff83cefaf742 Mon Sep 17 00:00:00 2001 From: Tom Price Date: Wed, 6 Apr 2016 21:53:38 +0100 Subject: [PATCH 8/9] PEP8 format files --- RIGS/admin.py | 49 ++++++++++-------- RIGS/versioning.py | 122 ++++++++++++++++++++++++--------------------- 2 files changed, 92 insertions(+), 79 deletions(-) diff --git a/RIGS/admin.py b/RIGS/admin.py index 0dfafd7a..a351aed0 100644 --- a/RIGS/admin.py +++ b/RIGS/admin.py @@ -6,7 +6,7 @@ import reversion from django.contrib.admin import helpers from django.template.response import TemplateResponse -from django.contrib import messages +from django.contrib import messages from django.db import transaction from django.core.exceptions import ObjectDoesNotExist from django.db.models import Count @@ -19,16 +19,17 @@ admin.site.register(models.EventItem, reversion.VersionAdmin) admin.site.register(models.Invoice) admin.site.register(models.Payment) + @admin.register(models.Profile) class ProfileAdmin(UserAdmin): fieldsets = ( (None, {'fields': ('username', 'password')}), (_('Personal info'), { - 'fields': ('first_name', 'last_name', 'email', 'initials', 'phone')}), + 'fields': ('first_name', 'last_name', 'email', 'initials', 'phone')}), (_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser', 'groups', 'user_permissions')}), (_('Important dates'), { - 'fields': ('last_login', 'date_joined')}), + 'fields': ('last_login', 'date_joined')}), ) add_fieldsets = ( (None, { @@ -39,41 +40,43 @@ class ProfileAdmin(UserAdmin): form = forms.ProfileChangeForm add_form = forms.ProfileCreationForm + class AssociateAdmin(reversion.VersionAdmin): - list_display = ('id', 'name','number_of_events') - search_fields = ['id','name'] - list_display_links = ['id','name'] + list_display = ('id', 'name', 'number_of_events') + search_fields = ['id', 'name'] + list_display_links = ['id', 'name'] actions = ['merge'] merge_fields = ['name'] def get_queryset(self, request): - return super(AssociateAdmin, self).get_queryset(request).annotate(event_count = Count('event')) + return super(AssociateAdmin, self).get_queryset(request).annotate(event_count=Count('event')) - def number_of_events(self,obj): + def number_of_events(self, obj): return obj.latest_events.count() + number_of_events.admin_order_field = 'event_count' def merge(self, request, queryset): - if request.POST.get('post'): # Has the user confirmed which is the master record? + if request.POST.get('post'): # Has the user confirmed which is the master record? try: masterObjectPk = request.POST.get('master') - masterObject = queryset.get(pk = masterObjectPk) + masterObject = queryset.get(pk=masterObjectPk) except ObjectDoesNotExist: - self.message_user(request, "An error occured. Did you select a 'master' record?",level=messages.ERROR) + self.message_user(request, "An error occured. Did you select a 'master' record?", level=messages.ERROR) return with transaction.atomic(), reversion.create_revision(): - for obj in queryset.exclude(pk = masterObjectPk): + for obj in queryset.exclude(pk=masterObjectPk): events = obj.event_set.all() for event in events: masterObject.event_set.add(event) - obj.delete() + obj.delete() reversion.set_comment('Merging Objects') self.message_user(request, "Objects successfully merged.") return - else: # Present the confirmation screen + else: # Present the confirmation screen class TempForm(ModelForm): class Meta: @@ -90,19 +93,23 @@ class AssociateAdmin(reversion.VersionAdmin): 'action_checkbox_name': helpers.ACTION_CHECKBOX_NAME, 'forms': forms } - return TemplateResponse(request, 'RIGS/admin_associate_merge.html', context, current_app=self.admin_site.name) + return TemplateResponse(request, 'RIGS/admin_associate_merge.html', context, + current_app=self.admin_site.name) + @admin.register(models.Person) class PersonAdmin(AssociateAdmin): - list_display = ('id', 'name','phone','email','number_of_events') - merge_fields = ['name','phone','email','address','notes'] + list_display = ('id', 'name', 'phone', 'email', 'number_of_events') + merge_fields = ['name', 'phone', 'email', 'address', 'notes'] + @admin.register(models.Venue) class VenueAdmin(AssociateAdmin): - list_display = ('id', 'name','phone','email','number_of_events') - merge_fields = ['name','phone','email','address','notes','three_phase_available'] + list_display = ('id', 'name', 'phone', 'email', 'number_of_events') + merge_fields = ['name', 'phone', 'email', 'address', 'notes', 'three_phase_available'] + @admin.register(models.Organisation) class OrganisationAdmin(AssociateAdmin): - list_display = ('id', 'name','phone','email','number_of_events') - merge_fields = ['name','phone','email','address','notes','union_account'] \ No newline at end of file + list_display = ('id', 'name', 'phone', 'email', 'number_of_events') + merge_fields = ['name', 'phone', 'email', 'address', 'notes', 'union_account'] diff --git a/RIGS/versioning.py b/RIGS/versioning.py index c537de87..36786a74 100644 --- a/RIGS/versioning.py +++ b/RIGS/versioning.py @@ -14,7 +14,7 @@ from django.core.exceptions import ObjectDoesNotExist import reversion import simplejson from reversion.models import Version -from django.contrib.contenttypes.models import ContentType # Used to lookup the content_type +from django.contrib.contenttypes.models import ContentType # Used to lookup the content_type from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.db.models import ForeignKey, IntegerField, EmailField, TextField from diff_match_patch import diff_match_patch @@ -29,11 +29,10 @@ logger = logging.getLogger('tec.pyrigs') def model_compare(oldObj, newObj, excluded_keys=[]): # recieves two objects of the same model, and compares them. Returns an array of FieldCompare objects try: - theFields = oldObj._meta.fields #This becomes deprecated in Django 1.8!!!!!!!!!!!!! (but an alternative becomes available) + theFields = oldObj._meta.fields # This becomes deprecated in Django 1.8!!!!!!!!!!!!! (but an alternative becomes available) except AttributeError: theFields = newObj._meta.fields - class FieldCompare(object): def __init__(self, field=None, old=None, new=None): self.field = field @@ -51,13 +50,13 @@ def model_compare(oldObj, newObj, excluded_keys=[]): @property def new(self): - return self.display_value(self._new) + return self.display_value(self._new) @property def long(self): if isinstance(self.field, EmailField): return True - return False + return False @property def linebreaks(self): @@ -76,21 +75,21 @@ def model_compare(oldObj, newObj, excluded_keys=[]): outputDiffs = [] for (op, data) in diffs: - if op == dmp.DIFF_INSERT: - outputDiffs.append({'type':'insert', 'text':data}) - elif op == dmp.DIFF_DELETE: - outputDiffs.append({'type':'delete', 'text':data}) - elif op == dmp.DIFF_EQUAL: - outputDiffs.append({'type':'equal', 'text':data}) + if op == dmp.DIFF_INSERT: + outputDiffs.append({'type': 'insert', 'text': data}) + elif op == dmp.DIFF_DELETE: + outputDiffs.append({'type': 'delete', 'text': data}) + elif op == dmp.DIFF_EQUAL: + outputDiffs.append({'type': 'equal', 'text': data}) return outputDiffs changes = [] for thisField in theFields: name = thisField.name - + if name in excluded_keys: - continue # if we're excluding this field, skip over it + continue # if we're excluding this field, skip over it try: oldValue = getattr(oldObj, name, None) @@ -101,17 +100,18 @@ def model_compare(oldObj, newObj, excluded_keys=[]): newValue = getattr(newObj, name, None) except ObjectDoesNotExist: newValue = None - + try: bothBlank = (not oldValue) and (not newValue) if oldValue != newValue and not bothBlank: - compare = FieldCompare(thisField,oldValue,newValue) + compare = FieldCompare(thisField, oldValue, newValue) changes.append(compare) - except TypeError: # logs issues with naive vs tz-aware datetimes + except TypeError: # logs issues with naive vs tz-aware datetimes logger.error('TypeError when comparing models') - + return changes + def compare_event_items(old, new): # Recieves two event version objects and compares their items, returns an array of ItemCompare objects @@ -126,39 +126,41 @@ def compare_event_items(old, new): self.changes = changes # Build some dicts of what we have - item_dict = {} # build a list of items, key is the item_pk - for version in old_item_versions: # put all the old versions in a list + item_dict = {} # build a list of items, key is the item_pk + for version in old_item_versions: # put all the old versions in a list compare = ItemCompare(old=version.object_version.object) item_dict[version.object_id] = compare - for version in new_item_versions: # go through the new versions - try: - compare = item_dict[version.object_id] # see if there's a matching old version - compare.new = version.object_version.object # then add the new version to the dictionary - except KeyError: # there's no matching old version, so add this item to the dictionary by itself + for version in new_item_versions: # go through the new versions + try: + compare = item_dict[version.object_id] # see if there's a matching old version + compare.new = version.object_version.object # then add the new version to the dictionary + except KeyError: # there's no matching old version, so add this item to the dictionary by itself compare = ItemCompare(new=version.object_version.object) - - item_dict[version.object_id] = compare # update the dictionary with the changes - changes = [] + item_dict[version.object_id] = compare # update the dictionary with the changes + + changes = [] for (_, compare) in item_dict.items(): - compare.changes = model_compare(compare.old, compare.new, ['id','event','order']) # see what's changed + compare.changes = model_compare(compare.old, compare.new, ['id', 'event', 'order']) # see what's changed if len(compare.changes) >= 1: - changes.append(compare) # transfer into a sequential array to make it easier to deal with later + changes.append(compare) # transfer into a sequential array to make it easier to deal with later return changes + def get_versions_for_model(models): content_types = [] for model in models: content_types.append(ContentType.objects.get_for_model(model)) - + versions = reversion.models.Version.objects.filter( - content_type__in = content_types, + content_type__in=content_types, ).select_related("revision").order_by("-pk") return versions + def get_previous_version(version): thisId = version.object_id thisVersionId = version.pk @@ -166,17 +168,19 @@ def get_previous_version(version): versions = reversion.get_for_object_reference(version.content_type.model_class(), thisId) try: - previousVersions = versions.filter(revision_id__lt=version.revision_id).latest(field_name='revision__date_created') + previousVersions = versions.filter(revision_id__lt=version.revision_id).latest( + field_name='revision__date_created') except ObjectDoesNotExist: return False return previousVersions -def get_changes_for_version(newVersion, oldVersion=None): - #Pass in a previous version if you already know it (for efficiancy) - #if not provided then it will be looked up in the database - if oldVersion == None: +def get_changes_for_version(newVersion, oldVersion=None): + # Pass in a previous version if you already know it (for efficiancy) + # if not provided then it will be looked up in the database + + if oldVersion == None: oldVersion = get_previous_version(newVersion) modelClass = newVersion.content_type.model_class() @@ -200,6 +204,7 @@ def get_changes_for_version(newVersion, oldVersion=None): return compare + class VersionHistory(generic.ListView): model = reversion.revisions.Version template_name = "RIGS/version_history.html" @@ -215,7 +220,7 @@ class VersionHistory(generic.ListView): def get_context_data(self, **kwargs): thisModel = self.kwargs['model'] - + context = super(VersionHistory, self).get_context_data(**kwargs) versions = context['object_list'] @@ -224,81 +229,82 @@ class VersionHistory(generic.ListView): items = [] for versionNo, thisVersion in enumerate(versions): - if versionNo >= len(versions)-1: + if versionNo >= len(versions) - 1: thisItem = get_changes_for_version(thisVersion, None) else: - thisItem = get_changes_for_version(thisVersion, versions[versionNo+1]) - + thisItem = get_changes_for_version(thisVersion, versions[versionNo + 1]) + items.append(thisItem) context['object_list'] = items context['object'] = thisObject - + return context + class ActivityTable(generic.ListView): model = reversion.revisions.Version template_name = "RIGS/activity_table.html" paginate_by = 25 - + def get_queryset(self): - versions = get_versions_for_model([models.Event,models.Venue,models.Person,models.Organisation]) + versions = get_versions_for_model([models.Event, models.Venue, models.Person, models.Organisation]) return versions def get_context_data(self, **kwargs): - # Call the base implementation first to get a context context = super(ActivityTable, self).get_context_data(**kwargs) - + items = [] for thisVersion in context['object_list']: thisItem = get_changes_for_version(thisVersion, None) items.append(thisItem) - context ['object_list'] = items - + context['object_list'] = items + return context + class ActivityFeed(generic.ListView): model = reversion.revisions.Version template_name = "RIGS/activity_feed_data.html" paginate_by = 25 - + def get_queryset(self): - versions = get_versions_for_model([models.Event,models.Venue,models.Person,models.Organisation]) + versions = get_versions_for_model([models.Event, models.Venue, models.Person, models.Organisation]) return versions def get_context_data(self, **kwargs): maxTimeDelta = [] - maxTimeDelta.append({ 'maxAge':datetime.timedelta(days=1), 'group':datetime.timedelta(hours=1)}) - maxTimeDelta.append({ 'maxAge':None, 'group':datetime.timedelta(days=1)}) + maxTimeDelta.append({'maxAge': datetime.timedelta(days=1), 'group': datetime.timedelta(hours=1)}) + maxTimeDelta.append({'maxAge': None, 'group': datetime.timedelta(days=1)}) # Call the base implementation first to get a context context = super(ActivityFeed, self).get_context_data(**kwargs) - + items = [] for thisVersion in context['object_list']: thisItem = get_changes_for_version(thisVersion, None) if thisItem['item_changes'] or thisItem['field_changes'] or thisItem['old'] == None: thisItem['withPrevious'] = False - if len(items)>=1: - timeAgo = datetime.datetime.now(thisItem['revision'].date_created.tzinfo) - thisItem['revision'].date_created + if len(items) >= 1: + timeAgo = datetime.datetime.now(thisItem['revision'].date_created.tzinfo) - thisItem[ + 'revision'].date_created timeDiff = items[-1]['revision'].date_created - thisItem['revision'].date_created timeTogether = False for params in maxTimeDelta: if params['maxAge'] is None or timeAgo <= params['maxAge']: timeTogether = timeDiff < params['group'] break - + sameUser = thisItem['revision'].user == items[-1]['revision'].user thisItem['withPrevious'] = timeTogether & sameUser items.append(thisItem) - context ['object_list'] = items - + context['object_list'] = items - return context \ No newline at end of file + return context From df61225b73f31dce3f1b17ea35a7cf6f1b9e32ea Mon Sep 17 00:00:00 2001 From: Tom Price Date: Wed, 6 Apr 2016 21:57:25 +0100 Subject: [PATCH 9/9] Optimise imports So many unused imports, these have been banished --- RIGS/versioning.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/RIGS/versioning.py b/RIGS/versioning.py index 36786a74..7677fa13 100644 --- a/RIGS/versioning.py +++ b/RIGS/versioning.py @@ -1,27 +1,18 @@ import logging -from django.views import generic -from django.core.urlresolvers import reverse_lazy -from django.shortcuts import get_object_or_404 -from django.template import RequestContext -from django.template.loader import get_template -from django.conf import settings -from django.http import HttpResponse -from django.db.models import Q -from django.contrib import messages + from django.core.exceptions import ObjectDoesNotExist +from django.shortcuts import get_object_or_404 +from django.views import generic # Versioning import reversion -import simplejson from reversion.models import Version from django.contrib.contenttypes.models import ContentType # Used to lookup the content_type -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger -from django.db.models import ForeignKey, IntegerField, EmailField, TextField +from django.db.models import IntegerField, EmailField, TextField from diff_match_patch import diff_match_patch -from RIGS import models, forms +from RIGS import models import datetime -import re logger = logging.getLogger('tec.pyrigs')