From 4187232eae76ab85587288ebe18e31430b0f1c36 Mon Sep 17 00:00:00 2001 From: Jorge Date: Fri, 27 Jun 2025 13:30:54 -0400 Subject: [PATCH] Allow new files and folders added to a shared folder to be automatically seen --- chris_backend/core/models.py | 67 +++++++++++++++++++ chris_backend/core/tests/test_models.py | 21 ++++++ chris_backend/feeds/models.py | 2 +- chris_backend/feeds/views.py | 4 +- chris_backend/filebrowser/serializers.py | 61 ++++++++++------- chris_backend/filebrowser/services.py | 6 +- chris_backend/userfiles/serializers.py | 56 +++++++++++----- .../userfiles/tests/test_serializers.py | 19 ++++-- chris_backend/workflows/models.py | 2 +- 9 files changed, 187 insertions(+), 51 deletions(-) diff --git a/chris_backend/core/models.py b/chris_backend/core/models.py index ac296169..c75495ec 100644 --- a/chris_backend/core/models.py +++ b/chris_backend/core/models.py @@ -5,6 +5,7 @@ import os from django.db import models +from django.db.models.functions import Length from django.db.models.signals import post_delete from django.dispatch import receiver from django.conf import settings @@ -190,6 +191,20 @@ def has_user_permission(self, user, permission=''): group__in=grp_qs) return qs.exists() + def get_groups_permissions_queryset(self): + """ + Custom method to get the queryset of permissions granted to groups to access the + folder. + """ + return FolderGroupPermission.objects.filter(folder=self) + + def get_users_permissions_queryset(self): + """ + Custom method to get the queryset of permissions granted to users to access the + folder. + """ + return FolderUserPermission.objects.filter(folder=self) + def grant_group_permission(self, group, permission): """ Custom method to grant a group a permission to access the folder and all its @@ -337,6 +352,30 @@ def _update_public_access(self, public_tf): lf.public = public_tf ChrisLinkFile.objects.bulk_update(link_files, ['public']) + @classmethod + def get_first_existing_folder_ancestor(cls, path): + """ + Custom class method to return the closest ancestor folder (by path prefix + including the passed path itself) that exists in the DB. + """ + if not path: + try: + return cls.objects.get(path='') + except cls.DoesNotExist: + return None + + parts = path.strip().strip('/').split('/') + ancestor_paths = ['/'.join(parts[:i]) for i in range(len(parts), 0, -1)] + ancestor_paths.append('') + + return ( + cls.objects + .filter(path__in=ancestor_paths) + .annotate(path_length=Length('path')) + .order_by('-path_length') + .first() + ) + @receiver(post_delete, sender=ChrisFolder) def auto_delete_folder_from_storage(sender, instance, **kwargs): @@ -609,6 +648,20 @@ def has_user_permission(self, user, permission=''): group__in=grp_qs) return qs.exists() + def get_groups_permissions_queryset(self): + """ + Custom method to get the queryset of permissions granted to groups to access the + file. + """ + return FileGroupPermission.objects.filter(file=self) + + def get_users_permissions_queryset(self): + """ + Custom method to get the queryset of permissions granted to users to access the + file. + """ + return FileUserPermission.objects.filter(file=self) + def grant_group_permission(self, group, permission): """ Custom method to grant a group a permission to access the file. @@ -896,6 +949,20 @@ def has_user_permission(self, user, permission=''): group__in=grp_qs) return qs.exists() + def get_groups_permissions_queryset(self): + """ + Custom method to get the queryset of permissions granted to groups to access the + link file. + """ + return LinkFileGroupPermission.objects.filter(link_file=self) + + def get_users_permissions_queryset(self): + """ + Custom method to get the queryset of permissions granted to users to access the + link file. + """ + return LinkFileUserPermission.objects.filter(link_file=self) + def grant_group_permission(self, group, permission): """ Custom method to grant a group a permission to access the link file. diff --git a/chris_backend/core/tests/test_models.py b/chris_backend/core/tests/test_models.py index bd064d90..6f9192aa 100755 --- a/chris_backend/core/tests/test_models.py +++ b/chris_backend/core/tests/test_models.py @@ -28,6 +28,27 @@ def tearDown(self): logging.disable(logging.NOTSET) +class ChrisFolderModelTests(ModelTests): + + def setUp(self): + super(ChrisFolderModelTests, self).setUp() + + def test_get_first_existing_folder_ancestor(self): + """ + Test whether custom get_first_existing_folder_ancestor method returns the + closest ancestor folder (by path prefix including the passed path itself) that + exists in the DB. + """ + folder = ChrisFolder.get_first_existing_folder_ancestor('') + self.assertEqual(folder.path, '') + folder = ChrisFolder.get_first_existing_folder_ancestor('home') + self.assertEqual(folder.path, 'home') + folder = ChrisFolder.get_first_existing_folder_ancestor('home/12345678/999999') + self.assertEqual(folder.path, 'home') + folder = ChrisFolder.get_first_existing_folder_ancestor('home/12345678/file.txt') + self.assertEqual(folder.path, 'home') + + class ChrisFileModelTests(ModelTests): def setUp(self): diff --git a/chris_backend/feeds/models.py b/chris_backend/feeds/models.py index dc0a44f9..a93952af 100644 --- a/chris_backend/feeds/models.py +++ b/chris_backend/feeds/models.py @@ -70,7 +70,7 @@ def add_jobs_status_count(feed_qs): then=1), output_field=IntegerField())), cancelled_jobs=Count(Case(When(plugin_instances__status='cancelled', then=1), output_field=IntegerField())) - ) + ).order_by('-creation_date') def get_jobs_status_count(self): """ diff --git a/chris_backend/feeds/views.py b/chris_backend/feeds/views.py index 8b489429..807aae55 100644 --- a/chris_backend/feeds/views.py +++ b/chris_backend/feeds/views.py @@ -167,7 +167,7 @@ def get_feeds_queryset(self, user): lookup = Q(owner=user) | Q(public=True) | Q(shared_users=user) | Q( shared_groups__in=user.groups.all()) - return Feed.add_jobs_status_count(tag.feeds.filter(lookup)) + return Feed.add_jobs_status_count(tag.feeds.filter(lookup).distinct()) class FeedTaggingList(generics.ListCreateAPIView): @@ -262,7 +262,7 @@ def get_taggings_queryset(self, user): lookup = Q(feed__owner=user) | (Q(feed__public=True) | Q( feed__shared_users=user) | Q(feed__shared_groups__in=user.groups.all())) - return Tagging.objects.filter(tag=tag).filter(lookup) + return Tagging.objects.filter(tag=tag).filter(lookup).distinct() class TaggingDetail(generics.RetrieveDestroyAPIView): diff --git a/chris_backend/filebrowser/serializers.py b/chris_backend/filebrowser/serializers.py index 1c11fb4e..2d2e275d 100644 --- a/chris_backend/filebrowser/serializers.py +++ b/chris_backend/filebrowser/serializers.py @@ -38,19 +38,42 @@ class Meta: def create(self, validated_data): """ - Overriden to set the parent folder. + Overriden to set the parent folder. It also creates non-existent ancestors and + sets their permissions to be the same as the first existing ancestor. """ path = validated_data['path'] parent_path = os.path.dirname(path) owner = validated_data['owner'] - try: - parent_folder = ChrisFolder.objects.get(path=parent_path) - except ChrisFolder.DoesNotExist: - parent_folder = ChrisFolder.objects.create(path=parent_path, owner=owner) + parent = ancestor = ChrisFolder.get_first_existing_folder_ancestor(path) + if ancestor.path != parent_path: + parent = ChrisFolder.objects.create(path=parent_path, owner=owner) + + validated_data['parent'] = parent + folder = super(FileBrowserFolderSerializer, self).create(validated_data) + + if ancestor.path == parent_path: + top_created_folder = folder + else: + path_parts = path.split('/') + ancestor_path_parts = ancestor.path.split('/') + next_part = path_parts[len(ancestor_path_parts)] + top_created_folder = ChrisFolder.objects.get( + path=ancestor.path + '/' + next_part) - validated_data['parent'] = parent_folder - return super(FileBrowserFolderSerializer, self).create(validated_data) + if ancestor.public: + top_created_folder.grant_public_access() + folder.public = True # update object before returning it + + for perm in ancestor.get_groups_permissions_queryset(): + top_created_folder.grant_group_permission(perm.group, perm.permission) + + for perm in ancestor.get_users_permissions_queryset(): + top_created_folder.grant_user_permission(perm.user, perm.permission) + + if owner != ancestor.owner: + top_created_folder.grant_user_permission(ancestor.owner, 'w') + return folder def update(self, instance, validated_data): """ @@ -98,29 +121,19 @@ def validate_path(self, path): if not path.startswith('home/'): raise serializers.ValidationError(["Invalid path. Path must start with " "'home/'."]) - try: - ChrisFolder.objects.get(path=path) - except ChrisFolder.DoesNotExist: - pass - else: + + ancestor = ChrisFolder.get_first_existing_folder_ancestor(path) + + if ancestor.path == path: raise serializers.ValidationError([f"Folder with path '{path}' already " f"exists."]) user = self.context['request'].user - parent_folder_path = os.path.dirname(path) - - while True: - try: - parent_folder = ChrisFolder.objects.get(path=parent_folder_path) - except ChrisFolder.DoesNotExist: - parent_folder_path = os.path.dirname(parent_folder_path) - else: - break - if not (parent_folder.owner == user or parent_folder.public or - parent_folder.has_user_permission(user, 'w')): + if not (ancestor.owner == user or ancestor.public or + ancestor.has_user_permission(user, 'w')): raise serializers.ValidationError([f"Invalid path. User do not have write " f"permission under the folder " - f"'{parent_folder_path}'."]) + f"'{ancestor.path}'."]) return path def validate_public(self, public): diff --git a/chris_backend/filebrowser/services.py b/chris_backend/filebrowser/services.py index a629ee44..ad55dbfb 100644 --- a/chris_backend/filebrowser/services.py +++ b/chris_backend/filebrowser/services.py @@ -37,7 +37,7 @@ def get_folder_children_queryset(folder, user=None): lookup = models.Q(owner=user) | models.Q(public=True) | models.Q( shared_users=user) | models.Q(shared_groups__in=user.groups.all()) - return folder.children.filter(lookup) + return folder.children.filter(lookup).distinct() def get_folder_files_queryset(folder, user=None): @@ -52,7 +52,7 @@ def get_folder_files_queryset(folder, user=None): lookup = models.Q(owner=user) | models.Q(public=True) | models.Q( shared_users=user) | models.Q(shared_groups__in=user.groups.all()) - return folder.chris_files.filter(lookup) + return folder.chris_files.filter(lookup).distinct() def get_folder_link_files_queryset(folder, user=None): @@ -67,4 +67,4 @@ def get_folder_link_files_queryset(folder, user=None): lookup = models.Q(owner=user) | models.Q(public=True) | models.Q( shared_users=user) | models.Q(shared_groups__in=user.groups.all()) - return folder.chris_link_files.filter(lookup) + return folder.chris_link_files.filter(lookup).distinct() diff --git a/chris_backend/userfiles/serializers.py b/chris_backend/userfiles/serializers.py index 77f9f6ea..acb54e50 100644 --- a/chris_backend/userfiles/serializers.py +++ b/chris_backend/userfiles/serializers.py @@ -29,7 +29,9 @@ def __init__(self, *args, **kwargs): def create(self, validated_data): """ - Overriden to set the file's saving path and parent folder. + Overriden to set the file's saving path and parent folder. It also creates + non-existent ancestor folders and sets their permissions to be the same as the + first existing ancestor folder. """ # user file will be stored at: SWIFT_CONTAINER_NAME/ # where must start with home// @@ -37,15 +39,41 @@ def create(self, validated_data): folder_path = os.path.dirname(upload_path) owner = validated_data['owner'] - try: - parent_folder = ChrisFolder.objects.get(path=folder_path) - except ChrisFolder.DoesNotExist: + parent_folder = ancestor_folder = ChrisFolder.get_first_existing_folder_ancestor( + upload_path) + if ancestor_folder.path != folder_path: parent_folder = ChrisFolder.objects.create(path=folder_path, owner=owner) validated_data['parent_folder'] = parent_folder user_file = UserFile(**validated_data) user_file.fname.name = upload_path user_file.save() + + if ancestor_folder.path == folder_path: + top_created_obj = user_file + else: + parent_folder_path_parts = folder_path.split('/') + ancestor_folder_path_parts = ancestor_folder.path.split('/') + next_part = parent_folder_path_parts[len(ancestor_folder_path_parts)] + top_created_obj_path = ancestor_folder.path + '/' + next_part + + if top_created_obj_path == folder_path: + top_created_obj = parent_folder + else: + top_created_obj = ChrisFolder.objects.get(path=top_created_obj_path) + + if ancestor_folder.public: + top_created_obj.grant_public_access() + user_file.public = True # update object before returning it + + for perm in ancestor_folder.get_groups_permissions_queryset(): + top_created_obj.grant_group_permission(perm.group, perm.permission) + + for perm in ancestor_folder.get_users_permissions_queryset(): + top_created_obj.grant_user_permission(perm.user, perm.permission) + + if owner != ancestor_folder.owner: + top_created_obj.grant_user_permission(ancestor_folder.owner, 'w') return user_file def update(self, instance, validated_data): @@ -93,22 +121,18 @@ def validate_upload_path(self, upload_path): if not upload_path.startswith('home/'): raise serializers.ValidationError(["Invalid path. Path must start with " "'home/'."]) - user = self.context['request'].user - folder_path = os.path.dirname(upload_path) - while True: - try: - folder = ChrisFolder.objects.get(path=folder_path) - except ChrisFolder.DoesNotExist: - folder_path = os.path.dirname(folder_path) - else: - break + ancestor_folder = ChrisFolder.get_first_existing_folder_ancestor(upload_path) - if not (folder.owner == user or folder.public or - folder.has_user_permission(user, 'w')): + if ancestor_folder.path == upload_path: + raise serializers.ValidationError([f"A folder with path '{upload_path}' " + f"already exists."]) + user = self.context['request'].user + if not (ancestor_folder.owner == user or ancestor_folder.public or + ancestor_folder.has_user_permission(user, 'w')): raise serializers.ValidationError([f"Invalid path. User does not have write " f"permission under the folder " - f"'{folder_path}'."]) + f"'{ancestor_folder.path}'."]) return upload_path def validate(self, data): diff --git a/chris_backend/userfiles/tests/test_serializers.py b/chris_backend/userfiles/tests/test_serializers.py index 3dfc9339..84b3cb75 100644 --- a/chris_backend/userfiles/tests/test_serializers.py +++ b/chris_backend/userfiles/tests/test_serializers.py @@ -42,17 +42,28 @@ def tearDown(self): def test_create(self): """ Test whether overriden 'create' method successfully creates a new UserFile with - the correct path and parent folder + the correct path, parent folder and permissions. """ + chris_user = User.objects.get(username=self.chris_username) user = User.objects.get(username=self.username) + ancestor_folder_path = f'home/{self.username}/uploads/ancestor' + (ancestor_folder, _) = ChrisFolder.objects.get_or_create(path=ancestor_folder_path, + owner=user) + ancestor_folder.grant_public_access() + ancestor_folder.grant_user_permission(chris_user, 'w') + f = ContentFile('Test file'.encode()) f.name = 'file1.txt' - validated_data = {'upload_path': f'home/{self.username}/uploads/file1.txt', + validated_data = {'upload_path': ancestor_folder_path + '/upload_folder/file1.txt', 'owner': user, 'fname': f} + userfiles_serializer = UserFileSerializer() user_file = userfiles_serializer.create(validated_data) - self.assertEqual(user_file.fname.name, f'home/{self.username}/uploads/file1.txt') - self.assertEqual(user_file.parent_folder.path, f'home/{self.username}/uploads') + + self.assertEqual(user_file.fname.name, ancestor_folder_path + '/upload_folder/file1.txt') + self.assertEqual(user_file.parent_folder.path, ancestor_folder_path + '/upload_folder') + self.assertTrue(user_file.public) + self.assertTrue(user_file.has_user_permission(chris_user, 'w')) user_file.delete() def test_update(self): diff --git a/chris_backend/workflows/models.py b/chris_backend/workflows/models.py index ba2f9dec..19be01dc 100644 --- a/chris_backend/workflows/models.py +++ b/chris_backend/workflows/models.py @@ -43,7 +43,7 @@ def add_jobs_status_count(workflow_qs): then=1), output_field=IntegerField())), cancelled_jobs=Count(Case(When(plugin_instances__status='cancelled', then=1), output_field=IntegerField())) - ) + ).order_by('-creation_date') def get_jobs_status_count(self): """