From 9e9840b24815c238cd52a079f564a1c35e63d3ea Mon Sep 17 00:00:00 2001 From: Tanner Prestegard <tanner.prestegard@ligo.org> Date: Fri, 25 May 2018 14:19:27 -0500 Subject: [PATCH] preferred_event is now required for all superevents and cannot be None --- gracedb/superevents/api/serializers.py | 8 +------ .../0005_preferred_event_required.py | 21 ++++++++++++++++ gracedb/superevents/models.py | 8 +++---- gracedb/superevents/tests/test_forms.py | 13 +--------- gracedb/superevents/utils.py | 24 +++++++++---------- gracedb/templates/superevents/view.html | 2 -- 6 files changed, 37 insertions(+), 39 deletions(-) create mode 100644 gracedb/superevents/migrations/0005_preferred_event_required.py diff --git a/gracedb/superevents/api/serializers.py b/gracedb/superevents/api/serializers.py index 8b9fd432b..36ef1f45d 100644 --- a/gracedb/superevents/api/serializers.py +++ b/gracedb/superevents/api/serializers.py @@ -25,8 +25,6 @@ logger = logging.getLogger(__name__) class SupereventSerializer(serializers.ModelSerializer): # Error messages default_error_messages = { - 'event_missing': _('Either "preferred_event" or "events" must be ' - 'specified for Superevent creation'), 'event_assigned': _('Event {graceid} is already assigned to a ' 'Superevent'), } @@ -34,7 +32,7 @@ class SupereventSerializer(serializers.ModelSerializer): # Fields submitter = serializers.SlugRelatedField(slug_field='username', read_only=True) - preferred_event = EventGraceidField(required=False) + preferred_event = EventGraceidField(required=True) created = serializers.DateTimeField(source='date_created', format=settings.GRACE_STRFTIME_FORMAT, read_only=True) # Add custom fields @@ -60,10 +58,6 @@ class SupereventSerializer(serializers.ModelSerializer): preferred_event = data.get('preferred_event') events = data.get('events') - # Require either events or preferred_event to be set (creation only) - if not self.instance and not (preferred_event or events): - self.fail('event_missing') - # Make sure preferred_event is not already assigned if preferred_event: if (preferred_event.superevent or hasattr(preferred_event, diff --git a/gracedb/superevents/migrations/0005_preferred_event_required.py b/gracedb/superevents/migrations/0005_preferred_event_required.py new file mode 100644 index 000000000..cfd2a2a4c --- /dev/null +++ b/gracedb/superevents/migrations/0005_preferred_event_required.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.5 on 2018-05-25 19:19 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('superevents', '0004_add_fields_for_superevent_date_ids'), + ] + + operations = [ + migrations.AlterField( + model_name='superevent', + name='preferred_event', + field=models.OneToOneField(on_delete=django.db.models.deletion.PROTECT, related_name='superevent_preferred_for', to='events.Event'), + ), + ] diff --git a/gracedb/superevents/models.py b/gracedb/superevents/models.py index 45d077ecd..435843eb7 100644 --- a/gracedb/superevents/models.py +++ b/gracedb/superevents/models.py @@ -56,11 +56,9 @@ class Superevent(CleanSaveModel, ModelToDictMixin, AutoIncrementModel): # One-to-one relationship with preferred event - an event can only be # preferred for a single superevent and a superevent can only have - # one preferred event. Ideally, this wouldn't be nullable, but it makes - # the logic a lot easier to handle. We will just have to check whether - # self.preferred_event is None in some cases. - preferred_event = models.OneToOneField(Event, null=True, blank=True, - on_delete=models.SET_NULL, related_name='superevent_preferred_for') + # one preferred event. + preferred_event = models.OneToOneField(Event, null=False, + on_delete=models.PROTECT, related_name='superevent_preferred_for') # Labels labels = models.ManyToManyField('events.label', through='Labelling') diff --git a/gracedb/superevents/tests/test_forms.py b/gracedb/superevents/tests/test_forms.py index 659480767..2838d5219 100644 --- a/gracedb/superevents/tests/test_forms.py +++ b/gracedb/superevents/tests/test_forms.py @@ -113,18 +113,7 @@ class TestSupereventForm(TestCase): form = SupereventForm(data_dict) # Form is valid? - self.assertTrue(form.is_valid()) - - # Make sure submitter field is set correctly - self.assertEqual(submitter, form.cleaned_data['submitter']) - - # preferred_event should be None - self.assertTrue(form.cleaned_data['preferred_event'] is None) - - # Make sure events are set correctly - self.assertEqual(events.count(), form.cleaned_data['events'].count()) - for ev in events: - self.assertIn(ev, form.cleaned_data['events']) + self.assertFalse(form.is_valid()) def test_form_no_preferred_event_no_events(self): """Test form with no preferred_event or events""" diff --git a/gracedb/superevents/utils.py b/gracedb/superevents/utils.py index dad9c1603..2e02dd16e 100644 --- a/gracedb/superevents/utils.py +++ b/gracedb/superevents/utils.py @@ -12,24 +12,23 @@ logger = logging.getLogger(__name__) # TODO: # Add decorator to check access permissions (??) not sure if we should do it here or in the viewset itself -def create_superevent(submitter, t_start, t_0, t_end, preferred_event=None, +def create_superevent(submitter, t_start, t_0, t_end, preferred_event, events=[], labels=[], add_log_message=True, issue_alert=True): """ - Utility method for creating superevents. Requires at least one event to be - specified. Can be a preferred_event, an event or list of events, or both. + Utility method for creating superevents. Arguments: - submitter: User object - t_start, t_0, t_end: floats - preferred_event: Event object + submitter: User object (required) + t_start, t_0, t_end: floats (required) + preferred_event: Event object (required) events: list or QuerySet of Event objects labels: list of QuerySet of Label objects Usage: create_superevent( UserModel.objects.get(username='albert.einstein@ligo.org'), - t_start=1, t_0=2, t_end=3, - events=[Event.objects.getByGraceid('G123400')] + t_start=1, t_0=2, t_end=3, preferred_event= + Event.objects.get(id='123400') ) """ @@ -121,11 +120,10 @@ def update_superevent(superevent, updater, issue_alert=True, **kwargs): if new_params.has_key('preferred_event') and \ (old_params['preferred_event'] != new_params['preferred_event']): # Old preferred event - if old_params['preferred_event'] is not None: - old_msg = ("Removed as preferred event for superevent: " - "{superevent_id}").format(superevent_id=superevent.superevent_id) - old_log = create_log(updater, old_msg, old_params['preferred_event'], - issue_alert=True) + old_msg = ("Removed as preferred event for superevent: " + "{superevent_id}").format(superevent_id=superevent.superevent_id) + old_log = create_log(updater, old_msg, old_params['preferred_event'], + issue_alert=True) # New preferred event new_msg = "Set as preferred event for superevent: {superevent_id}" \ diff --git a/gracedb/templates/superevents/view.html b/gracedb/templates/superevents/view.html index bd0099376..1a01ccf88 100644 --- a/gracedb/templates/superevents/view.html +++ b/gracedb/templates/superevents/view.html @@ -65,7 +65,6 @@ TBD: <br /> <br /> -{% if preferred_event %} {% block basic_info %} <h2>Preferred Event Info</h2> @@ -112,7 +111,6 @@ TBD: </tr> </table> {% endblock %} -{% endif %} </div> -- GitLab