Skip to content

Commit

Permalink
Remove param from get_queryset, use request to infer this
Browse files Browse the repository at this point in the history
If the user has passed ?include_related_frames=false, then do not prefetch related frames, as this is wasteful. Update tests.
  • Loading branch information
mgdaily committed Oct 9, 2024
1 parent 5cc79e5 commit eb489e5
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
10 changes: 10 additions & 0 deletions archive/frames/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,37 @@ def setUp(self):
self.client.force_login(user)
self.frames = FrameFactory.create_batch(5)
self.frame = self.frames[0]
# Used to check if the related frames are prefetched
self.patcher = patch('archive.frames.views.Prefetch')
self.mock_prefetch = self.patcher.start()

def test_get_frame(self):
response = self.client.get(reverse('frame-detail', args=(self.frame.id, )))
self.assertEqual(response.json()['basename'], self.frame.basename)
self.assertContains(response, 'related_frames')
self.assertEqual(self.mock_prefetch.call_count, 1)

def test_get_frame_exclude_related_frames(self):
response = self.client.get(reverse('frame-detail', args=(self.frame.id,)),
{'include_related_frames': False})

self.assertNotContains(response, 'related_frames')
self.assertEqual(self.mock_prefetch.call_count, 0)

def test_get_frame_list(self):
response = self.client.get(reverse('frame-list'))
self.assertEqual(response.json()['count'], 5)
self.assertContains(response, self.frame.basename)
self.assertContains(response, 'related_frames')
self.assertEqual(self.mock_prefetch.call_count, 1)

def test_get_frame_list_exclude_related_frames(self):
response = self.client.get(reverse('frame-list'),
{'include_related_frames': False})
self.assertEqual(response.json()['count'], 5)
self.assertContains(response, self.frame.basename)
self.assertNotContains(response, 'related_frames')
self.assertEqual(self.mock_prefetch.call_count, 0)

def test_get_frame_list_filter(self):
response = self.client.get(
Expand Down Expand Up @@ -90,6 +97,9 @@ def test_get_frame_list_exclude_empty_version_set(self):
self.assertEqual(Frame.objects.count(), 6)
self.assertEqual(len(response.json()['results']), 5)

def tearDown(self):
self.patcher.stop()


class TestFramePost(ReplicationTestCase):
def setUp(self):
Expand Down
6 changes: 3 additions & 3 deletions archive/frames/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class FrameViewSet(viewsets.ModelViewSet):
ordering_fields = ('id', 'basename', 'observation_date', 'primary_optical_element', 'configuration_type',
'proposal_id', 'instrument_id', 'target_name', 'reduction_level', 'exposure_time')

def get_queryset(self, include_related_frames=False):
def get_queryset(self):
"""
Filter frames depending on the logged in user.
Admin users see all frames, excluding ones which have no versions.
Expand All @@ -72,7 +72,7 @@ def get_queryset(self, include_related_frames=False):
.prefetch_related('thumbnails')
)
# Only prefetch related frames if we're including them in the response
if include_related_frames:
if self.request.query_params.get('include_related_frames', '').lower() != 'false':
queryset = queryset.prefetch_related(Prefetch('related_frames', queryset=Frame.objects.all().only('id')))
if self.request.user.is_superuser:
return queryset
Expand All @@ -92,7 +92,7 @@ def list(self, request, *args, **kwargs):
include_related_frames = False
include_thumbnails = True if request.query_params.get('include_thumbnails', '').lower() == 'true' else False

queryset = self.filter_queryset(self.get_queryset(include_related_frames))
queryset = self.filter_queryset(self.get_queryset())
page = self.paginate_queryset(queryset)
json_models = [model.as_dict(include_thumbnails, include_related_frames) for model in page]
json_models = [model for model in json_models if model['version_set']] # Filter out frames with no versions
Expand Down

0 comments on commit eb489e5

Please sign in to comment.