Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 25 additions & 1 deletion kpi/serializers/v2/paired_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import re

from django.utils.translation import gettext as t
from django_request_cache import cache_for_request
from drf_spectacular.utils import extend_schema_field
from rest_framework import serializers
from rest_framework.reverse import reverse
Expand Down Expand Up @@ -41,6 +42,9 @@ def create(self, validated_data):
return self.__save(validated_data)

def get_source__name(self, paired_data):
if self._source_is_deleted(paired_data):
return None

# To avoid multiple calls to DB, try to retrieve source names from
# the context.
source__name = None
Expand Down Expand Up @@ -120,6 +124,24 @@ def validate_source(self, source):
def update(self, instance, validated_data):
return self.__save(validated_data)

@cache_for_request
def _source_is_deleted(self, instance) -> bool:
"""
Return whether the source (parent) asset no longer exists.

A deleted source is not cleaned up from the destination's
`paired_data` because there is no cascading FK (see DEV-2034), so it
keeps appearing here. Reuse the `source__names` mapping the viewset
already built (uid absent means the row does not exist) to avoid an
extra query per row on the list endpoint; fall back to a direct
existence check when the context is not populated. Cached per request
so `get_source__name()` and `__get_source_asset_url()` share one lookup.
"""
source__names = self.context.get('source__names')
if source__names is not None:
return instance.source_uid not in source__names
return not Asset.objects.filter(uid=instance.source_uid).exists()

def _validate_fields(self, attrs: dict):

if 'fields' not in attrs:
Expand Down Expand Up @@ -237,7 +259,9 @@ def __get_download_url(self, instance: 'kpi.models.PairedData') -> str:
request=request,
)

def __get_source_asset_url(self, instance: 'kpi.models.PairedData') -> str:
def __get_source_asset_url(self, instance: 'kpi.models.PairedData') -> str | None:
if self._source_is_deleted(instance):
return None
request = self.context['request']
return reverse('asset-detail',
args=[instance.source_uid],
Expand Down
21 changes: 21 additions & 0 deletions kpi/tests/api/v2/test_api_paired_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,27 @@ def test_create_with_already_used_filename(self):
self.assertEqual('`paired_data` is already used',
str(response.data['filename'][0]))

def test_list_with_deleted_source(self):
# anotheruser pairs their form with someuser's source asset
self.toggle_source_sharing(enabled=True)
self.source_asset.assign_perm(self.anotheruser, PERM_VIEW_SUBMISSIONS)
response = self.paired_data()
self.assertEqual(response.status_code, status.HTTP_201_CREATED)

# someuser deletes the source (parent) project. There is no FK cascade,
# so the destination asset still references it in its `paired_data` dict.
self.source_asset.delete()

# The destination owner lists their paired data. The deleted source must
# be reported with a null `source` (not just a null `source__name`).
self.login_as_other_user('anotheruser', 'anotheruser')
response = self.client.get(self.destination_asset_paired_data_url)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data['count'], 1)
result = response.data['results'][0]
self.assertIsNone(result['source'])
self.assertIsNone(result['source__name'])

def test_create_paired_data_anonymous(self):
self.toggle_source_sharing(enabled=True)
payload = {
Expand Down
Loading