Skip to content

Commit

Permalink
Fix auto accepting pending requests upon verification (#3659)
Browse files Browse the repository at this point in the history
This fix relates to issue #2461 and PR #3514. The issue is that the
logic of auto accepting pending requests upon verification was
implemented in the save() method of the model. However, in Admin, the
actions are performed as an update on the queryset which does not
trigger the save() method, and similar behaviour affected the proper
handling of permissions since the queryset update did not trigger the
pre_save signals.
  • Loading branch information
ammar257ammar authored Oct 31, 2024
1 parent ed07a16 commit 9a35fba
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 8 deletions.
7 changes: 4 additions & 3 deletions app/grandchallenge/verifications/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
permissions=("change",),
)
def mark_verified(modeladmin, request, queryset):
queryset.filter(email_is_verified=True).update(
is_verified=True, verified_at=now()
)
for verification in queryset.filter(email_is_verified=True):
verification.is_verified = True
verification.verified_at = now()
verification.save()


@admin.action(
Expand Down
6 changes: 4 additions & 2 deletions app/grandchallenge/verifications/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,15 @@ def accept_pending_requests_for_verified_users(self):
object_name,
request_class,
) in permission_request_classes.items():
request_class.objects.filter(
for request_object in request_class.objects.filter(
**{
"user": self.user,
"status": request_class.PENDING,
f"{object_name}__access_request_handling": AccessRequestHandlingOptions.ACCEPT_VERIFIED_USERS,
}
).update(status=request_class.ACCEPTED)
):
request_object.status = request_class.ACCEPTED
request_object.save()


def create_verification(email_address, *_, **__):
Expand Down
94 changes: 91 additions & 3 deletions app/tests/verification_tests/test_admin.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,34 @@
import pytest

from grandchallenge.verifications.admin import deactivate_vus_users
from grandchallenge.verifications.models import VerificationUserSet
from tests.factories import UserFactory
from grandchallenge.core.models import RequestBase
from grandchallenge.core.utils.access_requests import (
AccessRequestHandlingOptions,
)
from grandchallenge.verifications.admin import (
deactivate_vus_users,
mark_verified,
)
from grandchallenge.verifications.models import (
Verification,
VerificationUserSet,
)
from tests.algorithms_tests.factories import (
AlgorithmFactory,
AlgorithmPermissionRequestFactory,
)
from tests.archives_tests.factories import (
ArchiveFactory,
ArchivePermissionRequestFactory,
)
from tests.factories import (
ChallengeFactory,
RegistrationRequestFactory,
UserFactory,
)
from tests.reader_studies_tests.factories import (
ReaderStudyFactory,
ReaderStudyPermissionRequestFactory,
)
from tests.verification_tests.factories import (
VerificationFactory,
VerificationUserSetFactory,
Expand Down Expand Up @@ -52,3 +78,65 @@ def test_deactivate_users(django_capture_on_commit_callbacks, settings):
assert users[3].is_active is True
assert users[3].verification.is_verified is True
assert users[4].is_active is True


@pytest.mark.django_db
@pytest.mark.parametrize(
"perm_request_factory, perm_request_entity_attr, entity_factory",
[
(AlgorithmPermissionRequestFactory, "algorithm", AlgorithmFactory),
(ArchivePermissionRequestFactory, "archive", ArchiveFactory),
(
ReaderStudyPermissionRequestFactory,
"reader_study",
ReaderStudyFactory,
),
(RegistrationRequestFactory, "challenge", ChallengeFactory),
],
)
@pytest.mark.parametrize(
"access_request_handling, expected_request_status_without_verification, expected_request_status_with_verification",
[
(
AccessRequestHandlingOptions.ACCEPT_ALL,
RequestBase.ACCEPTED,
RequestBase.ACCEPTED,
),
(
AccessRequestHandlingOptions.ACCEPT_VERIFIED_USERS,
RequestBase.PENDING,
RequestBase.ACCEPTED,
),
(
AccessRequestHandlingOptions.MANUAL_REVIEW,
RequestBase.PENDING,
RequestBase.PENDING,
),
],
)
def test_verify_users_and_accept_pending_requests(
perm_request_factory,
perm_request_entity_attr,
entity_factory,
access_request_handling,
expected_request_status_without_verification,
expected_request_status_with_verification,
):
usr = UserFactory()

t = entity_factory(access_request_handling=access_request_handling)
pr = perm_request_factory(**{"user": usr, perm_request_entity_attr: t})

VerificationFactory(user=usr, email_is_verified=True, is_verified=False)

assert pr.status == expected_request_status_without_verification

mark_verified(
None,
None,
Verification.objects.filter(user_id=usr.pk),
)

pr.refresh_from_db()

assert pr.status == expected_request_status_with_verification

0 comments on commit 9a35fba

Please sign in to comment.