diff --git a/castellum/static/style.css b/castellum/static/style.css index 091a17d5a1d56b3724aa0623e438c7360b1658a5..65abc6f81b2b031567921f14da1f8b0519690468 100644 --- a/castellum/static/style.css +++ b/castellum/static/style.css @@ -159,3 +159,13 @@ .dl-inline dd { margin: 0 1em 0 0; } + +.diff-changed { + color: var(--orange); +} +.diff-added { + color: var(--green); +} +.diff-removed { + color: var(--red); +} diff --git a/castellum/studies/migrations/0031_study_snapshot.py b/castellum/studies/migrations/0031_study_snapshot.py new file mode 100644 index 0000000000000000000000000000000000000000..4fbf493b46d2ac9865720eeeb29dae3a3b3f750a --- /dev/null +++ b/castellum/studies/migrations/0031_study_snapshot.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2 on 2021-04-19 13:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('studies', '0030_domain_context'), + ] + + operations = [ + migrations.AddField( + model_name='study', + name='snapshot', + field=models.TextField(blank=True, editable=False, verbose_name='Snapshot'), + ), + ] diff --git a/castellum/studies/models.py b/castellum/studies/models.py index f98fe4a1aa6d349fc83f6600cd7d85f89eb1e72f..cbda835f89f99bb87c42a4da044cad419a18971b 100644 --- a/castellum/studies/models.py +++ b/castellum/studies/models.py @@ -126,6 +126,7 @@ class Study(models.Model): blank=True, ) announce_status_changes = models.BooleanField(_('Announce status changes'), default=False) + snapshot = models.TextField(_('Snapshot'), blank=True, editable=False) members = models.ManyToManyField(User, through='StudyMembership') domains = GenericRelation(Domain) @@ -383,4 +384,4 @@ class StudyTag(models.Model): name = models.CharField(_('Name'), max_length=128) def __str__(self): - return '{} - {}'.format(self.study, self.name) + return self.name diff --git a/castellum/studies/templates/studies/study_base.html b/castellum/studies/templates/studies/study_base.html index be2af5b1ddfb747197a35c8266b3257c82d3fa41..bb783520a1758b4ee41873b4927efc3d8361a33e 100644 --- a/castellum/studies/templates/studies/study_base.html +++ b/castellum/studies/templates/studies/study_base.html @@ -110,6 +110,12 @@ {% translate 'Overview' %} + {% if can_access_study and can_approve_study %} +
  • + {% translate 'Changes' %} +
    • + {% endif %} + {% if can_access_study and can_change_study %}
  • {% translate 'General' %} diff --git a/castellum/studies/templates/studies/study_diff.html b/castellum/studies/templates/studies/study_diff.html new file mode 100644 index 0000000000000000000000000000000000000000..e64884fc56d492f2c384769b7e48fbe9fdc1ac2e --- /dev/null +++ b/castellum/studies/templates/studies/study_diff.html @@ -0,0 +1,155 @@ +{% extends "studies/study_base.html" %} +{% load static i18n utils diff %} + +{% block title %}{% translate "Changes" %} · {{ block.super }}{% endblock %} + +{% block content %} +

    {% translate 'This page provides an overview of all the settings for this study. Any changes that were made since the study was started are highlighted.' %}

    + +
    +
    {{ study|verbose_name:'name' }}
    +
    {% diff_get study 'name' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'contact_person' }}
    +
    {% diff_get study 'contact_person' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'email' }}
    +
    {% diff_get study 'email' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'phone' }}
    +
    {% diff_get study 'phone' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'principal_investigator' }}
    +
    {% diff_get study 'principal_investigator' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'affiliated_scientists' }}
    +
    {% diff_get study 'affiliated_scientists' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'affiliated_research_assistants' }}
    +
    {% diff_get study 'affiliated_research_assistants' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'description' }}
    +
    {% diff_get study 'description' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'data_sensitivity' }}
    +
    {% diff_get study 'data_sensitivity' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'min_subject_count' }}
    +
    {% diff_get study 'min_subject_count' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'session_instructions' }}
    +
    {% diff_get study 'session_instructions' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'sessions_start' }}
    +
    {% diff_get study 'sessions_start' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'sessions_end' }}
    +
    {% diff_get study 'sessions_end' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'excluded_studies' }}
    +
    {% diff_get study 'excluded_studies' as items %}{% diff_list items %}
    + +
    {% translate 'Tags' %}
    +
    {% diff_get study 'studytag_set' as items %}{% diff_list items %}
    + +
    {% translate 'Domains' %}
    +
    {% diff_get study 'domains' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'exportable_attributes' }}
    +
    {% diff_get study 'exportable_attributes' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'exclusion_criteria' }}
    +
    {% diff_get study 'exclusion_criteria' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'recruitment_text' }}
    +
    {% diff_get study 'recruitment_text' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'announce_status_changes' }}
    +
    {% diff_get study 'announce_status_changes' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'advanced_filtering' }}
    +
    {% diff_get study 'advanced_filtering' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'custom_filter' }}
    +
    {% diff_get study 'custom_filter' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'geo_filter' }}
    +
    {% diff_get study 'geo_filter' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'is_exclusive' }}
    +
    {% diff_get study 'is_exclusive' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'complete_matches_only' }}
    +
    {% diff_get study 'complete_matches_only' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'is_onetime_invitation' }}
    +
    {% diff_get study 'is_onetime_invitation' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'consent' }}
    +
    {% diff_get study 'consent' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'mail_subject' }}
    +
    {% diff_get study 'mail_subject' as items %}{% diff_list items %}
    + +
    {{ study|verbose_name:'mail_body' }}
    +
    {% diff_get study 'mail_body' as items %}{% diff_list items %}
    +
    + + {% diff_get study 'studymembership_set' as memberships %} + {% if memberships %} +

    {% translate 'Memberships' %}

    +
    + {% for membership, class in memberships %} +
    + <{{ class|diff_tag }} class="{{ class }}">{{ membership|display:'user' }} +
    +
    {% diff_get membership 'groups' as items %}{% diff_list items %}
    + {% endfor %} +
    + {% endif %} + + {% diff_get study 'studysession_set' as sessions %} + {% if sessions %} +

    {% translate 'Sessions' %}

    + {% for session, class in sessions %} + <{{ class|diff_tag }} class="{{ class }}"> +
    +
    {{ session|verbose_name:'name' }}
    +
    {% diff_get session 'name' as items %}{% diff_list items %}
    + +
    {{ session|verbose_name:'duration' }}
    +
    {% diff_get session 'duration' as items %}{% diff_list items %}
    + +
    {{ session|verbose_name:'type' }}
    +
    {% diff_get session 'type' as type %}{% diff_list items %}
    + +
    {{ session|verbose_name:'resource' }}
    +
    {% diff_get session 'resource' as items %}{% diff_list items %}
    + +
    {{ session|verbose_name:'reminder_text' }}
    +
    {% diff_get session 'reminder_text' as items %}{% diff_list items %}
    + +
    {{ session|verbose_name:'schedule_id' }}
    +
    {% diff_get session 'schedule_id' as items %}{% diff_list items %}
    +
    + + {% endfor %} + {% endif %} + + {% diff_get study 'subjectfiltergroup_set' as filtergroups %} + {% if filtergroups %} +

    {% translate 'Filters' %}

    + {% for filtergroup, class in filtergroups %} + <{{ class|diff_tag }} class="{{ class }}"> + + + {% endfor %} + {% endif %} +{% endblock %} diff --git a/castellum/studies/urls/__init__.py b/castellum/studies/urls/__init__.py index 79198b8a5c53021d8796b8284e099938d71fc4f9..e6ce01f01722b43e191bac759e98b32f1ce60fd6 100644 --- a/castellum/studies/urls/__init__.py +++ b/castellum/studies/urls/__init__.py @@ -27,6 +27,7 @@ from ..views.recruitment import OneTimeInvitationMailRecruitmentView from ..views.studies import StudyCreateView from ..views.studies import StudyDeleteView from ..views.studies import StudyDetailView +from ..views.studies import StudyDiffView from ..views.studies import StudyExportView from ..views.studies import StudyFinishRecruitmentView from ..views.studies import StudyImportView @@ -49,6 +50,7 @@ urlpatterns = [ path('/start/', StudyStartRecruitmentView.as_view(), name='start'), path('/finish/', StudyFinishRecruitmentView.as_view(), name='finish'), path('/mail/', OneTimeInvitationMailRecruitmentView.as_view(), name='mail'), + path('/diff/', StudyDiffView.as_view(), name='diff'), path('/members/', include(members_urls)), path('/recruitmentsettings/', include(recruitment_urls)), path('/sessions/', include(sessions_urls)), diff --git a/castellum/studies/views/studies.py b/castellum/studies/views/studies.py index a59cc84a4617cf64b60e6d0a4dda0004f00ca113..5974aa53374a5ce737a27962672865448ec362b1 100644 --- a/castellum/studies/views/studies.py +++ b/castellum/studies/views/studies.py @@ -24,6 +24,7 @@ import logging from django.conf import settings from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin +from django.core import serializers from django.core.exceptions import PermissionDenied from django.db import models from django.forms import modelform_factory @@ -49,7 +50,9 @@ from castellum.castellum_auth.mixins import PermissionRequiredMixin from castellum.pseudonyms.models import Domain from castellum.recruitment import filter_queries from castellum.recruitment.models import Participation +from castellum.recruitment.models import SubjectFilter from castellum.subjects.models import Subject +from castellum.utils import Exclusion from castellum.utils.mail import MailContext from castellum.utils.views import get_next_url @@ -100,6 +103,19 @@ def send_pr_mail(study, user_email): ) +def take_snapshot(study): + study.snapshot = serializers.serialize('json', [ + study, + *study.studymembership_set.all(), + *study.domains.all(), + *study.studytag_set.all(), + *study.studysession_set.all(), + *study.subjectfiltergroup_set.all(), + *SubjectFilter.objects.filter(group__study=study), + ], fields=Exclusion(['snapshot', 'status', 'previous_status'])) + study.save() + + class StudyIndexView(LoginRequiredMixin, ListView): model = Study ordering = 'name' @@ -319,6 +335,8 @@ class StudyStartRecruitmentView(StudyMixin, PermissionRequiredMixin, View): ) else: self.study.set_status(self.study.EXECUTION) + take_snapshot(self.study) + monitoring_logger.info( 'Study {} started by {}'.format(self.study.name, self.request.user.pk) ) @@ -424,3 +442,20 @@ class StudyExportView(StudyMixin, PermissionRequiredMixin, View): filename = 'castellum_{}.json'.format(slugify(self.study.name)) response['Content-Disposition'] = 'attachment; filename="{}"'.format(filename) return response + + +class StudyDiffView(StudyMixin, PermissionRequiredMixin, DetailView): + model = Study + permission_required = 'studies.approve_study' + template_name = 'studies/study_diff.html' + tab = 'diff' + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + try: + context['snapshot'] = list(serializers.deserialize( + 'json', self.study.snapshot, ignorenonexistent=True + )) + except serializers.base.DeserializationError: + context['snapshot'] = [] + return context diff --git a/castellum/utils/__init__.py b/castellum/utils/__init__.py index efca33b1656ac222586e4d196fa86adf84bbb23a..8f828e1d9996e5b031d023cfa45a2ccd37715bef 100644 --- a/castellum/utils/__init__.py +++ b/castellum/utils/__init__.py @@ -70,3 +70,11 @@ def get_parler_languages(site_id=None): names = dict(settings.LANGUAGES) codes = [lang['code'] for lang in settings.PARLER_LANGUAGES.get(site_id, [])] return [(code, names[code]) for code in codes] + + +class Exclusion: + def __init__(self, values): + self.values = values + + def __contains__(self, value): + return not (value in self.values) diff --git a/castellum/utils/templates/utils/__diff_list.html b/castellum/utils/templates/utils/__diff_list.html new file mode 100644 index 0000000000000000000000000000000000000000..1069d5f7d6b0b9d34a0b72f536cc665ec15be2eb --- /dev/null +++ b/castellum/utils/templates/utils/__diff_list.html @@ -0,0 +1,7 @@ +{% load utils diff %} + +{% for value, class in items %} + <{{ class|diff_tag }} class="{{ class }}">{{ value|display }}{% if not forloop.last %},{% endif %} +{% empty %} + — +{% endfor %} diff --git a/castellum/utils/templatetags/diff.py b/castellum/utils/templatetags/diff.py new file mode 100644 index 0000000000000000000000000000000000000000..24cce878da0b301011d01358bea8f7b82cd7f9aa --- /dev/null +++ b/castellum/utils/templatetags/diff.py @@ -0,0 +1,132 @@ +# (c) 2018-2021 +# MPIB , +# MPI-CBS , +# MPIP +# +# This file is part of Castellum. +# +# Castellum is free software; you can redistribute it and/or modify it +# under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Castellum is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public +# License along with Castellum. If not, see +# . + +from django import template + +register = template.Library() + +TAGS = { + 'diff-added': 'ins', + 'diff-removed': 'del', +} + + +def get_old(snapshot, new_obj): + # equality on django models checks only for pk, not actual values + for old in snapshot: + if old.object == new_obj: + return old + raise KeyError + + +def diff_class(snapshot, new_obj, fieldname): + new_value = getattr(new_obj, fieldname) + + try: + old = get_old(snapshot, new_obj) + old_value = getattr(old.object, fieldname) + except KeyError: + return 'diff-added' + + if new_value == old_value: + return 'diff-unchanged' + elif not new_value: + return 'diff-removed' + elif not old_value: + return 'diff-added' + else: + return 'diff-changed' + + +def diff_get_m2m(snapshot, new_obj, fieldname): + field = getattr(new_obj, fieldname) + model = field.model + + try: + old = get_old(snapshot, new_obj) + old_values = set(old.m2m_data[fieldname]) + except KeyError: + old_values = set() + + for obj in field.all(): + if obj.pk in old_values: + old_values.remove(obj.pk) + yield obj, 'diff-unchanged' + else: + yield obj, 'diff-added' + for pk in old_values: + try: + obj = model.objects.get(pk=pk) + yield obj, 'diff-removed' + except model.DoesNotExist: + # Deletion of related models is not relevant for the diff + pass + + +def diff_get_o2m(snapshot, new_obj, fieldname): + field = getattr(new_obj, fieldname) + model = field.model + + old_objects = set() + for old in snapshot: + if isinstance(old.object, model) and getattr(old.object, field.field.attname) == new_obj.pk: + old_objects.add(old.object) + + for obj in field.all(): + try: + old = get_old(snapshot, obj) + old_objects.remove(obj) + if str(obj) == str(old.object): + yield obj, 'diff-unchanged' + else: + yield obj, 'diff-changed' + except KeyError: + yield obj, 'diff-added' + for obj in old_objects: + # Diffing the contents of obj recursively will result in + # 'diff-unchanged' in this case because this already is the old + # object. + yield obj, 'diff-removed' + + +@register.simple_tag(takes_context=True) +def diff_get(context, new_obj, fieldname): + snapshot = context['snapshot'] + try: + rel = getattr(new_obj.__class__, fieldname).rel + if rel.many_to_many: + return list(diff_get_m2m(snapshot, new_obj, fieldname)) + else: + return list(diff_get_o2m(snapshot, new_obj, fieldname)) + except AttributeError: + cls = diff_class(snapshot, new_obj, fieldname) + value = getattr(new_obj, fieldname) + return [(value, cls)] + + +@register.filter +def diff_tag(cls): + return TAGS.get(cls, 'span') + + +@register.inclusion_tag('utils/__diff_list.html') +def diff_list(items): + return {'items': items} diff --git a/castellum/utils/templatetags/utils.py b/castellum/utils/templatetags/utils.py index f556d65248452a84d6d13644e887d773415f67bb..175122fff2667b1a061763d0a02a87218c959fc4 100644 --- a/castellum/utils/templatetags/utils.py +++ b/castellum/utils/templatetags/utils.py @@ -49,9 +49,11 @@ def verbose_name(instance, field_name): @register.filter -def display(instance, field_name): - getter = getattr(instance, 'get_{}_display'.format(field_name), None) - value = getter() if getter else getattr(instance, field_name) +def display(value, field_name=None): + if field_name: + getter = getattr(value, 'get_{}_display'.format(field_name), None) + value = getter() if getter else getattr(value, field_name) + if value is True: return _('Yes') elif value is False: diff --git a/tests/conftest.py b/tests/conftest.py index 7a25665f4d1af1fd33c2e5f0f0045ce84d95e789..8aac23632301819bf7a0b40a23a66f6f13ccb4dc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -204,6 +204,6 @@ def subject_filter(subject_filter_group, attribute_description): return SubjectFilter.objects.create( description=attribute_description, operator='exact', - value='left', + value=1, group=subject_filter_group, ) diff --git a/tests/studies/views/test_diff_view.py b/tests/studies/views/test_diff_view.py new file mode 100644 index 0000000000000000000000000000000000000000..d601499aac1ee7f0cac977ac7b5a1fa763d34e05 --- /dev/null +++ b/tests/studies/views/test_diff_view.py @@ -0,0 +1,81 @@ +import re + +from django.apps import apps + +from model_bakery import baker + +from castellum.studies.models import StudySession +from castellum.studies.models import StudyTag + +EXCLUDED = [ + 'admin.', + 'appointments.Appointment.', + 'auth.', + 'castellum_auth.User.', + 'contacts.', + 'contenttypes.', + 'geofilters.Geolocation.', + 'pseudonyms.Pseudonym.', + 'recruitment.AttributeCategory.', + 'recruitment.AttributeCategoryTranslation.', + 'recruitment.AttributeChoice.', + 'recruitment.AttributeChoiceTranslation.', + 'recruitment.AttributeDescription.', + 'recruitment.AttributeDescriptionTranslation.', + 'recruitment.MailBatch.', + 'recruitment.NewsMailBatch.', + 'recruitment.Participation.', + 'recruitment.SubjectFilterGroup.id', + 'recruitment.SubjectFilterGroup.study', + 'sessions.', + 'studies.Resource.', + 'studies.StudyGroup.', + 'studies.Study.id', + 'studies.Study.mailbatch', + 'studies.Study.newsmailbatch', + 'studies.Study.participation', + 'studies.StudyMembership.id', + 'studies.StudyMembership.study', + 'studies.StudySession.appointment', + 'studies.StudySession.id', + 'studies.StudySession.study', + 'studies.StudyType.', + 'studies.StudyTypeEventFeed.', + 'studies.StudyTypeTranslation.', + 'subjects.', + + # already covered by other fields (e.g. backrefs) + 'pseudonyms.Domain.', # 'studies.Study.domains' + 'recruitment.SubjectFilter.', # recruitment.SubjectFilterGroup.subjectfilter + 'studies.Study.members', # studies.Study.studymembership_set + 'studies.StudyTag.', # studies.Study.studytag + + # intentionally left out + 'studies.Study.previous_status', + 'studies.Study.snapshot', + 'studies.Study.status', + 'studies.StudySession.domains', +] + + +def iter_fields(): + for app in apps.get_app_configs(): + for model in app.get_models(): + for field in model._meta.get_fields(): + s = '{}.{}.{}'.format(app.label, model._meta.object_name, field.name) + if all(not s.startswith(pattern) for pattern in EXCLUDED): + yield s + + +def test_study_diff_complete(client, member, study, subject_filter): + client.force_login(member) + + baker.make(StudySession, study=study) + baker.make(StudyTag, study=study) + + response = client.get('/studies/{}/diff/'.format(study.pk)) + + expected = set(iter_fields()) + actual = {s.decode('utf-8') for s in re.findall(b'data-fieldname="([^"]*)"', response.content)} + + assert actual == expected diff --git a/tests/utils/templatetags/test_diff_tags.py b/tests/utils/templatetags/test_diff_tags.py new file mode 100644 index 0000000000000000000000000000000000000000..be897fc969d266a40a63e819ccca85e065bc5806 --- /dev/null +++ b/tests/utils/templatetags/test_diff_tags.py @@ -0,0 +1,67 @@ +from django.core import serializers + +from model_bakery import baker + +from castellum.studies.models import Study +from castellum.studies.views.studies import take_snapshot +from castellum.utils.templatetags import diff + + +def get_snapshot(study): + take_snapshot(study) + return list(serializers.deserialize('json', study.snapshot, ignorenonexistent=True)) + + +def test_diff_class(study): + snapshot = get_snapshot(study) + assert diff.diff_class(snapshot, study, 'name') == 'diff-unchanged' + study.name = 'test' + assert diff.diff_class(snapshot, study, 'name') == 'diff-changed' + study.name = '' + assert diff.diff_class(snapshot, study, 'name') == 'diff-removed' + + snapshot = get_snapshot(study) + study.name = 'test' + assert diff.diff_class(snapshot, study, 'name') == 'diff-added' + + +def test_diff_get_m2m(study): + excluded1 = baker.make(Study, name='excluded1') + excluded2 = baker.make(Study, name='excluded2') + + study.excluded_studies.add(excluded1) + snapshot = get_snapshot(study) + + study.excluded_studies.add(excluded2) + items = diff.diff_get_m2m(snapshot, study, 'excluded_studies') + items = [(value.name, cls) for value, cls in items] + assert items == [('excluded1', 'diff-unchanged'), ('excluded2', 'diff-added')] + + study.excluded_studies.clear() + items = diff.diff_get_m2m(snapshot, study, 'excluded_studies') + items = [(value.name, cls) for value, cls in items] + assert items == [('excluded1', 'diff-removed')] + + excluded1.delete() + items = diff.diff_get_m2m(snapshot, study, 'excluded_studies') + items = [(value.name, cls) for value, cls in items] + assert items == [] + + +def test_diff_get_o2m(study): + study.studytag_set.create(name='tag1') + study.studytag_set.create(name='tag2') + study.studytag_set.create(name='tag3') + snapshot = get_snapshot(study) + + study.studytag_set.filter(name='tag2').update(name='tag2-edited') + study.studytag_set.filter(name='tag3').delete() + study.studytag_set.create(name='tag4') + items = diff.diff_get_o2m(snapshot, study, 'studytag_set') + items = [(value.name, cls) for value, cls in items] + assert items == [ + ('tag1', 'diff-unchanged'), + ('tag2-edited', 'diff-changed'), + ('tag4', 'diff-added'), + ('tag3', 'diff-removed'), + ]