Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Dandiset star functionality with UI components #2123

Merged
merged 32 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ec7280b
Add Dandiset star functionality with UI components
bendichter Dec 26, 2024
c4c5afc
Delete web/src/components/NavigationDrawer.vue
bendichter Dec 26, 2024
b1fa860
Delete web/src/stores/auth.ts
bendichter Dec 26, 2024
9d11b34
Ruff compliance for DandisetStar model and admin integration
bendichter Dec 26, 2024
9d8bb01
Update pre-commit configuration to exclude package-lock.json from checks
bendichter Dec 26, 2024
ae147bc
Add star count and starred status to Dandiset and Version tests
bendichter Dec 26, 2024
eaf8f51
Add tests for Dandiset star functionality and update API responses
bendichter Dec 26, 2024
23adcb5
Enhance Dandiset model and StarredDandisetsView to support star funct…
bendichter Dec 26, 2024
84dd9fa
Refactor Dandiset data handling in StarredDandisetsView
bendichter Dec 26, 2024
ef20566
Refactor Dandiset data handling in StarredDandisetsView
bendichter Dec 26, 2024
8a667fe
Delete web/package-lock.json
bendichter Dec 26, 2024
4f749ba
Update .pre-commit-config.yaml
bendichter Dec 26, 2024
66069db
Implement star/unstar functionality for Dandiset model
bendichter Dec 26, 2024
0e039e0
Update dandiapi/api/services/dandiset/__init__.py
bendichter Dec 29, 2024
ebdce66
Update dandiapi/api/services/dandiset/__init__.py
bendichter Dec 29, 2024
7702c69
Update dandiapi/api/views/dandiset.py
bendichter Dec 29, 2024
48f5369
Fix N+1 query problem for dandiset star info
jjnesbitt Dec 30, 2024
ab45617
Reorder StarredDandisetView in router.ts
jjnesbitt Dec 30, 2024
456ac99
Add button for "Starred Dandisets" on app bar
jjnesbitt Dec 30, 2024
3181921
Incorporate starred dandisets into dandiset list endpoint
jjnesbitt Dec 30, 2024
31e1663
Fix behavior of starred dandisets page
jjnesbitt Dec 30, 2024
4996581
Revert change to yarn.lock
jjnesbitt Dec 31, 2024
056d90b
Fix missing context for version detail serializer
jjnesbitt Dec 31, 2024
a9a4af8
Fix behavior of StarButton
jjnesbitt Dec 31, 2024
3772254
Replace unique_together with UniqueConstraint
jjnesbitt Jan 3, 2025
0e7c7c3
Move authentication check into star service functions
jjnesbitt Jan 3, 2025
c3b922a
Fix conflict in migrations
jjnesbitt Jan 28, 2025
910e52f
Replace excessive return statements with variable assignment
jjnesbitt Jan 28, 2025
c35352d
Update dandiapi/api/tests/test_dandiset.py
jjnesbitt Feb 5, 2025
afe79a3
Use `DELETE` rest method for dandiset un-star
jjnesbitt Feb 5, 2025
4a02dbe
Add `starred_users` m2m relation to Dandiset model
jjnesbitt Feb 5, 2025
fe47e36
Unify star and unstar view functions
jjnesbitt Feb 5, 2025
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
10 changes: 10 additions & 0 deletions dandiapi/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
AssetBlob,
AuditRecord,
Dandiset,
DandisetStar,
GarbageCollectionEvent,
Upload,
UserMetadata,
Expand Down Expand Up @@ -272,3 +273,12 @@ def has_delete_permission(self, request, obj=None):
@admin.register(GarbageCollectionEvent)
class GarbageCollectionEventAdmin(admin.ModelAdmin):
pass


@admin.register(DandisetStar)
class DandisetStarAdmin(admin.ModelAdmin):
list_display = ('user', 'dandiset', 'created')
list_filter = ('created',)
search_fields = ('user__username', 'dandiset__id')
raw_id_fields = ('user', 'dandiset')
date_hierarchy = 'created'
59 changes: 59 additions & 0 deletions dandiapi/api/migrations/0016_dandisetstar.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Generated by Django 4.2.17 on 2025-02-05 18:25
from __future__ import annotations

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('api', '0015_unaccent_extension'),
]

operations = [
migrations.CreateModel(
name='DandisetStar',
fields=[
(
'id',
models.BigAutoField(
auto_created=True, primary_key=True, serialize=False, verbose_name='ID'
),
),
('created', models.DateTimeField(auto_now_add=True)),
(
'dandiset',
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name='stars',
to='api.dandiset',
),
),
(
'user',
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name='dandiset_stars',
to=settings.AUTH_USER_MODEL,
),
),
],
),
migrations.AddField(
model_name='dandiset',
name='starred_users',
field=models.ManyToManyField(
related_name='starred_dandisets',
through='api.DandisetStar',
to=settings.AUTH_USER_MODEL,
),
),
migrations.AddConstraint(
model_name='dandisetstar',
constraint=models.UniqueConstraint(
fields=('user', 'dandiset'), name='unique-user-dandiset-star'
),
),
]
3 changes: 2 additions & 1 deletion dandiapi/api/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from .asset import Asset, AssetBlob
from .asset_paths import AssetPath, AssetPathRelation
from .audit import AuditRecord
from .dandiset import Dandiset
from .dandiset import Dandiset, DandisetStar
from .garbage_collection import GarbageCollectionEvent, GarbageCollectionEventRecord
from .oauth import StagingApplication
from .upload import Upload
Expand All @@ -19,6 +19,7 @@
'Dandiset',
'GarbageCollectionEvent',
'GarbageCollectionEventRecord',
'DandisetStar',
'StagingApplication',
'Upload',
'UserMetadata',
Expand Down
27 changes: 27 additions & 0 deletions dandiapi/api/models/dandiset.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from django.contrib.auth.models import User
from django.db import models
from django_extensions.db.models import TimeStampedModel
from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase
Expand All @@ -19,6 +20,9 @@ class EmbargoStatus(models.TextChoices):
choices=EmbargoStatus.choices,
default=EmbargoStatus.OPEN,
)
starred_users = models.ManyToManyField(
to=User, through='DandisetStar', related_name='starred_dandisets'
)

class Meta:
ordering = ['id']
Expand Down Expand Up @@ -58,10 +62,33 @@ def published_count(cls):
def __str__(self) -> str:
return self.identifier

@property
def star_count(self):
return self.stars.count()

def is_starred_by(self, user):
if not user.is_authenticated:
return False
return self.stars.filter(user=user).exists()


class DandisetUserObjectPermission(UserObjectPermissionBase):
content_object = models.ForeignKey(Dandiset, on_delete=models.CASCADE)


class DandisetGroupObjectPermission(GroupObjectPermissionBase):
content_object = models.ForeignKey(Dandiset, on_delete=models.CASCADE)


class DandisetStar(models.Model):
user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='dandiset_stars')
dandiset = models.ForeignKey(Dandiset, on_delete=models.CASCADE, related_name='stars')
created = models.DateTimeField(auto_now_add=True)

class Meta:
constraints = [
models.UniqueConstraint(name='unique-user-dandiset-star', fields=['user', 'dandiset'])
]

def __str__(self) -> str:
return f'Star {self.user.username} ★ {self.dandiset.identifier}'
44 changes: 42 additions & 2 deletions dandiapi/api/services/dandiset/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

from django.db import transaction

from dandiapi.api.models.dandiset import Dandiset
from dandiapi.api.models.dandiset import Dandiset, DandisetStar
from dandiapi.api.models.version import Version
from dandiapi.api.services import audit
from dandiapi.api.services.dandiset.exceptions import DandisetAlreadyExistsError
from dandiapi.api.services.embargo.exceptions import DandisetUnembargoInProgressError
from dandiapi.api.services.exceptions import AdminOnlyOperationError, NotAllowedError
from dandiapi.api.services.exceptions import (
AdminOnlyOperationError,
NotAllowedError,
NotAuthenticatedError,
)
from dandiapi.api.services.permissions.dandiset import add_dandiset_owner, is_dandiset_owner
from dandiapi.api.services.version.metadata import _normalize_version_metadata

Expand Down Expand Up @@ -74,3 +78,39 @@ def delete_dandiset(*, user, dandiset: Dandiset) -> None:

dandiset.versions.all().delete()
dandiset.delete()


def star_dandiset(*, user, dandiset: Dandiset) -> int:
"""
Star a Dandiset for a user.

Args:
user: The user starring the Dandiset.
dandiset: The Dandiset to star.

Returns:
The new star count for the Dandiset.
"""
if not user.is_authenticated:
raise NotAuthenticatedError

DandisetStar.objects.get_or_create(user=user, dandiset=dandiset)
mvandenburgh marked this conversation as resolved.
Show resolved Hide resolved
return dandiset.star_count


def unstar_dandiset(*, user, dandiset: Dandiset) -> int:
"""
Unstar a Dandiset for a user.

Args:
user: The user unstarring the Dandiset.
dandiset: The Dandiset to unstar.

Returns:
The new star count for the Dandiset.
"""
if not user.is_authenticated:
raise NotAuthenticatedError

DandisetStar.objects.filter(user=user, dandiset=dandiset).delete()
mvandenburgh marked this conversation as resolved.
Show resolved Hide resolved
return dandiset.star_count
5 changes: 5 additions & 0 deletions dandiapi/api/services/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,10 @@ class NotAllowedError(DandiError):
http_status_code = status.HTTP_403_FORBIDDEN


class NotAuthenticatedError(DandiError):
message = 'Action requires authentication.'
http_status_code = status.HTTP_401_UNAUTHORIZED


class AdminOnlyOperationError(DandiError):
pass
105 changes: 102 additions & 3 deletions dandiapi/api/tests/test_dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ def test_dandiset_rest_list(api_client, user, dandiset):
'most_recent_published_version': None,
'contact_person': '',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}
],
}
Expand Down Expand Up @@ -167,6 +169,8 @@ def expected_serialization(dandiset: Dandiset):
'modified': TIMESTAMP_RE,
'contact_person': contact_person,
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
'draft_version': {
'version': draft_version.version,
'name': draft_version.name,
Expand Down Expand Up @@ -228,6 +232,8 @@ def test_dandiset_rest_list_for_user(api_client, user, dandiset_factory):
'most_recent_published_version': None,
'contact_person': '',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}
],
}
Expand All @@ -243,6 +249,8 @@ def test_dandiset_rest_retrieve(api_client, dandiset):
'most_recent_published_version': None,
'contact_person': '',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}


Expand Down Expand Up @@ -284,6 +292,8 @@ def test_dandiset_rest_embargo_access(
'most_recent_published_version': None,
'contact_person': '',
'embargo_status': embargo_status,
'star_count': 0,
'is_starred': False,
}
# This is what unauthorized users should get from the retrieve endpoint
expected_error_message = {'detail': 'Not found.'}
Expand Down Expand Up @@ -359,6 +369,8 @@ def test_dandiset_rest_create(api_client, user):
'modified': TIMESTAMP_RE,
'contact_person': 'Doe, John',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
'draft_version': {
'version': 'draft',
'name': name,
Expand Down Expand Up @@ -458,6 +470,8 @@ def test_dandiset_rest_create_with_identifier(api_client, admin_user):
},
'contact_person': 'Doe, John',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}

# Creating a Dandiset has side affects.
Expand Down Expand Up @@ -559,6 +573,8 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user):
},
'contact_person': 'Jane Doe',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}

# Creating a Dandiset has side affects.
Expand Down Expand Up @@ -587,9 +603,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user):
'version': 'draft',
'url': url,
'dateCreated': UTC_ISO_TIMESTAMP_RE,
'citation': (
f'Jane Doe ({year}) {name} ' f'(Version draft) [Data set]. DANDI Archive. {url}'
),
'citation': (f'Jane Doe ({year}) {name} (Version draft) [Data set]. DANDI Archive. {url}'),
'@context': f'https://raw.githubusercontent.com/dandi/schema/master/releases/{settings.DANDI_SCHEMA_VERSION}/context.json',
'schemaVersion': settings.DANDI_SCHEMA_VERSION,
'schemaKey': 'Dandiset',
Expand Down Expand Up @@ -631,6 +645,8 @@ def test_dandiset_rest_create_embargoed(api_client, user):
'modified': TIMESTAMP_RE,
'contact_person': 'Doe, John',
'embargo_status': 'EMBARGOED',
'star_count': 0,
'is_starred': False,
'draft_version': {
'version': 'draft',
'name': name,
Expand Down Expand Up @@ -1230,3 +1246,86 @@ def test_dandiset_rest_clear_active_uploads(
response = authenticated_api_client.get(f'/api/dandisets/{ds.identifier}/uploads/').json()
assert response['count'] == 0
assert len(response['results']) == 0


@pytest.mark.django_db
def test_dandiset_star(api_client, user, dandiset):
api_client.force_authenticate(user=user)
response = api_client.post(f'/api/dandisets/{dandiset.identifier}/star/')
assert response.status_code == 200
assert response.data == {'count': 1}
assert dandiset.stars.count() == 1
assert dandiset.stars.first().user == user


@pytest.mark.django_db
def test_dandiset_unstar(api_client, user, dandiset):
api_client.force_authenticate(user=user)
# First star it
api_client.post(f'/api/dandisets/{dandiset.identifier}/star/')
assert dandiset.stars.count() == 1

# Then unstar it
response = api_client.delete(f'/api/dandisets/{dandiset.identifier}/star/')
assert response.status_code == 200
assert response.data == {'count': 0}
assert dandiset.stars.count() == 0


@pytest.mark.django_db
def test_dandiset_star_unauthenticated(api_client, dandiset):
response = api_client.post(f'/api/dandisets/{dandiset.identifier}/star/')
assert response.status_code == 401


@pytest.mark.django_db
def test_dandiset_star_count(api_client, user_factory, dandiset):
users = [user_factory() for _ in range(3)]
for user in users:
api_client.force_authenticate(user=user)
api_client.post(f'/api/dandisets/{dandiset.identifier}/star/')

response = api_client.get(f'/api/dandisets/{dandiset.identifier}/')
assert response.data['star_count'] == len(users)


@pytest.mark.django_db
def test_dandiset_is_starred(api_client, user, dandiset):
# Test unauthenticated
response = api_client.get(f'/api/dandisets/{dandiset.identifier}/')
assert response.data['is_starred'] is False

# Test authenticated but not starred
api_client.force_authenticate(user=user)
response = api_client.get(f'/api/dandisets/{dandiset.identifier}/')
assert response.data['is_starred'] is False

# Test after starring
api_client.post(f'/api/dandisets/{dandiset.identifier}/star/')
response = api_client.get(f'/api/dandisets/{dandiset.identifier}/')
assert response.data['is_starred'] is True


@pytest.mark.django_db
def test_dandiset_list_starred(api_client, user, dandiset_factory):
api_client.force_authenticate(user=user)
dandisets = [dandiset_factory() for _ in range(3)]

# Star 2 out of 3 dandisets
api_client.post(f'/api/dandisets/{dandisets[0].identifier}/star/')
api_client.post(f'/api/dandisets/{dandisets[1].identifier}/star/')

# List starred dandisets
response = api_client.get('/api/dandisets/', {'starred': True})
assert response.status_code == 200
assert response.data['count'] == 2
assert {d['identifier'] for d in response.data['results']} == {
dandisets[0].identifier,
dandisets[1].identifier,
}


@pytest.mark.django_db
def test_dandiset_list_starred_unauthenticated(api_client):
response = api_client.get('/api/dandisets/', {'starred': True})
assert response.status_code == 401
Loading