From 28ffe1042914dc15455e895e659e9ff3bb6aad64 Mon Sep 17 00:00:00 2001 From: Dan Avner Date: Thu, 16 Nov 2023 15:02:23 -0800 Subject: [PATCH] Add observation deletion functionality. - Implement unified method for deleting observations and associated data products. - Resolve bug preventing deletion of thumbnails. - Introduce new template for observation deletion process. --- tom_common/utils.py | 35 ++++++++++++++++++ tom_dataproducts/views.py | 10 +---- .../observationrecord_confirm_delete.html | 12 ++++++ .../observationrecord_detail.html | 1 + tom_observations/tests/tests.py | 37 +++++++++++++++++++ tom_observations/urls.py | 3 +- tom_observations/views.py | 29 +++++++++++++++ 7 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 tom_common/utils.py create mode 100644 tom_observations/templates/tom_observations/observationrecord_confirm_delete.html diff --git a/tom_common/utils.py b/tom_common/utils.py new file mode 100644 index 000000000..b8b12f868 --- /dev/null +++ b/tom_common/utils.py @@ -0,0 +1,35 @@ +from typing import Union + +from tom_dataproducts.models import DataProduct, ReducedDatum +from tom_observations.models import ObservationRecord + + +def delete_associated_data_products(record_or_product: Union[ObservationRecord, DataProduct]) -> None: + """ + Utility function to delete associated DataProducts or a single DataProduct. + + Parameters + ---------- + record_or_product : Union[ObservationRecord, DataProduct] + The ObservationRecord object to find associated DataProducts, + or a single DataProduct to be deleted. + """ + if isinstance(record_or_product, ObservationRecord): + query = DataProduct.objects.filter(observation_record=record_or_product) + elif isinstance(record_or_product, DataProduct): + query = [record_or_product] + else: + raise ValueError("Invalid argument type. Must be ObservationRecord or DataProduct.") + + for data_product in query: + # Delete associated ReducedDatum objects. + ReducedDatum.objects.filter(data_product=data_product).delete() + + # Delete the file from the disk. + data_product.data.delete() + + # Delete thumbnail from the disk. + data_product.thumbnail.delete() + + # Delete the `DataProduct` object from the database. + data_product.delete() \ No newline at end of file diff --git a/tom_dataproducts/views.py b/tom_dataproducts/views.py index 95223f844..4e9f3017f 100644 --- a/tom_dataproducts/views.py +++ b/tom_dataproducts/views.py @@ -23,6 +23,7 @@ from tom_common.hooks import run_hook from tom_common.hints import add_hint from tom_common.mixins import Raise403PermissionRequiredMixin +from tom_common.utils import delete_associated_data_products from tom_dataproducts.models import DataProduct, DataProductGroup, ReducedDatum from tom_dataproducts.exceptions import InvalidFileFormatException from tom_dataproducts.forms import AddProductToGroupForm, DataProductUploadForm, DataShareForm @@ -279,14 +280,7 @@ def form_valid(self, form): """ # Fetch the DataProduct object data_product = self.get_object() - - # Delete associated ReducedDatum objects - ReducedDatum.objects.filter(data_product=data_product).delete() - - # Delete the file reference. - data_product.data.delete() - # Delete the `DataProduct` object from the database. - data_product.delete() + delete_associated_data_products(data_product) return HttpResponseRedirect(self.get_success_url()) diff --git a/tom_observations/templates/tom_observations/observationrecord_confirm_delete.html b/tom_observations/templates/tom_observations/observationrecord_confirm_delete.html new file mode 100644 index 000000000..dc9eee7c8 --- /dev/null +++ b/tom_observations/templates/tom_observations/observationrecord_confirm_delete.html @@ -0,0 +1,12 @@ +{% extends 'tom_common/base.html' %} +{% load bootstrap4 %} +{% block title %}Confirm delete{% endblock %} +{% block content %} +

Confirm Delete

+
{% csrf_token %} +

Are you sure you want to delete observation ID {{ object.observation_id }} for target {{ object }}"?

+ {% buttons %} + + {% endbuttons %} +
+{% endblock %} \ No newline at end of file diff --git a/tom_observations/templates/tom_observations/observationrecord_detail.html b/tom_observations/templates/tom_observations/observationrecord_detail.html index 88868f1fe..19c6de2e4 100644 --- a/tom_observations/templates/tom_observations/observationrecord_detail.html +++ b/tom_observations/templates/tom_observations/observationrecord_detail.html @@ -19,6 +19,7 @@

{{ object }}

Observation ID: {{ object.observation_id }}

Created: {{ object.created }} Modified: {{ object.modified }}

Status: {{ object.status }}

+ Delete

{% upload_dataproduct object %} diff --git a/tom_observations/tests/tests.py b/tom_observations/tests/tests.py index ff61f8739..cf42a562b 100644 --- a/tom_observations/tests/tests.py +++ b/tom_observations/tests/tests.py @@ -4,6 +4,7 @@ from django.contrib.auth.models import User from django.contrib.messages import get_messages +from django.core.files.uploadedfile import SimpleUploadedFile from django.forms import ValidationError from django.test import TestCase, override_settings from django.urls import reverse @@ -13,6 +14,7 @@ from astropy.time import Time from .factories import ObservingRecordFactory, ObservationTemplateFactory, SiderealTargetFactory, TargetNameFactory +from tom_dataproducts.models import DataProduct from tom_observations.utils import get_astroplan_sun_and_time, get_sidereal_visibility from tom_observations.tests.utils import FakeRoboticFacility from tom_observations.models import ObservationRecord, ObservationGroup, ObservationTemplate @@ -436,3 +438,38 @@ def test_get_visibility_sidereal(self, mock_facility): self.assertEqual(len(airmass_data), len(expected_airmass)) for i, expected_airmass_value in enumerate(expected_airmass): self.assertAlmostEqual(airmass_data[i], expected_airmass_value, places=3) + + +@override_settings(TOM_FACILITY_CLASSES=['tom_observations.tests.utils.FakeRoboticFacility']) +class TestObservationRecordDeleteView(TestCase): + def setUp(self): + self.user = User.objects.create_user(username='testuser', password='testpassword') + self.client.force_login(self.user) + self.target = SiderealTargetFactory.create() + self.target_name = TargetNameFactory.create(target=self.target) + self.observation_record = ObservingRecordFactory.create( + target_id=self.target.id, + facility=FakeRoboticFacility.name, + parameters={} + ) + self.data_product = DataProduct.objects.create( + product_id='testproductid', + target=self.target, + observation_record=self.observation_record, + data=SimpleUploadedFile('afile.fits', b'somedata') + ) + assign_perm('tom_observations.delete_observationrecord', self.user, self.observation_record) + assign_perm('tom_targets.delete_dataproduct', self.user, self.data_product) + + def test_delete_observation(self): + self.assertTrue(ObservationRecord.objects.filter(id=self.observation_record.id).exists()) + self.assertTrue(DataProduct.objects.filter(product_id='testproductid').exists()) + delete_url = reverse('tom_observations:delete', kwargs={'pk': self.observation_record.id}) + response = self.client.post(delete_url, follow=True) + + # Check if the response redirected to the success URL (observations list). + self.assertRedirects(response, reverse("tom_observations:list")) + + # Verify that the ObservationRecord and DataProduct no longer exists. + self.assertFalse(ObservationRecord.objects.filter(id=self.observation_record.id).exists()) + self.assertFalse(DataProduct.objects.filter(product_id='testproductid').exists()) diff --git a/tom_observations/urls.py b/tom_observations/urls.py index b9773a81f..118b303a9 100644 --- a/tom_observations/urls.py +++ b/tom_observations/urls.py @@ -5,7 +5,7 @@ ObservationGroupListView, ObservationListView, ObservationRecordCancelView, ObservationRecordDetailView, ObservationTemplateCreateView, ObservationTemplateDeleteView, ObservationTemplateListView, - ObservationTemplateUpdateView) + ObservationTemplateUpdateView, ObservationRecordDeleteView) from tom_observations.api_views import ObservationRecordViewSet from tom_common.api_router import SharedAPIRootRouter @@ -16,6 +16,7 @@ urlpatterns = [ path('add/', AddExistingObservationView.as_view(), name='add-existing'), + path('/delete', ObservationRecordDeleteView.as_view(), name='delete'), path('list/', ObservationListView.as_view(), name='list'), path('status/', FacilityStatusView.as_view(), name='facility-status'), path('template/list/', ObservationTemplateListView.as_view(), name='template-list'), diff --git a/tom_observations/views.py b/tom_observations/views.py index 7508d9eb0..0314dcba1 100644 --- a/tom_observations/views.py +++ b/tom_observations/views.py @@ -14,6 +14,7 @@ from django_filters import (CharFilter, ChoiceFilter, DateTimeFromToRangeFilter, FilterSet, ModelMultipleChoiceFilter, OrderingFilter) from django_filters.views import FilterView +from django.http import HttpResponse from django.shortcuts import redirect from django.urls import reverse, reverse_lazy from django.utils.safestring import mark_safe @@ -26,6 +27,7 @@ from tom_common.hints import add_hint from tom_common.mixins import Raise403PermissionRequiredMixin +from tom_common.utils import delete_associated_data_products from tom_dataproducts.forms import AddProductToGroupForm, DataProductUploadForm from tom_dataproducts.models import is_fits_image_file from tom_observations.cadence import CadenceForm, get_cadence_strategy @@ -667,3 +669,30 @@ class ObservationTemplateDeleteView(LoginRequiredMixin, DeleteView): class FacilityStatusView(TemplateView): template_name = 'tom_observations/facility_status.html' + + +class ObservationRecordDeleteView(Raise403PermissionRequiredMixin, DeleteView): + """View for deleting an observation.""" + permission_required = "tom_observations.delete_observationrecord" + success_url = reverse_lazy("observations:list") + model = ObservationRecord + + def form_valid(self, form: forms.Form) -> HttpResponse: + """Handle deletion of associated DataProducts upon valid form + submission. + + Parameters + ---------- + form : `Form` + The form object. + + Returns + ------- + `HttpResponse` + HTTP response indicating the outcome of the deletion process. + """ + # Fetch the ObservationRecord object. + observation_record = self.get_object() + delete_associated_data_products(observation_record) + + return super().form_valid(form)