From 0579cce6ee5cd5a44df0a7155d7e3a81cb554365 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Thu, 2 Jan 2025 17:42:36 -0500 Subject: [PATCH] Replace dandiset owners property with get_dandiset_owners Co-authored-by: Mike VanDenburgh --- dandiapi/api/mail.py | 6 ++++-- .../api/management/commands/extract_metadata.py | 3 ++- dandiapi/api/models/dandiset.py | 6 ------ dandiapi/api/tests/test_create_dev_dandiset.py | 3 ++- dandiapi/api/tests/test_dandiset.py | 15 ++++++++------- dandiapi/api/views/dandiset.py | 3 ++- dandiapi/zarr/tests/test_zarr.py | 8 ++++++-- 7 files changed, 24 insertions(+), 20 deletions(-) diff --git a/dandiapi/api/mail.py b/dandiapi/api/mail.py index 7b0a5b4d1..8fddd78fd 100644 --- a/dandiapi/api/mail.py +++ b/dandiapi/api/mail.py @@ -8,6 +8,8 @@ from django.template.loader import render_to_string from django.utils.html import strip_tags +from dandiapi.api.services.permissions.dandiset import get_dandiset_owners + if TYPE_CHECKING: from collections.abc import Iterable @@ -207,7 +209,7 @@ def build_dandiset_unembargoed_message(dandiset: Dandiset): subject='Your Dandiset has been unembargoed!', message=strip_tags(html_message), html_message=html_message, - to=[owner.email for owner in dandiset.owners], + to=[owner.email for owner in get_dandiset_owners(dandiset)], ) @@ -229,7 +231,7 @@ def build_dandiset_unembargo_failed_message(dandiset: Dandiset): subject=f'DANDI: Unembargo failed for dandiset {dandiset.identifier}', message=strip_tags(html_message), html_message=html_message, - to=[owner.email for owner in dandiset.owners], + to=[owner.email for owner in get_dandiset_owners(dandiset)], bcc=[settings.DANDI_DEV_EMAIL], reply_to=[ADMIN_EMAIL], ) diff --git a/dandiapi/api/management/commands/extract_metadata.py b/dandiapi/api/management/commands/extract_metadata.py index 0a9a9ba79..aa45ecee5 100644 --- a/dandiapi/api/management/commands/extract_metadata.py +++ b/dandiapi/api/management/commands/extract_metadata.py @@ -15,6 +15,7 @@ from dandiapi.api.models import Asset, Dandiset, Version from dandiapi.api.services.asset import change_asset +from dandiapi.api.services.permissions.dandiset import get_dandiset_owners if TYPE_CHECKING: from django.db.models import QuerySet @@ -55,7 +56,7 @@ def extract_asset_metadata(asset: Asset, draft_version: Version): # Use dandiset owner, default to some admin user user = ( - draft_version.dandiset.owners.first() + get_dandiset_owners(draft_version.dandiset).first() or User.objects.filter(is_superuser=True, is_staff=True).first() ) diff --git a/dandiapi/api/models/dandiset.py b/dandiapi/api/models/dandiset.py index f44a1bdd8..13ed9a67d 100644 --- a/dandiapi/api/models/dandiset.py +++ b/dandiapi/api/models/dandiset.py @@ -45,12 +45,6 @@ def most_recent_published_version(self): def draft_version(self): return self.versions.filter(version='draft').get() - @property - def owners(self): - from dandiapi.api.services.permissions.dandiset import get_dandiset_owners - - return get_dandiset_owners(self) - @classmethod def published_count(cls): """Return the number of Dandisets with published Versions.""" diff --git a/dandiapi/api/tests/test_create_dev_dandiset.py b/dandiapi/api/tests/test_create_dev_dandiset.py index b3204870b..c32f49d90 100644 --- a/dandiapi/api/tests/test_create_dev_dandiset.py +++ b/dandiapi/api/tests/test_create_dev_dandiset.py @@ -4,6 +4,7 @@ from dandiapi.api.management.commands.create_dev_dandiset import create_dev_dandiset from dandiapi.api.models import Asset, AssetBlob, Dandiset, Version +from dandiapi.api.services.permissions.dandiset import get_dandiset_owners @pytest.mark.django_db @@ -12,7 +13,7 @@ def test_create_dev_dandiset(user): assert Dandiset.objects.count() == 1 dandiset = Dandiset.objects.get() - assert user in dandiset.owners + assert user in get_dandiset_owners(dandiset) assert Version.objects.count() == 1 version = Version.objects.get() diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 669908911..19cc73379 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -10,6 +10,7 @@ from dandiapi.api.models import Dandiset, Version from dandiapi.api.services.permissions.dandiset import ( add_dandiset_owner, + get_dandiset_owners, get_visible_dandisets, replace_dandiset_owners, ) @@ -382,7 +383,7 @@ def test_dandiset_rest_create(api_client, user): # Creating a Dandiset has side affects. # Verify that the user is the only owner. dandiset = Dandiset.objects.get(id=dandiset_id) - assert list(dandiset.owners.all()) == [user] + assert list(get_dandiset_owners(dandiset).all()) == [user] # Verify that a draft Version and VersionMetadata were also created. assert dandiset.versions.count() == 1 @@ -476,7 +477,7 @@ def test_dandiset_rest_create_with_identifier(api_client, admin_user): # Creating a Dandiset has side affects. # Verify that the user is the only owner. dandiset = Dandiset.objects.get(id=identifier) - assert list(dandiset.owners.all()) == [admin_user] + assert list(get_dandiset_owners(dandiset).all()) == [admin_user] # Verify that a draft Version and VersionMetadata were also created. assert dandiset.versions.count() == 1 @@ -584,7 +585,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): # Creating a Dandiset has side affects. # Verify that the user is the only owner. dandiset = Dandiset.objects.get(id=identifier) - assert list(dandiset.owners.all()) == [admin_user] + assert list(get_dandiset_owners(dandiset).all()) == [admin_user] # Verify that a draft Version and VersionMetadata were also created. assert dandiset.versions.count() == 1 @@ -675,7 +676,7 @@ def test_dandiset_rest_create_embargoed(api_client, user): # Creating a Dandiset has side affects. # Verify that the user is the only owner. dandiset = Dandiset.objects.get(id=dandiset_id) - assert list(dandiset.owners.all()) == [user] + assert list(get_dandiset_owners(dandiset).all()) == [user] # Verify that a draft Version and VersionMetadata were also created. assert dandiset.versions.count() == 1 @@ -904,7 +905,7 @@ def test_dandiset_rest_change_owner( 'email': social_account2.extra_data['email'], } ] - assert list(dandiset.owners) == [user2] + assert list(get_dandiset_owners(dandiset)) == [user2] assert len(mailoutbox) == 2 assert mailoutbox[0].subject == f'Removed from Dandiset "{dandiset.draft_version.name}"' @@ -982,7 +983,7 @@ def test_dandiset_rest_add_owner( 'email': social_account2.extra_data['email'], }, ] - assert list(dandiset.owners) == [user1, user2] + assert list(get_dandiset_owners(dandiset)) == [user1, user2] assert len(mailoutbox) == 1 assert mailoutbox[0].subject == f'Added to Dandiset "{dandiset.draft_version.name}"' @@ -1041,7 +1042,7 @@ def test_dandiset_rest_remove_owner( 'email': social_account1.extra_data['email'], } ] - assert list(dandiset.owners) == [user1] + assert list(get_dandiset_owners(dandiset)) == [user1] assert len(mailoutbox) == 1 assert mailoutbox[0].subject == f'Removed from Dandiset "{dandiset.draft_version.name}"' diff --git a/dandiapi/api/views/dandiset.py b/dandiapi/api/views/dandiset.py index cfbe9095e..3df6a975d 100644 --- a/dandiapi/api/views/dandiset.py +++ b/dandiapi/api/views/dandiset.py @@ -31,6 +31,7 @@ ) from dandiapi.api.services.exceptions import NotAllowedError from dandiapi.api.services.permissions.dandiset import ( + get_dandiset_owners, get_owned_dandisets, get_visible_dandisets, is_dandiset_owner, @@ -444,7 +445,7 @@ def users(self, request, dandiset__pk): # noqa: C901 send_ownership_change_emails(dandiset, removed_owners, added_owners) owners = [] - for owner_user in dandiset.owners: + for owner_user in get_dandiset_owners(dandiset): try: owner_account = SocialAccount.objects.get(user=owner_user) owner_dict = {'username': owner_account.extra_data['login']} diff --git a/dandiapi/zarr/tests/test_zarr.py b/dandiapi/zarr/tests/test_zarr.py index 123865f3f..da13aabe0 100644 --- a/dandiapi/zarr/tests/test_zarr.py +++ b/dandiapi/zarr/tests/test_zarr.py @@ -5,7 +5,11 @@ from zarr_checksum.checksum import EMPTY_CHECKSUM from dandiapi.api.models.dandiset import Dandiset -from dandiapi.api.services.permissions.dandiset import add_dandiset_owner, replace_dandiset_owners +from dandiapi.api.services.permissions.dandiset import ( + add_dandiset_owner, + get_dandiset_owners, + replace_dandiset_owners, +) from dandiapi.api.tests.fuzzy import UUID_RE from dandiapi.zarr.models import ZarrArchive, ZarrArchiveStatus from dandiapi.zarr.tasks import ingest_zarr_archive @@ -129,7 +133,7 @@ def test_zarr_rest_get(authenticated_api_client, storage, zarr_archive_factory, @pytest.mark.django_db def test_zarr_rest_get_embargoed(authenticated_api_client, user, embargoed_zarr_archive): - assert user not in embargoed_zarr_archive.dandiset.owners + assert user not in get_dandiset_owners(embargoed_zarr_archive.dandiset) resp = authenticated_api_client.get(f'/api/zarr/{embargoed_zarr_archive.zarr_id}/') assert resp.status_code == 404