Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions osf/management/commands/process_manual_restart_approvals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion osf/models/registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 4 additions & 0 deletions osf/models/sanctions.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,10 @@

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():
"""
Expand Down Expand Up @@ -949,7 +953,7 @@

registration = self._get_registration()
if registration.is_spammy:
raise NodeStateError('Cannot approve a spammy registration')

Check failure on line 956 in osf/models/sanctions.py

View workflow job for this annotation

GitHub Actions / addons_and_admin

Cannot approve a spammy registration

Check failure on line 956 in osf/models/sanctions.py

View workflow job for this annotation

GitHub Actions / addons_and_admin

Cannot approve a spammy registration

super()._on_complete(event_data)
self.save()
Expand Down
35 changes: 35 additions & 0 deletions osf_tests/test_archiver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 20 additions & 11 deletions scripts/check_manual_restart_approval.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
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__)


@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(
Expand All @@ -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


Expand Down
47 changes: 47 additions & 0 deletions scripts/tests/test_check_manual_restart_approval.py
Original file line number Diff line number Diff line change
@@ -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')
15 changes: 7 additions & 8 deletions website/archiver/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
Expand Down