From f91b98c25e8f0b8712694d48482e6ff632acc049 Mon Sep 17 00:00:00 2001 From: Jose Buitron Date: Wed, 14 Feb 2024 14:15:07 -0500 Subject: [PATCH] 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(