diff --git a/admin/base/urls.py b/admin/base/urls.py index 9ff5e03a03e..1eb840bc362 100644 --- a/admin/base/urls.py +++ b/admin/base/urls.py @@ -38,6 +38,7 @@ re_path(r'^draft_registrations/', include('admin.draft_registrations.urls', namespace='draft_registrations')), re_path(r'^files/', include('admin.files.urls', namespace='files')), re_path(r'^share_reindex/', include('admin.share_reindex.urls', namespace='share_reindex')), + re_path(r'^notifications/', include('admin.notifications.urls', namespace='notifications')), ]), ), ] diff --git a/admin/maintenance/views.py b/admin/maintenance/views.py index 05e7a8372c9..7ca7a53cdef 100644 --- a/admin/maintenance/views.py +++ b/admin/maintenance/views.py @@ -1,7 +1,7 @@ import pytz import datetime -from osf.models import MaintenanceState +from osf.models import MaintenanceState, MaintenanceMode import website.maintenance as maintenance from admin.maintenance.forms import MaintenanceForm @@ -36,15 +36,17 @@ def get_context_data(self, **kwargs): maintenance = MaintenanceState.objects.first() kwargs['form'] = MaintenanceForm() kwargs['current_alert'] = model_to_dict(maintenance) if maintenance else None + kwargs['maintenance_mode'] = MaintenanceMode.is_under_maintenance() return super().get_context_data(**kwargs) def post(self, request, *args, **kwargs): data = request.POST - - start = convert_eastern_to_utc(data['start']).isoformat() if data.get('start') else None - end = convert_eastern_to_utc(data['end']).isoformat() if data.get('end') else None - - maintenance.set_maintenance(data.get('message', ''), data['level'], start, end) + if maintenance_mode := data.get('maintenance_mode'): + MaintenanceMode(maintenance_mode=False if maintenance_mode == 'True' else True).save() + else: + start = convert_eastern_to_utc(data['start']).isoformat() if data.get('start') else None + end = convert_eastern_to_utc(data['end']).isoformat() if data.get('end') else None + maintenance.set_maintenance(data.get('message', ''), data['level'], start, end) return redirect('maintenance:display') diff --git a/admin/management/views.py b/admin/management/views.py index 04034bfaa08..c1bdf19f333 100644 --- a/admin/management/views.py +++ b/admin/management/views.py @@ -194,7 +194,17 @@ def post(self, request): class SyncNotificationTemplates(ManagementCommandPermissionView): def post(self, request): - populate_notification_types() + run_type = request.POST.get('run_type') + if run_type == 'restore_one': + template_name = request.POST.get('template_name') + if not template_name: + messages.error(request, 'A template name must be specified when restoring one template. Check your inputs and try again') + return redirect(reverse('management:commands')) + populate_notification_types(restore_one=template_name) + elif run_type == 'restore_all': + populate_notification_types(restore_all=True) + else: + populate_notification_types() messages.success(request, 'Notification templates have been successfully synced.') return redirect(reverse('management:commands')) diff --git a/admin/nodes/views.py b/admin/nodes/views.py index 71db671620f..2adec026b21 100644 --- a/admin/nodes/views.py +++ b/admin/nodes/views.py @@ -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, ) @@ -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): @@ -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,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), + log_date=timezone.now(), + should_hide=False, + ) else: node.is_deleted = True node.deleted = timezone.now() @@ -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()) @@ -860,6 +888,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()) @@ -871,9 +908,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()) @@ -900,10 +946,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()) @@ -940,7 +1002,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()) @@ -967,8 +1040,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()) diff --git a/admin/notifications/forms.py b/admin/notifications/forms.py new file mode 100644 index 00000000000..946754415bb --- /dev/null +++ b/admin/notifications/forms.py @@ -0,0 +1,8 @@ +from django import forms +from osf.models import NotificationType + + +class NotificationTypeForm(forms.ModelForm): + class Meta: + model = NotificationType + fields = '__all__' diff --git a/admin/notifications/urls.py b/admin/notifications/urls.py new file mode 100644 index 00000000000..236059a577e --- /dev/null +++ b/admin/notifications/urls.py @@ -0,0 +1,14 @@ +from django.urls import re_path +from . import views + +app_name = 'admin' + +urlpatterns = [ + re_path(r'$', views.NotificationsList.as_view(), name='list'), + re_path(r'types/$', views.NotificationTypeList.as_view(), name='types_list'), + re_path(r'type_display/(?P\d+)/$', views.NotificationTypeDisplay.as_view(), name='type_display'), + re_path(r'type_detail/(?P\d+)/$', views.NotificationTypeDetail.as_view(), name='type_detail'), + re_path(r'types_preview/(?P\d+)/$', views.NotificationTypePreview.as_view(), name='types_preview'), + re_path(r'subscriptions/$', views.NotificationSubscriptionsList.as_view(), name='subscriptions_list'), + re_path(r'email_tasks/$', views.EmailTasksList.as_view(), name='email_tasks_list'), +] diff --git a/admin/notifications/views.py b/admin/notifications/views.py index 6719ac90a8a..e1c55c05f47 100644 --- a/admin/notifications/views.py +++ b/admin/notifications/views.py @@ -1,4 +1,329 @@ -from osf.models.notification_subscription import NotificationSubscription +from django.urls import reverse_lazy +from django.db.models import Q +from osf.models import NotificationSubscription, NotificationType, Notification, EmailTask +from django.views.generic import ListView, DetailView, UpdateView +from django.contrib.auth.mixins import PermissionRequiredMixin +from django.forms.models import model_to_dict +from .forms import NotificationTypeForm +from osf.email import _render_email_html +import json +from collections import defaultdict +from mako.lexer import Lexer +from mako.parsetree import ControlLine +import re def delete_selected_notifications(selected_ids): NotificationSubscription.objects.filter(id__in=selected_ids).delete() + +TEMPLATE_IDENTIFIER_BLACKLIST = { + 'if', 'else', 'and', 'or', 'not', 'in', + 'True', 'False', 'len', 'str', 'int', 'float', 'list', 'dict', 'set', 'tuple', +} + +def resolve_identifiers(identifier_structure): + structure = defaultdict(dict) + if hasattr(identifier_structure, 'nodes') and identifier_structure.nodes: + for node in identifier_structure.nodes: + if isinstance(node, ControlLine) and node.keyword == 'for': + match = re.match(r'for (\w+) in (.+):', node.text) + if match: + iterator, source = match.groups() + structure[node.text] = { + 'type': 'loop', + 'iterator': iterator, + 'source': source, + 'children': resolve_identifiers(node) + } + elif hasattr(node, 'text'): + field_match = re.match(r"(\w+)\['(.+)'\]", node.text) + if field_match: + source, field = field_match.groups() + structure[node.text] = { + 'type': 'field', + 'source': source, + 'field': field + } + return structure + +def generate_mock_json(structure, list_name=None): + item = {} + result = {} + for key, value in structure.items(): + # simple field + if isinstance(value, dict) and value.get('type') == 'field': + field_name = value['field'] + item[field_name] = f"mock_{field_name}" + + # nested loop + elif isinstance(value, dict) and value.get('type') == 'loop': + nested_source = value['source'] + nested_match = re.match(r"\w+\['(.+)'\]", nested_source) + if nested_match: + nested_field = nested_match.group(1) + item[nested_field] = [1, 2, 3, 4] + + # top-level loop wrapper + elif key.startswith('for '): + match = re.match(r'for (\w+) in (.+):', key) + if match: + _, source = match.groups() + # Extract final field name + field_match = re.search(r"(\w+)\['(.+?)'\]$", source) + if field_match: + field_name = field_match.group(1) + list_name = field_match.group(2) + return {field_name: generate_mock_json(value, list_name)} + else: + list_name = source + return generate_mock_json(value, list_name) + if list_name: + result[list_name] = [item, item, item] + + return result + + +def build_safe_context(template: str) -> dict: + templatenode = Lexer(text=template).parse() + identifiers_location = [] + for node in templatenode.get_children(): + if hasattr(node, 'nodes'): + identifiers_location.extend(node.nodes) + + if not identifiers_location: + identifiers_location = templatenode.get_children() + identifier_structure = defaultdict() + for control_structure in identifiers_location: + if isinstance(control_structure, ControlLine): + identifier_structure[control_structure.text] = resolve_identifiers(control_structure) + + identifiers = [x.undeclared_identifiers() for x in identifiers_location if hasattr(x, 'undeclared_identifiers')] + flatten_identifiers = set() + for indentifier_set in identifiers: + flatten_identifiers.update(indentifier_set) + mock_json = generate_mock_json(identifier_structure) + context = {identifier: f'mock_{identifier}' for identifier in flatten_identifiers if identifier not in TEMPLATE_IDENTIFIER_BLACKLIST} + context.update(mock_json) + return context + +class NotificationsList(PermissionRequiredMixin, ListView): + paginate_by = 25 + template_name = 'notifications/notifications_list.html' + ordering = 'id' + permission_required = 'osf.view_notification' + raise_exception = True + model = Notification + + def get_queryset(self): + qs = Notification.objects.all().order_by(self.ordering) + q = self.request.GET.get('q') + if q: + qs = qs.filter( + Q(subscription__notification_type__name__icontains=q) | + Q(subscription__user__username__icontains=q) | + Q(subscription__message_frequency__icontains=q) + ) + return qs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + q = self.request.GET.get('q', '') + context['q'] = q + # append search param to pagination links + if q: + context['extra_query_params'] = f"&q={q}" + else: + context['extra_query_params'] = '' + + context['notifications'] = context['object_list'] + context['page'] = context['page_obj'] + return context + +class NotificationSubscriptionsList(PermissionRequiredMixin, ListView): + paginate_by = 25 + template_name = 'notifications/notification_subscriptions_list.html' + ordering = 'id' + permission_required = 'osf.view_notificationsubscription' + raise_exception = True + model = NotificationSubscription + + def get_queryset(self): + qs = NotificationSubscription.objects.all().order_by(self.ordering) + q = self.request.GET.get('q') + if q: + qs = qs.filter( + Q(notification_type__name__icontains=q) | + Q(user__username__icontains=q) | + Q(message_frequency__icontains=q) + ) + return qs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + q = self.request.GET.get('q', '') + context['q'] = q + # append search param to pagination links + if q: + context['extra_query_params'] = f"&q={q}" + else: + context['extra_query_params'] = '' + context['subscriptions'] = context['object_list'] + context['page'] = context['page_obj'] + return context + +class EmailTasksList(PermissionRequiredMixin, ListView): + paginate_by = 25 + template_name = 'notifications/email_tasks_list.html' + ordering = 'task_id' + permission_required = 'osf.view_emailtask' + raise_exception = True + model = EmailTask + + def get_queryset(self): + qs = EmailTask.objects.all().order_by(self.ordering) + q = self.request.GET.get('q') + if q: + qs = qs.filter( + Q(task_id=q) | + Q(user__username__icontains=q) | + Q(status=q) + ) + return qs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + q = self.request.GET.get('q', '') + context['q'] = q + # append search param to pagination links + if q: + context['extra_query_params'] = f"&q={q}" + else: + context['extra_query_params'] = '' + context['email_tasks'] = context['object_list'] + context['page'] = context['page_obj'] + return context + +class NotificationTypeList(PermissionRequiredMixin, ListView): + paginate_by = 25 + template_name = 'notifications/notification_types_list.html' + ordering = 'name' + permission_required = 'osf.view_notificationtype' + raise_exception = True + model = NotificationType + + def get_queryset(self): + qs = NotificationType.objects.all().order_by(self.ordering) + q = self.request.GET.get('q') + if q: + qs = qs.filter( + Q(name__icontains=q) | + Q(subject__icontains=q) | + Q(notification_interval_choices__icontains=q) + ) + return qs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + q = self.request.GET.get('q', '') + context['q'] = q + # append search param to pagination links + if q: + context['extra_query_params'] = f"&q={q}" + else: + context['extra_query_params'] = '' + + context['notification_types'] = context['object_list'] + context['page'] = context['page_obj'] + return context + +class NotificationTypeDisplay(PermissionRequiredMixin, DetailView): + model = NotificationType + template_name = 'notifications/notification_type_detail.html' + permission_required = 'osf.view_notificationtype' + raise_exception = True + + def get_object(self, queryset=None): + return NotificationType.objects.get(id=self.kwargs.get('pk')) + + def get_context_data(self, *args, **kwargs): + notification_type = self.get_object() + notification_type_dict = model_to_dict(notification_type) + fields = notification_type_dict.copy() + kwargs.setdefault('page_number', self.request.GET.get('page', '1')) + notification_type_dict['is_digest_type'] = notification_type.is_digest_type + kwargs['notification_type'] = notification_type_dict + kwargs['template'] = notification_type_dict.pop('template', None) + kwargs['change_form'] = NotificationTypeForm(initial=fields) + + return kwargs + +class NotificationTypePreview(PermissionRequiredMixin, DetailView): + model = NotificationType + template_name = 'notifications/notification_type_preview.html' + permission_required = 'osf.view_notificationtype' + raise_exception = True + + def get_object(self, queryset=None): + return NotificationType.objects.get(id=self.kwargs.get('pk')) + + def get_context_data(self, *args, **kwargs): + notification_type = self.get_object() + raw_context = self.request.GET.get('context') + if raw_context: + try: + if notification_type.is_digest_type: + safe_context = {'notifications': [json.loads(raw_context)]} + else: + safe_context = json.loads(raw_context) + + return_context = json.loads(raw_context) + except json.JSONDecodeError as e: + kwargs['rendered_template'] = f"Error parsing JSON: {str(e)}" + kwargs['context'] = raw_context + return kwargs + else: + if notification_type.is_digest_type: + inner_context = build_safe_context(notification_type.template) + inner_template = _render_email_html(notification_type, ctx=inner_context, return_original_error=True) + safe_context = {'notifications': [inner_template]} + return_context = inner_context + else: + safe_context = build_safe_context(notification_type.template) + return_context = safe_context + + if notification_type.is_digest_type: + # Use user_digest template as a wrapper for digest notification preview. + template_obj = NotificationType.objects.get(name='user_digest') + else: + template_obj = notification_type + try: + kwargs['rendered_template'] = _render_email_html(template_obj, ctx=safe_context, return_original_error=True) + except Exception as e: + kwargs['rendered_template'] = f"Error rendering template: {str(e)}" + + kwargs['context'] = json.dumps(return_context, indent=4) + + return kwargs + +class NotificationTypeDetail(PermissionRequiredMixin, DetailView): + model = NotificationType + template_name = 'notifications/notification_type_detail.html' + permission_required = 'osf.view_notificationtype' + raise_exception = True + + def get(self, request, *args, **kwargs): + view = NotificationTypeDetail.as_view() + return view(request, *args, **kwargs) + + def post(self, request, *args, **kwargs): + view = NotificationTypeChangeForm.as_view() + return view(request, *args, **kwargs) + +class NotificationTypeChangeForm(PermissionRequiredMixin, UpdateView): + template_name = 'institutions/detail.html' + permission_required = 'osf.change_notificationtype' + raise_exception = True + model = NotificationType + form_class = NotificationTypeForm + + def get_success_url(self, *args, **kwargs): + return reverse_lazy('notifications:type_display', kwargs={'pk': self.kwargs.get('pk')}) diff --git a/admin/templates/base.html b/admin/templates/base.html index 31a89b74037..5f645ffe267 100644 --- a/admin/templates/base.html +++ b/admin/templates/base.html @@ -289,6 +289,28 @@ {% endif %} {% endif %} + {% if perms.osf.view_notification or perms.osf.view_notificationtype or perms.osf.view_notificationsubscription %} +
  • + Notifications +
    • +
    + +
    + {% endif %} {% if perms.osf.view_metrics %}
  • Metrics
    • {% endif %} diff --git a/admin/templates/maintenance/display.html b/admin/templates/maintenance/display.html index b29028fcbba..6064cf6f131 100644 --- a/admin/templates/maintenance/display.html +++ b/admin/templates/maintenance/display.html @@ -66,6 +66,21 @@

    Put up an alert:

    + + +
    +
    +
    + {% csrf_token %} + + {% if maintenance_mode %} + + {% else %} + + {% endif %} +
    +
    +
    {% endif %} {% endblock content %} diff --git a/admin/templates/management/commands.html b/admin/templates/management/commands.html index 03be151ddbb..799a7430cf2 100644 --- a/admin/templates/management/commands.html +++ b/admin/templates/management/commands.html @@ -172,6 +172,16 @@

    Sync Notification Templates

    {% csrf_token %} + + +
    + +
    + diff --git a/admin/templates/notifications/email_tasks_list.html b/admin/templates/notifications/email_tasks_list.html new file mode 100644 index 00000000000..cf4345f8907 --- /dev/null +++ b/admin/templates/notifications/email_tasks_list.html @@ -0,0 +1,34 @@ +{% extends "base.html" %} +{% load render_bundle from webpack_loader %} +{% load static %} +{% block title %} + List of Email Tasks +{% endblock title %} +{% block content %} +

    List of Email Tasks

    + + {% include "util/pagination.html" with items=page status=status %} + + + +
    + + + + + + + + + + {% for email_task in email_tasks %} + + + + + + {% endfor %} + +
    Task IDUserStatus
    {{ email_task.task_id }}{{ email_task.user }}{{ email_task.status }}
    + +{% endblock content %} diff --git a/admin/templates/notifications/notification_subscriptions_list.html b/admin/templates/notifications/notification_subscriptions_list.html new file mode 100644 index 00000000000..7ed0febd384 --- /dev/null +++ b/admin/templates/notifications/notification_subscriptions_list.html @@ -0,0 +1,37 @@ +{% extends "base.html" %} +{% load render_bundle from webpack_loader %} +{% load static %} +{% block title %} + List of Notification Subscriptions +{% endblock title %} +{% block content %} +

    List of Notification Subscriptions

    + + {% include "util/pagination.html" with items=page status=status %} +
    + + +
    + + + + + + + + + + + {% for subscription in subscriptions %} + + + + + + + + {% endfor %} + +
    Notification Type NameUserMessage FrequencySubscribed Object
    {{ subscription.notification_type.name }}{{ subscription.user }}{{ subscription.message_frequency }}{{ subscription.subscribed_object }}
    + +{% endblock content %} diff --git a/admin/templates/notifications/notification_type_detail.html b/admin/templates/notifications/notification_type_detail.html new file mode 100644 index 00000000000..1a571099762 --- /dev/null +++ b/admin/templates/notifications/notification_type_detail.html @@ -0,0 +1,109 @@ +{% extends "base.html" %} +{% load static %} +{% load render_bundle from webpack_loader %} +{% block title %} + Notification Type +{% endblock title %} +{% block content %} +
    +
    +
    + +
    +
    +
    +
    +

    {{ notification_type.name }}

    +
    +
    +
    +
    + +
    +
    +
    +
    + + {% for field, value in notification_type.items %} + + + + + {% endfor %} + + + + + +
    {{ field }}{{ value | safe }}
    template
    {{ template }}
    +
    + +
    +
    +
    +
    +
    + +{% endblock content %} + +{% block bottom_js %} + +{% endblock %} diff --git a/admin/templates/notifications/notification_type_preview.html b/admin/templates/notifications/notification_type_preview.html new file mode 100644 index 00000000000..e9aefbe3284 --- /dev/null +++ b/admin/templates/notifications/notification_type_preview.html @@ -0,0 +1,30 @@ +

    Notification Template Preview

    +

    Rendered Template

    + + + +
    + {{ rendered_template|safe }} +
    +

    Mock Data

    +
    + Edit the mock data and click "Render Preview" to see how changes affect the rendered template.
    + Note: The mock data should be a JSON object matching the expected context for the template. Please ensure that the JSON is properly formatted if error occurs during rendering. +
    +
    + +
    diff --git a/admin/templates/notifications/notification_types_list.html b/admin/templates/notifications/notification_types_list.html new file mode 100644 index 00000000000..655e8ec1ab2 --- /dev/null +++ b/admin/templates/notifications/notification_types_list.html @@ -0,0 +1,37 @@ +{% extends "base.html" %} +{% load render_bundle from webpack_loader %} +{% load static %} +{% block title %} + List of Notification Types +{% endblock title %} +{% block content %} +

    List of Notification Types

    + + {% include "util/pagination.html" with items=page status=status %} +
    + + +
    + + + + + + + + + + + {% for notification_type in notification_types %} + + + + + + + + {% endfor %} + +
    NameSubjectInterval ChoicesIs Digest
    {{ notification_type.name }}{{ notification_type.subject }}{{ notification_type.notification_interval_choices }}{{ notification_type.is_digest_type }}
    + +{% endblock content %} diff --git a/admin/templates/notifications/notifications_list.html b/admin/templates/notifications/notifications_list.html new file mode 100644 index 00000000000..76e747ca5e1 --- /dev/null +++ b/admin/templates/notifications/notifications_list.html @@ -0,0 +1,37 @@ +{% extends "base.html" %} +{% load render_bundle from webpack_loader %} +{% load static %} +{% block title %} + List of Notifications +{% endblock title %} +{% block content %} +

    List of Notifications

    + + {% include "util/pagination.html" with items=page status=status %} +
    + + +
    + + + + + + + + + + + {% for notification in notifications %} + + + + + + + + {% endfor %} + +
    Notification Type NameUserSentIs fake sent
    {{ notification.subscription.notification_type.name }}{{ notification.subscription.user }}{{ notification.sent }}{{ notification.fake_sent }}
    + +{% endblock content %} diff --git a/admin_tests/maintenance/test_views.py b/admin_tests/maintenance/test_views.py index abeaa6af677..f42ac73c0a1 100644 --- a/admin_tests/maintenance/test_views.py +++ b/admin_tests/maintenance/test_views.py @@ -8,7 +8,7 @@ from django.core.exceptions import PermissionDenied import website.maintenance as maintenance -from osf.models import MaintenanceState +from osf.models import MaintenanceState, MaintenanceMode from osf_tests.factories import AuthUserFactory from admin_tests.utilities import setup_view @@ -105,3 +105,59 @@ def test_correct_view_permissions(self, req, user, plain_view): res = plain_view.as_view()(req) assert res.status_code == 200 + + +@pytest.mark.urls('admin.base.urls') +class TestMaintenanceMode: + + @pytest.fixture() + def user(self): + user = AuthUserFactory() + view_permission = Permission.objects.get(codename='change_maintenancestate') + user.user_permissions.add(view_permission) + user.save() + return user + + @pytest.fixture() + def plain_view(self): + return views.MaintenanceDisplay + + @pytest.fixture() + def view(self, user, plain_view): + req = RequestFactory().get('/fake_path') + req.user = user + view = plain_view() + setup_view(view, req) + return view + + def test_get_context_data_includes_maintenance_mode(self, view): + MaintenanceMode(maintenance_mode=True).save() + context = view.get_context_data() + assert context['maintenance_mode'] is True + MaintenanceMode(maintenance_mode=False).save() + context = view.get_context_data() + assert context['maintenance_mode'] is False + + def test_post_toggles_maintenance_mode_on(self, user, plain_view): + MaintenanceMode(maintenance_mode=False).save() + req = RequestFactory().post('/fake_path', data={'maintenance_mode': 'False'}) + req.user = user + view = plain_view() + setup_view(view, req) + response = view.post(req) + # It should redirect back to the display page + assert response.status_code == 302 + # The database state should now be True + assert MaintenanceMode.is_under_maintenance() is True + + def test_post_toggles_maintenance_mode_off(self, user, plain_view): + MaintenanceMode(maintenance_mode=True).save() + req = RequestFactory().post('/fake_path', data={'maintenance_mode': 'True'}) + req.user = user + view = plain_view() + setup_view(view, req) + response = view.post(req) + # It should redirect back to the display page + assert response.status_code == 302 + # The database state should now be False + assert MaintenanceMode.is_under_maintenance() is False diff --git a/admin_tests/nodes/test_views.py b/admin_tests/nodes/test_views.py index 2c29b1af2c6..d7cdcf8bece 100644 --- a/admin_tests/nodes/test_views.py +++ b/admin_tests/nodes/test_views.py @@ -21,13 +21,18 @@ Sanction, SchemaResponse, DraftRegistration, + ) from admin.nodes.views import ( NodeAddOsfStorageFileView, NodeConfirmSpamView, NodeDeleteView, + NodeMakePrivate, + NodeMakePublic, NodeRemoveContributorView, NodeRemoveOsfStorageFileView, + NodeUpdatePermissionsView, + RegistrationUpdateDateView, NodeView, NodeReindexShare, NodeReindexElastic, @@ -77,6 +82,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): @@ -176,7 +189,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}) @@ -190,18 +205,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() @@ -237,12 +267,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): @@ -272,10 +304,16 @@ 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 + 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 @@ -941,7 +979,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): @@ -990,6 +1030,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): @@ -1015,7 +1060,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): @@ -1072,6 +1119,123 @@ 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) class TestEmbargoReportView(AdminTestCase): 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/base/authentication/drf.py b/api/base/authentication/drf.py index 9db2ca8faeb..9dd13ef15e6 100644 --- a/api/base/authentication/drf.py +++ b/api/base/authentication/drf.py @@ -21,6 +21,7 @@ from osf.models import OSFUser from osf.utils.fields import ensure_str from website import settings +from framework import sentry SessionStore = import_module(api_settings.SESSION_ENGINE).SessionStore @@ -92,6 +93,20 @@ def check_user(user): # For all other cases, the user status is invalid. Although such status can't be reached with # normal user-facing web application flow, it is still possible as a result of direct database # access, coding bugs, database corruption, etc. + extra_data = { + 'user_id': user.id, + 'user_guid': user._id, + 'is_active': user.is_active, + 'is_disabled': user.is_disabled, + 'is_merged': user.is_merged, + 'date_disabled': user.date_disabled, + 'is_confirmed': user.is_confirmed, + 'is_registered': user.is_registered, + 'can_login': user.has_usable_password() or ( + 'VERIFIED' in sum([list(each.values()) for each in user.external_identity.values()], []) + ), + } + sentry.log_message(f'Invalid user account status detected: user_id={user._id}', extra_data=extra_data) raise InvalidAccountError diff --git a/api/base/middleware.py b/api/base/middleware.py index bec771aba61..239d64194cb 100644 --- a/api/base/middleware.py +++ b/api/base/middleware.py @@ -5,6 +5,7 @@ from importlib import import_module from django.conf import settings +from django.http import JsonResponse from django.contrib.sessions.middleware import SessionMiddleware from django.utils.deprecation import MiddlewareMixin from sentry_sdk import init @@ -24,6 +25,7 @@ from .api_globals import api_globals from api.base import settings as api_settings from api.base.authentication.drf import drf_get_session_from_cookie +from osf.models import MaintenanceMode SessionStore = import_module(settings.SESSION_ENGINE).SessionStore @@ -132,3 +134,20 @@ def process_request(self, request): request.session = drf_get_session_from_cookie(cookie) else: request.session = SessionStore() + + +class MaintenanceModeMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + if MaintenanceMode.is_under_maintenance(): + return JsonResponse( + { + 'meta': { + 'maintenance_mode': True, + 'status_page': 'https://status.cos.io', + }, + }, status=503, + ) + return self.get_response(request) diff --git a/api/base/settings/defaults.py b/api/base/settings/defaults.py index ac9a9739f1b..1bb3f196429 100644 --- a/api/base/settings/defaults.py +++ b/api/base/settings/defaults.py @@ -232,6 +232,7 @@ 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'api.base.middleware.UnsignCookieSessionMiddleware', + 'api.base.middleware.MaintenanceModeMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', diff --git a/api/logs/serializers.py b/api/logs/serializers.py index 51567b991f3..7768074c6e0 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.CharField(read_only=True) params = ser.SerializerMethodField(read_only=True) links = LinksField({'self': 'get_absolute_url'}) diff --git a/api/subscriptions/views.py b/api/subscriptions/views.py index 52be8669231..14a97d50489 100644 --- a/api/subscriptions/views.py +++ b/api/subscriptions/views.py @@ -306,6 +306,9 @@ def update(self, request, *args, **kwargs): raise PermissionDenied for instance in qs: + instance.legacy_id = self.kwargs['subscription_id'] + instance.event_name = 'global_file_updated' + serializer = self.get_serializer(instance=instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) self.perform_update(serializer) @@ -326,6 +329,9 @@ def update(self, request, *args, **kwargs): raise PermissionDenied for instance in qs: + instance.legacy_id = self.kwargs['subscription_id'] + instance.event_name = 'global_reviews' + serializer = self.get_serializer(instance=instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) self.perform_update(serializer) @@ -354,6 +360,9 @@ def update(self, request, *args, **kwargs): raise PermissionDenied for instance in qs: + instance.legacy_id = self.kwargs['subscription_id'] + instance.event_name = 'file_updated' + serializer = self.get_serializer(instance=instance, data=request.data, partial=True) serializer.is_valid(raise_exception=True) self.perform_update(serializer) @@ -361,6 +370,7 @@ def update(self, request, *args, **kwargs): else: instance.event_name = instance.notification_type.name # Set event_name for serializer to use + instance.legacy_id = instance.notification_type.name # Set legacy_id for serializer to use partial = kwargs.pop('partial', False) serializer = self.get_serializer(instance, data=request.data, partial=partial) diff --git a/api_tests/notifications/test_notification_digest.py b/api_tests/notifications/test_notification_digest.py index a1065c27604..73067456f27 100644 --- a/api_tests/notifications/test_notification_digest.py +++ b/api_tests/notifications/test_notification_digest.py @@ -1,4 +1,5 @@ import pytest +from django.utils import timezone from django.contrib.contenttypes.models import ContentType from osf.models import Notification, NotificationType, NotificationTypeEnum, EmailTask, Email @@ -186,6 +187,26 @@ def test_get_users_emails(self): assert user_info['user_id'] == user._id assert any(msg['notification_id'] == notification1.id for msg in user_info['info']) + def test_get_users_emails_ignore_scheduled(self): + user = AuthUserFactory() + notification_type = NotificationType.objects.get(name=NotificationTypeEnum.USER_FILE_UPDATED) + notification1 = Notification.objects.create( + subscription=add_notification_subscription(user, notification_type, 'daily'), + event_context={}, + sent=None + ) + Notification.objects.create( + subscription=add_notification_subscription(user, notification_type, 'daily'), + event_context={}, + sent=None, + scheduled=timezone.now() + ) + res = list(get_users_emails('daily')) + assert len(res) == 1 + user_info = res[0] + assert user_info['user_id'] == user._id + assert any(msg['notification_id'] == notification1.id for msg in user_info['info']) + def test_get_moderators_emails(self): user = AuthUserFactory() provider = RegistrationProviderFactory() @@ -204,6 +225,30 @@ def test_get_moderators_emails(self): ] assert entry, 'Expected moderator digest group' + def test_get_moderators_emails_ignore_scheduled(self): + user = AuthUserFactory() + provider = RegistrationProviderFactory() + reg = RegistrationFactory(provider=provider) + notification_type = NotificationType.objects.get(name=NotificationTypeEnum.PROVIDER_NEW_PENDING_SUBMISSIONS) + subscription = add_notification_subscription(user, notification_type, 'daily', subscribed_object=reg) + Notification.objects.create( + subscription=subscription, + event_context={}, + sent=None + ) + Notification.objects.create( + subscription=subscription, + event_context={}, + sent=None, + scheduled=timezone.now() + ) + res = list(get_moderators_emails('daily')) + assert len(res) >= 1 + entry = [ + x for x in res if x['user_id'] == user._id and subscription.subscribed_object.id == reg.id + ] + assert entry, 'Expected moderator digest group' + def test_send_users_digest_email_end_to_end(self): user = AuthUserFactory() notification_type = NotificationType.objects.get(name=NotificationTypeEnum.USER_FILE_UPDATED) diff --git a/api_tests/subscriptions/views/test_subscriptions_detail.py b/api_tests/subscriptions/views/test_subscriptions_detail.py index a7246bbbd19..f14eeec3cfa 100644 --- a/api_tests/subscriptions/views/test_subscriptions_detail.py +++ b/api_tests/subscriptions/views/test_subscriptions_detail.py @@ -342,3 +342,5 @@ def test_subscription_detail_patch( res = app.patch_json_api(url_user_global_file_updated, payload, auth=user.auth) assert res.status_code == 200 assert res.json['data']['attributes']['frequency'] == 'none' + assert res.json['data'].get('id') is not None + assert res.json['data']['attributes'].get('event_name') is not None diff --git a/notifications/tasks.py b/notifications/tasks.py index 4f802285c93..9775a44fc7a 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -133,6 +133,7 @@ def send_user_email_task(self, user_id, notification_ids, **kwargs): email_task.error_message = email_task.error_message + f'Max retries reached: {str(e)} \n' email_task.save() logger.error(f'Max retries reached for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') + notifications_qs.update(scheduled=None) return email_task, _ = EmailTask.objects.get_or_create(task_id=self.request.id) @@ -291,6 +292,7 @@ def send_moderator_email_task(self, user_id, notification_ids, provider_content_ email_task.error_message = email_task.error_message + f'\nMax retries reached: {str(e)}' email_task.save() logger.error(f'Max retries reached for send_moderator_email_task for user {user_id}. Task id: {self.request.id}. Errors: {email_task.error_message}') + notifications_qs.update(scheduled=None) return email_task.status = 'RETRY' @@ -314,6 +316,8 @@ def send_users_digest_email(dry_run=False): user_id = group['user_id'] notification_ids = [msg['notification_id'] for msg in group['info']] if not dry_run: + notifications_qs = Notification.objects.filter(id__in=notification_ids) + notifications_qs.update(scheduled=timezone.now()) send_user_email_task.delay(user_id, notification_ids) @celery_app.task(name='notifications.tasks.send_moderators_digest_email') @@ -334,6 +338,8 @@ def send_moderators_digest_email(dry_run=False): provider_content_type_id = group['provider_content_type_id'] notification_ids = [msg['notification_id'] for msg in group['info']] if not dry_run: + notifications_qs = Notification.objects.filter(id__in=notification_ids) + notifications_qs.update(scheduled=timezone.now()) send_moderator_email_task.delay(user_id, notification_ids, provider_content_type_id, provider_id) def get_moderators_emails(message_freq: str): @@ -358,6 +364,7 @@ def get_moderators_emails(message_freq: str): INNER JOIN osf_notificationtype AS nt ON ns.notification_type_id = nt.id LEFT JOIN osf_guid ON ns.user_id = osf_guid.object_id WHERE n.sent IS NULL + AND n.scheduled IS NULL AND ns.message_frequency = %s AND nt.name IN (%s, %s) AND nt.name NOT IN (%s, %s, %s) @@ -401,6 +408,7 @@ def get_users_emails(message_freq): INNER JOIN osf_notificationtype AS nt ON ns.notification_type_id = nt.id LEFT JOIN osf_guid ON ns.user_id = osf_guid.object_id WHERE n.sent IS NULL + AND n.scheduled IS NULL AND ns.message_frequency = %s AND nt.name NOT IN (%s, %s, %s, %s, %s) AND osf_guid.content_type_id = ( diff --git a/osf/email/__init__.py b/osf/email/__init__.py index bb22bbcd637..1cf39af809b 100644 --- a/osf/email/__init__.py +++ b/osf/email/__init__.py @@ -149,7 +149,7 @@ def _read_lookup_uri(uri: str) -> str: 'domain': settings.DOMAIN, } -def _render_email_html(notification_type, ctx: dict) -> str: +def _render_email_html(notification_type, ctx: dict, return_original_error: bool = False) -> str: template_text = notification_type.template if not template_text: return '' @@ -172,7 +172,9 @@ def _render_email_html(notification_type, ctx: dict) -> str: strict_undefined=True, ).render(**(ctx or {})) - except Exception: + except Exception as e: + if return_original_error: + raise e logging.exception( f'Mako render failed. type {notification_type.name} provided_keys=%s inline_uri=%s base_uri=%s lookup_dirs=%s', sorted((ctx or {}).keys()), uri, NOTIFY_BASE_URI, LOOKUP_DIRS, diff --git a/osf/management/commands/populate_notification_types.py b/osf/management/commands/populate_notification_types.py index 302c1069a17..5f0145f08a9 100644 --- a/osf/management/commands/populate_notification_types.py +++ b/osf/management/commands/populate_notification_types.py @@ -1,6 +1,5 @@ import sys import yaml -from django.apps import apps from waffle import switch_is_active from osf import features @@ -19,7 +18,7 @@ 'email_transactional': 'instantly', } -def populate_notification_types(*args, **kwargs): +def populate_notification_types(*args, restore_one=None, restore_all=False, **kwargs): if kwargs.get('sender'): # exists when called as a post_migrate signal if not switch_is_active(features.POPULATE_NOTIFICATION_TYPES): if 'pytest' not in sys.modules: @@ -28,64 +27,89 @@ def populate_notification_types(*args, **kwargs): logger.info('Populating notification types...') from django.contrib.contenttypes.models import ContentType from osf.models.notification_type import NotificationType + try: with open(settings.NOTIFICATION_TYPES_YAML) as stream: notification_types = yaml.safe_load(stream) - for notification_type in notification_types['notification_types']: - notification_type.pop('__docs__', None) - notification_type.pop('tests', None) - object_content_type_model_name = notification_type.pop('object_content_type_model_name') + + notification_types_dict = { + nt['name']: nt for nt in notification_types['notification_types'] + } + + all_names = set(notification_types_dict.keys()) + existing_names = set( + NotificationType.objects.values_list('name', flat=True) + ) + + if restore_one: + if restore_one not in notification_types_dict: + raise ValueError(f'Notification type "{restore_one}" not found in YAML') + names_to_process = {restore_one} + + elif restore_all: + names_to_process = all_names + + else: + names_to_process = all_names - existing_names + + logger.info(f'Processing {len(names_to_process)} notification types') + + for name in names_to_process: + raw_nt = notification_types_dict[name].copy() + + raw_nt.pop('__docs__', None) + raw_nt.pop('tests', None) + + object_content_type_model_name = raw_nt.pop('object_content_type_model_name') if object_content_type_model_name == 'desk': content_type = None - elif object_content_type_model_name == 'osfuser': - OSFUser = apps.get_model('osf', 'OSFUser') - content_type = ContentType.objects.get_for_model(OSFUser) - elif object_content_type_model_name == 'preprint': - Preprint = apps.get_model('osf', 'Preprint') - content_type = ContentType.objects.get_for_model(Preprint) - elif object_content_type_model_name == 'collectionsubmission': - CollectionSubmission = apps.get_model('osf', 'CollectionSubmission') - content_type = ContentType.objects.get_for_model(CollectionSubmission) - elif object_content_type_model_name == 'abstractprovider': - AbstractProvider = apps.get_model('osf', 'abstractprovider') - content_type = ContentType.objects.get_for_model(AbstractProvider) - elif object_content_type_model_name == 'osfuser': - OSFUser = apps.get_model('osf', 'OSFUser') - content_type = ContentType.objects.get_for_model(OSFUser) - elif object_content_type_model_name == 'draftregistration': - DraftRegistration = apps.get_model('osf', 'DraftRegistration') - content_type = ContentType.objects.get_for_model(DraftRegistration) else: try: - content_type = ContentType.objects.get( - app_label='osf', - model=object_content_type_model_name - ) + content_type = ContentType.objects.get_by_natural_key(app_label='osf', model=object_content_type_model_name) except ContentType.DoesNotExist: raise ValueError(f'No content type for osf.{object_content_type_model_name}') - template_path = notification_type.pop('template') + template_path = raw_nt.pop('template') + template = None + if template_path: with open(template_path) as stream: template = stream.read() nt, _ = NotificationType.objects.update_or_create( - name=notification_type['name'], - defaults=notification_type, + name=name, + defaults=raw_nt, ) + nt.object_content_type = content_type - if not nt.template or settings.DEV_MODE: + if template: nt.template = template + nt.save() + except ProgrammingError: logger.info('Notification types failed potential side effect of reverse migration') logger.info('Finished populating notification types.') - class Command(BaseCommand): - help = 'Population notification types.' + help = 'Populate notification types.' + + def add_arguments(self, parser): + parser.add_argument( + '--restore-all', + action='store_true', + help='Restore all templates from files' + ) + parser.add_argument( + '--restore', + type=str, + help='Restore specific template by name' + ) def handle(self, *args, **options): with transaction.atomic(): - populate_notification_types(args, options) + populate_notification_types( + restore_all=options['restore_all'], + restore_one=options['restore'] + ) diff --git a/osf/management/commands/process_manual_restart_approvals.py b/osf/management/commands/process_manual_restart_approvals.py index e708320357f..3d01da98262 100644 --- a/osf/management/commands/process_manual_restart_approvals.py +++ b/osf/management/commands/process_manual_restart_approvals.py @@ -4,7 +4,6 @@ from django.utils import timezone from osf.models import Registration from osf.models.admin_log_entry import AdminLogEntry, MANUAL_ARCHIVE_RESTART -from website import settings from scripts.approve_registrations import approve_past_pendings logger = logging.getLogger(__name__) @@ -134,9 +133,9 @@ def should_auto_approve(self, registration): if approval.is_rejected: return 'approval was rejected' - time_since_initiation = timezone.now() - approval.initiation_date - if time_since_initiation < settings.REGISTRATION_APPROVAL_TIME: - remaining = settings.REGISTRATION_APPROVAL_TIME - time_since_initiation + auto_approval_time = approval.auto_approval_time + if timezone.now() < auto_approval_time: + remaining = auto_approval_time - timezone.now() return f'not ready yet ({remaining} remaining)' if registration.is_stuck_registration: diff --git a/osf/migrations/0040_maintenancemode.py b/osf/migrations/0040_maintenancemode.py new file mode 100644 index 00000000000..9d7a410ae0a --- /dev/null +++ b/osf/migrations/0040_maintenancemode.py @@ -0,0 +1,39 @@ +# Generated by Django 4.2.26 on 2026-05-22 14:57 +# Manually added `create_initial_record()` + +from django.db import migrations, models + + +def create_initial_record(apps, schema_editor): + MaintenanceMode = apps.get_model('osf', 'MaintenanceMode') + MaintenanceMode.objects.get_or_create( + pk=1, + defaults={'maintenance_mode': False} + ) + + +def reverse_initial_record(apps, schema_editor): + # the reverse 'reverse_initial_record' does nothing + # because the table will be removed + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0039_merge_20260427_1359'), + ] + + operations = [ + migrations.CreateModel( + name='MaintenanceMode', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('maintenance_mode', models.BooleanField(default=False)), + ], + ), + migrations.RunPython( + create_initial_record, + reverse_code=reverse_initial_record + ), + ] diff --git a/osf/migrations/0041_notification_scheduled.py b/osf/migrations/0041_notification_scheduled.py new file mode 100644 index 00000000000..c4e2b18b909 --- /dev/null +++ b/osf/migrations/0041_notification_scheduled.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.26 on 2026-06-08 09:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('osf', '0040_maintenancemode'), + ] + + operations = [ + migrations.AddField( + model_name='notification', + name='scheduled', + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/osf/models/__init__.py b/osf/models/__init__.py index 7f334a357cc..918ca9aa009 100644 --- a/osf/models/__init__.py +++ b/osf/models/__init__.py @@ -52,7 +52,7 @@ from .institution_affiliation import InstitutionAffiliation from .institution_storage_region import InstitutionStorageRegion from .licenses import NodeLicense, NodeLicenseRecord -from .maintenance_state import MaintenanceState +from .maintenance_state import MaintenanceState, MaintenanceMode from .metadata import GuidMetadataRecord from .metaschema import ( FileMetadataSchema, diff --git a/osf/models/maintenance_state.py b/osf/models/maintenance_state.py index ce8a5ce1786..134979b5f86 100644 --- a/osf/models/maintenance_state.py +++ b/osf/models/maintenance_state.py @@ -14,3 +14,16 @@ class MaintenanceState(models.Model): start = NonNaiveDateTimeField() end = NonNaiveDateTimeField() message = models.TextField(blank=True) + + +class MaintenanceMode(models.Model): + maintenance_mode = models.BooleanField(default=False) + + def save(self, *args, **kwargs): + self.pk = 1 + super().save(*args, **kwargs) + + @classmethod + def is_under_maintenance(cls): + obj, _ = cls.objects.get_or_create(pk=1) + return obj.maintenance_mode diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 692075ef4c6..d8c0b976565 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. @@ -1826,14 +1826,15 @@ def update_contributor(self, user, permission, visible, auth, save=False, skip_p 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: + 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' diff --git a/osf/models/notification.py b/osf/models/notification.py index 533a05a4e97..d3071442af6 100644 --- a/osf/models/notification.py +++ b/osf/models/notification.py @@ -5,6 +5,7 @@ from django.utils import timezone from api.base import settings as api_settings +from website import settings as osf_settings from osf import email, features @@ -18,10 +19,10 @@ class Notification(models.Model): sent = models.DateTimeField(null=True, blank=True) created = models.DateTimeField(auto_now_add=True) fake_sent = models.BooleanField(default=False) + scheduled = models.DateTimeField(null=True, blank=True) def send( self, - protocol_type='email', destination_address=None, email_context=None, save=True, @@ -36,22 +37,28 @@ def send( f"\ncontext={self.event_context}" f"\nemail_context={email_context}" ) - if protocol_type == 'email' and waffle.switch_is_active(features.ENABLE_MAILHOG): + + if waffle.switch_is_active(features.ENABLE_MAILHOG): email.send_email_over_smtp( recipient_address, self.subscription.notification_type, self.event_context, email_context ) - elif protocol_type == 'email': + + if not osf_settings.LOCAL_MODE: email.send_email_with_send_grid( recipient_address, self.subscription.notification_type, self.event_context, email_context ) - else: - raise NotImplementedError(f'protocol `{protocol_type}` is not supported.') + + if osf_settings.LOCAL_MODE and not waffle.switch_is_active(features.ENABLE_MAILHOG): + logging.warning( + 'Both ENABLE_MAILHOG and LOCAL_MODE are disabled. Emails will not be sent to MailHog or real email addresses. ' + 'Turn on ENABLE_MAILHOG to send emails to MailHog for testing, or turn on LOCAL_MODE to send emails with SendGrid.' + ) if save: self.mark_sent() diff --git a/osf/models/notification_type.py b/osf/models/notification_type.py index e0afdab7aea..f8162a08bce 100644 --- a/osf/models/notification_type.py +++ b/osf/models/notification_type.py @@ -160,6 +160,7 @@ def is_digest_type(self): NotificationTypeEnum.ADDON_FILE_COPIED.value, NotificationTypeEnum.ADDON_FILE_MOVED.value, NotificationTypeEnum.ADDON_FILE_RENAMED.value, + NotificationTypeEnum.ADDON_FILE_REMOVED.value, NotificationTypeEnum.FILE_ADDED.value, NotificationTypeEnum.FILE_REMOVED.value, NotificationTypeEnum.FILE_UPDATED.value, diff --git a/osf/models/registrations.py b/osf/models/registrations.py index f13489f1201..f2001175c11 100644 --- a/osf/models/registrations.py +++ b/osf/models/registrations.py @@ -238,7 +238,7 @@ def is_collection(self): @property def archive_job(self): - return self.archive_jobs.first() if self.archive_jobs.count() else None + return self.archive_jobs.first() @property def sanction(self): diff --git a/osf/models/sanctions.py b/osf/models/sanctions.py index 43dd8aece7b..9268a140b3d 100644 --- a/osf/models/sanctions.py +++ b/osf/models/sanctions.py @@ -878,6 +878,10 @@ class RegistrationApproval(SanctionCallbackMixin, EmailApprovableSanction): initiated_by = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, blank=True, on_delete=models.CASCADE) + @property + def auto_approval_time(self): + return self.initiation_date + osf_settings.REGISTRATION_APPROVAL_TIME + @staticmethod def find_approval_backlog(): """ diff --git a/osf_tests/test_archiver.py b/osf_tests/test_archiver.py index 94c8b4f2827..2ee659e00f5 100644 --- a/osf_tests/test_archiver.py +++ b/osf_tests/test_archiver.py @@ -451,6 +451,22 @@ def test_archive(self, mock_chain, mock_enqueue): ] ) + @mock.patch('website.archiver.tasks.delayed_manual_restart_approval.delay') + @mock.patch('osf.management.commands.force_archive.archive') + @mock.patch('osf.management.commands.force_archive.verify') + def test_force_archive_schedules_manual_restart_approval_check( + self, mock_verify, mock_archive, mock_delayed_check + ): + result = force_archive( + registration_id=self.dst._id, + permissible_addons=['osfstorage'], + ) + + assert result == f'Registration {self.dst._id} archive completed' + mock_verify.assert_called_once() + mock_archive.assert_called_once() + mock_delayed_check.assert_called_once_with(self.dst._id, delay_minutes=5) + def test_stat_addon(self): with mock.patch.object(BaseStorageAddon, '_get_file_tree') as mock_file_tree: mock_file_tree.return_value = FILE_TREE diff --git a/osf_tests/test_middleware.py b/osf_tests/test_middleware.py new file mode 100644 index 00000000000..32c16cabd41 --- /dev/null +++ b/osf_tests/test_middleware.py @@ -0,0 +1,105 @@ +import pytest +from unittest import mock +from tests.base import ApiTestCase +from osf.utils import permissions +from osf_tests.factories import ( + AuthUserFactory, + CollectionProviderFactory, + ProjectFactory, +) + + +@pytest.fixture() +def provider(): + provider = CollectionProviderFactory() + provider.update_group_permissions() + return provider + + +@pytest.fixture() +def admin(provider): + user = AuthUserFactory() + provider.get_group(permissions.ADMIN).user_set.add(user) + return user + + +@pytest.fixture() +def node(admin): + return ProjectFactory(creator=admin) + + +class TestMaintenanceModeMiddlewareIntegration(ApiTestCase): + MAINTENANCE_MOCK_PATH = 'api.base.middleware.MaintenanceMode.is_under_maintenance' + + def setUp(self): + super().setUp() + self.provider = CollectionProviderFactory() + self.provider.update_group_permissions() + self.admin = AuthUserFactory() + self.provider.get_group(permissions.ADMIN).user_set.add(self.admin) + self.node = ProjectFactory(creator=self.admin) + + @mock.patch(MAINTENANCE_MOCK_PATH, return_value=True) + def test_middleware_blocks_post_if_maintenance_mode_on(self, mock_maintenance): + url = f'/v2/nodes/{self.node._id}/' + response = self.app.post_json(url, {}, expect_errors=True) + assert response.status_code == 503 + assert response.json['meta']['maintenance_mode'] is True + assert response.json['meta']['status_page'] == 'https://status.cos.io' + + @mock.patch(MAINTENANCE_MOCK_PATH, return_value=True) + def test_middleware_blocks_patch_if_maintenance_mode_on(self, mock_maintenance): + url = f'/v2/nodes/{self.node._id}/' + original_title = self.node.title + payload = { + 'data': { + 'id': self.node._id, + 'type': 'nodes', + 'attributes': {'title': 'Updated Title'} + } + } + response = self.app.patch_json(url, payload, expect_errors=True) + assert response.status_code == 503 + assert response.json['meta']['maintenance_mode'] is True + self.node.reload() + assert self.node.title == original_title + + @mock.patch(MAINTENANCE_MOCK_PATH, return_value=True) + def test_middleware_blocks_delete_if_maintenance_mode_on(self, mock_maintenance): + url = f'/v2/nodes/{self.node._id}/' + response = self.app.delete(url, expect_errors=True) + assert response.status_code == 503 + assert response.json['meta']['maintenance_mode'] is True + self.node.reload() + assert self.node.is_deleted is False + + @mock.patch(MAINTENANCE_MOCK_PATH, return_value=False) + def test_go_to_post_view_if_maintenance_mode_off(self, mock_maintenance): + url = '/v2/nodes/' + payload = { + 'data': { + 'type': 'nodes', + 'attributes': {'title': 'New Node', 'category': 'project'} + } + } + response = self.app.post_json(url, payload, auth=self.admin.auth) + assert response.status_code == 201 + + @mock.patch(MAINTENANCE_MOCK_PATH, return_value=False) + def test_go_to_patch_view_if_maintenance_mode_off(self, mock_maintenance): + url = f'/v2/nodes/{self.node._id}/' + payload = { + 'data': { + 'id': self.node._id, + 'type': 'nodes', + 'attributes': {'title': 'Updated Title'} + } + } + response = self.app.patch_json(url, payload, auth=self.admin.auth) + assert response.status_code == 200 + + @mock.patch(MAINTENANCE_MOCK_PATH, return_value=False) + def test_go_to_delete_view_if_maintenance_mode_off(self, mock_maintenance): + url = f'/v2/nodes/{self.node._id}/' + response = self.app.delete(url, auth=self.admin.auth) + assert response.status_code == 204 diff --git a/scripts/check_manual_restart_approval.py b/scripts/check_manual_restart_approval.py index e9e6c70de0c..5dfcbdfa2eb 100644 --- a/scripts/check_manual_restart_approval.py +++ b/scripts/check_manual_restart_approval.py @@ -1,7 +1,10 @@ import logging +from framework import sentry from framework.celery_tasks import app as celery_app from django.core.management import call_command +from django.utils import timezone from osf.models import Registration +from scripts.approve_registrations import approve_past_pendings logger = logging.getLogger(__name__) @@ -9,37 +12,45 @@ @celery_app.task(name='scripts.check_manual_restart_approval') def check_manual_restart_approval(registration_id): try: - try: - registration = Registration.objects.get(_id=registration_id) - except Registration.DoesNotExist: + registration = Registration.load(registration_id) + if not registration: logger.error(f"Registration {registration_id} not found") return f"Registration {registration_id} not found" if registration.is_public or registration.is_registration_approved: return f"Registration {registration_id} already approved/public" + approval = registration.registration_approval + if not approval: + logger.error(f"Registration {registration_id} has no registration approval object") + return f"Registration {registration_id} has no registration approval object" + + if approval.is_rejected: + logger.info(f"Registration {registration_id} approval was rejected") + return f"Registration {registration_id} approval was rejected" + if registration.archiving: - logger.info(f"Registration {registration_id} still archiving, retrying in 10 minutes") + logger.debug(f"Registration {registration_id} still archiving, retrying in 10 minutes") check_manual_restart_approval.apply_async( args=[registration_id], countdown=600 ) return f"Registration {registration_id} still archiving, scheduled retry" - logger.info(f"Processing manual restart approval for registration {registration_id}") + if timezone.now() < approval.auto_approval_time: + logger.info(f"Registration {registration_id} not ready for auto-approval yet") + return f"Registration {registration_id} not ready for auto-approval yet" - call_command( - 'process_manual_restart_approvals', - registration_id=registration_id, - dry_run=False, - hours_back=24, - verbosity=1 - ) + logger.debug(f"Processing manual restart approval for registration {registration_id}") + approve_past_pendings([approval], dry_run=False) return f"Processed manual restart approval check for registration {registration_id}" except Exception as e: - logger.error(f"Error processing manual restart approval for {registration_id}: {e}") + msg = f"Error processing manual restart approval for {registration_id}: {str(e)}" + logger.error(msg) + sentry.log_message(msg) + sentry.log_exception(e) raise diff --git a/scripts/tests/test_check_manual_restart_approval.py b/scripts/tests/test_check_manual_restart_approval.py new file mode 100644 index 00000000000..f30c99c4616 --- /dev/null +++ b/scripts/tests/test_check_manual_restart_approval.py @@ -0,0 +1,47 @@ +from datetime import timedelta +from unittest import mock + +from django.utils import timezone + +from osf_tests.factories import RegistrationFactory, UserFactory +from scripts.check_manual_restart_approval import check_manual_restart_approval +from tests.base import OsfTestCase +from website import settings + + +class TestCheckManualRestartApproval(OsfTestCase): + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.registration = RegistrationFactory(creator=self.user, archive=False) + self.registration.require_approval(self.user) + + @mock.patch('scripts.check_manual_restart_approval.approve_past_pendings') + def test_skips_if_approval_window_not_elapsed(self, mock_approve): + self.registration.registration_approval.initiation_date = timezone.now() - timedelta(hours=47) + self.registration.registration_approval.save() + + result = check_manual_restart_approval(self.registration._id) + + assert 'not ready for auto-approval' in result + mock_approve.assert_not_called() + + @mock.patch('scripts.check_manual_restart_approval.approve_past_pendings') + def test_approves_when_approval_window_elapsed(self, mock_approve): + self.registration.registration_approval.initiation_date = ( + timezone.now() - settings.REGISTRATION_APPROVAL_TIME - timedelta(minutes=1) + ) + self.registration.registration_approval.save() + + result = check_manual_restart_approval(self.registration._id) + + assert 'Processed manual restart approval check' in result + mock_approve.assert_called_once_with([self.registration.registration_approval], dry_run=False) + + @mock.patch('scripts.check_manual_restart_approval.Registration.load', return_value=None) + def test_returns_not_found_when_registration_missing(self, mock_load): + result = check_manual_restart_approval('abc12') + + assert result == 'Registration abc12 not found' + mock_load.assert_called_once_with('abc12') diff --git a/website/archiver/tasks.py b/website/archiver/tasks.py index 7608b5c02c6..ded49d5bcb4 100644 --- a/website/archiver/tasks.py +++ b/website/archiver/tasks.py @@ -7,9 +7,6 @@ import celery from celery.utils.log import get_task_logger -from django.utils import timezone -from datetime import timedelta - from framework.celery_tasks import app as celery_app from framework.celery_tasks.utils import logged from framework.exceptions import HTTPError @@ -38,7 +35,6 @@ from website import settings from website.app import init_addons -from osf.models.admin_log_entry import AdminLogEntry, MANUAL_ARCHIVE_RESTART from osf.models import ( ArchiveJob, AbstractNode, @@ -445,23 +441,9 @@ def archive_success(self, dst_pk, job_pk): job.save() dst.sanction.ask(dst.get_active_contributors_recursive(unique_users=True)) - if was_manually_restarted(dst): - logger.info(f'Registration {dst._id} was manually restarted, scheduling approval check') - delayed_manual_restart_approval.delay(dst._id, delay_minutes=5) - dst.update_search() -def was_manually_restarted(registration): - recent_logs = AdminLogEntry.objects.filter( - object_id=registration.pk, - action_flag=MANUAL_ARCHIVE_RESTART, - action_time__gte=timezone.now() - timedelta(hours=48) - ) - - return recent_logs.exists() - - @celery_app.task(bind=True) def force_archive(self, registration_id, permissible_addons, allow_unconfigured=False, skip_collisions=False, delete_collisions=False): from osf.management.commands.force_archive import archive, verify @@ -482,6 +464,10 @@ def force_archive(self, registration_id, permissible_addons, allow_unconfigured= skip_collisions=skip_collisions, delete_collisions=delete_collisions, ) + + logger.info(f'Registration {registration._id} was manually restarted, scheduling approval check') + delayed_manual_restart_approval.delay(registration._id, delay_minutes=5) + return f'Registration {registration_id} archive completed' except Exception as exc: diff --git a/website/templates/pending_registration_admin.html.mako b/website/templates/pending_registration_admin.html.mako index ebf11fb1cd8..694a670c5e2 100644 --- a/website/templates/pending_registration_admin.html.mako +++ b/website/templates/pending_registration_admin.html.mako @@ -32,7 +32,7 @@ To cancel this registration: Click here.

    - % if not reviewable_provider__id != 'gfs': + % if reviewable_provider__id != 'gfs': Note: If any admin clicks their cancel link, the submission will be canceled immediately, and the pending registration will be reverted to draft state to revise and resubmit. This operation is irreversible. % else: