diff --git a/CHANGELOG b/CHANGELOG index ff834603cd5..6e95692d20e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,11 @@ We follow the CalVer (https://calver.org/) versioning scheme: YY.MINOR.MICRO. +26.8.1 (2026-04-30) +=================== + +- Hotfix for registrations sometimes fail to become public after approval + 26.8.0 (2026-04-23) =================== diff --git a/api/users/views.py b/api/users/views.py index b887ee7bdd6..9dc9f9475b3 100644 --- a/api/users/views.py +++ b/api/users/views.py @@ -1480,7 +1480,6 @@ def post(self, request, *args, **kwargs): user.date_last_logged_in = timezone.now() user.external_identity[provider][provider_id] = 'VERIFIED' - user.social[provider.lower()] = provider_id del user.email_verifications[token] user.verification_key = generate_verification_key() user.save() diff --git a/osf/models/node.py b/osf/models/node.py index 3aa04185aa5..d08a9d86f0a 100644 --- a/osf/models/node.py +++ b/osf/models/node.py @@ -1239,7 +1239,7 @@ def set_privacy(self, permissions, auth=None, log=True, save=True, meeting_creat action_flag=DOI_CREATION_FAILED ) raise NodeStateError( - 'Unable to make registration public: DOI creation failed. ' + f'Unable to make registration {self._id} public: DOI creation failed. ' 'This may be due to a temporary DataCite service outage. ' 'Please try again later or contact support if the issue persists.' ) diff --git a/osf/models/sanctions.py b/osf/models/sanctions.py index 3e41a02e02b..93941a8c41a 100644 --- a/osf/models/sanctions.py +++ b/osf/models/sanctions.py @@ -1,7 +1,7 @@ from django.apps import apps from django.utils import timezone from django.conf import settings -from django.db import models +from django.db import models, transaction from osf.utils.fields import NonNaiveDateTimeField @@ -951,31 +951,32 @@ def _on_complete(self, event_data): if registration.is_spammy: raise NodeStateError('Cannot approve a spammy registration') - super()._on_complete(event_data) - self.save() - registered_from = registration.registered_from - # Pass auth=None because the registration initiator may not be - # an admin on components (component admins had the opportunity - # to disapprove the registration by this point) - registration.set_privacy('public', auth=None, log=False) - for child in registration.get_descendants_recursive(primary_only=True): - child.set_privacy('public', auth=None, log=False) - # Accounts for system actions where no `User` performs the final approval - auth = Auth(user) if user else None - registered_from.add_log( - action=NodeLog.REGISTRATION_APPROVAL_APPROVED, - params={ - 'node': registered_from._id, - 'registration': registration._id, - 'registration_approval_id': self._id, - }, - auth=auth, - ) - for node in registration.root.node_and_primary_descendants(): - self._add_success_logs(node, user) - node.update_search() # update search if public + with transaction.atomic(): + super()._on_complete(event_data) + self.save() + registered_from = registration.registered_from + # Pass auth=None because the registration initiator may not be + # an admin on components (component admins had the opportunity + # to disapprove the registration by this point) + registration.set_privacy('public', auth=None, log=False) + for child in registration.get_descendants_recursive(primary_only=True): + child.set_privacy('public', auth=None, log=False) + # Accounts for system actions where no `User` performs the final approval + auth = Auth(user) if user else None + registered_from.add_log( + action=NodeLog.REGISTRATION_APPROVAL_APPROVED, + params={ + 'node': registered_from._id, + 'registration': registration._id, + 'registration_approval_id': self._id, + }, + auth=auth, + ) + for node in registration.root.node_and_primary_descendants(): + self._add_success_logs(node, user) + node.update_search() # update search if public - self.save() + self.save() def _on_reject(self, event_data): user = event_data.kwargs.get('user') diff --git a/osf_tests/test_registrations.py b/osf_tests/test_registrations.py index d64af521982..c8efdc938a1 100644 --- a/osf_tests/test_registrations.py +++ b/osf_tests/test_registrations.py @@ -395,7 +395,7 @@ def test_registration_cannot_become_public_when_doi_creation_fails(self, registr with pytest.raises(NodeStateError) as exc_info: registration.set_privacy(Node.PUBLIC, auth=auth, log=False) - assert 'Unable to make registration public: DOI creation failed' in str(exc_info.value) + assert f'Unable to make registration {registration._id} public: DOI creation failed' in str(exc_info.value) assert registration.is_public is False mock_client.create_identifier.assert_called_once() diff --git a/package.json b/package.json index 97351a098b8..2ac9cd620ac 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "OSF", - "version": "26.8.0", + "version": "26.8.1", "description": "Facilitating Open Science", "repository": "https://github.com/CenterForOpenScience/osf.io", "author": "Center for Open Science", diff --git a/scripts/approve_registrations.py b/scripts/approve_registrations.py index b7ac9651268..905b9756200 100644 --- a/scripts/approve_registrations.py +++ b/scripts/approve_registrations.py @@ -38,6 +38,14 @@ def approve_past_pendings(approvals_past_pending, dry_run=True): f'RegistrationApproval {registration_approval._id} is not attached to a registration' ) continue + + if pending_registration.is_spammy: + logger.warning( + f'Skipping RegistrationApproval {registration_approval._id} for ' + f'spammy registration {pending_registration._id}.' + ) + continue + logger.warning( 'RegistrationApproval {} automatically approved by system. Making registration {} public.' .format(registration_approval._id, pending_registration._id) diff --git a/scripts/tests/test_approve_registrations.py b/scripts/tests/test_approve_registrations.py index 4af39cccfa3..2a25287bca3 100644 --- a/scripts/tests/test_approve_registrations.py +++ b/scripts/tests/test_approve_registrations.py @@ -4,6 +4,7 @@ from tests.base import OsfTestCase from osf.models import NodeLog +from osf.models.spam import SpamStatus from osf_tests.factories import RegistrationFactory, UserFactory from scripts.approve_registrations import main @@ -73,3 +74,16 @@ def test_registration_adds_to_parent_projects_log(self): assert self.registration.registered_from.logs.filter( action=NodeLog.PROJECT_REGISTERED ).exists() + + def test_spammy_registration_is_not_auto_approved_or_made_public(self): + self.registration.registration_approval.initiation_date = timezone.now() - timedelta(days=365) + self.registration.is_public = False + self.registration.spam_status = SpamStatus.FLAGGED + self.registration.save() + + main(dry_run=False) + self.registration.registration_approval.reload() + self.registration.reload() + + assert not self.registration.is_registration_approved + assert not self.registration.is_public diff --git a/tests/test_registrations/test_registration_approvals.py b/tests/test_registrations/test_registration_approvals.py index 2802e0e5a30..60e4e72745b 100644 --- a/tests/test_registrations/test_registration_approvals.py +++ b/tests/test_registrations/test_registration_approvals.py @@ -318,3 +318,22 @@ def test_accept_raises_error_if_project_is_spam(self): with pytest.raises(NodeStateError): self.registration.registration_approval.accept() assert mock_notify.call_count == 0 + + def test_accept_rolls_back_approval_if_publish_fails(self): + self.registration.require_approval(self.user) + self.registration.save() + assert self.registration.is_pending_registration + + with mock.patch.object( + self.registration.__class__, + 'set_privacy', + side_effect=NodeStateError('forced publish failure') + ): + with pytest.raises(NodeStateError): + self.registration.registration_approval.accept() + + self.registration.registration_approval.reload() + self.registration.reload() + assert self.registration.is_pending_registration + assert not self.registration.is_registration_approved + assert not self.registration.is_public