From c6524a23ccbfd176dcee886768cabbd8d29238d6 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 8 Jan 2024 12:22:09 -0500 Subject: [PATCH 01/20] feat: Added shared uuid and access type to shared resource entities --- .../0049_sharedresource_share_fields.py | 57 +++++++++++++++++++ .../apps/core/models/shared_resources.py | 29 ++++++++++ .../apps/graphql/schema/schema.graphql | 14 +++++ .../apps/graphql/schema/shared_resources.py | 7 ++- .../tests/graphql/test_shared_data.py | 19 ++++++- 5 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py diff --git a/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py b/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py new file mode 100644 index 000000000..067394102 --- /dev/null +++ b/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py @@ -0,0 +1,57 @@ +# Copyright © 2024 Technology Matters +# +# This program 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. +# +# This program 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 this program. If not, see https://www.gnu.org/licenses/. + +# Generated by Django 5.0 on 2024-01-08 15:59 + +import uuid + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0048_group_membership_list"), + ] + + operations = [ + migrations.AddField( + model_name="sharedresource", + name="share_access", + field=models.CharField( + choices=[ + ("no", "No share access"), + ("all", "Anyone with the link"), + ("target_members", "Only tagert members"), + ], + default="no", + max_length=32, + ), + ), + migrations.AddField( + model_name="sharedresource", + name="share_uuid", + field=models.UUIDField(default=uuid.uuid4), + preserve_default=False, + ), + migrations.AddConstraint( + model_name="sharedresource", + constraint=models.UniqueConstraint( + condition=models.Q(("deleted_at__isnull", True)), + fields=("share_uuid",), + name="unique_share_uuid", + ), + ), + ] diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index 211117fde..bf38754ee 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -12,9 +12,12 @@ # # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see https://www.gnu.org/licenses/. +import uuid + from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.db import models +from django.utils.translation import gettext_lazy as _ from apps.core.models import BaseModel @@ -26,6 +29,17 @@ class SharedResource(BaseModel): Target represents the resource that is receiving the shared resource (Example: Landscape). """ + SHARE_ACCESS_NO = "no" + SHARE_ACCESS_ALL = "all" + SHARE_ACCESS_TARGET_MEMBERS = "target_members" + DEFAULT_SHARE_ACCESS = SHARE_ACCESS_NO + + SHARE_ACCESS_TYPES = ( + (SHARE_ACCESS_NO, _("No share access")), + (SHARE_ACCESS_ALL, _("Anyone with the link")), + (SHARE_ACCESS_TARGET_MEMBERS, _("Only tagert members")), + ) + source = GenericForeignKey("source_content_type", "source_object_id") source_content_type = models.ForeignKey( ContentType, on_delete=models.CASCADE, related_name="source_content_type" @@ -37,3 +51,18 @@ class SharedResource(BaseModel): ContentType, on_delete=models.CASCADE, related_name="target_content_type" ) target_object_id = models.UUIDField() + share_uuid = models.UUIDField(default=uuid.uuid4) + share_access = models.CharField( + max_length=32, + choices=SHARE_ACCESS_TYPES, + default=DEFAULT_SHARE_ACCESS, + ) + + class Meta: + constraints = ( + models.UniqueConstraint( + fields=("share_uuid",), + condition=models.Q(deleted_at__isnull=True), + name="unique_share_uuid", + ), + ) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index bcc577ef3..f95f7d9a6 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -430,8 +430,22 @@ type SharedResourceNodeEdge { type SharedResourceNode implements Node { id: ID! + shareAccess: CoreSharedResourceShareAccessChoices! source: SourceNode target: TargetNode + shareUrl: String +} + +"""An enumeration.""" +enum CoreSharedResourceShareAccessChoices { + """No share access""" + NO + + """Anyone with the link""" + ALL + + """Only tagert members""" + TARGET_MEMBERS } union SourceNode = VisualizationConfigNode | DataEntryNode diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index b13ecd65d..ba219c1e9 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -14,6 +14,7 @@ # along with this program. If not, see https://www.gnu.org/licenses/. import graphene +from django.conf import settings from graphene import relay from graphene_django import DjangoObjectType @@ -39,10 +40,11 @@ class SharedResourceNode(DjangoObjectType): id = graphene.ID(source="pk", required=True) source = graphene.Field(SourceNode) target = graphene.Field(TargetNode) + share_url = graphene.String() class Meta: model = SharedResource - fields = ["id"] + fields = ["id", "share_access"] interfaces = (relay.Node,) connection_class = TerrasoConnection @@ -51,3 +53,6 @@ def resolve_source(self, info, **kwargs): def resolve_target(self, info, **kwargs): return self.target + + def resolve_share_url(self, info, **kwargs): + return f"{settings.API_ENDPOINT}/shared-data/download/{self.share_uuid}" diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index 25aed428f..ddcac07cc 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -404,6 +404,8 @@ def test_data_entries_from_parent_query_by_resource_field( name } } + shareUrl + shareAccess } } } @@ -417,10 +419,23 @@ def test_data_entries_from_parent_query_by_resource_field( json_response = response.json() resources = json_response["data"][parent]["edges"][0]["node"]["sharedResources"]["edges"] - entries_result = [resource["node"]["source"]["name"] for resource in resources] + entries_result = [ + { + "name": resource["node"]["source"]["name"], + "share_url": resource["node"]["shareUrl"], + "share_access": resource["node"]["shareAccess"], + } + for resource in resources + ] for data_entry in data_entries: - assert data_entry.name in entries_result + share_uuid = data_entry.shared_resources.all()[0].share_uuid + share_url = f"http://127.0.0.1:8000/shared-data/download/{share_uuid}" + assert { + "name": data_entry.name, + "share_url": share_url, + "share_access": data_entry.shared_resources.all()[0].share_access.upper(), + } in entries_result @pytest.mark.parametrize( From 96b356e7f6de4e1b34a130a1b8e89614e05ca762 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 16 Jan 2024 11:10:29 -0500 Subject: [PATCH 02/20] fix: Fixed migration to populate share_uuid --- .../core/migrations/0049_sharedresource_share_fields.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py b/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py index 067394102..6b32e5612 100644 --- a/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py +++ b/terraso_backend/apps/core/migrations/0049_sharedresource_share_fields.py @@ -21,6 +21,14 @@ from django.db import migrations, models +def fill_shared_uuid(apps, schema_editor): + SharedResource = apps.get_model("core", "SharedResource") + shared_resources = SharedResource.objects.all() + for shared_resource in shared_resources: + shared_resource.share_uuid = uuid.uuid4() + SharedResource.objects.bulk_update(shared_resources, ["share_uuid"]) + + class Migration(migrations.Migration): dependencies = [ ("core", "0048_group_membership_list"), @@ -46,6 +54,7 @@ class Migration(migrations.Migration): field=models.UUIDField(default=uuid.uuid4), preserve_default=False, ), + migrations.RunPython(fill_shared_uuid), migrations.AddConstraint( model_name="sharedresource", constraint=models.UniqueConstraint( From 5af0432f7da61cc170ab52f1b042a9993204df28 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 24 Jan 2024 11:05:15 -0500 Subject: [PATCH 03/20] feat: Added shared resource update mutation --- .../apps/core/models/shared_resources.py | 11 +++ terraso_backend/apps/core/permission_rules.py | 17 ++++ .../apps/graphql/schema/__init__.py | 2 + .../apps/graphql/schema/schema.graphql | 13 ++++ .../apps/graphql/schema/shared_resources.py | 42 +++++++++- .../test_shared_resource_mutations.py | 77 +++++++++++++++++++ 6 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index bf38754ee..3ef9c4114 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -66,3 +66,14 @@ class Meta: name="unique_share_uuid", ), ) + + @classmethod + def get_share_access_from_text(cls, share_access): + if not share_access: + return cls.SHARE_ACCESS_NO + lowered = share_access.lower() + if lowered == cls.SHARE_ACCESS_ALL: + return cls.SHARE_ACCESS_ALL + if lowered == cls.SHARE_ACCESS_TARGET_MEMBERS: + return cls.SHARE_ACCESS_TARGET_MEMBERS + return cls.SHARE_ACCESS_NO diff --git a/terraso_backend/apps/core/permission_rules.py b/terraso_backend/apps/core/permission_rules.py index c9636df4e..118283e0c 100644 --- a/terraso_backend/apps/core/permission_rules.py +++ b/terraso_backend/apps/core/permission_rules.py @@ -214,6 +214,22 @@ def allowed_to_delete_group_membership(user, obj): return validate_delete_membership(user, group, membership) +@rules.predicate +def allowed_to_change_shared_resource(user, shared_resource): + from apps.shared_data.permission_rules import is_target_manager + + target = shared_resource.target + source = shared_resource.source + + if source.created_by == user: + return True + + if is_target_manager(user, target): + return True + + return False + + rules.add_rule("allowed_group_managers_count", allowed_group_managers_count) rules.add_rule("allowed_to_update_preferences", allowed_to_update_preferences) rules.add_rule("allowed_to_change_landscape", allowed_to_change_landscape) @@ -222,3 +238,4 @@ def allowed_to_delete_group_membership(user, obj): rules.add_rule("allowed_landscape_managers_count", allowed_landscape_managers_count) rules.add_rule("allowed_to_change_group_membership", allowed_to_change_group_membership) rules.add_rule("allowed_to_delete_group_membership", allowed_to_delete_group_membership) +rules.add_rule("allowed_to_change_shared_resource", allowed_to_change_shared_resource) diff --git a/terraso_backend/apps/graphql/schema/__init__.py b/terraso_backend/apps/graphql/schema/__init__.py index 526c08e85..6193bbd5e 100644 --- a/terraso_backend/apps/graphql/schema/__init__.py +++ b/terraso_backend/apps/graphql/schema/__init__.py @@ -80,6 +80,7 @@ LandscapeMembershipDeleteMutation, LandscapeMembershipSaveMutation, ) +from .shared_resources import SharedResourceUpdateMutation from .sites import ( SiteAddMutation, SiteDeleteMutation, @@ -202,6 +203,7 @@ class Mutations(graphene.ObjectType): delete_landscape_membership = LandscapeMembershipDeleteMutation.Field() save_group_membership = GroupMembershipSaveMutation.Field() delete_group_membership = GroupMembershipDeleteMutation.Field() + update_shared_resource = SharedResourceUpdateMutation.Field() schema = graphene.Schema(query=Query, mutation=Mutations) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index f95f7d9a6..e4fea7e91 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -1690,6 +1690,7 @@ type Mutations { deleteLandscapeMembership(input: LandscapeMembershipDeleteMutationInput!): LandscapeMembershipDeleteMutationPayload! saveGroupMembership(input: GroupMembershipSaveMutationInput!): GroupMembershipSaveMutationPayload! deleteGroupMembership(input: GroupMembershipDeleteMutationInput!): GroupMembershipDeleteMutationPayload! + updateSharedResource(input: SharedResourceUpdateMutationInput!): SharedResourceUpdateMutationPayload! } type GroupAddMutationPayload { @@ -2491,3 +2492,15 @@ input GroupMembershipDeleteMutationInput { groupSlug: String! clientMutationId: String } + +type SharedResourceUpdateMutationPayload { + errors: GenericScalar + sharedResource: SharedResourceNode + clientMutationId: String +} + +input SharedResourceUpdateMutationInput { + id: ID! + shareAccess: String! + clientMutationId: String +} diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index ba219c1e9..bbd30379e 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -14,17 +14,23 @@ # along with this program. If not, see https://www.gnu.org/licenses/. import graphene +import rules +import structlog from django.conf import settings from graphene import relay from graphene_django import DjangoObjectType from apps.core.models import SharedResource +from apps.graphql.exceptions import GraphQLNotAllowedException, GraphQLNotFoundException from . import GroupNode, LandscapeNode -from .commons import TerrasoConnection +from .commons import BaseWriteMutation, TerrasoConnection +from .constants import MutationTypes from .data_entries import DataEntryNode from .visualization_config import VisualizationConfigNode +logger = structlog.get_logger(__name__) + class SourceNode(graphene.Union): class Meta: @@ -56,3 +62,37 @@ def resolve_target(self, info, **kwargs): def resolve_share_url(self, info, **kwargs): return f"{settings.API_ENDPOINT}/shared-data/download/{self.share_uuid}" + + +class SharedResourceUpdateMutation(BaseWriteMutation): + shared_resource = graphene.Field(SharedResourceNode) + + model_class = SharedResource + + class Input: + id = graphene.ID(required=True) + share_access = graphene.String(required=True) + + @classmethod + def mutate_and_get_payload(cls, root, info, **kwargs): + user = info.context.user + + try: + shared_resource = SharedResource.objects.get(pk=kwargs["id"]) + except SharedResource.DoesNotExist: + logger.error( + "SharedResource not found", + extra={"shared_resource_id": kwargs["id"]}, + ) + raise GraphQLNotFoundException(field="id", model_name=SharedResource.__name__) + + if not rules.test_rule("allowed_to_change_shared_resource", user, shared_resource): + logger.info( + "Attempt to update a SharedResource, but user lacks permission", + extra={"user_id": user.pk, "shared_resource_id": str(shared_resource.pk)}, + ) + raise GraphQLNotAllowedException( + model_name=SharedResource.__name__, operation=MutationTypes.UPDATE + ) + kwargs["share_access"] = SharedResource.get_share_access_from_text(kwargs["share_access"]) + return super().mutate_and_get_payload(root, info, **kwargs) diff --git a/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py b/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py new file mode 100644 index 000000000..1cbac1c5b --- /dev/null +++ b/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py @@ -0,0 +1,77 @@ +# Copyright © 2024 Technology Matters +# +# This program 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. +# +# This program 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 this program. If not, see https://www.gnu.org/licenses/. + +import pytest + +pytestmark = pytest.mark.django_db + + +def test_shared_resource_update_by_source_works(client_query, data_entries): + data_entry = data_entries[0] + shared_resource = data_entry.shared_resources.all()[0] + + new_data = { + "id": str(shared_resource.id), + "shareAccess": "ALL", + } + response = client_query( + """ + mutation updateSharedResource($input: SharedResourceUpdateMutationInput!) { + updateSharedResource(input: $input) { + sharedResource { + id + shareAccess + } + } + } + """, + variables={"input": new_data}, + ) + json_result = response.json() + print(json_result) + result = json_result["data"]["updateSharedResource"]["sharedResource"] + + assert result == new_data + + +def test_shared_resource_update_by_non_creator_or_manager_fails_due_permission_check( + client_query, data_entries, users +): + data_entry = data_entries[0] + shared_resource = data_entry.shared_resources.all()[0] + + # Let's force old data creator be different from client query user + data_entry.created_by = users[2] + data_entry.save() + + new_data = { + "id": str(shared_resource.id), + "shareAccess": "ALL", + } + + response = client_query( + """ + mutation updateSharedResource($input: SharedResourceUpdateMutationInput!) { + updateSharedResource(input: $input) { + errors + } + } + """, + variables={"input": new_data}, + ) + response = response.json() + + assert "errors" in response["data"]["updateSharedResource"] + assert "update_not_allowed" in response["data"]["updateSharedResource"]["errors"][0]["message"] From f26a426d9a52ce6022fffd387534cc9a00ad128a Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 29 Jan 2024 17:37:20 -0500 Subject: [PATCH 04/20] feat: Shared resource download view --- .../apps/shared_data/permission_rules.py | 17 ++++- terraso_backend/apps/shared_data/urls.py | 7 +- terraso_backend/apps/shared_data/views.py | 38 +++++++++- .../tests/graphql/test_shared_data.py | 3 +- terraso_backend/tests/shared_data/conftest.py | 70 ++++++++++++++++++- .../tests/shared_data/test_views.py | 65 +++++++++++++++++ 6 files changed, 195 insertions(+), 5 deletions(-) diff --git a/terraso_backend/apps/shared_data/permission_rules.py b/terraso_backend/apps/shared_data/permission_rules.py index 5d86fb04b..91f659f3b 100644 --- a/terraso_backend/apps/shared_data/permission_rules.py +++ b/terraso_backend/apps/shared_data/permission_rules.py @@ -16,7 +16,7 @@ import rules from apps.core import group_collaboration_roles, landscape_collaboration_roles -from apps.core.models import Group, Landscape +from apps.core.models import Group, Landscape, SharedResource def is_target_manager(user, target): @@ -103,4 +103,19 @@ def allowed_to_delete_visualization_config(user, visualization_config): return is_user_allowed_to_change_data_entry(visualization_config.data_entry, user) +@rules.predicate +def allowed_to_download_data_entry_file(user, shared_resource): + target = shared_resource.target + + if shared_resource.share_access == SharedResource.SHARE_ACCESS_NO: + return False + + if shared_resource.share_access == SharedResource.SHARE_ACCESS_ALL: + return True + + if shared_resource.share_access == SharedResource.SHARE_ACCESS_TARGET_MEMBERS: + return is_target_member(user, target) + + rules.add_rule("allowed_to_add_data_entry", allowed_to_add_data_entry) +rules.add_rule("allowed_to_download_data_entry_file", allowed_to_download_data_entry_file) diff --git a/terraso_backend/apps/shared_data/urls.py b/terraso_backend/apps/shared_data/urls.py index 6f9b53053..c3edb10dc 100644 --- a/terraso_backend/apps/shared_data/urls.py +++ b/terraso_backend/apps/shared_data/urls.py @@ -16,10 +16,15 @@ from django.urls import path from django.views.decorators.csrf import csrf_exempt -from .views import DataEntryFileUploadView +from .views import DataEntryFileDownloadView, DataEntryFileUploadView app_name = "apps.shared_data" urlpatterns = [ path("upload/", csrf_exempt(DataEntryFileUploadView.as_view()), name="upload"), + path( + "download/", + csrf_exempt(DataEntryFileDownloadView.as_view()), + name="download", + ), ] diff --git a/terraso_backend/apps/shared_data/views.py b/terraso_backend/apps/shared_data/views.py index 035ef02c9..1add7b2f6 100644 --- a/terraso_backend/apps/shared_data/views.py +++ b/terraso_backend/apps/shared_data/views.py @@ -17,15 +17,18 @@ from dataclasses import asdict from pathlib import Path +import rules import structlog from config.settings import DATA_ENTRY_ACCEPTED_EXTENSIONS, MEDIA_UPLOAD_MAX_FILE_SIZE from django.contrib.contenttypes.models import ContentType from django.db import transaction -from django.http import JsonResponse +from django.http import HttpResponse, HttpResponseRedirect, JsonResponse +from django.views import View from django.views.generic.edit import FormView from apps.auth.mixins import AuthenticationRequiredMixin from apps.core.exceptions import ErrorContext, ErrorMessage +from apps.core.models import SharedResource from apps.storage.file_utils import has_multiple_files, is_file_upload_oversized from .forms import DataEntryForm @@ -38,6 +41,39 @@ mimetypes.init() +class DataEntryFileDownloadView(View): + def get(self, request, shared_resource_uuid, *args, **kwargs): + shared_resource = SharedResource.objects.filter(share_uuid=shared_resource_uuid).first() + + if shared_resource is None: + return HttpResponse("Not Found", status=404) + + not_shared = shared_resource.share_access == SharedResource.SHARE_ACCESS_NO + needs_authentication = ( + shared_resource.share_access != SharedResource.SHARE_ACCESS_ALL + and not request.user.is_authenticated + ) + + if not_shared or needs_authentication: + return HttpResponse("Not Found", status=404) + + source = shared_resource.source + + if not isinstance(source, DataEntry) or source.entry_type != DataEntry.ENTRY_TYPE_FILE: + # Only support download for data entries files + return HttpResponse("Not Found", status=404) + + if not rules.test_rule( + "allowed_to_download_data_entry_file", request.user, shared_resource + ): + return HttpResponse("Not Found", status=404) + + signed_url = source.signed_url + + # Redirect to the presigned URL + return HttpResponseRedirect(signed_url) + + class DataEntryFileUploadView(AuthenticationRequiredMixin, FormView): @transaction.atomic def post(self, request, **kwargs): diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index ddcac07cc..31cf07e75 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -22,6 +22,7 @@ import geopandas as gpd import pytest +from django.conf import settings from apps.collaboration.models import Membership as CollaborationMembership from apps.core import group_collaboration_roles @@ -430,7 +431,7 @@ def test_data_entries_from_parent_query_by_resource_field( for data_entry in data_entries: share_uuid = data_entry.shared_resources.all()[0].share_uuid - share_url = f"http://127.0.0.1:8000/shared-data/download/{share_uuid}" + share_url = f"{settings.API_ENDPOINT}/shared-data/download/{share_uuid}" assert { "name": data_entry.name, "share_url": share_url, diff --git a/terraso_backend/tests/shared_data/conftest.py b/terraso_backend/tests/shared_data/conftest.py index 4b01a4f6b..0d47d6e09 100644 --- a/terraso_backend/tests/shared_data/conftest.py +++ b/terraso_backend/tests/shared_data/conftest.py @@ -13,11 +13,15 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see https://www.gnu.org/licenses/. +import uuid + import pytest from django.conf import settings from mixer.backend.django import mixer -from apps.core.models import Group, Landscape, User +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core import group_collaboration_roles +from apps.core.models import Group, Landscape, SharedResource, User from apps.shared_data.models import DataEntry, VisualizationConfig @@ -117,3 +121,67 @@ def visualization_config_gpx(user): ), created_by=user, ) + + +@pytest.fixture +def shared_resource_data_entry_shared_all(users): + creator = users[0] + return mixer.blend( + SharedResource, + share_access=SharedResource.SHARE_ACCESS_ALL, + share_uuid=uuid.uuid4(), + target=mixer.blend(Group), + source=mixer.blend( + DataEntry, slug=None, created_by=creator, size=100, entry_type=DataEntry.ENTRY_TYPE_FILE + ), + ) + + +@pytest.fixture +def shared_resource_data_entry_shared_target_members(users): + creator = users[0] + creator_group = mixer.blend(Group) + creator_group.membership_list.save_membership( + creator.email, group_collaboration_roles.ROLE_MEMBER, CollaborationMembership.APPROVED + ) + return mixer.blend( + SharedResource, + share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS, + share_uuid=uuid.uuid4(), + target=creator_group, + source=mixer.blend( + DataEntry, slug=None, created_by=creator, size=100, entry_type=DataEntry.ENTRY_TYPE_FILE + ), + ) + + +@pytest.fixture +def shared_resource_data_entry_shared_target_members_user_1(users): + creator = users[1] + creator_group = mixer.blend(Group) + creator_group.membership_list.save_membership( + creator.email, group_collaboration_roles.ROLE_MEMBER, CollaborationMembership.APPROVED + ) + return mixer.blend( + SharedResource, + share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS, + share_uuid=uuid.uuid4(), + target=creator_group, + source=mixer.blend( + DataEntry, slug=None, created_by=creator, size=100, entry_type=DataEntry.ENTRY_TYPE_FILE + ), + ) + + +@pytest.fixture +def shared_resource_data_entry_shared_no_access(users): + creator = users[0] + return mixer.blend( + SharedResource, + share_access=SharedResource.SHARE_ACCESS_NO, + share_uuid=uuid.uuid4(), + target=mixer.blend(Group), + source=mixer.blend( + DataEntry, slug=None, created_by=creator, size=100, entry_type=DataEntry.ENTRY_TYPE_FILE + ), + ) diff --git a/terraso_backend/tests/shared_data/test_views.py b/terraso_backend/tests/shared_data/test_views.py index 6f32cb6bc..aa064f73e 100644 --- a/terraso_backend/tests/shared_data/test_views.py +++ b/terraso_backend/tests/shared_data/test_views.py @@ -165,3 +165,68 @@ def test_create_data_entry_file_invalid_type(logged_client, upload_url, data_ent response_data = response.json() assert "errors" in response_data + + +@mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") +def test_download_data_entry_file_shared_all( + get_url_mock, logged_client, shared_resource_data_entry_shared_all +): + redirect_url = "https://example.org/s3_file.json" + get_url_mock.return_value = redirect_url + url = reverse( + "shared_data:download", + kwargs={"shared_resource_uuid": shared_resource_data_entry_shared_all.share_uuid}, + ) + response = logged_client.get(url) + + assert response.status_code == 302 + assert response.url == redirect_url + + +@mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") +def test_download_data_entry_file_shared_target_members( + get_url_mock, logged_client, shared_resource_data_entry_shared_target_members +): + redirect_url = "https://example.org/s3_file.json" + get_url_mock.return_value = redirect_url + url = reverse( + "shared_data:download", + kwargs={ + "shared_resource_uuid": shared_resource_data_entry_shared_target_members.share_uuid + }, + ) + response = logged_client.get(url) + + assert response.status_code == 302 + assert response.url == redirect_url + + +@mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") +def test_download_data_entry_file_shared_target_members_fail( + get_url_mock, logged_client, shared_resource_data_entry_shared_target_members_user_1 +): + redirect_url = "https://example.org/s3_file.json" + get_url_mock.return_value = redirect_url + share_uuid = shared_resource_data_entry_shared_target_members_user_1.share_uuid + url = reverse( + "shared_data:download", + kwargs={"shared_resource_uuid": share_uuid}, + ) + response = logged_client.get(url) + + assert response.status_code == 404 + + +@mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") +def test_download_data_entry_file_shared_no_fail( + get_url_mock, logged_client, shared_resource_data_entry_shared_no_access +): + redirect_url = "https://example.org/s3_file.json" + get_url_mock.return_value = redirect_url + url = reverse( + "shared_data:download", + kwargs={"shared_resource_uuid": shared_resource_data_entry_shared_no_access.share_uuid}, + ) + response = logged_client.get(url) + + assert response.status_code == 404 From d9cb025524cf24253b98c21c92c00d87a149abeb Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 29 Jan 2024 17:41:20 -0500 Subject: [PATCH 05/20] fix: Removed not needed debug prints --- terraso_backend/tests/core/gis/test_parsers.py | 1 - .../graphql/mutations/test_shared_resource_mutations.py | 1 - terraso_backend/tests/graphql/test_shared_data.py | 7 ------- 3 files changed, 9 deletions(-) diff --git a/terraso_backend/tests/core/gis/test_parsers.py b/terraso_backend/tests/core/gis/test_parsers.py index 279a90902..e7f7a411f 100644 --- a/terraso_backend/tests/core/gis/test_parsers.py +++ b/terraso_backend/tests/core/gis/test_parsers.py @@ -202,7 +202,6 @@ def test_parse_shapefile(file_path_expected): with open(resources.files("tests").joinpath(expected_file_path), "rb") as file: expected_json = json.load(file) - print(f"shapefile_json: {shapefile_json}") assert json.dumps(shapefile_json) == json.dumps(expected_json) diff --git a/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py b/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py index 1cbac1c5b..71be149ab 100644 --- a/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py +++ b/terraso_backend/tests/graphql/mutations/test_shared_resource_mutations.py @@ -40,7 +40,6 @@ def test_shared_resource_update_by_source_works(client_query, data_entries): variables={"input": new_data}, ) json_result = response.json() - print(json_result) result = json_result["data"]["updateSharedResource"]["sharedResource"] assert result == new_data diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index 31cf07e75..b598aceff 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -147,9 +147,6 @@ def test_data_entries_filter_by_closed_group_slug_filters_successfuly( users[0].email, group_collaboration_roles.ROLE_MEMBER, CollaborationMembership.APPROVED ) - shared_resources = data_entry_a.shared_resources.all() - print(shared_resources) - response = client_query( """ {dataEntries(sharedResources_Target_Slug: "%s", sharedResources_TargetContentType: "%s") { @@ -345,10 +342,6 @@ def test_data_entries_empty_from_closed_group_query(client_query, groups_closed, data_entry_a.shared_resources.create(target=group) data_entry_b.shared_resources.create(target=group) - memberships = group.membership_list.memberships.all() - - print(memberships) - response = client_query( """ {groups(slug: "%s") { From 91db4a53f8342bb4f5b8802865fd56a353fb66d4 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Tue, 30 Jan 2024 14:09:49 -0500 Subject: [PATCH 06/20] fix: Make download url optional auth --- terraso_backend/apps/shared_data/urls.py | 4 +++- terraso_backend/tests/conftest.py | 5 +++++ terraso_backend/tests/shared_data/test_views.py | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/terraso_backend/apps/shared_data/urls.py b/terraso_backend/apps/shared_data/urls.py index c3edb10dc..ed411ab10 100644 --- a/terraso_backend/apps/shared_data/urls.py +++ b/terraso_backend/apps/shared_data/urls.py @@ -16,6 +16,8 @@ from django.urls import path from django.views.decorators.csrf import csrf_exempt +from apps.auth.middleware import auth_optional + from .views import DataEntryFileDownloadView, DataEntryFileUploadView app_name = "apps.shared_data" @@ -24,7 +26,7 @@ path("upload/", csrf_exempt(DataEntryFileUploadView.as_view()), name="upload"), path( "download/", - csrf_exempt(DataEntryFileDownloadView.as_view()), + csrf_exempt(auth_optional(DataEntryFileDownloadView.as_view())), name="download", ), ] diff --git a/terraso_backend/tests/conftest.py b/terraso_backend/tests/conftest.py index 1bc47b9d4..38cadf1d5 100644 --- a/terraso_backend/tests/conftest.py +++ b/terraso_backend/tests/conftest.py @@ -46,6 +46,11 @@ def logged_client(access_token): return Client(HTTP_AUTHORIZATION=f"Bearer {access_token}") +@pytest.fixture +def not_logged_client(access_token): + return Client() + + @pytest.fixture def unit_polygon(): """A polygon whose geographical area is roughly 1 km squared.""" diff --git a/terraso_backend/tests/shared_data/test_views.py b/terraso_backend/tests/shared_data/test_views.py index aa064f73e..f674191d2 100644 --- a/terraso_backend/tests/shared_data/test_views.py +++ b/terraso_backend/tests/shared_data/test_views.py @@ -169,7 +169,7 @@ def test_create_data_entry_file_invalid_type(logged_client, upload_url, data_ent @mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") def test_download_data_entry_file_shared_all( - get_url_mock, logged_client, shared_resource_data_entry_shared_all + get_url_mock, not_logged_client, shared_resource_data_entry_shared_all ): redirect_url = "https://example.org/s3_file.json" get_url_mock.return_value = redirect_url @@ -177,7 +177,7 @@ def test_download_data_entry_file_shared_all( "shared_data:download", kwargs={"shared_resource_uuid": shared_resource_data_entry_shared_all.share_uuid}, ) - response = logged_client.get(url) + response = not_logged_client.get(url) assert response.status_code == 302 assert response.url == redirect_url From ba2eb840b4f88ed111204e82ed18f613fd082dd7 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 31 Jan 2024 17:52:33 -0500 Subject: [PATCH 07/20] feat: WIP shared resource GQL node --- .../apps/graphql/schema/__init__.py | 4 +- .../apps/graphql/schema/shared_resources.py | 42 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/__init__.py b/terraso_backend/apps/graphql/schema/__init__.py index 6193bbd5e..7d092596e 100644 --- a/terraso_backend/apps/graphql/schema/__init__.py +++ b/terraso_backend/apps/graphql/schema/__init__.py @@ -80,7 +80,7 @@ LandscapeMembershipDeleteMutation, LandscapeMembershipSaveMutation, ) -from .shared_resources import SharedResourceUpdateMutation +from .shared_resources import SharedResourceRelayNode, SharedResourceUpdateMutation from .sites import ( SiteAddMutation, SiteDeleteMutation, @@ -140,6 +140,8 @@ class Query(graphene.ObjectType): site = TerrasoRelayNode.Field(SiteNode) sites = DjangoFilterConnectionField(SiteNode, required=True) audit_logs = DjangoFilterConnectionField(AuditLogNode) + shared_resource = SharedResourceRelayNode.Field() + from .shared_resources import resolve_shared_resource # All mutations should inherit from BaseWriteMutation or BaseDeleteMutation diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index bbd30379e..d7e81397c 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -17,10 +17,12 @@ import rules import structlog from django.conf import settings +from django.db.models import Q, Subquery from graphene import relay from graphene_django import DjangoObjectType -from apps.core.models import SharedResource +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core.models import Group, Landscape, SharedResource from apps.graphql.exceptions import GraphQLNotAllowedException, GraphQLNotFoundException from . import GroupNode, LandscapeNode @@ -64,6 +66,44 @@ def resolve_share_url(self, info, **kwargs): return f"{settings.API_ENDPOINT}/shared-data/download/{self.share_uuid}" +class SharedResourceRelayNode: + @classmethod + def Field(cls): + return graphene.Field(SharedResourceNode, share_uuid=graphene.String(required=True)) + + +def resolve_shared_resource(root, info, share_uuid=None): + if not share_uuid: + return None + + user_pk = getattr(info.context.user, "pk", False) + user_groups_ids = Subquery( + Group.objects.filter( + membership_list__memberships__deleted_at__isnull=True, + membership_list__memberships__user__id=user_pk, + membership_list__memberships__membership_status=CollaborationMembership.APPROVED, + ).values("id") + ) + user_landscape_ids = Subquery( + Landscape.objects.filter( + membership_list__memberships__deleted_at__isnull=True, + membership_list__memberships__user__id=user_pk, + membership_list__memberships__membership_status=CollaborationMembership.APPROVED, + ).values("id") + ) + + share_access_no = Q(share_access=SharedResource.SHARE_ACCESS_NO) + share_access_all = Q(share_access=SharedResource.SHARE_ACCESS_ALL) + share_access_members = Q( + Q(share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS) + & Q(Q(target_object_id__in=user_groups_ids) | Q(target_object_id__in=user_landscape_ids)) + ) + + return SharedResource.objects.filter( + Q(share_uuid=share_uuid) & ~share_access_no & Q(share_access_all | share_access_members) + ).first() + + class SharedResourceUpdateMutation(BaseWriteMutation): shared_resource = graphene.Field(SharedResourceNode) From 7c004c5fe09670408da55902b4c98d2104c84886 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 5 Feb 2024 21:25:21 -0500 Subject: [PATCH 08/20] chore: Added tests for shared resource query --- terraso_backend/tests/graphql/conftest.py | 2 + .../tests/graphql/test_shared_resource.py | 125 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 terraso_backend/tests/graphql/test_shared_resource.py diff --git a/terraso_backend/tests/graphql/conftest.py b/terraso_backend/tests/graphql/conftest.py index 9ff927f81..8884b2a9d 100644 --- a/terraso_backend/tests/graphql/conftest.py +++ b/terraso_backend/tests/graphql/conftest.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see https://www.gnu.org/licenses/. +import uuid from datetime import timedelta import pytest @@ -321,6 +322,7 @@ def group_data_entries(users, groups): resources = mixer.cycle(5).blend( SharedResource, target=creator_group, + share_uuid=lambda: uuid.uuid4(), source=lambda: mixer.blend(DataEntry, created_by=creator, size=100, resource_type="csv"), ) return [resource.source for resource in resources] diff --git a/terraso_backend/tests/graphql/test_shared_resource.py b/terraso_backend/tests/graphql/test_shared_resource.py new file mode 100644 index 000000000..61ac50dc8 --- /dev/null +++ b/terraso_backend/tests/graphql/test_shared_resource.py @@ -0,0 +1,125 @@ +# Copyright © 2024 Technology Matters +# +# This program 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. +# +# This program 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 this program. If not, see https://www.gnu.org/licenses/. + + +import pytest + +from apps.collaboration.models import Membership as CollaborationMembership +from apps.core import group_collaboration_roles +from apps.core.models import SharedResource + +pytestmark = pytest.mark.django_db + + +def test_shared_resource_access_no(client_query, data_entries): + data_entry = data_entries[0] + shared_resource = data_entry.shared_resources.all()[0] + + shared_resource.share_access = SharedResource.SHARE_ACCESS_NO + shared_resource.save() + + response = client_query( + """ + {sharedResource(shareUuid: "%s") { + shareAccess + }} + """ + % shared_resource.share_uuid + ) + + json_response = response.json() + + result = json_response["data"]["sharedResource"] + + assert result is None + + +def test_shared_resource_access_all(client_query, data_entries): + data_entry = data_entries[0] + shared_resource = data_entry.shared_resources.all()[0] + + shared_resource.target.membership_list.memberships.all().delete() + + shared_resource.share_access = SharedResource.SHARE_ACCESS_ALL + shared_resource.save() + + response = client_query( + """ + {sharedResource(shareUuid: "%s") { + shareAccess + }} + """ + % shared_resource.share_uuid + ) + + json_response = response.json() + + result = json_response["data"]["sharedResource"] + + assert shared_resource.share_access == result["shareAccess"].lower() + + +def test_shared_resource_access_members(client_query, data_entries, users): + data_entry = data_entries[0] + shared_resource = data_entry.shared_resources.all()[0] + + shared_resource.target.membership_list.memberships.all().delete() + + shared_resource.target.membership_list.save_membership( + users[0].email, group_collaboration_roles.ROLE_MEMBER, CollaborationMembership.APPROVED + ) + + shared_resource.share_access = SharedResource.SHARE_ACCESS_TARGET_MEMBERS + shared_resource.save() + + response = client_query( + """ + {sharedResource(shareUuid: "%s") { + shareAccess + }} + """ + % shared_resource.share_uuid + ) + + json_response = response.json() + + result = json_response["data"]["sharedResource"] + + assert shared_resource.share_access == result["shareAccess"].lower() + + +def test_shared_resource_access_members_fail(client_query, data_entries, users): + data_entry = data_entries[0] + shared_resource = data_entry.shared_resources.all()[0] + + shared_resource.target.membership_list.memberships.all().delete() + + shared_resource.share_access = SharedResource.SHARE_ACCESS_TARGET_MEMBERS + shared_resource.save() + + response = client_query( + """ + {sharedResource(shareUuid: "%s") { + shareAccess + }} + """ + % shared_resource.share_uuid + ) + + json_response = response.json() + + result = json_response["data"]["sharedResource"] + + assert result is None From 961aefb8fd648f2c640c9ccbba45ae1debc9be9e Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 5 Feb 2024 21:27:41 -0500 Subject: [PATCH 09/20] feat: Updated schema --- terraso_backend/apps/graphql/schema/schema.graphql | 1 + 1 file changed, 1 insertion(+) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index e4fea7e91..aa45eadfb 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -89,6 +89,7 @@ type Query { """Ordering""" orderBy: String ): AuditLogNodeConnection + sharedResource(shareUuid: String!): SharedResourceNode } type GroupNode implements Node { From 36688f2a90ec7e220bd6f3aaa86020a16f75417c Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Mon, 5 Feb 2024 22:00:24 -0500 Subject: [PATCH 10/20] feat: Added share url and download url --- terraso_backend/apps/graphql/schema/schema.graphql | 14 ++++++++------ .../apps/graphql/schema/shared_resources.py | 12 ++++++++++-- terraso_backend/tests/graphql/test_shared_data.py | 10 ++++++++-- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index aa45eadfb..48e210ce2 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -431,12 +431,20 @@ type SharedResourceNodeEdge { type SharedResourceNode implements Node { id: ID! + shareUuid: UUID! shareAccess: CoreSharedResourceShareAccessChoices! source: SourceNode target: TargetNode + downloadUrl: String shareUrl: String } +""" +Leverages the internal Python implementation of UUID (uuid.UUID) to provide native UUID objects +in fields, resolvers and input. +""" +scalar UUID + """An enumeration.""" enum CoreSharedResourceShareAccessChoices { """No share access""" @@ -522,12 +530,6 @@ type VisualizationConfigNodeEdge { cursor: String! } -""" -Leverages the internal Python implementation of UUID (uuid.UUID) to provide native UUID objects -in fields, resolvers and input. -""" -scalar UUID - union OwnerNode = GroupNode | LandscapeNode union TargetNode = GroupNode | LandscapeNode diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index d7e81397c..2e2457a06 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -48,11 +48,12 @@ class SharedResourceNode(DjangoObjectType): id = graphene.ID(source="pk", required=True) source = graphene.Field(SourceNode) target = graphene.Field(TargetNode) + download_url = graphene.String() share_url = graphene.String() class Meta: model = SharedResource - fields = ["id", "share_access"] + fields = ["id", "share_access", "share_uuid"] interfaces = (relay.Node,) connection_class = TerrasoConnection @@ -62,9 +63,16 @@ def resolve_source(self, info, **kwargs): def resolve_target(self, info, **kwargs): return self.target - def resolve_share_url(self, info, **kwargs): + def resolve_download_url(self, info, **kwargs): return f"{settings.API_ENDPOINT}/shared-data/download/{self.share_uuid}" + def resolve_share_url(self, info, **kwargs): + target = self.target + entity = "groups" if isinstance(target, Group) else "landscapes" + slug = target.slug + share_uuid = self.share_uuid + return f"{settings.WEB_CLIENT_URL}/{entity}/{slug}/shared-resource/download/{share_uuid}" + class SharedResourceRelayNode: @classmethod diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index b598aceff..70c26da60 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -399,6 +399,7 @@ def test_data_entries_from_parent_query_by_resource_field( } } shareUrl + downloadUrl shareAccess } } @@ -417,16 +418,21 @@ def test_data_entries_from_parent_query_by_resource_field( { "name": resource["node"]["source"]["name"], "share_url": resource["node"]["shareUrl"], + "download_url": resource["node"]["downloadUrl"], "share_access": resource["node"]["shareAccess"], } for resource in resources ] for data_entry in data_entries: - share_uuid = data_entry.shared_resources.all()[0].share_uuid - share_url = f"{settings.API_ENDPOINT}/shared-data/download/{share_uuid}" + uuid = data_entry.shared_resources.all()[0].share_uuid + download_url = f"{settings.API_ENDPOINT}/shared-data/download/{uuid}" + slug = parent_entity.slug + share_url = f"{settings.WEB_CLIENT_URL}/{parent}/{slug}/shared-resource/download/{uuid}" + assert { "name": data_entry.name, + "download_url": download_url, "share_url": share_url, "share_access": data_entry.shared_resources.all()[0].share_access.upper(), } in entries_result From e3813bd1d0d896106afffae955851b74d62af1db Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 7 Feb 2024 14:36:34 -0500 Subject: [PATCH 11/20] fix: Added SharedResource to admin, fixed typo --- terraso_backend/apps/core/admin.py | 6 ++++++ terraso_backend/apps/core/models/shared_resources.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/terraso_backend/apps/core/admin.py b/terraso_backend/apps/core/admin.py index 6eac68fd5..a3e0e3979 100644 --- a/terraso_backend/apps/core/admin.py +++ b/terraso_backend/apps/core/admin.py @@ -20,6 +20,7 @@ Landscape, LandscapeDevelopmentStrategy, LandscapeGroup, + SharedResource, TaxonomyTerm, User, UserPreference, @@ -69,3 +70,8 @@ class TaxonomyTermAdmin(admin.ModelAdmin): @admin.register(LandscapeDevelopmentStrategy) class LandscapeDevelopmentStrategyAdmin(admin.ModelAdmin): list_display = ("id", "landscape") + + +@admin.register(SharedResource) +class SharedResourceAdmin(admin.ModelAdmin): + list_display = ("id", "share_uuid", "share_access") diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index 3ef9c4114..e45d81523 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -37,7 +37,7 @@ class SharedResource(BaseModel): SHARE_ACCESS_TYPES = ( (SHARE_ACCESS_NO, _("No share access")), (SHARE_ACCESS_ALL, _("Anyone with the link")), - (SHARE_ACCESS_TARGET_MEMBERS, _("Only tagert members")), + (SHARE_ACCESS_TARGET_MEMBERS, _("Only target members")), ) source = GenericForeignKey("source_content_type", "source_object_id") From 421812278d2451ee39586dda8558244bfc5ba737 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 7 Feb 2024 15:57:48 -0500 Subject: [PATCH 12/20] fix: Fixed schema --- terraso_backend/apps/graphql/schema/schema.graphql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 48e210ce2..937fce74e 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -453,7 +453,7 @@ enum CoreSharedResourceShareAccessChoices { """Anyone with the link""" ALL - """Only tagert members""" + """Only target members""" TARGET_MEMBERS } From 80a7a4f7e2f481c75f69e23726cbac04e8f3c062 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 14 Feb 2024 14:15:07 -0500 Subject: [PATCH 13/20] fix: Renamed constant, improved share url, added explicit if for landscape entity --- .../apps/core/models/shared_resources.py | 10 +++++----- .../apps/graphql/schema/shared_resources.py | 16 ++++++++++++---- .../apps/shared_data/permission_rules.py | 2 +- terraso_backend/apps/shared_data/views.py | 2 +- .../tests/graphql/test_shared_data.py | 2 +- .../tests/graphql/test_shared_resource.py | 2 +- terraso_backend/tests/shared_data/conftest.py | 2 +- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index e45d81523..634198250 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -29,13 +29,13 @@ class SharedResource(BaseModel): Target represents the resource that is receiving the shared resource (Example: Landscape). """ - SHARE_ACCESS_NO = "no" + SHARE_ACCESS_NONE = "no" SHARE_ACCESS_ALL = "all" SHARE_ACCESS_TARGET_MEMBERS = "target_members" - DEFAULT_SHARE_ACCESS = SHARE_ACCESS_NO + DEFAULT_SHARE_ACCESS = SHARE_ACCESS_NONE SHARE_ACCESS_TYPES = ( - (SHARE_ACCESS_NO, _("No share access")), + (SHARE_ACCESS_NONE, _("No share access")), (SHARE_ACCESS_ALL, _("Anyone with the link")), (SHARE_ACCESS_TARGET_MEMBERS, _("Only target members")), ) @@ -70,10 +70,10 @@ class Meta: @classmethod def get_share_access_from_text(cls, share_access): if not share_access: - return cls.SHARE_ACCESS_NO + return cls.SHARE_ACCESS_NONE lowered = share_access.lower() if lowered == cls.SHARE_ACCESS_ALL: return cls.SHARE_ACCESS_ALL if lowered == cls.SHARE_ACCESS_TARGET_MEMBERS: return cls.SHARE_ACCESS_TARGET_MEMBERS - return cls.SHARE_ACCESS_NO + return cls.SHARE_ACCESS_NONE diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index 2e2457a06..487223619 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -68,10 +68,18 @@ def resolve_download_url(self, info, **kwargs): def resolve_share_url(self, info, **kwargs): target = self.target - entity = "groups" if isinstance(target, Group) else "landscapes" + entity = ( + "groups" + if isinstance(target, Group) + else "landscapes" + if isinstance(target, Landscape) + else None + ) + if not entity: + return None slug = target.slug share_uuid = self.share_uuid - return f"{settings.WEB_CLIENT_URL}/{entity}/{slug}/shared-resource/download/{share_uuid}" + return f"{settings.WEB_CLIENT_URL}/{entity}/{slug}/download/{share_uuid}" class SharedResourceRelayNode: @@ -100,7 +108,7 @@ def resolve_shared_resource(root, info, share_uuid=None): ).values("id") ) - share_access_no = Q(share_access=SharedResource.SHARE_ACCESS_NO) + share_access_none = Q(share_access=SharedResource.SHARE_ACCESS_NONE) share_access_all = Q(share_access=SharedResource.SHARE_ACCESS_ALL) share_access_members = Q( Q(share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS) @@ -108,7 +116,7 @@ def resolve_shared_resource(root, info, share_uuid=None): ) return SharedResource.objects.filter( - Q(share_uuid=share_uuid) & ~share_access_no & Q(share_access_all | share_access_members) + Q(share_uuid=share_uuid) & ~share_access_none & Q(share_access_all | share_access_members) ).first() diff --git a/terraso_backend/apps/shared_data/permission_rules.py b/terraso_backend/apps/shared_data/permission_rules.py index 91f659f3b..0809944d1 100644 --- a/terraso_backend/apps/shared_data/permission_rules.py +++ b/terraso_backend/apps/shared_data/permission_rules.py @@ -107,7 +107,7 @@ def allowed_to_delete_visualization_config(user, visualization_config): def allowed_to_download_data_entry_file(user, shared_resource): target = shared_resource.target - if shared_resource.share_access == SharedResource.SHARE_ACCESS_NO: + if shared_resource.share_access == SharedResource.SHARE_ACCESS_NONE: return False if shared_resource.share_access == SharedResource.SHARE_ACCESS_ALL: diff --git a/terraso_backend/apps/shared_data/views.py b/terraso_backend/apps/shared_data/views.py index 1add7b2f6..0dea84be0 100644 --- a/terraso_backend/apps/shared_data/views.py +++ b/terraso_backend/apps/shared_data/views.py @@ -48,7 +48,7 @@ def get(self, request, shared_resource_uuid, *args, **kwargs): if shared_resource is None: return HttpResponse("Not Found", status=404) - not_shared = shared_resource.share_access == SharedResource.SHARE_ACCESS_NO + not_shared = shared_resource.share_access == SharedResource.SHARE_ACCESS_NONE needs_authentication = ( shared_resource.share_access != SharedResource.SHARE_ACCESS_ALL and not request.user.is_authenticated diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index 70c26da60..13c139335 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -428,7 +428,7 @@ def test_data_entries_from_parent_query_by_resource_field( uuid = data_entry.shared_resources.all()[0].share_uuid download_url = f"{settings.API_ENDPOINT}/shared-data/download/{uuid}" slug = parent_entity.slug - share_url = f"{settings.WEB_CLIENT_URL}/{parent}/{slug}/shared-resource/download/{uuid}" + share_url = f"{settings.WEB_CLIENT_URL}/{parent}/{slug}/download/{uuid}" assert { "name": data_entry.name, diff --git a/terraso_backend/tests/graphql/test_shared_resource.py b/terraso_backend/tests/graphql/test_shared_resource.py index 61ac50dc8..275c4341d 100644 --- a/terraso_backend/tests/graphql/test_shared_resource.py +++ b/terraso_backend/tests/graphql/test_shared_resource.py @@ -27,7 +27,7 @@ def test_shared_resource_access_no(client_query, data_entries): data_entry = data_entries[0] shared_resource = data_entry.shared_resources.all()[0] - shared_resource.share_access = SharedResource.SHARE_ACCESS_NO + shared_resource.share_access = SharedResource.SHARE_ACCESS_NONE shared_resource.save() response = client_query( diff --git a/terraso_backend/tests/shared_data/conftest.py b/terraso_backend/tests/shared_data/conftest.py index 0d47d6e09..c44f9ec5e 100644 --- a/terraso_backend/tests/shared_data/conftest.py +++ b/terraso_backend/tests/shared_data/conftest.py @@ -178,7 +178,7 @@ def shared_resource_data_entry_shared_no_access(users): creator = users[0] return mixer.blend( SharedResource, - share_access=SharedResource.SHARE_ACCESS_NO, + share_access=SharedResource.SHARE_ACCESS_NONE, share_uuid=uuid.uuid4(), target=mixer.blend(Group), source=mixer.blend( From cf848bf4c82ca25f1915ea5d460d80cbd3835cf8 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 14 Feb 2024 15:00:27 -0500 Subject: [PATCH 14/20] fix: Removed share none option --- .../0050_sharedresource_remove_none.py | 40 +++++++++++++++++++ .../apps/core/models/shared_resources.py | 10 ++--- .../apps/graphql/schema/shared_resources.py | 3 +- .../apps/shared_data/permission_rules.py | 3 -- terraso_backend/apps/shared_data/views.py | 4 +- .../tests/graphql/test_shared_resource.py | 23 ----------- terraso_backend/tests/shared_data/conftest.py | 14 ------- .../tests/shared_data/test_views.py | 15 ------- 8 files changed, 45 insertions(+), 67 deletions(-) create mode 100644 terraso_backend/apps/core/migrations/0050_sharedresource_remove_none.py diff --git a/terraso_backend/apps/core/migrations/0050_sharedresource_remove_none.py b/terraso_backend/apps/core/migrations/0050_sharedresource_remove_none.py new file mode 100644 index 000000000..b66432cc5 --- /dev/null +++ b/terraso_backend/apps/core/migrations/0050_sharedresource_remove_none.py @@ -0,0 +1,40 @@ +# Copyright © 2024 Technology Matters +# +# This program 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. +# +# This program 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 this program. If not, see https://www.gnu.org/licenses/. + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0049_sharedresource_share_fields"), + ] + + operations = [ + migrations.RunSQL( + sql="UPDATE core_sharedresource SET share_access ='target_members' WHERE share_access='no';" + ), + migrations.AlterField( + model_name="sharedresource", + name="share_access", + field=models.CharField( + choices=[ + ("all", "Anyone with the link"), + ("target_members", "Only target members"), + ], + default="target_members", + max_length=32, + ), + ), + ] diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index 634198250..6028889eb 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -29,13 +29,11 @@ class SharedResource(BaseModel): Target represents the resource that is receiving the shared resource (Example: Landscape). """ - SHARE_ACCESS_NONE = "no" SHARE_ACCESS_ALL = "all" SHARE_ACCESS_TARGET_MEMBERS = "target_members" - DEFAULT_SHARE_ACCESS = SHARE_ACCESS_NONE + DEFAULT_SHARE_ACCESS = SHARE_ACCESS_TARGET_MEMBERS SHARE_ACCESS_TYPES = ( - (SHARE_ACCESS_NONE, _("No share access")), (SHARE_ACCESS_ALL, _("Anyone with the link")), (SHARE_ACCESS_TARGET_MEMBERS, _("Only target members")), ) @@ -70,10 +68,8 @@ class Meta: @classmethod def get_share_access_from_text(cls, share_access): if not share_access: - return cls.SHARE_ACCESS_NONE + return cls.SHARE_ACCESS_TARGET_MEMBERS lowered = share_access.lower() if lowered == cls.SHARE_ACCESS_ALL: return cls.SHARE_ACCESS_ALL - if lowered == cls.SHARE_ACCESS_TARGET_MEMBERS: - return cls.SHARE_ACCESS_TARGET_MEMBERS - return cls.SHARE_ACCESS_NONE + return cls.SHARE_ACCESS_TARGET_MEMBERS diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index 487223619..162ab7dd1 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -108,7 +108,6 @@ def resolve_shared_resource(root, info, share_uuid=None): ).values("id") ) - share_access_none = Q(share_access=SharedResource.SHARE_ACCESS_NONE) share_access_all = Q(share_access=SharedResource.SHARE_ACCESS_ALL) share_access_members = Q( Q(share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS) @@ -116,7 +115,7 @@ def resolve_shared_resource(root, info, share_uuid=None): ) return SharedResource.objects.filter( - Q(share_uuid=share_uuid) & ~share_access_none & Q(share_access_all | share_access_members) + Q(share_uuid=share_uuid) & Q(share_access_all | share_access_members) ).first() diff --git a/terraso_backend/apps/shared_data/permission_rules.py b/terraso_backend/apps/shared_data/permission_rules.py index 0809944d1..f35be2a4f 100644 --- a/terraso_backend/apps/shared_data/permission_rules.py +++ b/terraso_backend/apps/shared_data/permission_rules.py @@ -107,9 +107,6 @@ def allowed_to_delete_visualization_config(user, visualization_config): def allowed_to_download_data_entry_file(user, shared_resource): target = shared_resource.target - if shared_resource.share_access == SharedResource.SHARE_ACCESS_NONE: - return False - if shared_resource.share_access == SharedResource.SHARE_ACCESS_ALL: return True diff --git a/terraso_backend/apps/shared_data/views.py b/terraso_backend/apps/shared_data/views.py index 0dea84be0..61105ea62 100644 --- a/terraso_backend/apps/shared_data/views.py +++ b/terraso_backend/apps/shared_data/views.py @@ -48,13 +48,11 @@ def get(self, request, shared_resource_uuid, *args, **kwargs): if shared_resource is None: return HttpResponse("Not Found", status=404) - not_shared = shared_resource.share_access == SharedResource.SHARE_ACCESS_NONE needs_authentication = ( shared_resource.share_access != SharedResource.SHARE_ACCESS_ALL and not request.user.is_authenticated ) - - if not_shared or needs_authentication: + if needs_authentication: return HttpResponse("Not Found", status=404) source = shared_resource.source diff --git a/terraso_backend/tests/graphql/test_shared_resource.py b/terraso_backend/tests/graphql/test_shared_resource.py index 275c4341d..65f0cdf90 100644 --- a/terraso_backend/tests/graphql/test_shared_resource.py +++ b/terraso_backend/tests/graphql/test_shared_resource.py @@ -23,29 +23,6 @@ pytestmark = pytest.mark.django_db -def test_shared_resource_access_no(client_query, data_entries): - data_entry = data_entries[0] - shared_resource = data_entry.shared_resources.all()[0] - - shared_resource.share_access = SharedResource.SHARE_ACCESS_NONE - shared_resource.save() - - response = client_query( - """ - {sharedResource(shareUuid: "%s") { - shareAccess - }} - """ - % shared_resource.share_uuid - ) - - json_response = response.json() - - result = json_response["data"]["sharedResource"] - - assert result is None - - def test_shared_resource_access_all(client_query, data_entries): data_entry = data_entries[0] shared_resource = data_entry.shared_resources.all()[0] diff --git a/terraso_backend/tests/shared_data/conftest.py b/terraso_backend/tests/shared_data/conftest.py index c44f9ec5e..3946fffc2 100644 --- a/terraso_backend/tests/shared_data/conftest.py +++ b/terraso_backend/tests/shared_data/conftest.py @@ -171,17 +171,3 @@ def shared_resource_data_entry_shared_target_members_user_1(users): DataEntry, slug=None, created_by=creator, size=100, entry_type=DataEntry.ENTRY_TYPE_FILE ), ) - - -@pytest.fixture -def shared_resource_data_entry_shared_no_access(users): - creator = users[0] - return mixer.blend( - SharedResource, - share_access=SharedResource.SHARE_ACCESS_NONE, - share_uuid=uuid.uuid4(), - target=mixer.blend(Group), - source=mixer.blend( - DataEntry, slug=None, created_by=creator, size=100, entry_type=DataEntry.ENTRY_TYPE_FILE - ), - ) diff --git a/terraso_backend/tests/shared_data/test_views.py b/terraso_backend/tests/shared_data/test_views.py index f674191d2..fabb21c1a 100644 --- a/terraso_backend/tests/shared_data/test_views.py +++ b/terraso_backend/tests/shared_data/test_views.py @@ -215,18 +215,3 @@ def test_download_data_entry_file_shared_target_members_fail( response = logged_client.get(url) assert response.status_code == 404 - - -@mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") -def test_download_data_entry_file_shared_no_fail( - get_url_mock, logged_client, shared_resource_data_entry_shared_no_access -): - redirect_url = "https://example.org/s3_file.json" - get_url_mock.return_value = redirect_url - url = reverse( - "shared_data:download", - kwargs={"shared_resource_uuid": shared_resource_data_entry_shared_no_access.share_uuid}, - ) - response = logged_client.get(url) - - assert response.status_code == 404 From da11d701a0d3c331b0fcb0ca76296a8da128ac3d Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 14 Feb 2024 15:18:28 -0500 Subject: [PATCH 15/20] fix: Updated GQL schema --- terraso_backend/apps/graphql/schema/schema.graphql | 3 --- 1 file changed, 3 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 937fce74e..55c3c41bf 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -447,9 +447,6 @@ scalar UUID """An enumeration.""" enum CoreSharedResourceShareAccessChoices { - """No share access""" - NO - """Anyone with the link""" ALL From 1772abc85ef69bf0c90d5f708fef70b41997bb45 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 14 Feb 2024 16:02:07 -0500 Subject: [PATCH 16/20] fix: Linter fix --- terraso_backend/apps/graphql/schema/shared_resources.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index 162ab7dd1..94307118e 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -71,9 +71,7 @@ def resolve_share_url(self, info, **kwargs): entity = ( "groups" if isinstance(target, Group) - else "landscapes" - if isinstance(target, Landscape) - else None + else "landscapes" if isinstance(target, Landscape) else None ) if not entity: return None From aa74c4a50da25145dacbbef3119a4d2b19d18944 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 14 Feb 2024 16:11:24 -0500 Subject: [PATCH 17/20] fix: Fixed fixture name --- terraso_backend/tests/conftest.py | 2 +- terraso_backend/tests/shared_data/test_views.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/terraso_backend/tests/conftest.py b/terraso_backend/tests/conftest.py index 38cadf1d5..9b5cca3b6 100644 --- a/terraso_backend/tests/conftest.py +++ b/terraso_backend/tests/conftest.py @@ -47,7 +47,7 @@ def logged_client(access_token): @pytest.fixture -def not_logged_client(access_token): +def not_logged_in_client(access_token): return Client() diff --git a/terraso_backend/tests/shared_data/test_views.py b/terraso_backend/tests/shared_data/test_views.py index fabb21c1a..54d9413fe 100644 --- a/terraso_backend/tests/shared_data/test_views.py +++ b/terraso_backend/tests/shared_data/test_views.py @@ -169,7 +169,7 @@ def test_create_data_entry_file_invalid_type(logged_client, upload_url, data_ent @mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") def test_download_data_entry_file_shared_all( - get_url_mock, not_logged_client, shared_resource_data_entry_shared_all + get_url_mock, not_logged_in_client, shared_resource_data_entry_shared_all ): redirect_url = "https://example.org/s3_file.json" get_url_mock.return_value = redirect_url @@ -177,7 +177,7 @@ def test_download_data_entry_file_shared_all( "shared_data:download", kwargs={"shared_resource_uuid": shared_resource_data_entry_shared_all.share_uuid}, ) - response = not_logged_client.get(url) + response = not_logged_in_client.get(url) assert response.status_code == 302 assert response.url == redirect_url From 109fb235a55666eaa18a873ba56ad249f799b8d3 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Thu, 15 Feb 2024 17:15:42 -0500 Subject: [PATCH 18/20] fix: Renamed target_members to members --- ...1_sharedresource_renamed_target_members.py | 29 +++++++++++++++++++ .../apps/core/models/shared_resources.py | 10 +++---- .../apps/graphql/schema/shared_resources.py | 2 +- .../apps/shared_data/permission_rules.py | 2 +- .../tests/graphql/test_shared_resource.py | 4 +-- terraso_backend/tests/shared_data/conftest.py | 8 ++--- .../tests/shared_data/test_views.py | 14 ++++----- 7 files changed, 48 insertions(+), 21 deletions(-) create mode 100644 terraso_backend/apps/core/migrations/0051_sharedresource_renamed_target_members.py diff --git a/terraso_backend/apps/core/migrations/0051_sharedresource_renamed_target_members.py b/terraso_backend/apps/core/migrations/0051_sharedresource_renamed_target_members.py new file mode 100644 index 000000000..4cb604f06 --- /dev/null +++ b/terraso_backend/apps/core/migrations/0051_sharedresource_renamed_target_members.py @@ -0,0 +1,29 @@ +# Copyright © 2024 Technology Matters +# +# This program 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. +# +# This program 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 this program. If not, see https://www.gnu.org/licenses/. + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0050_sharedresource_remove_none"), + ] + + operations = [ + migrations.RunSQL( + sql="UPDATE core_sharedresource SET share_access ='members' WHERE share_access='target_members';" + ), + ] diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index 6028889eb..40b03e17b 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -30,12 +30,12 @@ class SharedResource(BaseModel): """ SHARE_ACCESS_ALL = "all" - SHARE_ACCESS_TARGET_MEMBERS = "target_members" - DEFAULT_SHARE_ACCESS = SHARE_ACCESS_TARGET_MEMBERS + SHARE_ACCESS_MEMBERS = "members" + DEFAULT_SHARE_ACCESS = SHARE_ACCESS_MEMBERS SHARE_ACCESS_TYPES = ( (SHARE_ACCESS_ALL, _("Anyone with the link")), - (SHARE_ACCESS_TARGET_MEMBERS, _("Only target members")), + (SHARE_ACCESS_MEMBERS, _("Only members")), ) source = GenericForeignKey("source_content_type", "source_object_id") @@ -68,8 +68,8 @@ class Meta: @classmethod def get_share_access_from_text(cls, share_access): if not share_access: - return cls.SHARE_ACCESS_TARGET_MEMBERS + return cls.SHARE_ACCESS_MEMBERS lowered = share_access.lower() if lowered == cls.SHARE_ACCESS_ALL: return cls.SHARE_ACCESS_ALL - return cls.SHARE_ACCESS_TARGET_MEMBERS + return cls.SHARE_ACCESS_MEMBERS diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index 94307118e..96ba0c049 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -108,7 +108,7 @@ def resolve_shared_resource(root, info, share_uuid=None): share_access_all = Q(share_access=SharedResource.SHARE_ACCESS_ALL) share_access_members = Q( - Q(share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS) + Q(share_access=SharedResource.SHARE_ACCESS_MEMBERS) & Q(Q(target_object_id__in=user_groups_ids) | Q(target_object_id__in=user_landscape_ids)) ) diff --git a/terraso_backend/apps/shared_data/permission_rules.py b/terraso_backend/apps/shared_data/permission_rules.py index f35be2a4f..0a888c6a0 100644 --- a/terraso_backend/apps/shared_data/permission_rules.py +++ b/terraso_backend/apps/shared_data/permission_rules.py @@ -110,7 +110,7 @@ def allowed_to_download_data_entry_file(user, shared_resource): if shared_resource.share_access == SharedResource.SHARE_ACCESS_ALL: return True - if shared_resource.share_access == SharedResource.SHARE_ACCESS_TARGET_MEMBERS: + if shared_resource.share_access == SharedResource.SHARE_ACCESS_MEMBERS: return is_target_member(user, target) diff --git a/terraso_backend/tests/graphql/test_shared_resource.py b/terraso_backend/tests/graphql/test_shared_resource.py index 65f0cdf90..41c9b3d74 100644 --- a/terraso_backend/tests/graphql/test_shared_resource.py +++ b/terraso_backend/tests/graphql/test_shared_resource.py @@ -58,7 +58,7 @@ def test_shared_resource_access_members(client_query, data_entries, users): users[0].email, group_collaboration_roles.ROLE_MEMBER, CollaborationMembership.APPROVED ) - shared_resource.share_access = SharedResource.SHARE_ACCESS_TARGET_MEMBERS + shared_resource.share_access = SharedResource.SHARE_ACCESS_MEMBERS shared_resource.save() response = client_query( @@ -83,7 +83,7 @@ def test_shared_resource_access_members_fail(client_query, data_entries, users): shared_resource.target.membership_list.memberships.all().delete() - shared_resource.share_access = SharedResource.SHARE_ACCESS_TARGET_MEMBERS + shared_resource.share_access = SharedResource.SHARE_ACCESS_MEMBERS shared_resource.save() response = client_query( diff --git a/terraso_backend/tests/shared_data/conftest.py b/terraso_backend/tests/shared_data/conftest.py index 3946fffc2..e09b9b219 100644 --- a/terraso_backend/tests/shared_data/conftest.py +++ b/terraso_backend/tests/shared_data/conftest.py @@ -138,7 +138,7 @@ def shared_resource_data_entry_shared_all(users): @pytest.fixture -def shared_resource_data_entry_shared_target_members(users): +def shared_resource_data_entry_shared_members(users): creator = users[0] creator_group = mixer.blend(Group) creator_group.membership_list.save_membership( @@ -146,7 +146,7 @@ def shared_resource_data_entry_shared_target_members(users): ) return mixer.blend( SharedResource, - share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS, + share_access=SharedResource.SHARE_ACCESS_MEMBERS, share_uuid=uuid.uuid4(), target=creator_group, source=mixer.blend( @@ -156,7 +156,7 @@ def shared_resource_data_entry_shared_target_members(users): @pytest.fixture -def shared_resource_data_entry_shared_target_members_user_1(users): +def shared_resource_data_entry_shared_members_user_1(users): creator = users[1] creator_group = mixer.blend(Group) creator_group.membership_list.save_membership( @@ -164,7 +164,7 @@ def shared_resource_data_entry_shared_target_members_user_1(users): ) return mixer.blend( SharedResource, - share_access=SharedResource.SHARE_ACCESS_TARGET_MEMBERS, + share_access=SharedResource.SHARE_ACCESS_MEMBERS, share_uuid=uuid.uuid4(), target=creator_group, source=mixer.blend( diff --git a/terraso_backend/tests/shared_data/test_views.py b/terraso_backend/tests/shared_data/test_views.py index 54d9413fe..26e35b7d0 100644 --- a/terraso_backend/tests/shared_data/test_views.py +++ b/terraso_backend/tests/shared_data/test_views.py @@ -184,16 +184,14 @@ def test_download_data_entry_file_shared_all( @mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") -def test_download_data_entry_file_shared_target_members( - get_url_mock, logged_client, shared_resource_data_entry_shared_target_members +def test_download_data_entry_file_shared_members( + get_url_mock, logged_client, shared_resource_data_entry_shared_members ): redirect_url = "https://example.org/s3_file.json" get_url_mock.return_value = redirect_url url = reverse( "shared_data:download", - kwargs={ - "shared_resource_uuid": shared_resource_data_entry_shared_target_members.share_uuid - }, + kwargs={"shared_resource_uuid": shared_resource_data_entry_shared_members.share_uuid}, ) response = logged_client.get(url) @@ -202,12 +200,12 @@ def test_download_data_entry_file_shared_target_members( @mock.patch("apps.shared_data.models.data_entries.data_entry_file_storage.url") -def test_download_data_entry_file_shared_target_members_fail( - get_url_mock, logged_client, shared_resource_data_entry_shared_target_members_user_1 +def test_download_data_entry_file_shared_members_fail( + get_url_mock, logged_client, shared_resource_data_entry_shared_members_user_1 ): redirect_url = "https://example.org/s3_file.json" get_url_mock.return_value = redirect_url - share_uuid = shared_resource_data_entry_shared_target_members_user_1.share_uuid + share_uuid = shared_resource_data_entry_shared_members_user_1.share_uuid url = reverse( "shared_data:download", kwargs={"shared_resource_uuid": share_uuid}, From f5170e6a4399a4b9ee7a1453ded5285a28881e8e Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Thu, 15 Feb 2024 17:16:26 -0500 Subject: [PATCH 19/20] fix: Updated GQL schema --- terraso_backend/apps/graphql/schema/schema.graphql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/terraso_backend/apps/graphql/schema/schema.graphql b/terraso_backend/apps/graphql/schema/schema.graphql index 55c3c41bf..def8172fe 100644 --- a/terraso_backend/apps/graphql/schema/schema.graphql +++ b/terraso_backend/apps/graphql/schema/schema.graphql @@ -450,8 +450,8 @@ enum CoreSharedResourceShareAccessChoices { """Anyone with the link""" ALL - """Only target members""" - TARGET_MEMBERS + """Only members""" + MEMBERS } union SourceNode = VisualizationConfigNode | DataEntryNode From ad6ae4e4d23b9bb85e5b97467c26e65a3f866c56 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Thu, 15 Feb 2024 17:50:01 -0500 Subject: [PATCH 20/20] refactor: Removed duplication for urls creation --- .../apps/core/models/shared_resources.py | 19 +++++++++++++++++++ .../apps/graphql/schema/shared_resources.py | 15 ++------------- .../tests/graphql/test_shared_data.py | 18 +++++++----------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/terraso_backend/apps/core/models/shared_resources.py b/terraso_backend/apps/core/models/shared_resources.py index 40b03e17b..41de5f445 100644 --- a/terraso_backend/apps/core/models/shared_resources.py +++ b/terraso_backend/apps/core/models/shared_resources.py @@ -14,6 +14,7 @@ # along with this program. If not, see https://www.gnu.org/licenses/. import uuid +from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.db import models @@ -65,6 +66,24 @@ class Meta: ), ) + def get_download_url(self): + return f"{settings.API_ENDPOINT}/shared-data/download/{self.share_uuid}" + + def get_share_url(self): + from apps.core.models import Group, Landscape + + target = self.target + entity = ( + "groups" + if isinstance(target, Group) + else "landscapes" if isinstance(target, Landscape) else None + ) + if not entity: + return None + slug = target.slug + share_uuid = self.share_uuid + return f"{settings.WEB_CLIENT_URL}/{entity}/{slug}/download/{share_uuid}" + @classmethod def get_share_access_from_text(cls, share_access): if not share_access: diff --git a/terraso_backend/apps/graphql/schema/shared_resources.py b/terraso_backend/apps/graphql/schema/shared_resources.py index 96ba0c049..b6469b7d1 100644 --- a/terraso_backend/apps/graphql/schema/shared_resources.py +++ b/terraso_backend/apps/graphql/schema/shared_resources.py @@ -16,7 +16,6 @@ import graphene import rules import structlog -from django.conf import settings from django.db.models import Q, Subquery from graphene import relay from graphene_django import DjangoObjectType @@ -64,20 +63,10 @@ def resolve_target(self, info, **kwargs): return self.target def resolve_download_url(self, info, **kwargs): - return f"{settings.API_ENDPOINT}/shared-data/download/{self.share_uuid}" + return self.get_download_url() def resolve_share_url(self, info, **kwargs): - target = self.target - entity = ( - "groups" - if isinstance(target, Group) - else "landscapes" if isinstance(target, Landscape) else None - ) - if not entity: - return None - slug = target.slug - share_uuid = self.share_uuid - return f"{settings.WEB_CLIENT_URL}/{entity}/{slug}/download/{share_uuid}" + return self.get_share_url() class SharedResourceRelayNode: diff --git a/terraso_backend/tests/graphql/test_shared_data.py b/terraso_backend/tests/graphql/test_shared_data.py index 13c139335..5add53eb1 100644 --- a/terraso_backend/tests/graphql/test_shared_data.py +++ b/terraso_backend/tests/graphql/test_shared_data.py @@ -22,7 +22,6 @@ import geopandas as gpd import pytest -from django.conf import settings from apps.collaboration.models import Membership as CollaborationMembership from apps.core import group_collaboration_roles @@ -425,17 +424,14 @@ def test_data_entries_from_parent_query_by_resource_field( ] for data_entry in data_entries: - uuid = data_entry.shared_resources.all()[0].share_uuid - download_url = f"{settings.API_ENDPOINT}/shared-data/download/{uuid}" - slug = parent_entity.slug - share_url = f"{settings.WEB_CLIENT_URL}/{parent}/{slug}/download/{uuid}" - - assert { + shared_resource = data_entry.shared_resources.all()[0] + expected = { "name": data_entry.name, - "download_url": download_url, - "share_url": share_url, - "share_access": data_entry.shared_resources.all()[0].share_access.upper(), - } in entries_result + "download_url": shared_resource.get_download_url(), + "share_url": shared_resource.get_share_url(), + "share_access": shared_resource.share_access.upper(), + } + assert expected in entries_result @pytest.mark.parametrize(