From 14592f7906368a8f01cd93b39807e428631c3339 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Wed, 15 Feb 2023 12:43:19 -0500 Subject: [PATCH 01/14] Created a New Model to Support Event Agreements What is Event Agreements? -->Event Agreements are like DUAs but specific to Event. Idea behind creating a new model: My understanding is that each event will be assigned a Event Agreements and the event host can choose which Event Agreements to link to event. One assumption i made was we dont want two different event editors to be able to edit each others Event Agreements(So i tied each Event Agreements to the Event Agreements creator) So the reason i decided to create a new model instead of using the existing DUA are 1. To keep things simple 2. The existing DUAs are tied to project type like `database`,`software` etc. So to make them work for events, we would have to come up with weird hack. So i thought using a new model will be cleaner and simpler. --- physionet-django/events/models.py | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/physionet-django/events/models.py b/physionet-django/events/models.py index 2140fc901b..985c6433cb 100644 --- a/physionet-django/events/models.py +++ b/physionet-django/events/models.py @@ -6,6 +6,8 @@ from django.contrib.auth.models import Permission from events.enums import EventCategory from events import validators +from project.modelcomponents.fields import SafeHTMLField +from project.validators import validate_version class Event(models.Model): @@ -155,6 +157,37 @@ def withdraw(self, comment_to_applicant=None): self._apply_decision(self.EventApplicationStatus.WITHDRAWN, comment_to_applicant) +class EventAgreement(models.Model): + name = models.CharField(max_length=100) + slug = models.SlugField(max_length=120, unique=True) + version = models.CharField(max_length=15, default='', validators=[validate_version]) + is_active = models.BooleanField(default=True) + html_content = SafeHTMLField(default='') + access_template = SafeHTMLField(default='') + creator = models.ForeignKey("user.User", on_delete=models.CASCADE) + + class Meta: + default_permissions = ('add',) + unique_together = (('name', 'version', 'creator'),) + + def __str__(self): + return self.name + + +class EventAgreementSignature(models.Model): + """ + Log of user signing EventAgreement + """ + event = models.ForeignKey('events.Event', on_delete=models.CASCADE) + event_agreement = models.ForeignKey('events.EventAgreement', on_delete=models.CASCADE) + user = models.ForeignKey('user.User', on_delete=models.CASCADE, + related_name='event_agreement_signatures') + sign_datetime = models.DateTimeField(auto_now_add=True) + + class Meta: + default_permissions = () + + class EventDataset(models.Model): """ Captures information about datasets for events. From 8b292f814edb705e9c6f62035f058ba2b4b63848 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Wed, 15 Feb 2023 12:46:08 -0500 Subject: [PATCH 02/14] Provide the Event Host access to Admin console Because historically we have been using Admin Console for stuff like creating DUA, license and the Event Agreement is kind of same, it makes sense to put the code behind admin console. So for that the first step is to provide the `Event Host` access to Admin console. This should not pose any security risk as all the other views or links in templates are secured by different permission check. So after this change, the `Event Host` should be able to admin console and only see the `Event Participant stuff`(The code for that is coming after this commit) --- physionet-django/user/fixtures/demo-user.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/physionet-django/user/fixtures/demo-user.json b/physionet-django/user/fixtures/demo-user.json index c4293d0190..e2688ac689 100644 --- a/physionet-django/user/fixtures/demo-user.json +++ b/physionet-django/user/fixtures/demo-user.json @@ -14553,6 +14553,11 @@ "view_event_menu", "events", "event" + ], + [ + "can_view_admin_console", + "user", + "user" ] ] } From 8049364ee357ef666ff0a97beaf414a263635d89 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Wed, 15 Feb 2023 12:46:57 -0500 Subject: [PATCH 03/14] Create an EventAgreement form --- physionet-django/events/forms.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/physionet-django/events/forms.py b/physionet-django/events/forms.py index b675183de1..30819f01a4 100644 --- a/physionet-django/events/forms.py +++ b/physionet-django/events/forms.py @@ -1,10 +1,9 @@ from django import forms from events.widgets import DatePickerInput -from events.models import Event, EventApplication, EventDataset +from events.models import Event, EventApplication, EventAgreement, EventDataset from project.models import PublishedProject - class AddEventForm(forms.ModelForm): """ A form for adding events. @@ -73,3 +72,18 @@ class EventDatasetForm(forms.ModelForm): class Meta: model = EventDataset fields = ('dataset', 'access_type') + + +class EventAgreementForm(forms.ModelForm): + class Meta: + model = EventAgreement + fields = ( + 'name', + 'version', + 'slug', + 'is_active', + 'html_content', + 'access_template', + ) + labels = {'html_content': 'Content'} + From b0a4b90f5bacbe7531c9eef377726ea6266e78cb Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Wed, 15 Feb 2023 12:57:39 -0500 Subject: [PATCH 04/14] CRUD for Event Agreement Just like the DUAs, here i copied code from DUA CRUD stuff and refactored it to work for Event Agreement. Although this looks intimidating as we have quite a large number of lines of code, its just refactored code, and i figured breaking down the changes to each step of CRUD would take more time than refactoring, testing. So to speed things up, i made a giant commit. --- .../templates/console/console_navbar.html | 9 +- .../console/event_agreement_detail.html | 23 +++++ .../console/event_agreement_list.html | 93 ++++++++++++++++++ .../console/event_agreement_new_version.html | 23 +++++ physionet-django/console/urls.py | 7 +- physionet-django/console/views.py | 97 ++++++++++++++++++- 6 files changed, 247 insertions(+), 5 deletions(-) create mode 100644 physionet-django/console/templates/console/event_agreement_detail.html create mode 100644 physionet-django/console/templates/console/event_agreement_list.html create mode 100644 physionet-django/console/templates/console/event_agreement_new_version.html diff --git a/physionet-django/console/templates/console/console_navbar.html b/physionet-django/console/templates/console/console_navbar.html index 29248def66..6026a0df83 100644 --- a/physionet-django/console/templates/console/console_navbar.html +++ b/physionet-django/console/templates/console/console_navbar.html @@ -136,7 +136,7 @@ {% endif %} - {% if perms.project.add_dua or perms.project.add_codeofconduct or perms.project.add_license %} + {% if perms.project.add_dua or perms.project.add_codeofconduct or perms.project.add_license or perms.events.add_event %}
  • {% if license_nav or dua_nav or code_of_conduct_nav %} @@ -147,7 +147,7 @@ Legal - {% if license_nav or dua_nav or code_of_conduct_nav %} + {% if license_nav or dua_nav or code_of_conduct_nav or perms.events.add_event %}
      {% else %} {% endif %} diff --git a/physionet-django/console/templates/console/event_agreement_detail.html b/physionet-django/console/templates/console/event_agreement_detail.html new file mode 100644 index 0000000000..d514cedfa0 --- /dev/null +++ b/physionet-django/console/templates/console/event_agreement_detail.html @@ -0,0 +1,23 @@ +{% extends "console/base_console.html" %} + +{% load static %} + +{% block title %}{{ SITE_NAME }} - Event Agreements{% endblock %} + +{% block content %} +
      +
      + {{ event_agreement.name }} - edit +
      +
      +
      + {% include "inline_form_snippet.html" with form=event_agreement_form %} + +
      +
      +
      +{% endblock %} + +{% block local_js_bottom %} + +{% endblock %} diff --git a/physionet-django/console/templates/console/event_agreement_list.html b/physionet-django/console/templates/console/event_agreement_list.html new file mode 100644 index 0000000000..298c8c6910 --- /dev/null +++ b/physionet-django/console/templates/console/event_agreement_list.html @@ -0,0 +1,93 @@ +{% extends "console/base_console.html" %} + +{% load static %} + +{% block local_css %} + +{% endblock %} + +{% block title %}{{ SITE_NAME }} - Event Agreements{% endblock %} + +{% block content %} +
      +
      + Event Agreement {{ event_agreements|length }} +
      +
      + {% if event_agreements %} +
      + + + + + + + + + + {% for event_agreement in event_agreements %} + + + + + + {% endfor %} + +
      NameVersionActions
      + {{ event_agreement.name }} + {% if event_agreement.is_active %} + Active + {% else %} + Inactive + {% endif %} + {{ event_agreement.version }} + Edit + New version + + + +
      + {% include "console/pagination.html" with pagination=event_agreements %} +
      + {% else %} +

      There aren't any data Event agreements.

      + {% endif %} +
      +
      +
      +
      + New Event Agreement +
      +
      +
      + {% include "inline_form_snippet.html" with form=event_agreement_form %} + +
      +
      +
      +{% endblock %} + +{% block local_js_bottom %} + +{% endblock %} diff --git a/physionet-django/console/templates/console/event_agreement_new_version.html b/physionet-django/console/templates/console/event_agreement_new_version.html new file mode 100644 index 0000000000..3562dbc33d --- /dev/null +++ b/physionet-django/console/templates/console/event_agreement_new_version.html @@ -0,0 +1,23 @@ +{% extends "console/base_console.html" %} + +{% load static %} + +{% block title %}{{ SITE_NAME }} - Event Agreements{% endblock %} + +{% block content %} +
      +
      + {{ event_agreement.name }} - new version +
      +
      +
      + {% include "inline_form_snippet.html" with form=event_agreement_form %} + +
      +
      +
      +{% endblock %} + +{% block local_js_bottom %} + +{% endblock %} diff --git a/physionet-django/console/urls.py b/physionet-django/console/urls.py index 7f2e8547a8..159c21119d 100644 --- a/physionet-django/console/urls.py +++ b/physionet-django/console/urls.py @@ -144,5 +144,10 @@ # Lists of event components path('event/', views.event, name='event'), - path('event/manage/', views.event_management, name='event_management') + path('event/manage/', views.event_management, name='event_management'), + path('event_agreements/', views.event_agreement_list, name='event_agreement_list'), + path('event_agreements//', views.event_agreement_detail, name='event_agreement_detail'), + path('event_agreements//delete/', views.event_agreement_delete, name='event_agreement_delete'), + path('event_agreements//new-version/', views.event_agreement_new_version, + name='event_agreement_new_version'), ] diff --git a/physionet-django/console/views.py b/physionet-django/console/views.py index 01b295a7da..10335378bc 100644 --- a/physionet-django/console/views.py +++ b/physionet-django/console/views.py @@ -24,8 +24,8 @@ from django.urls import reverse from django.utils import timezone from django.core.exceptions import PermissionDenied -from events.models import Event, EventDataset -from events.forms import EventDatasetForm +from events.forms import EventAgreementForm, EventDatasetForm +from events.models import Event, EventAgreement, EventDataset from notification.models import News from physionet.forms import set_saved_fields_cookie from physionet.middleware.maintenance import ServiceUnavailable @@ -2924,3 +2924,96 @@ def event_management(request, event_slug): 'event_dataset_form': event_dataset_form, 'event_datasets': event_datasets, }) + + +@permission_required('events.add_event', raise_exception=True) +def event_agreement_list(request): + if request.method == 'POST': + event_agreement_form = EventAgreementForm(data=request.POST) + if event_agreement_form.is_valid(): + event_agreement_form.instance.creator = request.user + event_agreement_form.save() + event_agreement_form = EventAgreementForm() + messages.success(request, "The Event Agreement has been created.") + else: + messages.error(request, "Invalid submission. Check errors below.") + else: + event_agreement_form = EventAgreementForm() + + event_agreements = EventAgreement.objects.filter(creator=request.user).order_by('name') + event_agreements = paginate(request, event_agreements, 20) + + return render( + request, + 'console/event_agreement_list.html', + { + 'event_agreement_nav': True, + 'event_agreements': event_agreements, + 'event_agreement_form': event_agreement_form + } + ) + + +@permission_required('events.add_event', raise_exception=True) +def event_agreement_detail(request, pk): + event_agreement = get_object_or_404(EventAgreement, pk=pk) + + if request.method == 'POST': + event_agreement_form = EventAgreementForm(data=request.POST, instance=event_agreement) + if event_agreement_form.is_valid(): + event_agreement_form.save() + messages.success(request, "The Event Agreement has been updated.") + else: + messages.error(request, "Invalid submission. Check errors below.") + + else: + event_agreement_form = EventAgreementForm(instance=event_agreement) + + return render( + request, + 'console/event_agreement_detail.html', + { + 'event_agreement_nav': True, + 'event_agreement': event_agreement, + 'event_agreement_form': event_agreement_form + } + ) + + +@permission_required('events.add_event', raise_exception=True) +def event_agreement_delete(request, pk): + if request.method == 'POST': + event_agreement = get_object_or_404(EventAgreement, pk=pk) + event_agreement.delete() + + return redirect("event_agreement_list") + + +@permission_required('events.add_event', raise_exception=True) +def event_agreement_new_version(request, pk): + event_agreement = get_object_or_404(EventAgreement, pk=pk) + + if request.method == 'POST': + event_agreement_form = EventAgreementForm(data=request.POST) + if event_agreement_form.is_valid(): + event_agreement_form.instance.creator = request.user + event_agreement_form.save() + messages.success(request, "The Event Agreement has been created.") + return redirect("event_agreement_list") + else: + messages.error(request, "Invalid submission. Check errors below.") + else: + event_agreement_data = model_to_dict(event_agreement) + event_agreement_data['id'] = None + event_agreement_data['version'] = None + event_agreement_form = EventAgreementForm(initial=event_agreement_data) + + return render( + request, + 'console/event_agreement_new_version.html', + { + 'event_agreement_nav': True, + 'event_agreement': event_agreement, + 'event_agreement_form': event_agreement_form + } + ) From e74221d1bbb23eea473e81cd3cec51989c401840 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Thu, 16 Feb 2023 15:21:37 -0500 Subject: [PATCH 05/14] add tests to make sure event agreements works as expected --- physionet-django/console/test_views.py | 157 +++++++++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/physionet-django/console/test_views.py b/physionet-django/console/test_views.py index b6ce3975fc..71bc8dac26 100644 --- a/physionet-django/console/test_views.py +++ b/physionet-django/console/test_views.py @@ -8,6 +8,7 @@ from background_task.tasks import tasks from django.test.utils import get_runner from django.urls import reverse +from events.models import EventAgreement from project.models import ( ActiveProject, ArchivedProject, @@ -614,3 +615,159 @@ def test_frontpage_button_delete(self): reverse("frontpage_button_delete", args=(self.button_1.pk,)), follow=True) self.assertRedirects(response, reverse("frontpage_buttons"), status_code=302) self.assertEqual(FrontPageButton.objects.count(), frontpage_button_count - 1) + + +class TestEventAgreements(TestMixin): + """ Test that all views are behaving as expected """ + + def setUp(self): + """Setup for tests""" + + super().setUp() + self.event_agreement_name = "test event agreement" + self.event_agreement_version = "0.1" + self.event_agreement_version_invalid = "1" + self.event_agreement_version_new_version = "0.2" + self.event_agreement_slug = "pyvO3g6Nuc" + self.event_agreement_slug_new_version = "a1b2c3d4e5" + self.updated_event_agreement_name = "updated test event agreement" + self.event_agreement_html_content = "

      My test Event Agreement test content

      " + self.event_agreement_access_template = "

      My test Event Agreement test content

      " + + self.client.login(username='rgmark', password='Tester11!') + + def test_add_event_agreement_valid(self): + """tests the view that adds a valid event agreement""" + + # Create an event Agreement + response = self.client.post( + reverse('event_agreement_list'), + data={ + 'name': self.event_agreement_name, + 'version': self.event_agreement_version, + 'slug': self.event_agreement_slug, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + self.assertEqual(response.status_code, 200) + event_agreement = EventAgreement.objects.get(slug=self.event_agreement_slug) + self.assertEqual(event_agreement.name, self.event_agreement_name) + return event_agreement + + def test_add_event_agreement_invalid(self): + """tests the view that adds an invalid event agreement""" + + # Try to Create an Invalid event Agreement + response = self.client.post( + reverse('event_agreement_list'), + data={ + 'name': self.event_agreement_name, + 'version': self.event_agreement_version_invalid, + 'slug': self.event_agreement_slug, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'console/event_agreement_list.html') + self.assertContains(response, + 'Version may only contain numbers and dots, and must begin and end with a number.') + + def test_edit_event_agreement_valid(self): + """tests the view that edits a valid event agreement""" + + event_agreement = self.test_add_event_agreement_valid() + + # Edit the event Agreement + response = self.client.post( + reverse('event_agreement_detail', args=[event_agreement.pk]), + data={ + 'name': self.updated_event_agreement_name, + 'version': self.event_agreement_version, + 'slug': self.event_agreement_slug, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + + self.assertEqual(response.status_code, 200) + event_agreement = EventAgreement.objects.get(slug=self.event_agreement_slug) + self.assertEqual(event_agreement.name, self.updated_event_agreement_name) + + def test_edit_event_agreement_invalid(self): + """tests the view that edits an invalid event agreement""" + + event_agreement = self.test_add_event_agreement_valid() + + # Edit the event Agreement + response = self.client.post( + reverse('event_agreement_detail', args=[event_agreement.pk]), + data={ + 'name': self.updated_event_agreement_name, + 'version': self.event_agreement_version_invalid, + 'slug': self.event_agreement_slug, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'console/event_agreement_detail.html') + self.assertContains(response, + 'Version may only contain numbers and dots, and must begin and end with a number.') + + def test_delete_event_agreement(self): + """tests the view that deletes an event agreement""" + + event_agreement = self.test_add_event_agreement_valid() + + # Delete the event Agreement + response = self.client.post( + reverse('event_agreement_delete', args=[event_agreement.pk])) + + self.assertEqual(response.status_code, 302) + self.assertEqual(EventAgreement.objects.filter(slug=self.event_agreement_slug).exists(), False) + + def test_event_agreement_new_version_valid(self): + """tests the view that adds a valid new version of event agreement""" + + event_agreement = self.test_add_event_agreement_valid() + + # Create an event Agreement + response = self.client.post( + reverse('event_agreement_new_version', args=[event_agreement.pk]), + data={ + 'name': self.event_agreement_name, + 'version': self.event_agreement_version_new_version, + 'slug': self.event_agreement_slug_new_version, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + + self.assertEqual(response.status_code, 302) + event_agreement = EventAgreement.objects.get(slug=self.event_agreement_slug_new_version) + self.assertEqual(event_agreement.version, self.event_agreement_version_new_version) + + def test_event_agreement_new_version_invalid(self): + """tests the view that adds an invalid new version of event agreement""" + + event_agreement = self.test_add_event_agreement_valid() + + # Create an event Agreement + response = self.client.post( + reverse('event_agreement_new_version', args=[event_agreement.pk]), + data={ + 'name': self.event_agreement_name, + 'version': self.event_agreement_version_new_version, + 'slug': self.event_agreement_slug, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'console/event_agreement_new_version.html') + self.assertContains(response, 'Event agreement with this Slug already exists.') From 8c8dc2b5c8bfac1962b8fb05cc37b4212aef9d0b Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Tue, 21 Feb 2023 14:36:44 -0500 Subject: [PATCH 06/14] Refactor template, views for code consistency and better wording --- .../templates/console/console_navbar.html | 6 +-- .../console/event_agreement_list.html | 8 +-- physionet-django/console/views.py | 50 +++++++++---------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/physionet-django/console/templates/console/console_navbar.html b/physionet-django/console/templates/console/console_navbar.html index 6026a0df83..4d1be3686d 100644 --- a/physionet-django/console/templates/console/console_navbar.html +++ b/physionet-django/console/templates/console/console_navbar.html @@ -136,7 +136,7 @@ {% endif %} - {% if perms.project.add_dua or perms.project.add_codeofconduct or perms.project.add_license or perms.events.add_event %} + {% if perms.project.add_dua or perms.project.add_codeofconduct or perms.project.add_license or perms.events.add_event %}
    • {% if license_nav or dua_nav or code_of_conduct_nav %} @@ -147,7 +147,7 @@ Legal - {% if license_nav or dua_nav or code_of_conduct_nav or perms.events.add_event %} + {% if license_nav or dua_nav or code_of_conduct_nav or event_agreement_nav %}
        {% else %} diff --git a/physionet-django/console/templates/console/event_agreement_list.html b/physionet-django/console/templates/console/event_agreement_list.html index 298c8c6910..27c40f0d69 100644 --- a/physionet-django/console/templates/console/event_agreement_list.html +++ b/physionet-django/console/templates/console/event_agreement_list.html @@ -39,7 +39,7 @@ Edit New version - +
        - {% include "inline_form_snippet.html" with form=event_agreement_form %} + {% include "project/content_inline_form_snippet.html" with form=event_agreement_form %}
        @@ -90,4 +90,5 @@
        Delete agreement
        {% block local_js_bottom %} + {% endblock %} diff --git a/physionet-django/console/templates/console/event_agreement_new_version.html b/physionet-django/console/templates/console/event_agreement_new_version.html index 3562dbc33d..d1439ee756 100644 --- a/physionet-django/console/templates/console/event_agreement_new_version.html +++ b/physionet-django/console/templates/console/event_agreement_new_version.html @@ -7,17 +7,18 @@ {% block content %}
        - {{ event_agreement.name }} - new version + {{ event_agreement.name }} - new version
        -
        - {% include "inline_form_snippet.html" with form=event_agreement_form %} - -
        +
        + {% include "project/content_inline_form_snippet.html" with form=event_agreement_form %} + +
        {% endblock %} {% block local_js_bottom %} + {% endblock %} diff --git a/physionet-django/events/forms.py b/physionet-django/events/forms.py index 30819f01a4..cc7502ee86 100644 --- a/physionet-django/events/forms.py +++ b/physionet-django/events/forms.py @@ -86,4 +86,11 @@ class Meta: 'access_template', ) labels = {'html_content': 'Content'} - + help_texts = { + 'name': '* The displayed name of the agreement.', + 'version': '* The version number of the agreement.', + 'slug': '* A simple string for use in the URL displaying the agreement. Should include the version number.', + 'is_active': '* Only active agreements are usable in future events.', + 'html_content': '* The agreement text displayed to the participant.', + 'access_template': '* Instructions on accessing the dataset.' + } From 40e1073ed1c869cfb4da3bf62e1e18cb9aa09db5 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Tue, 21 Feb 2023 15:54:20 -0500 Subject: [PATCH 08/14] update/add test for events Changes 1. Updated few test name , docstring with better explanation 2. Added a new test that lets us test add new version of agreement with duplicate version number. For this test, i had to override the `clean` method in form because django was throwing internal server error due to `unique_together` constraint fail(we dont allow duplicate based on `agreement_name`,`version`,`creator`. --- physionet-django/console/test_views.py | 30 ++++++++++++++++++++++---- physionet-django/console/views.py | 14 ++++++------ physionet-django/events/forms.py | 14 ++++++++++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/physionet-django/console/test_views.py b/physionet-django/console/test_views.py index 71bc8dac26..c4a076ac4b 100644 --- a/physionet-django/console/test_views.py +++ b/physionet-django/console/test_views.py @@ -696,8 +696,8 @@ def test_edit_event_agreement_valid(self): event_agreement = EventAgreement.objects.get(slug=self.event_agreement_slug) self.assertEqual(event_agreement.name, self.updated_event_agreement_name) - def test_edit_event_agreement_invalid(self): - """tests the view that edits an invalid event agreement""" + def test_edit_event_agreement_invalid_version(self): + """tests the view that edits an invalid event agreement(invalid version)""" event_agreement = self.test_add_event_agreement_valid() @@ -751,8 +751,8 @@ def test_event_agreement_new_version_valid(self): event_agreement = EventAgreement.objects.get(slug=self.event_agreement_slug_new_version) self.assertEqual(event_agreement.version, self.event_agreement_version_new_version) - def test_event_agreement_new_version_invalid(self): - """tests the view that adds an invalid new version of event agreement""" + def test_event_agreement_new_version_invalid_slug(self): + """tests the view that adds an invalid new version of event agreement(invalid slug)""" event_agreement = self.test_add_event_agreement_valid() @@ -771,3 +771,25 @@ def test_event_agreement_new_version_invalid(self): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'console/event_agreement_new_version.html') self.assertContains(response, 'Event agreement with this Slug already exists.') + + def test_event_agreement_new_version_invalid_version(self): + """tests the view that adds an invalid new version of event agreement(invalid version)""" + + event_agreement = self.test_add_event_agreement_valid() + + # Create an event Agreement + response = self.client.post( + reverse('event_agreement_new_version', args=[event_agreement.pk]), + data={ + 'name': self.event_agreement_name, + 'version': self.event_agreement_version_invalid, + 'slug': self.event_agreement_slug_new_version, + 'is_active': True, + 'html_content': self.event_agreement_html_content, + 'access_template': self.event_agreement_access_template + }) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'console/event_agreement_new_version.html') + self.assertContains(response, + 'Version may only contain numbers and dots, and must begin and end with a number.') diff --git a/physionet-django/console/views.py b/physionet-django/console/views.py index 0fd06bf094..22cad102e1 100644 --- a/physionet-django/console/views.py +++ b/physionet-django/console/views.py @@ -2929,16 +2929,16 @@ def event_management(request, event_slug): @permission_required('events.add_event', raise_exception=True) def event_agreement_list(request): if request.method == 'POST': - event_agreement_form = EventAgreementForm(data=request.POST) + event_agreement_form = EventAgreementForm(user=request.user, data=request.POST) if event_agreement_form.is_valid(): event_agreement_form.instance.creator = request.user event_agreement_form.save() - event_agreement_form = EventAgreementForm() + event_agreement_form = EventAgreementForm(user=request.user) messages.success(request, "The Event Agreement has been created.") else: messages.error(request, "Invalid submission. Check errors below.") else: - event_agreement_form = EventAgreementForm() + event_agreement_form = EventAgreementForm(user=request.user) event_agreements = EventAgreement.objects.filter(creator=request.user).order_by('name') event_agreements = paginate(request, event_agreements, 20) @@ -2959,7 +2959,7 @@ def event_agreement_new_version(request, pk): event_agreement = get_object_or_404(EventAgreement, pk=pk) if request.method == 'POST': - event_agreement_form = EventAgreementForm(data=request.POST) + event_agreement_form = EventAgreementForm(user=request.user, data=request.POST) if event_agreement_form.is_valid(): event_agreement_form.instance.creator = request.user event_agreement_form.save() @@ -2971,7 +2971,7 @@ def event_agreement_new_version(request, pk): event_agreement_data = model_to_dict(event_agreement) event_agreement_data['id'] = None event_agreement_data['version'] = None - event_agreement_form = EventAgreementForm(initial=event_agreement_data) + event_agreement_form = EventAgreementForm(user=request.user, initial=event_agreement_data) return render( request, @@ -2989,7 +2989,7 @@ def event_agreement_detail(request, pk): event_agreement = get_object_or_404(EventAgreement, pk=pk) if request.method == 'POST': - event_agreement_form = EventAgreementForm(data=request.POST, instance=event_agreement) + event_agreement_form = EventAgreementForm(user=request.user, data=request.POST, instance=event_agreement) if event_agreement_form.is_valid(): event_agreement_form.save() messages.success(request, "The Event Agreement has been updated.") @@ -2997,7 +2997,7 @@ def event_agreement_detail(request, pk): messages.error(request, "Invalid submission. Check errors below.") else: - event_agreement_form = EventAgreementForm(instance=event_agreement) + event_agreement_form = EventAgreementForm(user=request.user, instance=event_agreement) return render( request, diff --git a/physionet-django/events/forms.py b/physionet-django/events/forms.py index cc7502ee86..1745b94e29 100644 --- a/physionet-django/events/forms.py +++ b/physionet-django/events/forms.py @@ -94,3 +94,17 @@ class Meta: 'html_content': '* The agreement text displayed to the participant.', 'access_template': '* Instructions on accessing the dataset.' } + + def __init__(self, user, *args, **kwargs): + self.creator = user + super(EventAgreementForm, self).__init__(*args, **kwargs) + + def clean(self): + cleaned_data = super(EventAgreementForm, self).clean() + if EventAgreement.objects.filter(name=cleaned_data['name'], version=cleaned_data['version'], + creator=self.creator).exists(): + raise forms.ValidationError( + {"name": ["Agreement with this name and version already exists for this user."], + "version": ["Agreement with this name and version already exists for this user."] + }) + return cleaned_data From 7e7830853b517b2f4b588f45b6c5b9f1c11e807c Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Tue, 21 Feb 2023 15:56:34 -0500 Subject: [PATCH 09/14] minor bug fix: add a message telling the user the event is deleted --- physionet-django/console/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/physionet-django/console/views.py b/physionet-django/console/views.py index 22cad102e1..547ad03390 100644 --- a/physionet-django/console/views.py +++ b/physionet-django/console/views.py @@ -3015,5 +3015,6 @@ def event_agreement_delete(request, pk): if request.method == 'POST': event_agreement = get_object_or_404(EventAgreement, pk=pk) event_agreement.delete() + messages.success(request, "The Event Agreement has been deleted.") return redirect("event_agreement_list") From d5d3bea53c3a7069ffa62f6014d3ba51ee29d1ac Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Tue, 21 Feb 2023 16:21:16 -0500 Subject: [PATCH 10/14] minor bug fix: add a slug validator to event agreement slug - also updated the slug in test as the slug validator doesnot allow uppercase letters --- physionet-django/console/test_views.py | 2 +- physionet-django/events/models.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/physionet-django/console/test_views.py b/physionet-django/console/test_views.py index c4a076ac4b..69c65021c0 100644 --- a/physionet-django/console/test_views.py +++ b/physionet-django/console/test_views.py @@ -628,7 +628,7 @@ def setUp(self): self.event_agreement_version = "0.1" self.event_agreement_version_invalid = "1" self.event_agreement_version_new_version = "0.2" - self.event_agreement_slug = "pyvO3g6Nuc" + self.event_agreement_slug = "pyvo3g6nuc" self.event_agreement_slug_new_version = "a1b2c3d4e5" self.updated_event_agreement_name = "updated test event agreement" self.event_agreement_html_content = "

        My test Event Agreement test content

        " diff --git a/physionet-django/events/models.py b/physionet-django/events/models.py index 985c6433cb..65462af3d5 100644 --- a/physionet-django/events/models.py +++ b/physionet-django/events/models.py @@ -7,7 +7,7 @@ from events.enums import EventCategory from events import validators from project.modelcomponents.fields import SafeHTMLField -from project.validators import validate_version +from project.validators import validate_version, validate_slug class Event(models.Model): @@ -159,7 +159,7 @@ def withdraw(self, comment_to_applicant=None): class EventAgreement(models.Model): name = models.CharField(max_length=100) - slug = models.SlugField(max_length=120, unique=True) + slug = models.SlugField(max_length=120, unique=True, validators=[validate_slug]) version = models.CharField(max_length=15, default='', validators=[validate_version]) is_active = models.BooleanField(default=True) html_content = SafeHTMLField(default='') From b699e72f85679a6de9e89d54c0d776d874b9bae3 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Tue, 21 Feb 2023 16:29:32 -0500 Subject: [PATCH 11/14] refactor model As discussed with @alistairewj https://github.com/MIT-LCP/physionet-build/pull/1876#discussion_r1113451979 We dont want to have the Event Agreement tied to 1 user. So i refactored the model, and removed validation code from form,view --- physionet-django/console/views.py | 14 +++++++------- physionet-django/events/forms.py | 10 +++------- physionet-django/events/models.py | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/physionet-django/console/views.py b/physionet-django/console/views.py index 547ad03390..d835de54f4 100644 --- a/physionet-django/console/views.py +++ b/physionet-django/console/views.py @@ -2929,16 +2929,16 @@ def event_management(request, event_slug): @permission_required('events.add_event', raise_exception=True) def event_agreement_list(request): if request.method == 'POST': - event_agreement_form = EventAgreementForm(user=request.user, data=request.POST) + event_agreement_form = EventAgreementForm(data=request.POST) if event_agreement_form.is_valid(): event_agreement_form.instance.creator = request.user event_agreement_form.save() - event_agreement_form = EventAgreementForm(user=request.user) + event_agreement_form = EventAgreementForm() messages.success(request, "The Event Agreement has been created.") else: messages.error(request, "Invalid submission. Check errors below.") else: - event_agreement_form = EventAgreementForm(user=request.user) + event_agreement_form = EventAgreementForm() event_agreements = EventAgreement.objects.filter(creator=request.user).order_by('name') event_agreements = paginate(request, event_agreements, 20) @@ -2959,7 +2959,7 @@ def event_agreement_new_version(request, pk): event_agreement = get_object_or_404(EventAgreement, pk=pk) if request.method == 'POST': - event_agreement_form = EventAgreementForm(user=request.user, data=request.POST) + event_agreement_form = EventAgreementForm(data=request.POST) if event_agreement_form.is_valid(): event_agreement_form.instance.creator = request.user event_agreement_form.save() @@ -2971,7 +2971,7 @@ def event_agreement_new_version(request, pk): event_agreement_data = model_to_dict(event_agreement) event_agreement_data['id'] = None event_agreement_data['version'] = None - event_agreement_form = EventAgreementForm(user=request.user, initial=event_agreement_data) + event_agreement_form = EventAgreementForm(initial=event_agreement_data) return render( request, @@ -2989,7 +2989,7 @@ def event_agreement_detail(request, pk): event_agreement = get_object_or_404(EventAgreement, pk=pk) if request.method == 'POST': - event_agreement_form = EventAgreementForm(user=request.user, data=request.POST, instance=event_agreement) + event_agreement_form = EventAgreementForm(data=request.POST, instance=event_agreement) if event_agreement_form.is_valid(): event_agreement_form.save() messages.success(request, "The Event Agreement has been updated.") @@ -2997,7 +2997,7 @@ def event_agreement_detail(request, pk): messages.error(request, "Invalid submission. Check errors below.") else: - event_agreement_form = EventAgreementForm(user=request.user, instance=event_agreement) + event_agreement_form = EventAgreementForm(instance=event_agreement) return render( request, diff --git a/physionet-django/events/forms.py b/physionet-django/events/forms.py index 1745b94e29..624ae37763 100644 --- a/physionet-django/events/forms.py +++ b/physionet-django/events/forms.py @@ -95,16 +95,12 @@ class Meta: 'access_template': '* Instructions on accessing the dataset.' } - def __init__(self, user, *args, **kwargs): - self.creator = user - super(EventAgreementForm, self).__init__(*args, **kwargs) def clean(self): cleaned_data = super(EventAgreementForm, self).clean() - if EventAgreement.objects.filter(name=cleaned_data['name'], version=cleaned_data['version'], - creator=self.creator).exists(): + if EventAgreement.objects.filter(name=cleaned_data['name'], version=cleaned_data['version']).exists(): raise forms.ValidationError( - {"name": ["Agreement with this name and version already exists for this user."], - "version": ["Agreement with this name and version already exists for this user."] + {"name": ["An agreement with this name and version already exists."], + "version": ["An agreement with this name and version already exists."] }) return cleaned_data diff --git a/physionet-django/events/models.py b/physionet-django/events/models.py index 65462af3d5..7bf1f8979e 100644 --- a/physionet-django/events/models.py +++ b/physionet-django/events/models.py @@ -168,7 +168,7 @@ class EventAgreement(models.Model): class Meta: default_permissions = ('add',) - unique_together = (('name', 'version', 'creator'),) + unique_together = (('name', 'version'),) def __str__(self): return self.name From 5de2a69050941a2ad4b27cb5ea70343cea405acc Mon Sep 17 00:00:00 2001 From: Amit Date: Tue, 21 Feb 2023 20:25:00 -0500 Subject: [PATCH 12/14] remove text assertion from test_views as suggested here https://github.com/MIT-LCP/physionet-build/pull/1876#discussion_r1113709658 with assertion of text in the template, we will need to maintain this in future, if the error messages are updated for EventDataset. So to save hassale in future, we decided to avoid using the text assertion. Currently we are validating that the form returned error or the event agreement creation/update was not successful by checking status code and template used. --- physionet-django/console/test_views.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/physionet-django/console/test_views.py b/physionet-django/console/test_views.py index 69c65021c0..c9f160b87a 100644 --- a/physionet-django/console/test_views.py +++ b/physionet-django/console/test_views.py @@ -672,8 +672,6 @@ def test_add_event_agreement_invalid(self): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'console/event_agreement_list.html') - self.assertContains(response, - 'Version may only contain numbers and dots, and must begin and end with a number.') def test_edit_event_agreement_valid(self): """tests the view that edits a valid event agreement""" @@ -715,8 +713,6 @@ def test_edit_event_agreement_invalid_version(self): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'console/event_agreement_detail.html') - self.assertContains(response, - 'Version may only contain numbers and dots, and must begin and end with a number.') def test_delete_event_agreement(self): """tests the view that deletes an event agreement""" @@ -770,7 +766,6 @@ def test_event_agreement_new_version_invalid_slug(self): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'console/event_agreement_new_version.html') - self.assertContains(response, 'Event agreement with this Slug already exists.') def test_event_agreement_new_version_invalid_version(self): """tests the view that adds an invalid new version of event agreement(invalid version)""" @@ -791,5 +786,3 @@ def test_event_agreement_new_version_invalid_version(self): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'console/event_agreement_new_version.html') - self.assertContains(response, - 'Version may only contain numbers and dots, and must begin and end with a number.') From 10a2d69af263ceafe04d342ff73e7926083e4483 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Wed, 22 Feb 2023 17:38:47 -0500 Subject: [PATCH 13/14] use `add_eventagreement` to control access to who can add/change Agreement and update tests As discussed with Tom, Benjamin; only admin or some should with `add_eventagreement` permission can add/change Agreement. The test is also updated as earlier it was assumed that Event Host can add/change Agreement. --- .../console/templates/console/console_navbar.html | 4 ++-- physionet-django/console/test_views.py | 2 +- physionet-django/console/views.py | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/physionet-django/console/templates/console/console_navbar.html b/physionet-django/console/templates/console/console_navbar.html index 4d1be3686d..3ded9cd23c 100644 --- a/physionet-django/console/templates/console/console_navbar.html +++ b/physionet-django/console/templates/console/console_navbar.html @@ -136,7 +136,7 @@ {% endif %} - {% if perms.project.add_dua or perms.project.add_codeofconduct or perms.project.add_license or perms.events.add_event %} + {% if perms.project.add_dua or perms.project.add_codeofconduct or perms.project.add_license or perms.events.add_eventagreement %}
      • {% if license_nav or dua_nav or code_of_conduct_nav %} @@ -167,7 +167,7 @@ Code of Conduct
        • {% endif %} - {% if perms.events.add_event %} + {% if perms.events.add_eventagreement %}
      • Event Agreements
        • diff --git a/physionet-django/console/test_views.py b/physionet-django/console/test_views.py index c9f160b87a..0c92538d69 100644 --- a/physionet-django/console/test_views.py +++ b/physionet-django/console/test_views.py @@ -634,7 +634,7 @@ def setUp(self): self.event_agreement_html_content = "

        My test Event Agreement test content

        " self.event_agreement_access_template = "

        My test Event Agreement test content

        " - self.client.login(username='rgmark', password='Tester11!') + self.client.login(username='admin', password='Tester11!') def test_add_event_agreement_valid(self): """tests the view that adds a valid event agreement""" diff --git a/physionet-django/console/views.py b/physionet-django/console/views.py index d835de54f4..dfd7093078 100644 --- a/physionet-django/console/views.py +++ b/physionet-django/console/views.py @@ -2926,7 +2926,7 @@ def event_management(request, event_slug): }) -@permission_required('events.add_event', raise_exception=True) +@permission_required('events.add_eventagreement', raise_exception=True) def event_agreement_list(request): if request.method == 'POST': event_agreement_form = EventAgreementForm(data=request.POST) @@ -2954,7 +2954,7 @@ def event_agreement_list(request): ) -@permission_required('events.add_event', raise_exception=True) +@permission_required('events.add_eventagreement', raise_exception=True) def event_agreement_new_version(request, pk): event_agreement = get_object_or_404(EventAgreement, pk=pk) @@ -2984,7 +2984,7 @@ def event_agreement_new_version(request, pk): ) -@permission_required('events.add_event', raise_exception=True) +@permission_required('events.add_eventagreement', raise_exception=True) def event_agreement_detail(request, pk): event_agreement = get_object_or_404(EventAgreement, pk=pk) @@ -3010,7 +3010,7 @@ def event_agreement_detail(request, pk): ) -@permission_required('events.add_event', raise_exception=True) +@permission_required('events.add_eventagreement', raise_exception=True) def event_agreement_delete(request, pk): if request.method == 'POST': event_agreement = get_object_or_404(EventAgreement, pk=pk) From 718fa2b35a772f9cb8ad774320d9cd268dfa5f36 Mon Sep 17 00:00:00 2001 From: Amit Upreti Date: Tue, 21 Feb 2023 16:31:46 -0500 Subject: [PATCH 14/14] add migration file and linter concerns fix --- physionet-django/console/views.py | 2 +- physionet-django/events/forms.py | 5 +- ..._eventagreement_eventagreementsignature.py | 53 +++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 physionet-django/events/migrations/0006_eventagreement_eventagreementsignature.py diff --git a/physionet-django/console/views.py b/physionet-django/console/views.py index dfd7093078..927eb9709f 100644 --- a/physionet-django/console/views.py +++ b/physionet-django/console/views.py @@ -2950,7 +2950,7 @@ def event_agreement_list(request): 'event_agreement_nav': True, 'event_agreements': event_agreements, 'event_agreement_form': event_agreement_form - } + } ) diff --git a/physionet-django/events/forms.py b/physionet-django/events/forms.py index 624ae37763..3f44219afb 100644 --- a/physionet-django/events/forms.py +++ b/physionet-django/events/forms.py @@ -4,6 +4,7 @@ from events.models import Event, EventApplication, EventAgreement, EventDataset from project.models import PublishedProject + class AddEventForm(forms.ModelForm): """ A form for adding events. @@ -89,13 +90,13 @@ class Meta: help_texts = { 'name': '* The displayed name of the agreement.', 'version': '* The version number of the agreement.', - 'slug': '* A simple string for use in the URL displaying the agreement. Should include the version number.', + 'slug': '* A simple string for use in the URL displaying the agreement. ' + 'Should include the version number.', 'is_active': '* Only active agreements are usable in future events.', 'html_content': '* The agreement text displayed to the participant.', 'access_template': '* Instructions on accessing the dataset.' } - def clean(self): cleaned_data = super(EventAgreementForm, self).clean() if EventAgreement.objects.filter(name=cleaned_data['name'], version=cleaned_data['version']).exists(): diff --git a/physionet-django/events/migrations/0006_eventagreement_eventagreementsignature.py b/physionet-django/events/migrations/0006_eventagreement_eventagreementsignature.py new file mode 100644 index 0000000000..5297f5467a --- /dev/null +++ b/physionet-django/events/migrations/0006_eventagreement_eventagreementsignature.py @@ -0,0 +1,53 @@ +# Generated by Django 3.2.16 on 2023-02-22 22:46 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import project.modelcomponents.fields +import project.validators + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('events', '0005_eventdataset'), + ] + + operations = [ + migrations.CreateModel( + name='EventAgreement', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=100)), + ('slug', models.SlugField(max_length=120, unique=True, + validators=[project.validators.validate_slug])), + ('version', models.CharField(default='', max_length=15, + validators=[project.validators.validate_version])), + ('is_active', models.BooleanField(default=True)), + ('html_content', project.modelcomponents.fields.SafeHTMLField(default='')), + ('access_template', project.modelcomponents.fields.SafeHTMLField(default='')), + ('creator', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL)), + ], + options={ + 'default_permissions': ('add',), + 'unique_together': {('name', 'version')}, + }, + ), + migrations.CreateModel( + name='EventAgreementSignature', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('sign_datetime', models.DateTimeField(auto_now_add=True)), + ('event', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='events.event')), + ('event_agreement', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + to='events.eventagreement')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + related_name='event_agreement_signatures', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'default_permissions': (), + }, + ), + ]