fix: Renamed constant, improved share url, added explicit if for land…

…scape entity
techmatters · Feb 14, 2024 · f91b98c · f91b98c
1 parent 9f90fc0
commit f91b98c
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 14 deletions.
diff --git 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
@@ -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,15 +108,15 @@ 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)
         & 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)
+        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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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(