diff --git a/osf/management/commands/process_manual_restart_approvals.py b/osf/management/commands/process_manual_restart_approvals.py index e708320357f..6a2d7b37f78 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_approve_at = approval.auto_approve_at + if timezone.now() < auto_approve_at: + remaining = auto_approve_at - timezone.now() return f'not ready yet ({remaining} remaining)' if registration.is_stuck_registration: diff --git a/osf/models/registrations.py b/osf/models/registrations.py index e1d819b43bf..7fb6f380615 100644 --- a/osf/models/registrations.py +++ b/osf/models/registrations.py @@ -235,7 +235,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 3e41a02e02b..86b77d19299 100644 --- a/osf/models/sanctions.py +++ b/osf/models/sanctions.py @@ -833,6 +833,10 @@ class RegistrationApproval(SanctionCallbackMixin, EmailApprovableSanction): initiated_by = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, blank=True, on_delete=models.CASCADE) + @property + def auto_approve_at(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 5b67375a5a2..8ffd45c42cd 100644 --- a/osf_tests/test_archiver.py +++ b/osf_tests/test_archiver.py @@ -451,6 +451,41 @@ def test_archive(self, mock_chain, mock_enqueue): ] ) + @mock.patch('website.archiver.tasks.delayed_manual_restart_approval.delay') + @mock.patch('website.archiver.tasks.was_manually_restarted', return_value=True) + @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_was_manually_restarted, 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_was_manually_restarted.assert_called_once() + mock_delayed_check.assert_called_once_with(self.dst._id, delay_minutes=5) + + @mock.patch('website.archiver.tasks.delayed_manual_restart_approval.delay') + @mock.patch('website.archiver.tasks.was_manually_restarted', return_value=False) + @mock.patch('osf.management.commands.force_archive.archive') + @mock.patch('osf.management.commands.force_archive.verify') + def test_force_archive_does_not_schedule_when_not_manually_restarted( + self, mock_verify, mock_archive, mock_was_manually_restarted, mock_delayed_check + ): + force_archive( + registration_id=self.dst._id, + permissible_addons=['osfstorage'], + ) + + mock_verify.assert_called_once() + mock_archive.assert_called_once() + mock_was_manually_restarted.assert_called_once() + mock_delayed_check.assert_not_called() + 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/scripts/check_manual_restart_approval.py b/scripts/check_manual_restart_approval.py index e9e6c70de0c..4772a4939dc 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,15 +12,23 @@ @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.info(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") check_manual_restart_approval.apply_async( @@ -26,20 +37,18 @@ def check_manual_restart_approval(registration_id): ) 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_approve_at: + 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.info(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}") + 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 407585527e2..1940c207d70 100644 --- a/website/archiver/tasks.py +++ b/website/archiver/tasks.py @@ -387,21 +387,15 @@ 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( + return AdminLogEntry.objects.filter( object_id=registration.pk, action_flag=MANUAL_ARCHIVE_RESTART, action_time__gte=timezone.now() - timedelta(hours=48) - ) - - return recent_logs.exists() + ).exists() @celery_app.task(bind=True) @@ -424,6 +418,11 @@ def force_archive(self, registration_id, permissible_addons, allow_unconfigured= skip_collisions=skip_collisions, delete_collisions=delete_collisions, ) + # The force-archive path bypasses `archive_success`; schedule manual restart follow-up + # in the force_archive so restarted registrations still get auto-approval checks + if was_manually_restarted(registration): + 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: