From 4d293a5b80ebac65dc714c624556f540c2cf7403 Mon Sep 17 00:00:00 2001 From: nirmeen Date: Mon, 15 May 2023 14:15:03 +0200 Subject: [PATCH 1/5] Refactor 'is_published' field as abstract method and define it based on exposures_list in dataset and project, while other entities retain 'is_publishes' field with getter and setter methods. --- core/migrations/0033_auto_20230515_1353.py | 32 +++++++++++++++++++ core/models/cohort.py | 4 +-- core/models/dataset.py | 5 +++ core/models/partner.py | 4 +-- core/models/project.py | 4 ++- core/models/utils.py | 37 +++++++++++++++++----- elixir_daisy/settings.py | 1 + web/templates/search/_items/datasets.html | 1 + 8 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 core/migrations/0033_auto_20230515_1353.py diff --git a/core/migrations/0033_auto_20230515_1353.py b/core/migrations/0033_auto_20230515_1353.py new file mode 100644 index 00000000..15187a7c --- /dev/null +++ b/core/migrations/0033_auto_20230515_1353.py @@ -0,0 +1,32 @@ +# Generated by Django 3.2.19 on 2023-05-15 11:53 + +import core.models.utils +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0032_auto_20230508_1200'), + ] + + operations = [ + migrations.RenameField( + model_name='cohort', + old_name='is_published', + new_name='_is_published', + ), + migrations.RenameField( + model_name='partner', + old_name='is_published', + new_name='_is_published', + ), + migrations.RemoveField( + model_name='dataset', + name='is_published', + ), + migrations.RemoveField( + model_name='project', + name='is_published', + ), + ] diff --git a/core/models/cohort.py b/core/models/cohort.py index 31d873c2..6482d865 100644 --- a/core/models/cohort.py +++ b/core/models/cohort.py @@ -1,10 +1,10 @@ from django.db import models from django.conf import settings -from .utils import CoreTrackedModel, TextFieldWithInputWidget +from .utils import CoreTrackedDBModel, TextFieldWithInputWidget -class Cohort(CoreTrackedModel): +class Cohort(CoreTrackedDBModel): class Meta: app_label = 'core' get_latest_by = "added" diff --git a/core/models/dataset.py b/core/models/dataset.py index d41bed7d..906b8580 100644 --- a/core/models/dataset.py +++ b/core/models/dataset.py @@ -60,6 +60,11 @@ class AppMeta: verbose_name='Sensitivity class', help_text='Sensitivity denotes the security classification of this dataset.') + @property + def is_published(self): + exposures_list = self.exposures.all() + return len(exposures_list) > 0 + @property def data_types(self): all_data_types = set() diff --git a/core/models/partner.py b/core/models/partner.py index d5baf46d..5f3cac1c 100644 --- a/core/models/partner.py +++ b/core/models/partner.py @@ -3,7 +3,7 @@ from django_countries.fields import CountryField from model_utils import Choices -from .utils import CoreTrackedModel, TextFieldWithInputWidget +from .utils import CoreTrackedDBModel, TextFieldWithInputWidget from elixir_daisy import settings @@ -21,7 +21,7 @@ ) -class Partner(CoreTrackedModel): +class Partner(CoreTrackedDBModel): """ Represents a partner. { diff --git a/core/models/project.py b/core/models/project.py index 28ec540c..8265ecb7 100644 --- a/core/models/project.py +++ b/core/models/project.py @@ -151,7 +151,9 @@ def __str__(self): return self.acronym or self.title or "undefined" - + @property + def is_published(self): + return any(dataset.is_published for dataset in self.datasets.all()) def to_dict(self): contact_dicts = [] diff --git a/core/models/utils.py b/core/models/utils.py index f9a3f132..a843690f 100644 --- a/core/models/utils.py +++ b/core/models/utils.py @@ -1,3 +1,4 @@ +from abc import abstractmethod from json import loads from json.decoder import JSONDecodeError @@ -11,6 +12,7 @@ COMPANY = getattr(settings, "COMPANY", 'Company') + def validate_json(value): if len(value) == 0: return value @@ -18,12 +20,13 @@ def validate_json(value): try: loads(value) if '{' not in value: # Very inaccurate, but should do the trick when the user tries to save e.g. '123' - raise ValidationError(f'`scientific_metadata` field must be a valid JSON containing a dictionary!') + raise ValidationError(f'`scientific_metadata` field must be a valid JSON containing a dictionary!') return value except JSONDecodeError as ex: msg = str(ex) raise ValidationError(f'`scientific_metadata` field must contain a valid JSON! ({msg})') + class classproperty(property): def __get__(self, cls, owner): return self.fget.__get__(None, owner)() @@ -43,11 +46,6 @@ class CoreTrackedModel(CoreModel): blank=True, null=True, max_length=20) - - is_published = models.BooleanField( - default=False, - blank=False, - verbose_name='Is published?') scientific_metadata = models.TextField( default='{}', @@ -56,9 +54,14 @@ class CoreTrackedModel(CoreModel): verbose_name='Additional scientific metadata (in JSON format)', validators=[validate_json] # This will work in ModelForm only ) + class Meta: abstract = True + @abstractmethod + def is_published(self): + pass + def publish(self, save=True): generate_id_function_path = getattr(settings, 'IDSERVICE_FUNCTION') generate_id_function = import_string(generate_id_function_path) @@ -87,6 +90,25 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) +class CoreTrackedDBModel(CoreTrackedModel): + _is_published = models.BooleanField( + default=False, + blank=False, + verbose_name='Is published?') + + class Meta: + abstract = True + @property + def is_published(self): + # Getter method + return self._is_published + + @is_published.setter + def is_published(self, value): + # Setter method + self._is_published = value + + class TextFieldWithInputWidget(TextField): def formfield(self, **kwargs): @@ -105,11 +127,10 @@ class HashedField(models.CharField): A custom field that will store a hash of the provided value. """ description = "Keeps the hash of the string in the DB" - + def pre_save(self, model_instance, add): """ This function is called when the value is about to be saved to the DB. We hash the value and return it. """ value = getattr(model_instance, self.attname) return make_password(value, salt=settings.SECRET_KEY) - diff --git a/elixir_daisy/settings.py b/elixir_daisy/settings.py index 09236663..74c1584c 100644 --- a/elixir_daisy/settings.py +++ b/elixir_daisy/settings.py @@ -282,6 +282,7 @@ 'consent_status', 'data_types', 'deidentification_method', + 'is_published', ), 'contract': ( 'contacts', diff --git a/web/templates/search/_items/datasets.html b/web/templates/search/_items/datasets.html index 0aafb272..3a4b743a 100644 --- a/web/templates/search/_items/datasets.html +++ b/web/templates/search/_items/datasets.html @@ -6,6 +6,7 @@

{{ dataset.title }}

diff --git a/web/tests/views/test_project_views.py b/web/tests/views/test_project_views.py index 613d32bc..070fb86a 100644 --- a/web/tests/views/test_project_views.py +++ b/web/tests/views/test_project_views.py @@ -150,7 +150,7 @@ def test_project_edit_protected_documents(permissions, group): os.remove(document.content.name) @pytest.mark.parametrize('group', [VIPGroup, DataStewardGroup, LegalGroup, AuditorGroup]) -@pytest.mark.parametrize('url_name', ['project_publish', 'project_unpublish', 'projects_export']) +@pytest.mark.parametrize('url_name', ['projects_export']) def test_projects_publications_and_export(permissions, group, url_name): user = UserFactory(groups=[group()]) kwargs = {} diff --git a/web/urls.py b/web/urls.py index 0cac011f..4e02d282 100644 --- a/web/urls.py +++ b/web/urls.py @@ -24,7 +24,7 @@ PartnerEditView, partner_search_view, \ publish_partner, unpublish_partner from web.views.projects import ProjectCreateView, ProjectEditView, ProjectDetailView, \ - ProjectDelete, publish_project, unpublish_project, dsw_list_projects + ProjectDelete, dsw_list_projects from web.views.publication import PublicationCreateView, PublicationListView, \ PublicationEditView, add_publication_to_project, \ remove_publication_from_project, pick_publication_for_project @@ -164,8 +164,6 @@ path('project//', ProjectDetailView.as_view(), name="project"), path('project//delete', ProjectDelete.as_view(), name="project_delete"), path('project//edit', ProjectEditView.as_view(), name="project_edit"), - path('project//publish', publish_project, name="project_publish"), - path('project//unpublish', unpublish_project, name="project_unpublish"), path('project//add-contact', add_contact_to_project, name="add_contact_to_project"), path('project//add-dataset', datasets.DatasetCreateView.as_view(), name="datasets_add_to_project"), path('project//add-personnel', add_personnel_to_project, name="add_personnel_to_project"), diff --git a/web/views/api.py b/web/views/api.py index 62beacad..fffc5781 100644 --- a/web/views/api.py +++ b/web/views/api.py @@ -11,7 +11,7 @@ from django.core.paginator import Paginator from django.http import JsonResponse, HttpResponse, HttpResponseBadRequest from django.views.decorators.csrf import csrf_exempt -from django.db.models import Q +from django.db.models import Q, Count from django.contrib.auth.hashers import get_hasher from stronghold.decorators import public @@ -174,7 +174,7 @@ def get_filtered_entities(request, model_name): @protect_with_api_key def contracts(request): objects = get_filtered_entities(request, 'Contract') - objects = objects.filter(project__is_published=True) + objects = objects.anotate(c=Count("project__datasets__exposures")).filter(project__is_published=True, c__gt=0) if 'project_id' in request.GET: project_id = request.GET.get('project_id', '') objects = objects.filter(project__id=project_id) diff --git a/web/views/partner.py b/web/views/partner.py index 6e1e2619..39672c11 100644 --- a/web/views/partner.py +++ b/web/views/partner.py @@ -115,7 +115,7 @@ def unpublish_partner(request, pk): partner = get_object_or_404(Partner, pk=pk) if partner.is_published: partner.is_published = False - partner.save(update_fields=['is_published']) + partner.save(update_fields=['_is_published']) return HttpResponseRedirect(reverse_lazy('partner', kwargs={'pk': pk})) diff --git a/web/views/projects.py b/web/views/projects.py index 651e7192..03a35fec 100644 --- a/web/views/projects.py +++ b/web/views/projects.py @@ -3,6 +3,7 @@ from django.contrib.auth.decorators import user_passes_test from django.contrib.contenttypes.models import ContentType from django.db import transaction, IntegrityError +from django.db.models import Count from django.http import HttpResponse from django.shortcuts import render, get_object_or_404, redirect from django.urls import reverse_lazy @@ -267,26 +268,12 @@ def get_context_data(self, **kwargs): return context -@user_passes_test(is_data_steward) -def publish_project(request, pk): - project = get_object_or_404(Project, pk=pk) - project.publish() - return redirect(reverse_lazy('project', kwargs={'pk': project.id})) - - -@user_passes_test(is_data_steward) -def unpublish_project(request, pk): - project = get_object_or_404(Project, pk=pk) - project.is_published = False - project.save() - return redirect(reverse_lazy('project', kwargs={'pk': project.id})) - - def dsw_list_projects(request): #if data steward or admin -> list all the public projects # if(request.user.is_admin() | request.user.is_datasteward()): # objects = Project.objects.all().filter(is_published=True) - objects = (Project.objects.filter(local_custodians=request.user, is_published=True) | Project.objects.filter(company_personnel=request.user, is_published=True)).distinct() + objects = (Project.objects.filter(local_custodians=request.user) | Project.objects.filter(company_personnel=request.user)).distinct() + objects = objects.annotate(c=Count('datasets__exposures')).filter(c__gt=0) return render(request, 'integrations/dsw/project_list.html', { 'dsw_origin': getattr(settings, 'DSW_ORIGIN', 'localhost'), 'projects': [{'url': reverse('project', args=[str(project.id)]), From b89e528a34a13f3341b66dbd1173c02779262149 Mon Sep 17 00:00:00 2001 From: nirmeen Date: Tue, 16 May 2023 14:30:26 +0200 Subject: [PATCH 3/5] refactored publish method as an abstract method in coretrackedmodel and used it in dataset view --- core/models/project.py | 3 +++ core/models/utils.py | 4 ++++ web/views/exposure.py | 7 +++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/core/models/project.py b/core/models/project.py index 8265ecb7..bc5511e2 100644 --- a/core/models/project.py +++ b/core/models/project.py @@ -219,6 +219,9 @@ def serialize_to_export(self): d['publications'] = ','.join(publications) return d + def publish(self): + pass + # faster lookup for permissions # https://django-guardian.readthedocs.io/en/stable/userguide/performance.html#direct-foreign-keys class ProjectUserObjectPermission(UserObjectPermissionBase): diff --git a/core/models/utils.py b/core/models/utils.py index 28a37625..b9a52d1a 100644 --- a/core/models/utils.py +++ b/core/models/utils.py @@ -62,6 +62,10 @@ class Meta: def is_published(self): pass + @abstractmethod + def publish(self): + pass + def generate_elu_accession(self, save=True): generate_id_function_path = getattr(settings, 'IDSERVICE_FUNCTION') generate_id_function = import_string(generate_id_function_path) diff --git a/web/views/exposure.py b/web/views/exposure.py index 01df2349..7f6e7d33 100644 --- a/web/views/exposure.py +++ b/web/views/exposure.py @@ -20,9 +20,11 @@ class DataStewardGroupRequiredMixin(UserPassesTestMixin): def test_func(self): return can_publish(self.request.user) + class ExposureCreateView(DataStewardGroupRequiredMixin, CreateView, AjaxViewMixin): model = Exposure form_class = ExposureForm + def dispatch(self, request, *args, **kwargs): """ Hook method to save related dataset. @@ -41,6 +43,8 @@ def form_valid(self, form): self.object.save() messages.add_message(self.request, messages.SUCCESS, 'exposure endpoint created') self.dataset.generate_elu_accession() + # publish subentities + self.dataset.publish() return super().form_valid(form) def get_form_kwargs(self): @@ -56,6 +60,7 @@ def get_success_url(self, **kwargs): class ExposureEditView(DataStewardGroupRequiredMixin, UpdateView, AjaxViewMixin): model = Exposure form_class = ExposureEditForm + def dispatch(self, request, *args, **kwargs): """ Hook method to save related dataset and endpoint. @@ -102,5 +107,3 @@ def remove_exposure(request, dataset_pk, exposure_pk): exposure.delete() messages.add_message(request, messages.SUCCESS, 'exposure record deleted.') return HttpResponse("exposure deleted") - - From 2fcfa378b70e71fcc8737aad06628429f89bd68a Mon Sep 17 00:00:00 2001 From: nirmeen Date: Tue, 16 May 2023 16:35:04 +0200 Subject: [PATCH 4/5] added generate_elu_accession to publish in dataset model --- core/models/dataset.py | 11 ++++++++++- core/models/utils.py | 8 -------- web/views/exposure.py | 3 +-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/core/models/dataset.py b/core/models/dataset.py index f17a8eca..ef990c7e 100644 --- a/core/models/dataset.py +++ b/core/models/dataset.py @@ -3,6 +3,8 @@ from django.conf import settings from django.db import models from django.urls import reverse +from django.utils.module_loading import import_string + from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase from core import constants from core.permissions.mapping import PERMISSION_MAPPING @@ -167,7 +169,14 @@ def serialize_to_export(self): d['legal_bases'] = legal_bases return d - def publish(self): + def publish(self, save=True): + generate_id_function_path = getattr(settings, 'IDSERVICE_FUNCTION') + generate_id_function = import_string(generate_id_function_path) + if not self.elu_accession: + self.elu_accession = generate_id_function(self) + if save: + self.save(update_fields=['elu_accession']) + for data_declaration in self.data_declarations.all(): data_declaration.publish_subentities() diff --git a/core/models/utils.py b/core/models/utils.py index b9a52d1a..0e93bc0b 100644 --- a/core/models/utils.py +++ b/core/models/utils.py @@ -66,14 +66,6 @@ def is_published(self): def publish(self): pass - def generate_elu_accession(self, save=True): - generate_id_function_path = getattr(settings, 'IDSERVICE_FUNCTION') - generate_id_function = import_string(generate_id_function_path) - if not self.elu_accession: - self.elu_accession = generate_id_function(self) - if save: - self.save(update_fields=['elu_accession']) - def clean(self): cleaned_data = super().clean() validate_json(self.scientific_metadata) diff --git a/web/views/exposure.py b/web/views/exposure.py index 7f6e7d33..f62c20f0 100644 --- a/web/views/exposure.py +++ b/web/views/exposure.py @@ -42,8 +42,7 @@ def form_valid(self, form): self.object.dataset = self.dataset self.object.save() messages.add_message(self.request, messages.SUCCESS, 'exposure endpoint created') - self.dataset.generate_elu_accession() - # publish subentities + # generate elu accession for dataset & publish subentities self.dataset.publish() return super().form_valid(form) From dc2a8516e148171b3a812581734459025b289308 Mon Sep 17 00:00:00 2001 From: nirmeen Date: Wed, 17 May 2023 12:31:27 +0200 Subject: [PATCH 5/5] saved dataset on exposure delete to trigger reindex and renamed is_published migration and linked it to last migration on develop --- .../{0033_auto_20230515_1353.py => 0034_auto_20230515_1353.py} | 2 +- web/views/exposure.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) rename core/migrations/{0033_auto_20230515_1353.py => 0034_auto_20230515_1353.py} (94%) diff --git a/core/migrations/0033_auto_20230515_1353.py b/core/migrations/0034_auto_20230515_1353.py similarity index 94% rename from core/migrations/0033_auto_20230515_1353.py rename to core/migrations/0034_auto_20230515_1353.py index 15187a7c..f079b5ab 100644 --- a/core/migrations/0033_auto_20230515_1353.py +++ b/core/migrations/0034_auto_20230515_1353.py @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ - ('core', '0032_auto_20230508_1200'), + ('core', '0033_auto_20230515_1147'), ] operations = [ diff --git a/web/views/exposure.py b/web/views/exposure.py index f62c20f0..fdbdfcf1 100644 --- a/web/views/exposure.py +++ b/web/views/exposure.py @@ -104,5 +104,8 @@ def remove_exposure(request, dataset_pk, exposure_pk): exposure = get_object_or_404(Exposure, pk=exposure_pk) if exposure.dataset == dataset: exposure.delete() + if len(dataset.exposures.all()) < 1: + # needed to trigger reindex of dataset and changing dataset.is_published to false + dataset.save() messages.add_message(request, messages.SUCCESS, 'exposure record deleted.') return HttpResponse("exposure deleted")