From 83ee3281330f226ddbb12c23cf046b11bafbb660 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Thu, 16 Apr 2026 18:52:10 +0300 Subject: [PATCH 1/3] expose foreign_user in logs response; add missing logs for admin actions --- admin/nodes/views.py | 163 ++++++++++++++++++++++++++++++++-------- api/logs/serializers.py | 2 + osf/models/mixins.py | 25 +++--- osf/models/nodelog.py | 1 + 4 files changed, 147 insertions(+), 44 deletions(-) diff --git a/admin/nodes/views.py b/admin/nodes/views.py index a97fec18597..8465dde6369 100644 --- a/admin/nodes/views.py +++ b/admin/nodes/views.py @@ -112,7 +112,8 @@ def post(self, request, *args, **kwargs): node.add_log( action=NodeLog.REGISTRATION_DATE_UPDATED, - auth=request, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, params={ 'last_date': str(last_date), 'new_date': str(new_date) @@ -225,22 +226,20 @@ def post(self, request, *args, **kwargs): message=f'User {user.pk} removed from {node.__class__.__name__.lower()} {node.pk}.', action_flag=CONTRIBUTOR_REMOVED ) - # Log invisibly on the OSF. - self.add_contributor_removed_log(node, user) - return redirect(self.get_success_url()) + node.add_log( + action=NodeLog.CONTRIB_REMOVED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params={ + 'project': node.parent_id, + 'node': node.pk, + 'contributors': user.pk + }, + log_date=timezone.now(), + should_hide=False, + ) - def add_contributor_removed_log(self, node, user): - NodeLog( - action=NodeLog.CONTRIB_REMOVED, - user=None, - params={ - 'project': node.parent_id, - 'node': node.pk, - 'contributors': user.pk - }, - date=timezone.now(), - should_hide=True, - ).save() + return redirect(self.get_success_url()) class NodeUpdatePermissionsView(NodeMixin, View): @@ -266,6 +265,7 @@ def post(self, request, *args, **kwargs): new_permissions_to_add = data.get('new-permissions', []) new_permission_indexes_to_remove = [] + added_contributor_ids = [] for email, permission in zip(new_emails_to_add, new_permissions_to_add): contributor_user = OSFUser.objects.filter(emails__address=email.lower()).first() if not contributor_user: @@ -281,8 +281,10 @@ def post(self, request, *args, **kwargs): auth=request, user_id=contributor_user._id, permissions=permission, - notification_type=None + notification_type=None, + log=False, ) + added_contributor_ids.append(contributor_user._id) messages.success(self.request, f'User with email {email} was successfully added.') # should remove permissions of invalid emails because @@ -291,6 +293,19 @@ def post(self, request, *args, **kwargs): for permission_index in new_permission_indexes_to_remove: new_permissions_to_add.pop(permission_index) + # Log support-added contributors, if any + if added_contributor_ids: + params = resource.log_params + params['contributors'] = added_contributor_ids + resource.add_log( + action=NodeLog.CONTRIB_ADDED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params=params, + log_date=timezone.now(), + should_hide=False, + ) + updated_permissions = data.get('updated-permissions', []) all_permissions = updated_permissions + new_permissions_to_add has_admin = list(filter(lambda permission: ADMIN in permission, all_permissions)) @@ -298,6 +313,7 @@ def post(self, request, *args, **kwargs): messages.error(self.request, 'Must be at least one admin on this node.') return redirect(self.get_success_url()) + permissions_changed = {} for contributor_permission in updated_permissions: guid, permission = contributor_permission.split('-') user = OSFUser.load(guid) @@ -307,7 +323,21 @@ def post(self, request, *args, **kwargs): resource.get_visible(user), request, save=True, - skip_permission=True + skip_permission=True, + log=False, + ) + permissions_changed[user._id] = permission + + if permissions_changed: + params = resource.log_params + params['contributors'] = permissions_changed + resource.add_log( + action=NodeLog.PERMISSIONS_UPDATED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params=params, + log_date=timezone.now(), + should_hide=False, ) return redirect(self.get_success_url()) @@ -332,15 +362,16 @@ def post(self, request, *args, **kwargs): message=f'Node {node.pk} restored.', action_flag=NODE_RESTORED ) - NodeLog( + node.add_log( action=NodeLog.NODE_CREATED, - user=None, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, params={ 'project': node.parent_id, }, - date=timezone.now(), - should_hide=True, - ).save() + log_date=timezone.now(), + should_hide=False, + ) else: node.is_deleted = True node.deleted = timezone.now() @@ -352,15 +383,17 @@ def post(self, request, *args, **kwargs): message=f'Node {node.pk} removed.', action_flag=NODE_REMOVED ) - NodeLog( + node.add_log( action=NodeLog.NODE_REMOVED, - user=None, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, params={ 'project': node.parent_id, + 'node': node.pk, }, - date=timezone.now(), - should_hide=True, - ).save() + log_date=timezone.now(), + should_hide=False, + ) node.save() return redirect(self.get_success_url()) @@ -852,6 +885,18 @@ def post(self, request, *args, **kwargs): node.save() + node.add_log( + action=NodeLog.MADE_PRIVATE, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params={ + 'project': node.parent_id, + 'node': node._primary_key, + }, + log_date=timezone.now(), + should_hide=False, + ) + return redirect(self.get_success_url()) @@ -863,9 +908,21 @@ class NodeMakePublic(NodeMixin, View): def post(self, request, *args, **kwargs): node = self.get_object() try: - node.set_privacy('public') + node.set_privacy('public', auth=None, log=False) except NodeStateError as e: messages.error(request, str(e)) + else: + node.add_log( + action=NodeLog.MADE_PUBLIC, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params={ + 'project': node.parent_id, + 'node': node._primary_key, + }, + log_date=timezone.now(), + should_hide=False, + ) return redirect(self.get_success_url()) @@ -892,10 +949,26 @@ def _remove_file_from_schema_response_blocks(registration, removed_file_id): # delete file from registration and metadata and keep it for original project if guid and (file := guid.referent) and (file.target == node) and not isinstance(file, TrashedFile): + file_id = file._id + file_path = getattr(file, 'materialized_path', None) or getattr(file, 'path', None) or '' + copied_from_id = getattr(file, 'copied_from_id', None) or getattr(getattr(file, 'copied_from', None), '_id', None) with transaction.atomic(): file.delete() _update_schema_meta(file.target) - _remove_file_from_schema_response_blocks(node, [file._id, file.copied_from._id]) + _remove_file_from_schema_response_blocks(node, [file_id, copied_from_id]) + node.add_log( + action=NodeLog.FILE_REMOVED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params={ + 'project': node.parent_id, + 'node': node._primary_key, + 'pathType': 'file', + 'path': file_path, + }, + log_date=timezone.now(), + should_hide=False, + ) return redirect(self.get_success_url()) @@ -932,7 +1005,20 @@ def post(self, request, *args, **kwargs): parent=osfstorage.get_root(), name=osfstorage.archive_folder_name ).first() - file.copy_under(archive_folder) + copied = file.copy_under(archive_folder) + copied_path = getattr(copied, 'materialized_path', None) or getattr(copied, 'path', None) or '' + registration.add_log( + action=NodeLog.FILE_ADDED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params={ + 'project': registration.parent_id, + 'node': registration._primary_key, + 'path': copied_path, + }, + log_date=timezone.now(), + should_hide=False, + ) messages.success(request, 'The file was successfully added.') return redirect(self.get_success_url()) @@ -959,8 +1045,21 @@ def post(self, request, *args, **kwargs): if not registration_file.exists(): messages.error(request, 'The file with the provided guid is not part of the registration.') return redirect(self.get_success_url()) - + file_path = getattr(file, 'materialized_path', None) or getattr(file, 'path', None) or '' registration_file.delete() + registration.add_log( + action=NodeLog.FILE_REMOVED, + auth=None, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + params={ + 'project': registration.parent_id, + 'node': registration._primary_key, + 'pathType': 'file', + 'path': file_path, + }, + log_date=timezone.now(), + should_hide=False, + ) messages.success(request, 'The file was successfully removed.') return redirect(self.get_success_url()) diff --git a/api/logs/serializers.py b/api/logs/serializers.py index 51567b991f3..c27655a862f 100644 --- a/api/logs/serializers.py +++ b/api/logs/serializers.py @@ -203,11 +203,13 @@ class NodeLogSerializer(JSONAPISerializer): 'id', 'date', 'action', + 'foreign_user', ] id = ser.CharField(read_only=True, source='_id') date = VersionedDateTimeField(read_only=True) action = ser.CharField(read_only=True) + foreign_user = ser.SerializerMethodField(read_only=True) params = ser.SerializerMethodField(read_only=True) links = LinksField({'self': 'get_absolute_url'}) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 692075ef4c6..1d0f2a70a83 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1796,7 +1796,7 @@ def copy_unclaimed_records(self, resource): contributor.save() # TODO: optimize me - def update_contributor(self, user, permission, visible, auth, save=False, skip_permission=False): + def update_contributor(self, user, permission, visible, auth, save=False, skip_permission=False, log=True): """ TODO: this method should be updated as a replacement for the main loop of Node#manage_contributors. Right now there are redundancies, but to avoid major feature creep this will not be included as this time. @@ -1823,17 +1823,18 @@ def update_contributor(self, user, permission, visible, auth, save=False, skip_p ) if not self.get_group(permission).user_set.filter(id=user.id).exists(): self.set_permissions(user, permission, save=False) - permissions_changed = { - user._id: permission - } - params = self.log_params - params['contributors'] = permissions_changed - self.add_log( - action=self.log_class.PERMISSIONS_UPDATED, - params=params, - auth=auth, - save=False - ) + if log: + permissions_changed = { + user._id: permission + } + params = self.log_params + params['contributors'] = permissions_changed + self.add_log( + action=self.log_class.PERMISSIONS_UPDATED, + params=params, + auth=auth, + save=False + ) with transaction.atomic(): if [READ] in permissions_changed.values(): project_signals.write_permissions_revoked.send(self) diff --git a/osf/models/nodelog.py b/osf/models/nodelog.py index 6aa14f461e5..c40390ed6d4 100644 --- a/osf/models/nodelog.py +++ b/osf/models/nodelog.py @@ -16,6 +16,7 @@ class NodeLog(ObjectIDMixin, BaseModel): } DATE_FORMAT = '%m/%d/%Y %H:%M UTC' + SUPPORT_USER_LABEL = 'an OSF Support Team Member' # Log action constants -- NOTE: templates stored in log_templates.mako CREATED_FROM = 'created_from' From 85f02eeacec59c406a439d073d8ada22fe519b00 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Fri, 17 Apr 2026 14:33:02 +0300 Subject: [PATCH 2/3] fix tests --- admin/nodes/views.py | 17 +++++------------ admin_tests/nodes/test_views.py | 5 +++-- admin_tests/preprints/test_views.py | 5 +++-- api/logs/serializers.py | 2 +- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/admin/nodes/views.py b/admin/nodes/views.py index 8465dde6369..d0372b7dc29 100644 --- a/admin/nodes/views.py +++ b/admin/nodes/views.py @@ -226,15 +226,13 @@ def post(self, request, *args, **kwargs): message=f'User {user.pk} removed from {node.__class__.__name__.lower()} {node.pk}.', action_flag=CONTRIBUTOR_REMOVED ) + params = dict(node.log_params) + params['contributors'] = user.pk node.add_log( action=NodeLog.CONTRIB_REMOVED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': node.parent_id, - 'node': node.pk, - 'contributors': user.pk - }, + params=params, log_date=timezone.now(), should_hide=False, ) @@ -366,9 +364,7 @@ def post(self, request, *args, **kwargs): action=NodeLog.NODE_CREATED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': node.parent_id, - }, + params=dict(node.log_params), log_date=timezone.now(), should_hide=False, ) @@ -387,10 +383,7 @@ def post(self, request, *args, **kwargs): action=NodeLog.NODE_REMOVED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': node.parent_id, - 'node': node.pk, - }, + params=dict(node.log_params), log_date=timezone.now(), should_hide=False, ) diff --git a/admin_tests/nodes/test_views.py b/admin_tests/nodes/test_views.py index a81d1102b7a..7b5a7f236c3 100644 --- a/admin_tests/nodes/test_views.py +++ b/admin_tests/nodes/test_views.py @@ -269,10 +269,11 @@ def test_do_not_remove_last_admin(self): assert len(list(self.node.get_admin_contributors(self.node.contributors))) == 1 assert AdminLogEntry.objects.count() == count - def test_no_log(self): + def test_log(self): + assert not self.node.logs.filter(action=NodeLog.CONTRIB_REMOVED).exists() view = setup_log_view(self.view(), self.request, guid=self.node._id, user_id=self.user_2.id) view.post(self.request) - assert self.node.logs.latest().action != NodeLog.CONTRIB_REMOVED + assert self.node.logs.filter(action=NodeLog.CONTRIB_REMOVED).exists() def test_no_user_permissions_raises_error(self): guid = self.node._id diff --git a/admin_tests/preprints/test_views.py b/admin_tests/preprints/test_views.py index 5582c1b07eb..02519290733 100644 --- a/admin_tests/preprints/test_views.py +++ b/admin_tests/preprints/test_views.py @@ -542,7 +542,8 @@ def test_do_not_remove_last_admin(self): assert len(list(self.preprint.get_admin_contributors(self.preprint.contributors))) == 1 assert AdminLogEntry.objects.count() == count - def test_no_log(self): + def test_log(self): + assert not self.preprint.logs.filter(action=PreprintLog.CONTRIB_REMOVED).exists() view = setup_log_view( self.view(), self.request, @@ -550,7 +551,7 @@ def test_no_log(self): user_id=self.user_2.id ) view.post(self.request) - assert self.preprint.logs.latest().action != PreprintLog.CONTRIB_REMOVED + assert self.preprint.logs.filter(action=PreprintLog.CONTRIB_REMOVED).exists() @pytest.mark.urls('admin.base.urls') diff --git a/api/logs/serializers.py b/api/logs/serializers.py index c27655a862f..7768074c6e0 100644 --- a/api/logs/serializers.py +++ b/api/logs/serializers.py @@ -209,7 +209,7 @@ class NodeLogSerializer(JSONAPISerializer): id = ser.CharField(read_only=True, source='_id') date = VersionedDateTimeField(read_only=True) action = ser.CharField(read_only=True) - foreign_user = ser.SerializerMethodField(read_only=True) + foreign_user = ser.CharField(read_only=True) params = ser.SerializerMethodField(read_only=True) links = LinksField({'self': 'get_absolute_url'}) From c2a67ad0f4f818280ffad30975a76e415ca2b103 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Tue, 2 Jun 2026 19:15:50 +0300 Subject: [PATCH 3/3] minor fixes; add tests --- admin/nodes/views.py | 54 +++++------ admin_tests/nodes/test_views.py | 165 +++++++++++++++++++++++++++++++- osf/models/mixins.py | 6 +- 3 files changed, 191 insertions(+), 34 deletions(-) diff --git a/admin/nodes/views.py b/admin/nodes/views.py index d0372b7dc29..9ec37e6f797 100644 --- a/admin/nodes/views.py +++ b/admin/nodes/views.py @@ -110,14 +110,16 @@ def post(self, request, *args, **kwargs): node.created = new_date node.save() + params = dict(node.log_params) + params.update({ + 'last_date': str(last_date), + 'new_date': str(new_date), + }) node.add_log( action=NodeLog.REGISTRATION_DATE_UPDATED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'last_date': str(last_date), - 'new_date': str(new_date) - }, + params=params, log_date=timezone.now(), should_hide=False, ) @@ -227,7 +229,7 @@ def post(self, request, *args, **kwargs): action_flag=CONTRIBUTOR_REMOVED ) params = dict(node.log_params) - params['contributors'] = user.pk + params['contributors'] = [user._id] node.add_log( action=NodeLog.CONTRIB_REMOVED, auth=None, @@ -882,10 +884,7 @@ def post(self, request, *args, **kwargs): action=NodeLog.MADE_PRIVATE, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': node.parent_id, - 'node': node._primary_key, - }, + params=dict(node.log_params), log_date=timezone.now(), should_hide=False, ) @@ -909,10 +908,7 @@ def post(self, request, *args, **kwargs): action=NodeLog.MADE_PUBLIC, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': node.parent_id, - 'node': node._primary_key, - }, + params=dict(node.log_params), log_date=timezone.now(), should_hide=False, ) @@ -949,16 +945,16 @@ def _remove_file_from_schema_response_blocks(registration, removed_file_id): file.delete() _update_schema_meta(file.target) _remove_file_from_schema_response_blocks(node, [file_id, copied_from_id]) + params = dict(node.log_params) + params.update({ + 'pathType': 'file', + 'path': file_path, + }) node.add_log( action=NodeLog.FILE_REMOVED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': node.parent_id, - 'node': node._primary_key, - 'pathType': 'file', - 'path': file_path, - }, + params=params, log_date=timezone.now(), should_hide=False, ) @@ -1000,15 +996,13 @@ def post(self, request, *args, **kwargs): ).first() copied = file.copy_under(archive_folder) copied_path = getattr(copied, 'materialized_path', None) or getattr(copied, 'path', None) or '' + params = dict(registration.log_params) + params['path'] = copied_path registration.add_log( action=NodeLog.FILE_ADDED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': registration.parent_id, - 'node': registration._primary_key, - 'path': copied_path, - }, + params=params, log_date=timezone.now(), should_hide=False, ) @@ -1040,16 +1034,16 @@ def post(self, request, *args, **kwargs): return redirect(self.get_success_url()) file_path = getattr(file, 'materialized_path', None) or getattr(file, 'path', None) or '' registration_file.delete() + params = dict(registration.log_params) + params.update({ + 'pathType': 'file', + 'path': file_path, + }) registration.add_log( action=NodeLog.FILE_REMOVED, auth=None, foreign_user=NodeLog.SUPPORT_USER_LABEL, - params={ - 'project': registration.parent_id, - 'node': registration._primary_key, - 'pathType': 'file', - 'path': file_path, - }, + params=params, log_date=timezone.now(), should_hide=False, ) diff --git a/admin_tests/nodes/test_views.py b/admin_tests/nodes/test_views.py index 7b5a7f236c3..1f0afe18e5d 100644 --- a/admin_tests/nodes/test_views.py +++ b/admin_tests/nodes/test_views.py @@ -20,13 +20,18 @@ Embargo, SchemaResponse, DraftRegistration, + ) from admin.nodes.views import ( NodeAddOsfStorageFileView, NodeConfirmSpamView, NodeDeleteView, + NodeMakePrivate, + NodeMakePublic, NodeRemoveContributorView, NodeRemoveOsfStorageFileView, + NodeUpdatePermissionsView, + RegistrationUpdateDateView, NodeView, NodeReindexShare, NodeReindexElastic, @@ -74,6 +79,14 @@ def patch_messages(request): setattr(request, '_messages', messages) +def assert_support_attributed_log(log, admin_user=None): + """Support admin actions attribute logs to the support label, not the staff user.""" + assert log.user is None + assert log.foreign_user == NodeLog.SUPPORT_USER_LABEL + if admin_user is not None: + assert log.user_id is None + + class TestNodeView(AdminTestCase): def test_get_flagged_spam(self): @@ -173,7 +186,9 @@ class TestNodeDeleteView(AdminTestCase): def setUp(self): super().setUp() self.node = ProjectFactory() + self.admin_user = AuthUserFactory() self.request = RequestFactory().post('/fake_path') + self.request.user = self.admin_user self.plain_view = NodeDeleteView self.view = setup_log_view(self.plain_view(), self.request, guid=self.node._id) self.url = reverse('nodes:remove', kwargs={'guid': self.node._id}) @@ -187,18 +202,33 @@ def test_remove_node(self): assert self.node.is_deleted assert AdminLogEntry.objects.count() == count + 1 assert self.node.deleted == mock_now + log = self.node.logs.filter( + action=NodeLog.NODE_REMOVED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) def test_restore_node(self): self.view.post(self.request) self.node.refresh_from_db() assert self.node.is_deleted assert self.node.deleted is not None + remove_log = self.node.logs.filter( + action=NodeLog.NODE_REMOVED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(remove_log, self.admin_user) count = AdminLogEntry.objects.count() self.view.post(self.request) self.node.reload() assert not self.node.is_deleted assert self.node.deleted is None assert AdminLogEntry.objects.count() == count + 1 + restore_log = self.node.logs.filter( + action=NodeLog.NODE_CREATED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(restore_log, self.admin_user) def test_no_user_permissions_raises_error(self): user = AuthUserFactory() @@ -234,12 +264,14 @@ class TestRemoveContributor(AdminTestCase): def setUp(self): super().setUp() self.user = AuthUserFactory() + self.admin_user = AuthUserFactory() self.node = ProjectFactory(creator=self.user) self.user_2 = AuthUserFactory() self.node.add_contributor(self.user_2) self.node.save() self.view = NodeRemoveContributorView self.request = RequestFactory().post('/fake_path') + self.request.user = self.admin_user self.url = reverse('nodes:remove-user', kwargs={'guid': self.node._id, 'user_id': self.user.id}) def test_remove_contributor(self): @@ -273,7 +305,12 @@ def test_log(self): assert not self.node.logs.filter(action=NodeLog.CONTRIB_REMOVED).exists() view = setup_log_view(self.view(), self.request, guid=self.node._id, user_id=self.user_2.id) view.post(self.request) - assert self.node.logs.filter(action=NodeLog.CONTRIB_REMOVED).exists() + log = self.node.logs.filter( + action=NodeLog.CONTRIB_REMOVED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) + assert self.user_2._id in log.params['contributors'] def test_no_user_permissions_raises_error(self): guid = self.node._id @@ -939,7 +976,9 @@ def setUp(self): self.registration_without_registered_from.registered_from = None self.registration_without_registered_from.save() + self.admin_user = AuthUserFactory() self.request = RequestFactory().get('/fake_path') + self.request.user = self.admin_user patch_messages(self.request) def test_no_guid_found(self): @@ -988,6 +1027,11 @@ def test_file_is_added_to_registration_osfstorage(self): assert registration_osfstorage.get_root().children.get( name=registration_osfstorage.archive_folder_name ).children.filter(name=file.name).exists() + log = self.registration_registered_from.logs.filter( + action=NodeLog.FILE_ADDED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) class TestOsfStorageRegistrationFileRemove(AdminTestCase): @@ -1013,7 +1057,9 @@ def setUp(self): self.project = ProjectFactory() self.registration_registered_from = RegistrationFactory(project=self.project) + self.admin_user = AuthUserFactory() self.request = RequestFactory().get('/fake_path') + self.request.user = self.admin_user patch_messages(self.request) def test_no_guid_found(self): @@ -1070,3 +1116,120 @@ def test_file_is_removed_from_registration_osfstorage(self): name=registration_osfstorage.archive_folder_name ).children.exists() assert not self.registration_registered_from.files.exists() + remove_log = self.registration_registered_from.logs.filter( + action=NodeLog.FILE_REMOVED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(remove_log, self.admin_user) + + +class TestNodeMakePrivate(AdminTestCase): + def setUp(self): + super().setUp() + self.admin_user = AuthUserFactory() + self.node = ProjectFactory(is_public=True) + self.request = RequestFactory().post('/fake_path') + self.request.user = self.admin_user + + def test_make_private(self): + view = setup_log_view(NodeMakePrivate(), self.request, guid=self.node._id) + view.post(self.request) + self.node.refresh_from_db() + assert self.node.is_public is False + log = self.node.logs.filter( + action=NodeLog.MADE_PRIVATE, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) + + +class TestNodeMakePublic(AdminTestCase): + def setUp(self): + super().setUp() + self.admin_user = AuthUserFactory() + self.node = ProjectFactory(is_public=False) + self.request = RequestFactory().post('/fake_path') + self.request.user = self.admin_user + + def test_make_public(self): + view = setup_log_view(NodeMakePublic(), self.request, guid=self.node._id) + view.post(self.request) + self.node.refresh_from_db() + assert self.node.is_public is True + log = self.node.logs.filter( + action=NodeLog.MADE_PUBLIC, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) + + +class TestNodeUpdatePermissions(AdminTestCase): + def setUp(self): + super().setUp() + self.admin_user = AuthUserFactory() + self.node = ProjectFactory(creator=self.admin_user) + self.other_admin = AuthUserFactory() + self.node.add_contributor(self.other_admin, permissions=permissions.ADMIN) + self.node.save() + self.request = RequestFactory().post('/fake_path') + patch_messages(self.request) + self.request.user = self.admin_user + + def test_update_permissions(self): + # Form must include at least one admin permission or the view rejects the POST. + self.request.POST = { + 'updated-permissions': [ + f'{self.admin_user._id}-{permissions.ADMIN}', + f'{self.other_admin._id}-{permissions.READ}', + ], + } + view = setup_log_view(NodeUpdatePermissionsView(), self.request, guid=self.node._id) + view.post(self.request) + log = self.node.logs.filter( + action=NodeLog.PERMISSIONS_UPDATED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) + assert log.params['contributors'] == { + self.admin_user._id: permissions.ADMIN, + self.other_admin._id: permissions.READ, + } + + def test_add_contributor(self): + new_user = AuthUserFactory() + self.request.POST = { + 'updated-permissions': [f'{self.admin_user._id}-{permissions.ADMIN}'], + 'new-emails': [new_user.emails.first().address], + 'new-permissions': [permissions.READ], + } + view = setup_log_view(NodeUpdatePermissionsView(), self.request, guid=self.node._id) + view.post(self.request) + log = self.node.logs.filter( + action=NodeLog.CONTRIB_ADDED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) + assert log.params['contributors'] == [new_user._id] + + +class TestRegistrationUpdateDate(AdminTestCase): + def setUp(self): + super().setUp() + self.admin_user = AuthUserFactory() + self.registration = RegistrationFactory() + self.request = RequestFactory().post('/fake_path') + patch_messages(self.request) + self.request.user = self.admin_user + + def test_update_registration_date(self): + new_date = (timezone.now() - datetime.timedelta(days=30)).replace(microsecond=0) + self.request.POST = {'registered_date': new_date.strftime('%Y-%m-%d %H:%M:%S')} + view = setup_log_view(RegistrationUpdateDateView(), self.request, guid=self.registration._id) + view.post(self.request) + self.registration.refresh_from_db() + assert self.registration.registered_date == new_date + log = self.registration.logs.filter( + action=NodeLog.REGISTRATION_DATE_UPDATED, + foreign_user=NodeLog.SUPPORT_USER_LABEL, + ).latest('date') + assert_support_attributed_log(log, self.admin_user) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 1d0f2a70a83..d8c0b976565 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -1823,10 +1823,10 @@ def update_contributor(self, user, permission, visible, auth, save=False, skip_p ) if not self.get_group(permission).user_set.filter(id=user.id).exists(): self.set_permissions(user, permission, save=False) + permissions_changed = { + user._id: permission + } if log: - permissions_changed = { - user._id: permission - } params = self.log_params params['contributors'] = permissions_changed self.add_log(