From e0c0741069fd83183fbbef31cb512a347a5b92f3 Mon Sep 17 00:00:00 2001 From: Natasha Singh Date: Tue, 9 Feb 2021 17:26:31 -0500 Subject: [PATCH 1/5] :sparkles: Skeleton Data Reviews app --- creator/data_reviews/__init__.py | 0 creator/data_reviews/apps.py | 5 + creator/data_reviews/factories.py | 17 +++ creator/data_reviews/models.py | 31 ++++++ creator/data_reviews/mutations.py | 83 ++++++++++++++ creator/data_reviews/nodes.py | 29 +++++ creator/data_reviews/queries.py | 35 ++++++ creator/data_reviews/schema.py | 2 + creator/groups.py | 5 + creator/schema.py | 3 + creator/settings/development.py | 3 +- creator/settings/production.py | 1 + creator/settings/testing.py | 1 + .../test_data_reviews_mutations.py | 102 ++++++++++++++++++ .../data_reviews/test_data_reviews_queries.py | 79 ++++++++++++++ 15 files changed, 395 insertions(+), 1 deletion(-) create mode 100644 creator/data_reviews/__init__.py create mode 100644 creator/data_reviews/apps.py create mode 100644 creator/data_reviews/factories.py create mode 100644 creator/data_reviews/models.py create mode 100644 creator/data_reviews/mutations.py create mode 100644 creator/data_reviews/nodes.py create mode 100644 creator/data_reviews/queries.py create mode 100644 creator/data_reviews/schema.py create mode 100644 tests/data_reviews/test_data_reviews_mutations.py create mode 100644 tests/data_reviews/test_data_reviews_queries.py diff --git a/creator/data_reviews/__init__.py b/creator/data_reviews/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/creator/data_reviews/apps.py b/creator/data_reviews/apps.py new file mode 100644 index 000000000..2b2e34a23 --- /dev/null +++ b/creator/data_reviews/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class DataReviewsConfig(AppConfig): + name = "creator.data_reviews" diff --git a/creator/data_reviews/factories.py b/creator/data_reviews/factories.py new file mode 100644 index 000000000..b983c3b98 --- /dev/null +++ b/creator/data_reviews/factories.py @@ -0,0 +1,17 @@ +import pytz +import factory +from django.contrib.auth import get_user_model +from creator.data_reviews.models import DataReview +from creator.users.factories import UserFactory + +User = get_user_model() + + +class DataReviewFactory(factory.DjangoModelFactory): + class Meta: + model = DataReview + + created_at = factory.Faker( + "date_time_between", start_date="-2y", end_date="now", tzinfo=pytz.UTC + ) + creator = factory.SubFactory(UserFactory) diff --git a/creator/data_reviews/models.py b/creator/data_reviews/models.py new file mode 100644 index 000000000..b093dc724 --- /dev/null +++ b/creator/data_reviews/models.py @@ -0,0 +1,31 @@ +from django.db import models +from django.contrib.auth import get_user_model + +User = get_user_model() + + +class DataReview(models.Model): + """ + The iterative process between a data contributor and the Data Resource + Center where study data is continually supplied, validated, and refined + to meet quality standards. + """ + + class Meta: + permissions = [ + ("list_all_datareview", "Show all data_reviews"), + ] + + created_at = models.DateTimeField( + auto_now_add=True, + null=False, + help_text="Time when the data_review was created" + ) + creator = models.ForeignKey( + User, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="data_reviews", + help_text="The user who created the data review", + ) diff --git a/creator/data_reviews/mutations.py b/creator/data_reviews/mutations.py new file mode 100644 index 000000000..6fe440edb --- /dev/null +++ b/creator/data_reviews/mutations.py @@ -0,0 +1,83 @@ +import graphene +from graphql import GraphQLError +from graphql_relay import from_global_id + +from django.conf import settings +from creator.data_reviews.nodes import DataReviewNode +from creator.data_reviews.models import DataReview + + +class CreateDataReviewInput(graphene.InputObjectType): + """ Parameters used when creating a new data_review """ + + name = graphene.String(description="The name of the data_review") + + +class UpdateDataReviewInput(graphene.InputObjectType): + """ Parameters used when updating an existing data_review """ + + name = graphene.String(description="The name of the data_review") + + +class CreateDataReviewMutation(graphene.Mutation): + """ Creates a new data_review """ + + class Arguments: + input = CreateDataReviewInput( + required=True, description="Attributes for the new data_review" + ) + + data_review = graphene.Field(DataReviewNode) + + def mutate(self, info, input): + """ + Creates a new data_review. + """ + user = info.context.user + if not user.has_perm("data_reviews.add_datareview"): + raise GraphQLError("Not allowed") + + data_review = DataReview() + return CreateDataReviewMutation(data_review=data_review) + + +class UpdateDataReviewMutation(graphene.Mutation): + """ Update an existing data_review """ + + class Arguments: + id = graphene.ID( + required=True, description="The ID of the data_review to update" + ) + input = UpdateDataReviewInput( + required=True, description="Attributes for the data_review" + ) + + data_review = graphene.Field(DataReviewNode) + + def mutate(self, info, id, input): + """ + Updates an existing data_review + """ + user = info.context.user + if not user.has_perm("data_reviews.change_datareview"): + raise GraphQLError("Not allowed") + + model, node_id = from_global_id(id) + + try: + data_review = DataReview.objects.get(id=node_id) + except DataReview.DoesNotExist: + raise GraphQLError("DataReview was not found") + + return UpdateDataReviewMutation(data_review=data_review) + + +class Mutation: + """ Mutations for data_reviews """ + + create_data_review = CreateDataReviewMutation.Field( + description="Create a new data_review." + ) + update_data_review = UpdateDataReviewMutation.Field( + description="Update a given data_review" + ) diff --git a/creator/data_reviews/nodes.py b/creator/data_reviews/nodes.py new file mode 100644 index 000000000..50d8fa021 --- /dev/null +++ b/creator/data_reviews/nodes.py @@ -0,0 +1,29 @@ +from graphene import relay +from graphql import GraphQLError +from graphene_django import DjangoObjectType + +from creator.data_reviews.models import DataReview + + +class DataReviewNode(DjangoObjectType): + class Meta: + model = DataReview + interfaces = (relay.Node,) + filter_fields = () + + @classmethod + def get_node(cls, info, id): + """ + Check permissions and return + """ + user = info.context.user + + if not (user.has_perm("data_reviews.view_datareview")): + raise GraphQLError("Not allowed") + + try: + data_review = cls._meta.model.objects.get(id=id) + except cls._meta.model.DoesNotExist: + raise GraphQLError("DataReviews was not found") + + return data_review diff --git a/creator/data_reviews/queries.py b/creator/data_reviews/queries.py new file mode 100644 index 000000000..13beaf83e --- /dev/null +++ b/creator/data_reviews/queries.py @@ -0,0 +1,35 @@ +from graphene import relay +from graphene_django.filter import DjangoFilterConnectionField +from graphql import GraphQLError +from django_filters import FilterSet, OrderingFilter + +from creator.data_reviews.nodes import DataReviewNode +from creator.data_reviews.models import DataReview + + +class DataReviewFilter(FilterSet): + order_by = OrderingFilter(fields=("created_on",)) + + class Meta: + model = DataReview + fields = [] + + +class Query(object): + data_review = relay.Node.Field(DataReviewNode, description="Get a single data_review") + all_data_reviews = DjangoFilterConnectionField( + DataReviewNode, + filterset_class=DataReviewFilter, + description="Get all data_reviews", + ) + + def resolve_all_data_reviews(self, info, **kwargs): + """ + Return all data_reviews + """ + user = info.context.user + + if not user.has_perm("data_reviews.list_all_datareview"): + raise GraphQLError("Not allowed") + + return DataReview.objects.all() diff --git a/creator/data_reviews/schema.py b/creator/data_reviews/schema.py new file mode 100644 index 000000000..82d1b8261 --- /dev/null +++ b/creator/data_reviews/schema.py @@ -0,0 +1,2 @@ +from creator.data_reviews.queries import Query +from creator.data_reviews.mutations import Mutation diff --git a/creator/groups.py b/creator/groups.py index 2fbacd9f4..36e0dc3bc 100644 --- a/creator/groups.py +++ b/creator/groups.py @@ -79,6 +79,11 @@ "change_banner", "view_banner", "list_all_banner", + "add_datareview", + "delete_datareview", + "change_datareview", + "view_datareview", + "list_all_datareview", ], "Developers": [ "view_study", diff --git a/creator/schema.py b/creator/schema.py index 5c00a93f5..c939b54cc 100644 --- a/creator/schema.py +++ b/creator/schema.py @@ -17,6 +17,7 @@ import creator.status.schema import creator.jobs.schema import creator.releases.schema +import creator.data_reviews.schema class Query( @@ -31,6 +32,7 @@ class Query( creator.status.schema.Query, creator.jobs.schema.Query, creator.releases.schema.Query, + creator.data_reviews.schema.Query, graphene.ObjectType, ): """ Root query schema combining all apps' schemas """ @@ -48,6 +50,7 @@ class Mutation( creator.referral_tokens.schema.Mutation, creator.status.schema.Mutation, creator.releases.schema.Mutation, + creator.data_reviews.schema.Mutation, graphene.ObjectType, ): """ Root mutation schema combining all apps' schemas """ diff --git a/creator/settings/development.py b/creator/settings/development.py index 7bbe5ad7b..c8665881d 100644 --- a/creator/settings/development.py +++ b/creator/settings/development.py @@ -63,6 +63,7 @@ "creator.extract_configs", "creator.jobs", "creator.releases", + "creator.data_reviews", "creator.events.apps.EventsConfig", "creator", "corsheaders", @@ -253,7 +254,7 @@ STATIC_ROOT = os.path.join(BASE_DIR, "static") -## Email +# Email EMAIL_BACKEND = os.environ.get( "EMAIL_BACKEND", "django.core.mail.backends.smtp.EmailBackend" ) diff --git a/creator/settings/production.py b/creator/settings/production.py index 1f5413501..381e03d16 100644 --- a/creator/settings/production.py +++ b/creator/settings/production.py @@ -99,6 +99,7 @@ def traces_sampler(sampling_context): "creator.extract_configs", "creator.jobs", "creator.releases", + "creator.data_reviews", "creator.events.apps.EventsConfig", "creator", "corsheaders", diff --git a/creator/settings/testing.py b/creator/settings/testing.py index 7a2b94038..599658458 100644 --- a/creator/settings/testing.py +++ b/creator/settings/testing.py @@ -64,6 +64,7 @@ "creator.extract_configs", "creator.jobs", "creator.releases", + "creator.data_reviews", "creator.events.apps.EventsConfig", "creator", "corsheaders", diff --git a/tests/data_reviews/test_data_reviews_mutations.py b/tests/data_reviews/test_data_reviews_mutations.py new file mode 100644 index 000000000..6647be629 --- /dev/null +++ b/tests/data_reviews/test_data_reviews_mutations.py @@ -0,0 +1,102 @@ +import pytest +from graphql_relay import to_global_id + +from creator.data_reviews.models import DataReview +from creator.data_reviews.factories import DataReviewFactory + + +CREATE_DATA_REVIEW = """ +mutation ($input: CreateDataReviewInput!) { + createDataReview(input: $input) { + dataReview { + id + } + } +} +""" + +UPDATE_DATA_REVIEW = """ +mutation ($id: ID!, $input: UpdateDataReviewInput!) { + updateDataReview(id: $id, input: $input) { + dataReview { + id + } + } +} +""" + + +@pytest.mark.parametrize( + "user_group,allowed", + [ + ("Administrators", True), + ("Services", False), + ("Developers", False), + ("Investigators", False), + ("Bioinformatics", False), + (None, False), + ], +) +def test_create_data_review(db, clients, user_group, allowed): + """ + Test the create mutation. + """ + client = clients.get(user_group) + + resp = client.post( + "/graphql", + data={ + "query": CREATE_DATA_REVIEW, + "variables": {"input": {"name": "Test"}} + }, + content_type="application/json", + ) + + if allowed: + assert ( + resp.json()["data"]["createDataReview"][ + "dataReview"] + is not None + ) + else: + assert resp.json()["errors"][0]["message"] == "Not allowed" + + +@pytest.mark.parametrize( + "user_group,allowed", + [ + ("Administrators", True), + ("Services", False), + ("Developers", False), + ("Investigators", False), + ("Bioinformatics", False), + (None, False), + ], +) +def test_update_data_review(db, clients, user_group, allowed): + """ + Test the update mutation. + """ + client = clients.get(user_group) + + data_review = DataReviewFactory() + + resp = client.post( + "/graphql", + data={ + "query": UPDATE_DATA_REVIEW, + "variables": { + "id": to_global_id("DataReviewNode}}", data_review.id), + "input": {"name": "test"}, + }, + }, + content_type="application/json", + ) + + if allowed: + assert ( + resp.json()["data"]["updateDataReview"]["dataReview"] + is not None + ) + else: + assert resp.json()["errors"][0]["message"] == "Not allowed" diff --git a/tests/data_reviews/test_data_reviews_queries.py b/tests/data_reviews/test_data_reviews_queries.py new file mode 100644 index 000000000..1892cc423 --- /dev/null +++ b/tests/data_reviews/test_data_reviews_queries.py @@ -0,0 +1,79 @@ +import pytest +from graphql_relay import to_global_id +from creator.data_reviews.factories import DataReviewFactory + +DATA_REVIEW = """ +query ($id: ID!) { + dataReview(id: $id) { + id + } +} +""" + +ALL_DATA_REVIEWS = """ +query { + allDataReviews { + edges { node { id } } + } +} +""" + + +@pytest.mark.parametrize( + "user_group,allowed", + [ + ("Administrators", True), + ("Services", False), + ("Developers", False), + ("Investigators", False), + ("Bioinformatics", False), + (None, False), + ], +) +def test_query_data_review(db, clients, user_group, allowed): + client = clients.get(user_group) + + data_review = DataReviewFactory() + + variables = {"id": to_global_id("DataReviewNode", data_review.id)} + + resp = client.post( + "/graphql", + data={"query": DATA_REVIEW, "variables": variables}, + content_type="application/json", + ) + + if allowed: + assert ( + resp.json()["data"]["dataReview"]["id"] == + to_global_id("DataReviewNode", data_review.id) + ) + else: + assert resp.json()["errors"][0]["message"] == "Not allowed" + + +@pytest.mark.parametrize( + "user_group,allowed", + [ + ("Administrators", True), + ("Services", False), + ("Developers", False), + ("Investigators", False), + ("Bioinformatics", False), + (None, False), + ], +) +def test_query_all_data_reviews(db, clients, user_group, allowed): + client = clients.get(user_group) + + data_review = DataReviewFactory.create_batch(5) + + resp = client.post( + "/graphql", + data={"query": ALL_DATA_REVIEWS}, content_type="application/json" + ) + + if allowed: + assert len(resp.json()["data"]["allDataReviews"]["edges"]) == 5 + else: + assert resp.json()["errors"][0]["message"] == "Not allowed" From 0d6078ea99db84e59d40ed82e26dc0e56fa625ef Mon Sep 17 00:00:00 2001 From: Natasha Singh Date: Tue, 23 Feb 2021 14:58:00 -0500 Subject: [PATCH 2/5] :sparkles: Add DataReview model and event signals --- creator/data_reviews/__init__.py | 1 + creator/data_reviews/apps.py | 3 + creator/data_reviews/models.py | 130 ++++++++++++++++++++++++++++++- creator/data_reviews/signals.py | 102 ++++++++++++++++++++++++ creator/events/models.py | 15 ++++ 5 files changed, 247 insertions(+), 4 deletions(-) create mode 100644 creator/data_reviews/signals.py diff --git a/creator/data_reviews/__init__.py b/creator/data_reviews/__init__.py index e69de29bb..f858d6020 100644 --- a/creator/data_reviews/__init__.py +++ b/creator/data_reviews/__init__.py @@ -0,0 +1 @@ +default_app_config = "creator.data_reviews.apps.DataReviewsConfig" diff --git a/creator/data_reviews/apps.py b/creator/data_reviews/apps.py index 2b2e34a23..a4a4b32da 100644 --- a/creator/data_reviews/apps.py +++ b/creator/data_reviews/apps.py @@ -3,3 +3,6 @@ class DataReviewsConfig(AppConfig): name = "creator.data_reviews" + + def ready(self): + import creator.data_reviews.signals diff --git a/creator/data_reviews/models.py b/creator/data_reviews/models.py index b093dc724..1b662eed3 100644 --- a/creator/data_reviews/models.py +++ b/creator/data_reviews/models.py @@ -1,13 +1,31 @@ +import uuid from django.db import models from django.contrib.auth import get_user_model +from django_fsm import FSMField, transition + +from creator.fields import kf_id_generator +from creator.studies.models import Study +from creator.files.models import Version User = get_user_model() +def data_review_id(): + return kf_id_generator("DR") + + +class State(object): + NOT_STARTED = "not_started" + IN_REVIEW = "in_review" + WAITING = "awaiting_updates" + COMPLETED = "completed" + CLOSED = "closed" + + class DataReview(models.Model): """ - The iterative process between a data contributor and the Data Resource - Center where study data is continually supplied, validated, and refined + The iterative process between a data contributor and the Data Resource + Center where study data is continually supplied, validated, and refined to meet quality standards. """ @@ -16,10 +34,19 @@ class Meta: ("list_all_datareview", "Show all data_reviews"), ] + kf_id = models.CharField( + max_length=11, + primary_key=True, + default=data_review_id, + help_text="Kids First ID assigned to the data review", + ) + uuid = models.UUIDField( + default=uuid.uuid4, help_text="UUID used internally" + ) created_at = models.DateTimeField( auto_now_add=True, - null=False, - help_text="Time when the data_review was created" + null=True, + help_text="Time when the data_review was created", ) creator = models.ForeignKey( User, @@ -29,3 +56,98 @@ class Meta: related_name="data_reviews", help_text="The user who created the data review", ) + name = models.CharField( + max_length=256, help_text="Name of the data review" + ) + description = models.TextField( + max_length=10000, + null=True, + blank=True, + help_text="Description of data review", + ) + state = FSMField( + default=State.NOT_STARTED, + help_text="The current state of the data review", + ) + study = models.ForeignKey( + Study, + on_delete=models.CASCADE, + null=True, + related_name="data_reviews", + ) + versions = models.ManyToManyField( + Version, + related_name="data_reviews", + help_text="kf_ids of the file versions in this data review", + ) + + @transition(field=state, source=State.NOT_STARTED, target=State.IN_REVIEW) + def start(self): + """ Start the data review """ + + self._save_event( + "DR_STA", + f"{self.creator.username} started a data review", + ) + + @transition(field=state, source=State.IN_REVIEW, target=State.WAITING) + def wait_for_updates(self): + """ Feedback was sent to data contributors, now wait for updates """ + + self._save_event( + "DR_WAI", + "Data review is waiting for updates", + ) + + @transition(field=state, source=State.WAITING, target=State.IN_REVIEW) + def receive_updates(self): + """ Receive and process updates to data review """ + + self._save_event( + "DR_UPD", + "Data review received file updates", + ) + + @transition(field=state, source=State.IN_REVIEW, target=State.COMPLETED) + def approve(self): + """ Approve/complete the data review """ + + self._save_event( + "DR_APP", + f"{self.creator.username} completed the data review", + ) + + @transition( + field=state, + source=[State.NOT_STARTED, State.IN_REVIEW, State.WAITING], + target=State.CLOSED, + ) + def close(self): + """ Close the data review before it is completed """ + + self._save_event( + "DR_CLO", + f"{self.creator.username} closed the data review", + ) + + @transition(field=state, source=State.CLOSED, target=State.IN_REVIEW) + def reopen(self): + """ Re-open a closed data review """ + + self._save_event( + "DR_REO", + f"{self.creator.username} re-opened the data review", + ) + + def _save_event(self, event_type, message): + """ Create and save an event for a data review state transition """ + + from creator.events.models import Event + + return Event( + study=self.study, + user=self.creator, + data_review=self, + description=message, + event_type=event_type, + ).save() diff --git a/creator/data_reviews/signals.py b/creator/data_reviews/signals.py new file mode 100644 index 000000000..38e5624b9 --- /dev/null +++ b/creator/data_reviews/signals.py @@ -0,0 +1,102 @@ +from django.dispatch import receiver +from django.db.models.signals import pre_delete, post_save + +from creator.files.models import Version +from creator.events.models import Event +from creator.data_reviews.models import DataReview, State + + +@receiver(pre_delete, sender=Version) +def file_version_pre_delete(sender, instance, using, *args, **kwargs): + """ + When a file Version is deleted, update related DataReview states and fire + Data Review update events + + Scenario 1 + ----------------------------------- + Create Study - fa.v1, fb.v1, fc.v1 + Start DataReview - fa.v1, fb.v1 + Version deleted - fc.v1 + + Is version part of any Data Review + No - do nothing + + Scenario 2 - continuation of 1 + ----------------------------------- + Study - fa.v1, fb.v1 + DataReview - fa.v1, fb.v1 + Version deleted - fa.v1 + + Is version part of any Data Review + Yes - update all related review's state, emit event + """ + for dr in instance.data_reviews.all(): + if dr.state == State.WAITING: + dr.receive_updates() + dr.save() + Event( + study=dr.study, + file=instance.root_file, + user=dr.creator, + data_review=dr, + description=( + "File version {instance.pk} was deleted from data review " + f"{dr.pk}" + ), + event_type="DR_UPD", + ).save() + + +@receiver(post_save, sender=Version) +def file_version_post_save(sender, instance, created, *args, **kwargs): + """ + When a file Version is created, update the states of the related + DataReview and fire events + + Scenario 1 + ----------------------------------- + Create Study - fa.v1, fb.v1, fc.v1 + Start DataReview - fa.v1, fb.v1 + Receive new version - fc.v2 + + Check if instance.root_file has a version that is part of any Data Review + No - do nothing + + Scenario 2 - continuation of 1 + ----------------------------------- + Study - fa.v1, fb.v1, fc.v2 + DataReview - fa.v1, fb.v1 + Receive new version - fa.v2 + + Check if instance.root_file has a version that is part of any Data Review + Yes - update all related review's state, emit event + """ + # Only process new versions with root files + if (not created) or (not instance.root_file): + return + + # For non-terminal data reviews involving the root file of the version + # that was uploaded, update the reviews' state + emit events + for dr in ( + DataReview.objects.exclude(state__in={State.COMPLETED, State.CLOSED}) + .filter( + versions__in=Version.objects.filter(root_file=instance.root_file) + ) + .distinct() + ): + if dr.state == State.WAITING: + dr.receive_updates() + dr.save() + Event( + study=instance.root_file.study, + file=instance.root_file, + version=instance, + user=instance.creator, + data_review=dr, + description=( + f"New version {instance.pk} received for file " + f"{instance.root_file.pk} which is part of open data " + f"review {dr.pk}" + ), + event_type="DR_UPD", + ).save() diff --git a/creator/events/models.py b/creator/events/models.py index 364966e09..edcceabaf 100644 --- a/creator/events/models.py +++ b/creator/events/models.py @@ -6,6 +6,7 @@ from creator.files.models import File, Version from creator.projects.models import Project from creator.buckets.models import Bucket +from creator.data_reviews.models import DataReview User = get_user_model() @@ -67,6 +68,12 @@ class Meta: ("SL_STR", "Slack Channel Creation Start"), ("SL_ERR", "Slack Channel Creation Error"), ("SL_SUC", "Slack Channel Creation Success"), + ("DR_STA", "Data Review Started"), + ("DR_WAI", "Data Review Waiting for Updates"), + ("DR_UPD", "Data Review Updated"), + ("DR_APP", "Data Review Approved"), + ("DR_CLO", "Data Review Closed"), + ("DR_REO", "Data Review Re-opened"), ("OTH", "Other"), ), default="OTH", @@ -125,3 +132,11 @@ class Meta: related_name="events", help_text="User related to this event", ) + data_review = models.ForeignKey( + DataReview, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="events", + help_text="Data Review related to this event", + ) From fa90ea78072a6e7dd9847979d621558ce1287d6d Mon Sep 17 00:00:00 2001 From: Natasha Singh Date: Tue, 23 Feb 2021 14:58:42 -0500 Subject: [PATCH 3/5] :sparkles: Add DataReview GraphQL API --- creator/data_reviews/mutations.py | 298 +++++++++++++++++++++++++++++- creator/data_reviews/nodes.py | 2 +- creator/data_reviews/queries.py | 7 +- 3 files changed, 295 insertions(+), 12 deletions(-) diff --git a/creator/data_reviews/mutations.py b/creator/data_reviews/mutations.py index 6fe440edb..729a0c266 100644 --- a/creator/data_reviews/mutations.py +++ b/creator/data_reviews/mutations.py @@ -1,22 +1,102 @@ +from django_fsm import TransitionNotAllowed import graphene from graphql import GraphQLError from graphql_relay import from_global_id +from django.db import transaction -from django.conf import settings from creator.data_reviews.nodes import DataReviewNode -from creator.data_reviews.models import DataReview +from creator.studies.models import Study +from creator.files.models import Version +from creator.data_reviews.models import DataReview, State + + +def check_review_files(input, data_review): + """ + Validate version ids in the create/update data review mutation input + + Convert base64 encoded Version ids to Version primary keys + Check if file versions exist and they are from same study + Check if input file versions are different than existing review versions + + :param input: input parameters from create/update data review mutation + :type input: dict + :param data_review: current data review object being created or updated + :type data_review: DataReview + + :returns: list of version ids if version ids are valid or None if + there are no version ids in the input or the input versions didn't change + from the existing version ids in the data_review + """ + if "versions" not in input: + return None + + input_version_ids = [] + for v in input["versions"]: + _, version_id = from_global_id(v) + input_version_ids.append(version_id) + + # Check all versions exist + versions = ( + Version.objects.filter(pk__in=input_version_ids) + .select_related("root_file__study") + .all() + ) + if len(versions) != len(input["versions"]): + raise GraphQLError( + "Error in modifying data_review. All file versions in data " + "review must exist." + ) + + # Check all versions come from same study + studies = set( + [v.root_file.study.pk for v in versions] + [data_review.study.pk] + ) + if len(studies) > 1: + raise GraphQLError( + "Error in modifying data_review. All file versions in data " + "review must have files that belong to the same study." + ) + + # Check if data review file versions changed + if data_review: + if set(data_review.versions.values_list("pk", flat=True)) == set( + input_version_ids + ): + input_version_ids = None + + return input_version_ids class CreateDataReviewInput(graphene.InputObjectType): """ Parameters used when creating a new data_review """ - name = graphene.String(description="The name of the data_review") + name = graphene.String( + required=True, description="The name of the data_review" + ) + description = graphene.String( + required=True, description="The description of the data_review" + ) + study = graphene.ID( + required=True, + description="The ID of the study this data_review is for", + ) + versions = graphene.List( + graphene.ID, + description="File versions in this data_review", + ) class UpdateDataReviewInput(graphene.InputObjectType): """ Parameters used when updating an existing data_review """ name = graphene.String(description="The name of the data_review") + description = graphene.String( + description="The description of the data_review" + ) + versions = graphene.List( + graphene.ID, + description="File versions in this data_review", + ) class CreateDataReviewMutation(graphene.Mutation): @@ -34,10 +114,48 @@ def mutate(self, info, input): Creates a new data_review. """ user = info.context.user - if not user.has_perm("data_reviews.add_datareview"): + + # Check if study exists + _, study_id = from_global_id(input["study"]) + try: + study = Study.objects.get(pk=study_id) + except Study.DoesNotExist: + raise GraphQLError(f"Study {study_id} not found.") + + # Check permissions + # Must have general add permission for any study OR + # permission to add review to user's studies + if not ( + user.has_perm("data_reviews.add_datareview") + or ( + user.has_perm("data_reviews.add_my_study_datareview") + and user.studies.filter(kf_id=study.kf_id).exists() + ) + ): raise GraphQLError("Not allowed") - data_review = DataReview() + # Create data_review + with transaction.atomic(): + data_review = DataReview( + **{ + k: input[k] + for k in input + if k not in {"study", "versions"} + } + ) + data_review.study = study + data_review.creator = user + data_review.save() + + # Check files in review + review_version_ids = check_review_files(input, data_review) + + if review_version_ids: + data_review.versions.set(review_version_ids) + # Start review if we have files + data_review.start() + data_review.save() + return CreateDataReviewMutation(data_review=data_review) @@ -59,19 +177,169 @@ def mutate(self, info, id, input): Updates an existing data_review """ user = info.context.user - if not user.has_perm("data_reviews.change_datareview"): - raise GraphQLError("Not allowed") model, node_id = from_global_id(id) - try: - data_review = DataReview.objects.get(id=node_id) + data_review = DataReview.objects.get(pk=node_id) except DataReview.DoesNotExist: raise GraphQLError("DataReview was not found") + # Check permissions + # Must have general change review permission for any study OR + # permission to change review for user's studies + if not ( + user.has_perm("data_reviews.change_datareview") + or ( + user.has_perm("data_reviews.change_my_study_datareview") + and user.studies.filter(kf_id=data_review.study.kf_id).exists() + ) + ): + raise GraphQLError("Not allowed") + + # Closed/Completed reviews cannot be modified + if data_review.state in {State.CLOSED, State.COMPLETED}: + raise GraphQLError( + "Cannot modify a data review that has been closed or " + "completed. However, a closed review can be re-opened and " + "then modified." + ) + + # Update data_review + with transaction.atomic(): + for attr in ["name", "description"]: + if attr in input: + setattr(data_review, attr, input[attr]) + + # Check files in review + review_version_ids = check_review_files(input, data_review) + if review_version_ids is not None: + # Start review if not started + if data_review.state == State.NOT_STARTED: + data_review.start() + + # We received updates while waiting, go back to reviewing state + elif data_review.state == State.WAITING: + data_review.receive_updates() + + data_review.versions.set(review_version_ids) + data_review.save() + return UpdateDataReviewMutation(data_review=data_review) +def mutate_state_helper(info, id, state_change_method_name): + """ + Helper for a take action review mutation (e.g. approve, close, reopen) + Mutate data_review.state + """ + user = info.context.user + + data_review = None + model, node_id = from_global_id(id) + try: + data_review = DataReview.objects.get(pk=node_id) + except DataReview.DoesNotExist: + raise GraphQLError("DataReview was not found") + + # Check permissions + # Must have general change review permission for any study OR + # permission to change review for user's studies + if not ( + user.has_perm("data_reviews.change_datareview") + or ( + user.has_perm("data_reviews.change_my_study_datareview") + and user.studies.filter(kf_id=data_review.study.kf_id).exists() + ) + ): + raise GraphQLError("Not allowed") + + # Take review action - e.g. approve, close, reopen + state_change_method = getattr(data_review, state_change_method_name) + try: + state_change_method() + except TransitionNotAllowed: + raise GraphQLError( + f"Cannot {state_change_method_name} data review when data review " + f"is in state: {data_review.state}" + ) + + data_review.save() + + return data_review + + +class AwaitDataReviewMutation(graphene.Mutation): + """ Wait for updates for a data_review """ + + class Arguments: + id = graphene.ID( + required=True, description="The ID of the data_review" + ) + + data_review = graphene.Field(DataReviewNode) + + def mutate(self, info, id): + """ + Update data_review state to State.WAITING + """ + data_review = mutate_state_helper(info, id, "wait_for_updates") + return AwaitDataReviewMutation(data_review=data_review) + + +class ApproveDataReviewMutation(graphene.Mutation): + """ Approve a data_review """ + + class Arguments: + id = graphene.ID( + required=True, description="The ID of the data_review to approve" + ) + + data_review = graphene.Field(DataReviewNode) + + def mutate(self, info, id): + """ + Update data_review state to State.COMPLETE + """ + data_review = mutate_state_helper(info, id, "approve") + return ApproveDataReviewMutation(data_review=data_review) + + +class CloseDataReviewMutation(graphene.Mutation): + """ Close an incomplete data_review """ + + class Arguments: + id = graphene.ID( + required=True, description="The ID of the data_review to close" + ) + + data_review = graphene.Field(DataReviewNode) + + def mutate(self, info, id): + """ + Update data_review state to State.CLOSED + """ + data_review = mutate_state_helper(info, id, "close") + return CloseDataReviewMutation(data_review=data_review) + + +class ReopenDataReviewMutation(graphene.Mutation): + """ Re-open a closed data_review """ + + class Arguments: + id = graphene.ID( + required=True, description="The ID of the data_review to re-open" + ) + + data_review = graphene.Field(DataReviewNode) + + def mutate(self, info, id): + """ + Update data_review state to State.IN_REVIEW + """ + data_review = mutate_state_helper(info, id, "reopen") + return ReopenDataReviewMutation(data_review=data_review) + + class Mutation: """ Mutations for data_reviews """ @@ -81,3 +349,15 @@ class Mutation: update_data_review = UpdateDataReviewMutation.Field( description="Update a given data_review" ) + await_data_review = AwaitDataReviewMutation.Field( + description="Wait for updates for a data_review." + ) + approve_data_review = ApproveDataReviewMutation.Field( + description="Approve a data_review." + ) + close_data_review = CloseDataReviewMutation.Field( + description="Close an incomplete data_review." + ) + reopen_data_review = ReopenDataReviewMutation.Field( + description="Re-open a closed data_review." + ) diff --git a/creator/data_reviews/nodes.py b/creator/data_reviews/nodes.py index 50d8fa021..80c300841 100644 --- a/creator/data_reviews/nodes.py +++ b/creator/data_reviews/nodes.py @@ -22,7 +22,7 @@ def get_node(cls, info, id): raise GraphQLError("Not allowed") try: - data_review = cls._meta.model.objects.get(id=id) + data_review = cls._meta.model.objects.get(pk=id) except cls._meta.model.DoesNotExist: raise GraphQLError("DataReviews was not found") diff --git a/creator/data_reviews/queries.py b/creator/data_reviews/queries.py index 13beaf83e..6985278b3 100644 --- a/creator/data_reviews/queries.py +++ b/creator/data_reviews/queries.py @@ -1,7 +1,7 @@ from graphene import relay from graphene_django.filter import DjangoFilterConnectionField from graphql import GraphQLError -from django_filters import FilterSet, OrderingFilter +from django_filters import FilterSet, OrderingFilter, CharFilter from creator.data_reviews.nodes import DataReviewNode from creator.data_reviews.models import DataReview @@ -9,6 +9,7 @@ class DataReviewFilter(FilterSet): order_by = OrderingFilter(fields=("created_on",)) + study_kf_id = CharFilter(field_name="study__kf_id", lookup_expr="exact") class Meta: model = DataReview @@ -16,7 +17,9 @@ class Meta: class Query(object): - data_review = relay.Node.Field(DataReviewNode, description="Get a single data_review") + data_review = relay.Node.Field( + DataReviewNode, description="Get a single data_review" + ) all_data_reviews = DjangoFilterConnectionField( DataReviewNode, filterset_class=DataReviewFilter, From 41fbceefbdca285e22cef916cc76de608810d7a9 Mon Sep 17 00:00:00 2001 From: Natasha Singh Date: Tue, 23 Feb 2021 14:59:18 -0500 Subject: [PATCH 4/5] :white_check_mark: Test DataReview GraphQL API and event signals --- creator/data_reviews/factories.py | 21 +- .../data_reviews/test_data_reviews_events.py | 42 +++ .../test_data_reviews_mutations.py | 349 ++++++++++++++++-- .../data_reviews/test_data_reviews_queries.py | 108 +++++- .../data_reviews/test_data_reviews_signals.py | 104 ++++++ 5 files changed, 592 insertions(+), 32 deletions(-) create mode 100644 tests/data_reviews/test_data_reviews_events.py create mode 100644 tests/data_reviews/test_data_reviews_signals.py diff --git a/creator/data_reviews/factories.py b/creator/data_reviews/factories.py index b983c3b98..5b355f869 100644 --- a/creator/data_reviews/factories.py +++ b/creator/data_reviews/factories.py @@ -1,8 +1,9 @@ import pytz import factory from django.contrib.auth import get_user_model -from creator.data_reviews.models import DataReview from creator.users.factories import UserFactory +from creator.studies.factories import StudyFactory +from creator.data_reviews.models import DataReview, State User = get_user_model() @@ -15,3 +16,21 @@ class Meta: "date_time_between", start_date="-2y", end_date="now", tzinfo=pytz.UTC ) creator = factory.SubFactory(UserFactory) + name = factory.Faker("name") + description = factory.Faker("paragraph", nb_sentences=3) + state = factory.fuzzy.FuzzyChoice( + [ + attr + for attr in dir(State) + if not callable(getattr(State, attr)) and not attr.startswith("__") + ] + ) + study = factory.SubFactory(StudyFactory) + + @factory.post_generation + def files(self, create, extracted, **kwargs): + if not create: + return + + if extracted: + self.files.set(extracted) diff --git a/tests/data_reviews/test_data_reviews_events.py b/tests/data_reviews/test_data_reviews_events.py new file mode 100644 index 000000000..9f40f94c3 --- /dev/null +++ b/tests/data_reviews/test_data_reviews_events.py @@ -0,0 +1,42 @@ +import pytest + +from django.contrib.auth import get_user_model + +from creator.data_reviews.factories import DataReview +from creator.data_reviews.models import State +from creator.events.models import Event + +User = get_user_model() + + +@pytest.mark.parametrize( + "start_state,state_transition_method,expected_msg", + [ + (State.NOT_STARTED, None, None), + (State.NOT_STARTED, "start", "started a data review"), + (State.IN_REVIEW, "wait_for_updates", "is waiting"), + (State.WAITING, "receive_updates", "received file updates"), + (State.IN_REVIEW, "close", "closed the data review"), + (State.CLOSED, "reopen", "re-opened the data review"), + (State.IN_REVIEW, "approve", "completed the data review"), + ], +) +def test_state_update_signals( + db, clients, start_state, state_transition_method, expected_msg +): + """ + Test that correct events are fired on DataReview state transitions + """ + event_count_before = Event.objects.count() + creator = User.objects.first() + dr = DataReview(creator=creator, state=start_state) + dr.save() + + if state_transition_method: + transition = getattr(dr, state_transition_method) + transition() + dr.save() + e = Event.objects.filter(data_review__pk=dr.pk).all()[0] + assert expected_msg in e.description + else: + assert event_count_before == Event.objects.count() diff --git a/tests/data_reviews/test_data_reviews_mutations.py b/tests/data_reviews/test_data_reviews_mutations.py index 6647be629..848dc6a86 100644 --- a/tests/data_reviews/test_data_reviews_mutations.py +++ b/tests/data_reviews/test_data_reviews_mutations.py @@ -1,8 +1,12 @@ import pytest from graphql_relay import to_global_id +from graphql import GraphQLError -from creator.data_reviews.models import DataReview +from creator.studies.models import Study +from creator.files.models import File +from creator.data_reviews.models import DataReview, State from creator.data_reviews.factories import DataReviewFactory +from creator.data_reviews.mutations import check_review_files CREATE_DATA_REVIEW = """ @@ -10,6 +14,13 @@ createDataReview(input: $input) { dataReview { id + kfId + createdAt + name + description + state + study { id kfId } + versions { edges { node { id kfId } } } } } } @@ -20,6 +31,61 @@ updateDataReview(id: $id, input: $input) { dataReview { id + kfId + createdAt + name + description + state + study { id kfId } + versions { edges { node { id kfId } } } + } + } +} +""" + +AWAIT_DATA_REVIEW = """ +mutation ($id: ID!) { + awaitDataReview(id: $id) { + dataReview { + id + kfId + state + } + } +} + +""" +APPROVE_DATA_REVIEW = """ +mutation ($id: ID!) { + approveDataReview(id: $id) { + dataReview { + id + kfId + state + } + } +} +""" + +CLOSE_DATA_REVIEW = """ +mutation ($id: ID!) { + closeDataReview(id: $id) { + dataReview { + id + kfId + state + } + } +} +""" + +REOPEN_DATA_REVIEW = """ +mutation ($id: ID!) { + reopenDataReview(id: $id) { + dataReview { + id + kfId + state } } } @@ -37,27 +103,61 @@ (None, False), ], ) -def test_create_data_review(db, clients, user_group, allowed): +@pytest.mark.parametrize( + "end_state,expected_error", + [ + (State.NOT_STARTED, None), + (State.IN_REVIEW, None), + ], +) +def test_create_data_review( + db, clients, prep_file, end_state, expected_error, user_group, allowed +): """ Test the create mutation. + + 1) Create a review with no files. State should be State.NOT_STARTED + 2) Create a review with files. State should be State.IN_REVIEW """ client = clients.get(user_group) + simple_update = False + if end_state == State.NOT_STARTED: + simple_update = True + + # Create a study with some files + study_file_versions = [] + for i in range(2): + study_id, file_id, _ = prep_file(authed=True) + study_file_versions.append( + to_global_id( + "VersionNode", File.objects.get(pk=file_id).versions.first().pk + ) + ) + # Send request + input_ = { + "name": "A Data Review", + "description": "A description", + "study": to_global_id("StudyNode", study_id), + } + if not simple_update: + input_["versions"] = study_file_versions resp = client.post( "/graphql", - data={ - "query": CREATE_DATA_REVIEW, - "variables": {"input": {"name": "Test"}} - }, + data={"query": CREATE_DATA_REVIEW, "variables": {"input": input_}}, content_type="application/json", ) if allowed: - assert ( - resp.json()["data"]["createDataReview"][ - "dataReview"] - is not None - ) + # Check values + dr = resp.json()["data"]["createDataReview"]["dataReview"] + assert dr is not None + for k, v in input_.items(): + assert v is not None + + # Check proper state was set and correct task queued + data_review = DataReview.objects.get(pk=dr["kfId"]) + assert data_review.state == end_state else: assert resp.json()["errors"][0]["message"] == "Not allowed" @@ -73,30 +173,237 @@ def test_create_data_review(db, clients, user_group, allowed): (None, False), ], ) -def test_update_data_review(db, clients, user_group, allowed): +@pytest.mark.parametrize( + "start_state,end_state,expected_error", + [ + (State.NOT_STARTED, State.IN_REVIEW, None), + (State.NOT_STARTED, State.NOT_STARTED, None), + (State.WAITING, State.IN_REVIEW, None), + (State.CLOSED, State.CLOSED, "Cannot modify"), + (State.COMPLETED, State.COMPLETED, "Cannot modify"), + ], +) +def test_update_data_review( + db, + clients, + prep_file, + start_state, + end_state, + expected_error, + user_group, + allowed, +): """ - Test the update mutation. + Test the update mutation with multiple updates: + + 0) Update name, description to a review not started + 1) Add new file to a review that hasn't been started + 2) Add new file to review in waiting for updates state + + Both 1,2, ^ should result in state = State.IN_REVIEW + + 3) Try to modify a closed review + 4) Try to modify a complete review + + Both 3,4 ^ should result in error """ client = clients.get(user_group) + simple_update = False + if start_state == end_state and (start_state == State.NOT_STARTED): + simple_update = True - data_review = DataReviewFactory() + # Create a study with some files + study_file_versions = [] + for i in range(2): + study_id, file_id, _ = prep_file(authed=True) + study_file_versions.append( + to_global_id( + "VersionNode", File.objects.get(pk=file_id).versions.first().pk + ) + ) + + # Create data review with no files + data_review = DataReviewFactory( + study=Study.objects.get(pk=study_id), state=start_state + ) + + # Send request + input_ = { + "name": "foobar", + "description": "foobar", + } + if not simple_update: + input_["versions"] = study_file_versions resp = client.post( "/graphql", data={ "query": UPDATE_DATA_REVIEW, "variables": { - "id": to_global_id("DataReviewNode}}", data_review.id), - "input": {"name": "test"}, + "id": to_global_id("DataReviewNode", data_review.pk), + "input": input_, }, }, content_type="application/json", ) - if allowed: - assert ( - resp.json()["data"]["updateDataReview"]["dataReview"] - is not None - ) + # Check permissions + if not allowed: + assert resp.json()["errors"][0]["message"] == "Not allowed" + return + + # Check expected end_state + data_review.refresh_from_db() + assert data_review.state == end_state + + # Error + if expected_error: + assert expected_error in resp.json()["errors"][0]["message"] + # Success else: + dr = resp.json()["data"]["updateDataReview"]["dataReview"] + assert dr is not None + for k, v in input_.items(): + assert v is not None + + +@pytest.mark.parametrize( + "user_group,allowed", + [ + ("Administrators", True), + ("Services", False), + ("Developers", False), + ("Investigators", False), + ("Bioinformatics", False), + (None, False), + ], +) +@pytest.mark.parametrize( + "start_state,end_state,mutation,expected_error", + [ + (State.IN_REVIEW, State.WAITING, AWAIT_DATA_REVIEW, None), + (State.IN_REVIEW, State.COMPLETED, APPROVE_DATA_REVIEW, None), + (State.IN_REVIEW, State.CLOSED, CLOSE_DATA_REVIEW, None), + (State.CLOSED, State.IN_REVIEW, REOPEN_DATA_REVIEW, None), + ( + State.NOT_STARTED, + State.NOT_STARTED, + APPROVE_DATA_REVIEW, + "when data review is in state", + ), + ], +) +def test_review_actions( + db, + clients, + start_state, + end_state, + mutation, + expected_error, + user_group, + allowed, +): + client = clients.get(user_group) + + # Create data review with no files + data_review = DataReviewFactory(state=start_state) + + # Send request + resp = client.post( + "/graphql", + data={ + "query": mutation, + "variables": { + "id": to_global_id("DataReviewNode", data_review.pk), + }, + }, + content_type="application/json", + ) + + # Check permissions + if not allowed: assert resp.json()["errors"][0]["message"] == "Not allowed" + return + + # Check expected end_state + data_review.refresh_from_db() + assert data_review.state == end_state + + # Error + if expected_error: + assert expected_error in resp.json()["errors"][0]["message"] + # Success + else: + for k in resp.json()["data"]: + mutation_name = k + dr = resp.json()["data"][mutation_name]["dataReview"] + assert dr is not None + + +def test_update_review_bad_file(db, clients, prep_file): + """ + Test check_review_file method with valid / invalid file versions + """ + # Valid case - New data review with files from same study + study_id, file_id, _ = prep_file(authed=True) + dr = DataReviewFactory(study=Study.objects.get(pk=study_id)) + version = File.objects.get(pk=file_id).versions.first() + review_file_ids = check_review_files( + {"versions": [to_global_id("VersionNode", version.pk)]}, dr + ) + assert len(review_file_ids) == 1 + dr.versions.set(review_file_ids) + dr.save() + + # Invalid case - data review with files from different study + study_id, file_id, _ = prep_file(authed=False) + version = File.objects.get(pk=file_id).versions.first() + with pytest.raises(GraphQLError) as e: + review_file_ids = check_review_files( + {"versions": [to_global_id("VersionNode", version.pk)]}, dr + ) + assert "Error in modifying data_review" in str(e) + + +def test_update_review_files_changed(db, clients, prep_file): + """ + Test check_review_file method with review file version changes + """ + # Add files to data review w no files + study_id, file_id, _ = prep_file(authed=True) + dr = DataReviewFactory(study=Study.objects.get(pk=study_id)) + version1_id = to_global_id( + "VersionNode", File.objects.get(pk=file_id).versions.first().pk + ) + review_file_ids = check_review_files({"versions": [version1_id]}, dr) + assert len(review_file_ids) == 1 + dr.versions.set(review_file_ids) + dr.save() + + # Add same file to data review + review_file_ids = check_review_files({"versions": [version1_id]}, dr) + assert review_file_ids is None + + # Add new files to data review with files + vids = [] + for i in range(2): + study_id, file_id, _ = prep_file(authed=True) + vids.append( + to_global_id( + "VersionNode", File.objects.get(pk=file_id).versions.first().pk + ) + ) + review_file_ids = check_review_files({"versions": vids}, dr) + assert len(review_file_ids) == 2 + dr.versions.set(review_file_ids) + dr.save() + + # Delete all files from data review + review_file_ids = check_review_files({"versions": []}, dr) + assert len(review_file_ids) == 0 + dr.versions.set(review_file_ids) + dr.save() + + # Delete all files from data review w no files + review_file_ids = check_review_files({"versions": []}, dr) + assert review_file_ids is None diff --git a/tests/data_reviews/test_data_reviews_queries.py b/tests/data_reviews/test_data_reviews_queries.py index 1892cc423..54ed61b00 100644 --- a/tests/data_reviews/test_data_reviews_queries.py +++ b/tests/data_reviews/test_data_reviews_queries.py @@ -1,11 +1,19 @@ import pytest from graphql_relay import to_global_id from creator.data_reviews.factories import DataReviewFactory +from creator.studies.models import Study DATA_REVIEW = """ query ($id: ID!) { dataReview(id: $id) { id + kfId + createdAt + name + description + state + study { id kfId } + versions { edges { node { id kfId } } } } } """ @@ -13,7 +21,37 @@ ALL_DATA_REVIEWS = """ query { allDataReviews { - edges { node { id } } + edges { + node { + id + kfId + createdAt + name + description + state + study { id kfId } + versions { edges { node { id kfId } } } + } + } + } +} +""" + +ALL_DATA_REVIEWS_BY_STUDY = """ +query($studyKfId: String) { + allDataReviews(studyKfId: $studyKfId) { + edges { + node { + id + kfId + createdAt + name + description + state + study { id kfId } + versions { edges { node { id kfId } } } + } + } } } """ @@ -30,13 +68,17 @@ (None, False), ], ) -def test_query_data_review(db, clients, user_group, allowed): +def test_query_data_review(db, clients, prep_file, user_group, allowed): client = clients.get(user_group) - data_review = DataReviewFactory() + # Create a study with some files + for i in range(2): + prep_file(authed=True) - variables = {"id": to_global_id("DataReviewNode", data_review.id)} + # Create data review + data_review = DataReviewFactory() + variables = {"id": to_global_id("DataReviewNode", data_review.pk)} resp = client.post( "/graphql", data={"query": DATA_REVIEW, "variables": variables}, @@ -44,9 +86,8 @@ def test_query_data_review(db, clients, user_group, allowed): ) if allowed: - assert ( - resp.json()["data"]["dataReview"]["id"] == - to_global_id("DataReviewNode", data_review.id) + assert resp.json()["data"]["dataReview"]["id"] == to_global_id( + "DataReviewNode", data_review.pk ) else: assert resp.json()["errors"][0]["message"] == "Not allowed" @@ -63,17 +104,64 @@ def test_query_data_review(db, clients, user_group, allowed): (None, False), ], ) -def test_query_all_data_reviews(db, clients, user_group, allowed): +def test_query_all_data_reviews(db, clients, prep_file, user_group, allowed): client = clients.get(user_group) - data_review = DataReviewFactory.create_batch(5) + # Create a study with some files + for i in range(2): + prep_file(authed=True) + + # Create Data Review + DataReviewFactory.create_batch(5) resp = client.post( "/graphql", - data={"query": ALL_DATA_REVIEWS}, content_type="application/json" + data={"query": ALL_DATA_REVIEWS}, + content_type="application/json", ) if allowed: assert len(resp.json()["data"]["allDataReviews"]["edges"]) == 5 else: assert resp.json()["errors"][0]["message"] == "Not allowed" + + +@pytest.mark.parametrize( + "user_group,allowed", + [ + ("Administrators", True), + ("Services", False), + ("Developers", False), + ("Investigators", False), + ("Bioinformatics", False), + (None, False), + ], +) +def test_query_filter_data_reviews( + db, clients, prep_file, user_group, allowed +): + client = clients.get(user_group) + + # Create a study with some files + for i in range(2): + prep_file(authed=True) + + # Create Data Review + for s in Study.objects.all(): + dr = DataReviewFactory(study=s) + + resp = client.post( + "/graphql", + data={ + "query": ALL_DATA_REVIEWS, + "variables": {"studyKfId": dr.study.pk}, + }, + content_type="application/json", + ) + + if allowed: + drs = resp.json()["data"]["allDataReviews"]["edges"] + assert len(drs) == 1 + assert drs[0]["node"]["study"]["kfId"] == dr.study.pk + else: + assert resp.json()["errors"][0]["message"] == "Not allowed" diff --git a/tests/data_reviews/test_data_reviews_signals.py b/tests/data_reviews/test_data_reviews_signals.py new file mode 100644 index 000000000..5d53f3755 --- /dev/null +++ b/tests/data_reviews/test_data_reviews_signals.py @@ -0,0 +1,104 @@ +from django.contrib.auth import get_user_model + +from creator.data_reviews.factories import DataReview +from creator.data_reviews.models import State +from creator.studies.models import Study +from creator.files.models import File, Version + +User = get_user_model() + + +def test_file_version_delete_signal(db, clients, prep_file): + """ + Test correct event is fired when file versions are deleted + """ + # Create two DataReviews with files + for i in range(2): + study_id, file_id, _ = prep_file(authed=True) + f = File.objects.get(pk=file_id).versions.first() + + drs = [] + for i in range(3): + dr = DataReview( + creator=User.objects.first(), + study=Study.objects.get(pk=study_id), + ) + if i != 0: + dr.versions.add(f) + dr.save() + drs.append(dr) + + # Delete file version + f.delete() + + # Check events fired for the right data reviews + for i, dr in enumerate(drs): + if i == 0: + event_count = 0 + else: + event_count = 1 + assert ( + dr.events.filter( + description__contains="was deleted from data review" + ).count() + == event_count + ) + + +def test_file_post_save_signal(db, clients, prep_file): + """ + Test for expected data review state change and event firing when + new file version is uploaded + """ + # Create a study with files + files = [] + for i in range(2): + study_id, file_id, version_id = prep_file(authed=True) + files.append(file_id) + f0v0 = File.objects.get(pk=files[0]).versions.first() + + # Create some data reviews with one version + drs = [] + states = [State.WAITING, State.IN_REVIEW, State.COMPLETED, State.CLOSED] + for i in range(len(states)): + dr = DataReview( + state=states[i], + creator=User.objects.first(), + study=Study.objects.get(pk=study_id), + ) + dr.versions.add(f0v0) + dr.save() + drs.append(dr) + + # Upload new version to a file not involved in data reviews + f1v1 = Version(root_file=File.objects.get(pk=files[1]), size=100) + f1v1.save() + for dr in drs: + assert dr.events.count() == 0 + + # Upload new version to file involved in data reviews + f0v1 = Version(root_file=File.objects.get(pk=files[0]), size=100) + f0v1.save() + for dr in drs: + dr.refresh_from_db() + + # Non-terminal reviews should have events + for i, dr in enumerate(drs[0:2]): + for e in dr.events.filter(version=f0v1).all(): + assert f0v1.pk in e.description + assert "which is part of open data review" in e.description + if i == 0: + assert dr.state == State.IN_REVIEW + assert dr.events.count() == 2 + else: + assert dr.events.count() == 1 + + # Terminal reviews should have 0 events + for dr in drs[2:]: + assert dr.events.count() == 0 + + # Save existing version - should be no new data review events + f0v1.size = 200 + f0v1.save() + for dr in drs: + assert dr.events.count() <= 2 From 635c728ee241cad71f21e0136e7097ceb31238da Mon Sep 17 00:00:00 2001 From: Natasha Singh Date: Tue, 23 Feb 2021 14:59:50 -0500 Subject: [PATCH 5/5] :card_file_box: Add data review related migrations --- .../data_reviews/migrations/0001_initial.py | 106 ++++++++++++++++++ creator/data_reviews/migrations/__init__.py | 0 .../events/migrations/0014_add_data_review.py | 25 +++++ 3 files changed, 131 insertions(+) create mode 100644 creator/data_reviews/migrations/0001_initial.py create mode 100644 creator/data_reviews/migrations/__init__.py create mode 100644 creator/events/migrations/0014_add_data_review.py diff --git a/creator/data_reviews/migrations/0001_initial.py b/creator/data_reviews/migrations/0001_initial.py new file mode 100644 index 000000000..42acd505e --- /dev/null +++ b/creator/data_reviews/migrations/0001_initial.py @@ -0,0 +1,106 @@ +# Generated by Django 2.2.13 on 2021-02-23 19:30 + +import creator.data_reviews.models +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django_fsm +import uuid + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ("files", "0022_add_genomic_workflow_output_type"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="DataReview", + fields=[ + ( + "kf_id", + models.CharField( + default=creator.data_reviews.models.data_review_id, + help_text="Kids First ID assigned to the data review", + max_length=11, + primary_key=True, + serialize=False, + ), + ), + ( + "uuid", + models.UUIDField( + default=uuid.uuid4, help_text="UUID used internally" + ), + ), + ( + "created_at", + models.DateTimeField( + auto_now_add=True, + help_text="Time when the data_review was created", + null=True, + ), + ), + ( + "name", + models.CharField( + help_text="Name of the data review", max_length=256 + ), + ), + ( + "description", + models.TextField( + blank=True, + help_text="Description of data review", + max_length=10000, + null=True, + ), + ), + ( + "state", + django_fsm.FSMField( + default="not_started", + help_text="The current state of the data review", + max_length=50, + ), + ), + ( + "creator", + models.ForeignKey( + blank=True, + help_text="The user who created the data review", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="data_reviews", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "study", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="data_reviews", + to="studies.Study", + ), + ), + ( + "versions", + models.ManyToManyField( + help_text="kf_ids of the file versions in this data review", + related_name="data_reviews", + to="files.Version", + ), + ), + ], + options={ + "permissions": [ + ("list_all_datareview", "Show all data_reviews") + ], + }, + ), + ] diff --git a/creator/data_reviews/migrations/__init__.py b/creator/data_reviews/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/creator/events/migrations/0014_add_data_review.py b/creator/events/migrations/0014_add_data_review.py new file mode 100644 index 000000000..44a652b5a --- /dev/null +++ b/creator/events/migrations/0014_add_data_review.py @@ -0,0 +1,25 @@ +# Generated by Django 2.2.13 on 2021-02-23 19:31 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('data_reviews', '0001_initial'), + ('events', '0013_updated_event_types'), + ] + + operations = [ + migrations.AddField( + model_name='event', + name='data_review', + field=models.ForeignKey(blank=True, help_text='Data Review related to this event', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='events', to='data_reviews.DataReview'), + ), + migrations.AlterField( + model_name='event', + name='event_type', + field=models.CharField(choices=[('SF_CRE', 'Study File Created'), ('SF_UPD', 'Study File Updated'), ('SF_DEL', 'Study File Deleted'), ('FV_CRE', 'File Version Created'), ('FV_UPD', 'File Version Updated'), ('SD_CRE', 'Study Created'), ('SD_UPD', 'Study Updated'), ('PR_CRE', 'Project Created'), ('PR_UPD', 'Project Updated'), ('PR_DEL', 'Project Deleted'), ('PR_LIN', 'Project Linked'), ('PR_UNL', 'Project Unlinked'), ('PR_STR', 'Project Creation Start'), ('PR_ERR', 'Project Creation Error'), ('PR_SUC', 'Project Creation Success'), ('BK_STR', 'Bucket Creation Start'), ('BK_ERR', 'Bucket Creation Error'), ('BK_SUC', 'Bucket Creation Success'), ('BK_LIN', 'Bucket Linked'), ('BK_UNL', 'Bucket Unlinked'), ('IM_STR', 'File Import Start'), ('IM_ERR', 'File Import Error'), ('IM_SUC', 'File Import Success'), ('CB_ADD', 'Collaborator Added'), ('CB_UPD', 'Collaborator Updated'), ('CB_REM', 'Collaborator Removed'), ('IN_UPD', 'Ingestion Status Updated'), ('PH_UPD', 'Phenotype Status Updated'), ('ST_UPD', 'Sequencing Status Updated'), ('RT_CRE', 'Referral Token Created'), ('RT_CLA', 'Referral Token Claimed'), ('SL_STR', 'Slack Channel Creation Start'), ('SL_ERR', 'Slack Channel Creation Error'), ('SL_SUC', 'Slack Channel Creation Success'), ('DR_STA', 'Data Review Started'), ('DR_WAI', 'Data Review Waiting for Updates'), ('DR_UPD', 'Data Review Updated'), ('DR_APP', 'Data Review Approved'), ('DR_CLO', 'Data Review Closed'), ('DR_REO', 'Data Review Re-opened'), ('OTH', 'Other')], default='OTH', max_length=6), + ), + ]