Skip to content

Commit

Permalink
Replace dandiset owners property with get_dandiset_owners
Browse files Browse the repository at this point in the history
Co-authored-by: Mike VanDenburgh <michael.vandenburgh@kitware.com>
  • Loading branch information
jjnesbitt and mvandenburgh committed Jan 2, 2025
1 parent 265f0f2 commit 0579cce
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 20 deletions.
6 changes: 4 additions & 2 deletions dandiapi/api/mail.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)],
)


Expand All @@ -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],
)
Expand Down
3 changes: 2 additions & 1 deletion dandiapi/api/management/commands/extract_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
)

Expand Down
6 changes: 0 additions & 6 deletions dandiapi/api/models/dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
3 changes: 2 additions & 1 deletion dandiapi/api/tests/test_create_dev_dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
15 changes: 8 additions & 7 deletions dandiapi/api/tests/test_dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}"'
Expand Down Expand Up @@ -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}"'
Expand Down Expand Up @@ -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}"'
Expand Down
3 changes: 2 additions & 1 deletion dandiapi/api/views/dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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']}
Expand Down
8 changes: 6 additions & 2 deletions dandiapi/zarr/tests/test_zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0579cce

Please sign in to comment.