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

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2bfa8d1
Add Dandiset star functionality with UI components
bendichter Dec 26, 2024
d5546c3
Delete web/src/components/NavigationDrawer.vue
bendichter Dec 26, 2024
645c326
Delete web/src/stores/auth.ts
bendichter Dec 26, 2024
db92523
Ruff compliance for DandisetStar model and admin integration
bendichter Dec 26, 2024
05ed15b
Update pre-commit configuration to exclude package-lock.json from checks
bendichter Dec 26, 2024
61295ab
Add star count and starred status to Dandiset and Version tests
bendichter Dec 26, 2024
dd9bd64
Add tests for Dandiset star functionality and update API responses
bendichter Dec 26, 2024
effd30b
Enhance Dandiset model and StarredDandisetsView to support star funct…
bendichter Dec 26, 2024
3122129
Refactor Dandiset data handling in StarredDandisetsView
bendichter Dec 26, 2024
a44586d
Refactor Dandiset data handling in StarredDandisetsView
bendichter Dec 26, 2024
9308df8
Delete web/package-lock.json
bendichter Dec 26, 2024
cd74534
Update .pre-commit-config.yaml
bendichter Dec 26, 2024
e4dbf30
Implement star/unstar functionality for Dandiset model
bendichter Dec 26, 2024
ba15139
Update dandiapi/api/services/dandiset/__init__.py
bendichter Dec 29, 2024
655e3a4
Update dandiapi/api/services/dandiset/__init__.py
bendichter Dec 29, 2024
988888d
Update dandiapi/api/views/dandiset.py
bendichter Dec 29, 2024
02dfba1
Fix N+1 query problem for dandiset star info
jjnesbitt Dec 30, 2024
b713e30
Fix linting error
jjnesbitt Dec 30, 2024
b96d9e7
Reorder StarredDandisetView in router.ts
jjnesbitt Dec 30, 2024
78e6295
Add button for "Starred Dandisets" on app bar
jjnesbitt Dec 30, 2024
bb14cbf
Incorporate starred dandisets into dandiset list endpoint
jjnesbitt Dec 30, 2024
c50f705
Fix behavior of starred dandisets page
jjnesbitt Dec 30, 2024
7b4441d
Revert change to yarn.lock
jjnesbitt Dec 31, 2024
226b5ba
Fix missing context for version detail serializer
jjnesbitt Dec 31, 2024
0d1aace
Fix behavior of StarButton
jjnesbitt Dec 31, 2024
3e4008a
Replace unique_together with UniqueConstraint
jjnesbitt Jan 3, 2025
a83ba49
Move authentication check into star service functions
jjnesbitt Jan 3, 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
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
exclude: (^\.codespellrc|yarn.lock)
exclude: (^\.codespellrc|yarn.lock|package-lock.json)
bendichter marked this conversation as resolved.
Show resolved Hide resolved
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
Expand Down
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,
Upload,
UserMetadata,
Version,
Expand Down Expand Up @@ -266,3 +267,12 @@ def has_change_permission(self, request, obj=None):

def has_delete_permission(self, request, obj=None):
return False


@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'
50 changes: 50 additions & 0 deletions dandiapi/api/migrations/0014_dandisetstar.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Generated by Django 4.2.17 on 2024-12-26 14:24
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', '0013_remove_assetpath_consistent_slash_and_more'),
]

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='starred_dandisets',
to=settings.AUTH_USER_MODEL,
),
),
],
options={
'unique_together': {('user', 'dandiset')},
},
),
]
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 .oauth import StagingApplication
from .upload import Upload
from .user import UserMetadata
Expand All @@ -16,6 +16,7 @@
'AssetPathRelation',
'AuditRecord',
'Dandiset',
'DandisetStar',
'StagingApplication',
'Upload',
'UserMetadata',
Expand Down
30 changes: 30 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 Down Expand Up @@ -112,10 +113,39 @@ 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()

def star(self, user):
if user.is_authenticated:
self.stars.get_or_create(user=user)

def unstar(self, user):
if user.is_authenticated:
self.stars.filter(user=user).delete()


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='starred_dandisets')
dandiset = models.ForeignKey(Dandiset, on_delete=models.CASCADE, related_name='stars')
created = models.DateTimeField(auto_now_add=True)

class Meta:
unique_together = ('user', 'dandiset')

def __str__(self) -> str:
return f'Star {self.user.username} ★ {self.dandiset.identifier}'
109 changes: 109 additions & 0 deletions dandiapi/api/tests/test_dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,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 @@ -162,6 +164,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 @@ -223,6 +227,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 @@ -238,6 +244,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 @@ -279,6 +287,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 @@ -354,6 +364,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 All @@ -366,6 +378,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,
},
'status': 'Pending',
'created': TIMESTAMP_RE,
Expand Down Expand Up @@ -460,13 +474,17 @@ def test_dandiset_rest_create_with_identifier(api_client, admin_user):
'modified': TIMESTAMP_RE,
'contact_person': 'Doe, John',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
},
'status': 'Pending',
'created': TIMESTAMP_RE,
'modified': TIMESTAMP_RE,
},
'contact_person': 'Doe, John',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}

# Creating a Dandiset has side affects.
Expand Down Expand Up @@ -568,13 +586,17 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user):
'modified': TIMESTAMP_RE,
'contact_person': 'Jane Doe',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
},
'status': 'Pending',
'created': TIMESTAMP_RE,
'modified': TIMESTAMP_RE,
},
'contact_person': 'Jane Doe',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
}

# Creating a Dandiset has side affects.
Expand Down Expand Up @@ -647,6 +669,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 All @@ -659,6 +683,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,
},
'status': 'Pending',
'created': TIMESTAMP_RE,
Expand Down Expand Up @@ -1199,3 +1225,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.post(f'/api/dandisets/{dandiset.identifier}/unstar/')
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'] == 3


@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/')
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/')
assert response.status_code == 401
8 changes: 8 additions & 0 deletions dandiapi/api/tests/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,8 @@ def test_version_rest_list(api_client, version, draft_version_factory):
'modified': TIMESTAMP_RE,
'contact_person': version.metadata['contributor'][0]['name'],
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
},
'version': version.version,
'name': version.name,
Expand Down Expand Up @@ -455,6 +457,8 @@ def test_version_rest_info(api_client, version):
'modified': TIMESTAMP_RE,
'contact_person': version.metadata['contributor'][0]['name'],
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
},
'version': version.version,
'name': version.name,
Expand Down Expand Up @@ -506,6 +510,8 @@ def test_version_rest_info_with_asset(
'modified': TIMESTAMP_RE,
'contact_person': version.metadata['contributor'][0]['name'],
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
},
'version': version.version,
'name': version.name,
Expand Down Expand Up @@ -583,6 +589,8 @@ def test_version_rest_update(api_client, user, draft_version):
'modified': TIMESTAMP_RE,
'contact_person': 'Vargas, Getúlio',
'embargo_status': 'OPEN',
'star_count': 0,
'is_starred': False,
},
'version': draft_version.version,
'name': new_name,
Expand Down
Loading
Loading