Skip to content

[ENG-10146] Changes in Admin by Support Staff should be visible in activity logs #11697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cslzchen merged 3 commits into from
Jun 3, 2026
+310 −55
Merged

[ENG-10146] Changes in Admin by Support Staff should be visible in activity logs #11697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83ee328
expose foreign_user in logs response; add missing logs for admin actions
antkryt Apr 16, 2026
85f02ee
fix tests
antkryt Apr 17, 2026
c2a67ad
minor fixes; add tests
antkryt Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 128 additions & 42 deletions admin/nodes/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +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=request,
params={
'last_date': str(last_date),
'new_date': str(new_date)
},
auth=None,
foreign_user=NodeLog.SUPPORT_USER_LABEL,
params=params,
log_date=timezone.now(),
should_hide=False,
)
Expand Down Expand Up @@ -225,22 +228,18 @@ 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())
params = dict(node.log_params)
params['contributors'] = [user._id]
node.add_log(
action=NodeLog.CONTRIB_REMOVED,
auth=None,
foreign_user=NodeLog.SUPPORT_USER_LABEL,
params=params,
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):
Expand All @@ -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:
Expand All @@ -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,
Comment thread
cslzchen marked this conversation as resolved.
)
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
Expand All @@ -291,13 +293,27 @@ 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))
if not has_admin:
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)
Expand All @@ -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
Copy link
Copy Markdown
Collaborator

@cslzchen cslzchen Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am noticed that the type of params['contributors'] can be a integer (user.pk) earlier and a dict here. Seems inconsistent to me?

Copy link
Copy Markdown
Contributor Author

@antkryt antkryt Jun 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, permissions_changed is the dict with user.pk, so technically it would be better to use some iterable (e.g., [user.pk]) instead of a single user.pk. I will update it

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())
Expand All @@ -332,15 +362,14 @@ 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,
params={
'project': node.parent_id,
},
date=timezone.now(),
should_hide=True,
).save()
auth=None,
foreign_user=NodeLog.SUPPORT_USER_LABEL,
params=dict(node.log_params),
Copy link
Copy Markdown
Collaborator

@cslzchen cslzchen Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to an earlier question, node.log_params vs params={'project': node.parent_id}. Is this a required functional change? i.e. passing everything instead of just the one needed.

Copy link
Copy Markdown
Contributor Author

@antkryt antkryt Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my first comment

log_date=timezone.now(),
should_hide=False,
)
else:
node.is_deleted = True
node.deleted = timezone.now()
Expand All @@ -352,15 +381,14 @@ 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,
params={
'project': node.parent_id,
},
date=timezone.now(),
should_hide=True,
).save()
auth=None,
foreign_user=NodeLog.SUPPORT_USER_LABEL,
params=dict(node.log_params),
log_date=timezone.now(),
should_hide=False,
)
node.save()

return redirect(self.get_success_url())
Expand Down Expand Up @@ -852,6 +880,15 @@ def post(self, request, *args, **kwargs):

node.save()

node.add_log(
action=NodeLog.MADE_PRIVATE,
auth=None,
foreign_user=NodeLog.SUPPORT_USER_LABEL,
params=dict(node.log_params),
log_date=timezone.now(),
should_hide=False,
)

return redirect(self.get_success_url())


Expand All @@ -863,9 +900,18 @@ 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=dict(node.log_params),
log_date=timezone.now(),
should_hide=False,
)
return redirect(self.get_success_url())


Expand All @@ -892,10 +938,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])
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=params,
log_date=timezone.now(),
should_hide=False,
)
return redirect(self.get_success_url())


Expand Down Expand Up @@ -932,7 +994,18 @@ 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 ''
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=params,
log_date=timezone.now(),
should_hide=False,
)
messages.success(request, 'The file was successfully added.')
return redirect(self.get_success_url())

Expand All @@ -959,8 +1032,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()
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=params,
log_date=timezone.now(),
should_hide=False,
)
messages.success(request, 'The file was successfully removed.')
return redirect(self.get_success_url())

Expand Down
Loading