From 3a902e322d412e4008989536d0651706faaec54c Mon Sep 17 00:00:00 2001 From: Jon Date: Wed, 1 Nov 2023 19:16:11 -0100 Subject: [PATCH 01/56] Add user object into facility class so it can be used for per-user authentication --- tom_observations/facilities/lco.py | 4 ++-- tom_observations/facilities/ocs.py | 4 ++-- tom_observations/facilities/soar.py | 4 ++-- tom_observations/facility.py | 5 +++++ .../management/commands/updatestatus.py | 14 ++++++++++++- .../templatetags/observation_extras.py | 18 ++++++++--------- tom_observations/views.py | 20 +++++++++---------- 7 files changed, 43 insertions(+), 26 deletions(-) diff --git a/tom_observations/facilities/lco.py b/tom_observations/facilities/lco.py index 8b2d225f3..51224a945 100644 --- a/tom_observations/facilities/lco.py +++ b/tom_observations/facilities/lco.py @@ -1076,8 +1076,8 @@ class LCOFacility(OCSFacility): 'SPECTROSCOPIC_SEQUENCE': LCOSpectroscopicSequenceForm } - def __init__(self, facility_settings=LCOSettings('LCO')): - super().__init__(facility_settings=facility_settings) + def __init__(self, user=None, facility_settings=LCOSettings('LCO')): + super().__init__(user=user, facility_settings=facility_settings) # TODO: this should be called get_form_class def get_form(self, observation_type): diff --git a/tom_observations/facilities/ocs.py b/tom_observations/facilities/ocs.py index 463284d65..2d23ee340 100644 --- a/tom_observations/facilities/ocs.py +++ b/tom_observations/facilities/ocs.py @@ -1235,9 +1235,9 @@ class OCSFacility(BaseRoboticObservationFacility): 'ALL': OCSFullObservationForm, } - def __init__(self, facility_settings=OCSSettings('OCS')): + def __init__(self, user=None, facility_settings=OCSSettings('OCS')): self.facility_settings = facility_settings - super().__init__() + super().__init__(user=user) # TODO: this should be called get_form_class def get_form(self, observation_type): diff --git a/tom_observations/facilities/soar.py b/tom_observations/facilities/soar.py index 1da83e3c6..d1f87f920 100644 --- a/tom_observations/facilities/soar.py +++ b/tom_observations/facilities/soar.py @@ -76,8 +76,8 @@ class SOARFacility(LCOFacility): 'SPECTRA': SOARSpectroscopyObservationForm } - def __init__(self, facility_settings=SOARSettings('LCO')): - super().__init__(facility_settings=facility_settings) + def __init__(self, user=None, facility_settings=SOARSettings('LCO')): + super().__init__(user=user, facility_settings=facility_settings) def get_form(self, observation_type): return self.observation_forms.get(observation_type, SOARImagingObservationForm) diff --git a/tom_observations/facility.py b/tom_observations/facility.py index e7a7254c1..618844e24 100644 --- a/tom_observations/facility.py +++ b/tom_observations/facility.py @@ -186,6 +186,11 @@ class BaseObservationFacility(ABC): """ name = 'BaseObservation' + def __init__(self, user=None): + if self.__class__ == BaseObservationFacility: + raise Exception("Cannot instantiate abstract class BaseObservationFacility") + self.user = user + def all_data_products(self, observation_record): from tom_dataproducts.models import DataProduct products = {'saved': [], 'unsaved': []} diff --git a/tom_observations/management/commands/updatestatus.py b/tom_observations/management/commands/updatestatus.py index 6ef98e8e6..662e18892 100644 --- a/tom_observations/management/commands/updatestatus.py +++ b/tom_observations/management/commands/updatestatus.py @@ -1,5 +1,6 @@ from django.core.management.base import BaseCommand from django.core.exceptions import ObjectDoesNotExist +from django.contrib.auth.models import User from tom_targets.models import Target from tom_observations import facility @@ -18,19 +19,30 @@ def add_arguments(self, parser): '--target_id', help='Update observation statuses for a single target' ) + parser.add_argument( + '--username', + required=False, + help='The username of a user to use if the facility requires per user-based authentication for its API calls' + ) def handle(self, *args, **options): target = None + user = None if options['target_id']: try: target = Target.objects.get(pk=options['target_id']) except ObjectDoesNotExist: raise Exception('Invalid target id provided') + if options.get('username'): + try: + user = User.objects.get(username=options['username']) + except User.DoesNotExist: + raise Exception('Invalid username provided') failed_records = {} for facility_name in facility.get_service_classes(): clazz = facility.get_service_class(facility_name) - failed_records[facility_name] = clazz().update_all_observation_statuses(target=target) + failed_records[facility_name] = clazz(user=user).update_all_observation_statuses(target=target) success = True for facility_name, errors in failed_records.items(): if len(errors) > 0: diff --git a/tom_observations/templatetags/observation_extras.py b/tom_observations/templatetags/observation_extras.py index 60a3d5d95..445887002 100644 --- a/tom_observations/templatetags/observation_extras.py +++ b/tom_observations/templatetags/observation_extras.py @@ -79,12 +79,12 @@ def observation_type_tabs(context): } -@register.inclusion_tag('tom_observations/partials/facility_observation_form.html') -def facility_observation_form(target, facility, observation_type): +@register.inclusion_tag('tom_observations/partials/facility_observation_form.html', takes_context=True) +def facility_observation_form(context, target, facility, observation_type): """ Displays a form for submitting an observation for a specific facility and observation type, e.g., imaging. """ - facility_class = get_service_class(facility)() + facility_class = get_service_class(facility)(user=context['request'].user) initial_fields = { 'target_id': target.id, 'facility': facility, @@ -259,8 +259,8 @@ def observation_distribution(observations): return {'figure': figure} -@register.inclusion_tag('tom_observations/partials/facility_status.html') -def facility_status(): +@register.inclusion_tag('tom_observations/partials/facility_status.html', takes_context=True) +def facility_status(context): """ Collect the facility status from the registered facilities and pass them to the facility_status.html partial template. @@ -270,7 +270,7 @@ def facility_status(): facility_statuses = [] for facility_class in get_service_classes().values(): - facility = facility_class() + facility = facility_class(user=context['request'].user) weather_urls = facility.get_facility_weather_urls() status = facility.get_facility_status() @@ -286,11 +286,11 @@ def facility_status(): return {'facilities': facility_statuses} -@register.inclusion_tag('tom_observations/partials/facility_map.html') -def facility_map(): +@register.inclusion_tag('tom_observations/partials/facility_map.html', takes_context=True) +def facility_map(context): facility_locations = [] for facility_class in get_service_classes().values(): - facility = facility_class() + facility = facility_class(user=context['request'].user) sites = facility.get_observing_sites() # Flatten each facility site dictionary and add text label for use in facility map diff --git a/tom_observations/views.py b/tom_observations/views.py index 7508d9eb0..4a86d1e81 100644 --- a/tom_observations/views.py +++ b/tom_observations/views.py @@ -223,7 +223,7 @@ def get_context_data(self, **kwargs): # allow the Facility class to add data to the context facility_class = self.get_facility_class() - facility_context = facility_class().get_facility_context_data(target=target) + facility_context = facility_class(user=self.request.user).get_facility_context_data(target=target) context.update(facility_context) return context @@ -241,7 +241,7 @@ def get_form_class(self): elif self.request.method == 'POST': observation_type = self.request.POST.get('observation_type') form_class = type(f'Composite{observation_type}Form', - (self.get_facility_class()().get_form(observation_type), self.get_cadence_strategy_form()), + (self.get_facility_class()(user=self.request.user).get_form(observation_type), self.get_cadence_strategy_form()), {}) return form_class @@ -311,7 +311,7 @@ def form_valid(self, form): # Submit the observation facility = self.get_facility_class() target = self.get_target() - observation_ids = facility().submit_observation(form.observation_payload()) + observation_ids = facility(user=self.request.user).submit_observation(form.observation_payload()) records = [] for observation_id in observation_ids: @@ -376,7 +376,7 @@ class ObservationRecordCancelView(LoginRequiredMixin, View): def get(self, request, *args, **kwargs): obsr_id = self.kwargs.get('pk') obsr = ObservationRecord.objects.get(id=obsr_id) - facility = get_service_class(obsr.facility)() + facility = get_service_class(obsr.facility)(user=request.user) try: success = facility.cancel_observation(obsr.observation_id) if success: @@ -500,10 +500,10 @@ def get_context_data(self, *args, **kwargs): """ context = super().get_context_data(*args, **kwargs) context['form'] = AddProductToGroupForm() - service_class = get_service_class(self.object.facility) - context['editable'] = isinstance(service_class(), BaseManualObservationFacility) - context['data_products'] = service_class().all_data_products(self.object) - context['can_be_cancelled'] = self.object.status not in service_class().get_terminal_observing_states() + service_class = get_service_class(self.object.facility)(user=self.request.user) + context['editable'] = isinstance(service_class, BaseManualObservationFacility) + context['data_products'] = service_class.all_data_products(self.object) + context['can_be_cancelled'] = self.object.status not in service_class.get_terminal_observing_states() newest_image = None for data_product in context['data_products']['saved']: newest_image = data_product if (not newest_image or data_product.modified > newest_image.modified) and \ @@ -607,7 +607,7 @@ def get_form_class(self): raise ValueError('Must provide a facility name') # TODO: modify this to work with all LCO forms - return get_service_class(facility_name)().get_template_form(None) + return get_service_class(facility_name)(user=self.request.user).get_template_form(None) def get_form(self, form_class=None): form = super().get_form() @@ -637,7 +637,7 @@ def get_object(self): def get_form_class(self): self.object = self.get_object() - return get_service_class(self.object.facility)().get_template_form(None) + return get_service_class(self.object.facility)(user=self.request.user).get_template_form(None) def get_form(self, form_class=None): form = super().get_form() From c3c0a596a72499d9adc8012bf8c3aeeda85bec05 Mon Sep 17 00:00:00 2001 From: Jon Date: Wed, 1 Nov 2023 19:20:46 -0100 Subject: [PATCH 02/56] Fix linting errors --- tom_observations/management/commands/updatestatus.py | 2 +- tom_observations/views.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tom_observations/management/commands/updatestatus.py b/tom_observations/management/commands/updatestatus.py index 662e18892..5036cf472 100644 --- a/tom_observations/management/commands/updatestatus.py +++ b/tom_observations/management/commands/updatestatus.py @@ -22,7 +22,7 @@ def add_arguments(self, parser): parser.add_argument( '--username', required=False, - help='The username of a user to use if the facility requires per user-based authentication for its API calls' + help='The username of a user to use if the facility uses per user-based authentication for its API calls' ) def handle(self, *args, **options): diff --git a/tom_observations/views.py b/tom_observations/views.py index 4a86d1e81..c7a1c4d2a 100644 --- a/tom_observations/views.py +++ b/tom_observations/views.py @@ -240,8 +240,9 @@ def get_form_class(self): observation_type = self.request.GET.get('observation_type') elif self.request.method == 'POST': observation_type = self.request.POST.get('observation_type') + facility = self.get_facility_class()(user=self.request.user) form_class = type(f'Composite{observation_type}Form', - (self.get_facility_class()(user=self.request.user).get_form(observation_type), self.get_cadence_strategy_form()), + (facility.get_form(observation_type), self.get_cadence_strategy_form()), {}) return form_class From 118c3a0931184e0522580538cc33042a3e7cd825 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 2 Nov 2023 21:14:37 -0100 Subject: [PATCH 03/56] add doc for user in facility --- docs/observing/observation_module.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/observing/observation_module.rst b/docs/observing/observation_module.rst index 7ed7265d3..8a7ad224c 100644 --- a/docs/observing/observation_module.rst +++ b/docs/observing/observation_module.rst @@ -114,7 +114,11 @@ from two other classes. logic” for interacting with the remote observatory. This includes methods to submit observations, check observation status, etc. It inherits from ``BaseRoboticObservationFacility``, which contains some -functionality that all observation facility classes will want. +functionality that all observation facility classes will want. The +``BaseObservationFacility`` init method takes in a User object, which +you can access from your facility implementation if you need it for any +api requests. Remember to pass this parameter through if you need to override +the init method in your facility implementation. ``MyObservationFacilityForm`` is the class that will display a GUI form for our users to create an observation. We can submit observations From f4ef9863d878e0f633fa2374d440685afbd95090 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 2 Nov 2023 21:35:22 -0100 Subject: [PATCH 04/56] Add unit test showing user ends up in the facility instantiation --- tom_observations/tests/tests.py | 19 +++++++++++++++++++ tom_observations/tests/utils.py | 7 +++++++ 2 files changed, 26 insertions(+) diff --git a/tom_observations/tests/tests.py b/tom_observations/tests/tests.py index ff61f8739..96249a88a 100644 --- a/tom_observations/tests/tests.py +++ b/tom_observations/tests/tests.py @@ -144,6 +144,25 @@ def test_submit_observation_robotic(self): self.assertTrue(ObservationRecord.objects.filter(observation_id='fakeid').exists()) self.assertEqual(ObservationRecord.objects.filter(observation_id='fakeid').first().user, self.user) + @mock.patch('tom_observations.tests.utils.FakeRoboticFacility.test_user') + def test_submit_observation_robotic_gets_user(self, mock_method): + form_data = { + 'target_id': self.target.id, + 'test_input': 'gnomes', + 'facility': 'FakeRoboticFacility', + 'observation_type': 'OBSERVATION' + } + self.client.post( + '{}?target_id={}'.format( + reverse('tom_observations:create', kwargs={'facility': 'FakeRoboticFacility'}), + self.target.id + ), + data=form_data, + follow=True + ) + calls = [mock.call(self.user)] + mock_method.assert_has_calls(calls) + # TODO: this test # def test_submit_observation_cadence(self): # form_data = { diff --git a/tom_observations/tests/utils.py b/tom_observations/tests/utils.py index 25c7a5289..86bed3d84 100644 --- a/tom_observations/tests/utils.py +++ b/tom_observations/tests/utils.py @@ -40,6 +40,13 @@ class FakeRoboticFacility(BaseRoboticObservationFacility): 'OBSERVATION': FakeFacilityForm } + def __init__(self, user=None): + self.test_user(user) + super().__init__(user) + + def test_user(self, user): + return user + def get_form(self, observation_type): return self.observation_forms[observation_type] From 35df0134574d8a56379209340c49972890c54048 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 2 Nov 2023 22:43:53 -0100 Subject: [PATCH 05/56] Add user in a backwards compatible way to facilities --- docs/observing/observation_module.rst | 8 ++--- tom_observations/facilities/lco.py | 4 +-- tom_observations/facilities/ocs.py | 4 +-- tom_observations/facilities/soar.py | 4 +-- tom_observations/facility.py | 7 ++-- .../management/commands/updatestatus.py | 5 +-- .../templatetags/observation_extras.py | 11 ++++--- tom_observations/tests/tests.py | 3 +- tom_observations/tests/utils.py | 7 ---- tom_observations/views.py | 33 ++++++++++++------- 10 files changed, 46 insertions(+), 40 deletions(-) diff --git a/docs/observing/observation_module.rst b/docs/observing/observation_module.rst index 8a7ad224c..5b2db6f2b 100644 --- a/docs/observing/observation_module.rst +++ b/docs/observing/observation_module.rst @@ -114,11 +114,9 @@ from two other classes. logic” for interacting with the remote observatory. This includes methods to submit observations, check observation status, etc. It inherits from ``BaseRoboticObservationFacility``, which contains some -functionality that all observation facility classes will want. The -``BaseObservationFacility`` init method takes in a User object, which -you can access from your facility implementation if you need it for any -api requests. Remember to pass this parameter through if you need to override -the init method in your facility implementation. +functionality that all observation facility classes will want. You +can access the user within your facility implementation using +``self.user`` if you need it for any api requests. ``MyObservationFacilityForm`` is the class that will display a GUI form for our users to create an observation. We can submit observations diff --git a/tom_observations/facilities/lco.py b/tom_observations/facilities/lco.py index 51224a945..8b2d225f3 100644 --- a/tom_observations/facilities/lco.py +++ b/tom_observations/facilities/lco.py @@ -1076,8 +1076,8 @@ class LCOFacility(OCSFacility): 'SPECTROSCOPIC_SEQUENCE': LCOSpectroscopicSequenceForm } - def __init__(self, user=None, facility_settings=LCOSettings('LCO')): - super().__init__(user=user, facility_settings=facility_settings) + def __init__(self, facility_settings=LCOSettings('LCO')): + super().__init__(facility_settings=facility_settings) # TODO: this should be called get_form_class def get_form(self, observation_type): diff --git a/tom_observations/facilities/ocs.py b/tom_observations/facilities/ocs.py index 2d23ee340..463284d65 100644 --- a/tom_observations/facilities/ocs.py +++ b/tom_observations/facilities/ocs.py @@ -1235,9 +1235,9 @@ class OCSFacility(BaseRoboticObservationFacility): 'ALL': OCSFullObservationForm, } - def __init__(self, user=None, facility_settings=OCSSettings('OCS')): + def __init__(self, facility_settings=OCSSettings('OCS')): self.facility_settings = facility_settings - super().__init__(user=user) + super().__init__() # TODO: this should be called get_form_class def get_form(self, observation_type): diff --git a/tom_observations/facilities/soar.py b/tom_observations/facilities/soar.py index d1f87f920..1da83e3c6 100644 --- a/tom_observations/facilities/soar.py +++ b/tom_observations/facilities/soar.py @@ -76,8 +76,8 @@ class SOARFacility(LCOFacility): 'SPECTRA': SOARSpectroscopyObservationForm } - def __init__(self, user=None, facility_settings=SOARSettings('LCO')): - super().__init__(user=user, facility_settings=facility_settings) + def __init__(self, facility_settings=SOARSettings('LCO')): + super().__init__(facility_settings=facility_settings) def get_form(self, observation_type): return self.observation_forms.get(observation_type, SOARImagingObservationForm) diff --git a/tom_observations/facility.py b/tom_observations/facility.py index 618844e24..cff1ae4aa 100644 --- a/tom_observations/facility.py +++ b/tom_observations/facility.py @@ -186,9 +186,10 @@ class BaseObservationFacility(ABC): """ name = 'BaseObservation' - def __init__(self, user=None): - if self.__class__ == BaseObservationFacility: - raise Exception("Cannot instantiate abstract class BaseObservationFacility") + def __init__(self): + self.user = None + + def set_user(self, user): self.user = user def all_data_products(self, observation_record): diff --git a/tom_observations/management/commands/updatestatus.py b/tom_observations/management/commands/updatestatus.py index 5036cf472..2863cb98f 100644 --- a/tom_observations/management/commands/updatestatus.py +++ b/tom_observations/management/commands/updatestatus.py @@ -41,8 +41,9 @@ def handle(self, *args, **options): failed_records = {} for facility_name in facility.get_service_classes(): - clazz = facility.get_service_class(facility_name) - failed_records[facility_name] = clazz(user=user).update_all_observation_statuses(target=target) + instance = facility.get_service_class(facility_name)() + instance.set_user(user) + failed_records[facility_name] = instance.update_all_observation_statuses(target=target) success = True for facility_name, errors in failed_records.items(): if len(errors) > 0: diff --git a/tom_observations/templatetags/observation_extras.py b/tom_observations/templatetags/observation_extras.py index 445887002..40091dfa2 100644 --- a/tom_observations/templatetags/observation_extras.py +++ b/tom_observations/templatetags/observation_extras.py @@ -84,13 +84,14 @@ def facility_observation_form(context, target, facility, observation_type): """ Displays a form for submitting an observation for a specific facility and observation type, e.g., imaging. """ - facility_class = get_service_class(facility)(user=context['request'].user) + facility_instance = get_service_class(facility)() + facility_instance.set_user(context['request'].user) initial_fields = { 'target_id': target.id, 'facility': facility, 'observation_type': observation_type } - obs_form = facility_class.get_form(observation_type)(initial=initial_fields) + obs_form = facility_instance.get_form(observation_type)(initial=initial_fields) obs_form.helper.form_action = reverse('tom_observations:create', kwargs={'facility': facility}) return {'obs_form': obs_form} @@ -270,7 +271,8 @@ def facility_status(context): facility_statuses = [] for facility_class in get_service_classes().values(): - facility = facility_class(user=context['request'].user) + facility = facility_class() + facility.set_user(context['request'].user) weather_urls = facility.get_facility_weather_urls() status = facility.get_facility_status() @@ -290,7 +292,8 @@ def facility_status(context): def facility_map(context): facility_locations = [] for facility_class in get_service_classes().values(): - facility = facility_class(user=context['request'].user) + facility = facility_class() + facility.set_user(context['request'].user) sites = facility.get_observing_sites() # Flatten each facility site dictionary and add text label for use in facility map diff --git a/tom_observations/tests/tests.py b/tom_observations/tests/tests.py index 96249a88a..0f7907764 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.utils.functional import SimpleLazyObject from django.forms import ValidationError from django.test import TestCase, override_settings from django.urls import reverse @@ -144,7 +145,7 @@ def test_submit_observation_robotic(self): self.assertTrue(ObservationRecord.objects.filter(observation_id='fakeid').exists()) self.assertEqual(ObservationRecord.objects.filter(observation_id='fakeid').first().user, self.user) - @mock.patch('tom_observations.tests.utils.FakeRoboticFacility.test_user') + @mock.patch('tom_observations.tests.utils.FakeRoboticFacility.set_user') def test_submit_observation_robotic_gets_user(self, mock_method): form_data = { 'target_id': self.target.id, diff --git a/tom_observations/tests/utils.py b/tom_observations/tests/utils.py index 86bed3d84..25c7a5289 100644 --- a/tom_observations/tests/utils.py +++ b/tom_observations/tests/utils.py @@ -40,13 +40,6 @@ class FakeRoboticFacility(BaseRoboticObservationFacility): 'OBSERVATION': FakeFacilityForm } - def __init__(self, user=None): - self.test_user(user) - super().__init__(user) - - def test_user(self, user): - return user - def get_form(self, observation_type): return self.observation_forms[observation_type] diff --git a/tom_observations/views.py b/tom_observations/views.py index c7a1c4d2a..6ea0dcf26 100644 --- a/tom_observations/views.py +++ b/tom_observations/views.py @@ -222,8 +222,9 @@ def get_context_data(self, **kwargs): context['target'] = target # allow the Facility class to add data to the context - facility_class = self.get_facility_class() - facility_context = facility_class(user=self.request.user).get_facility_context_data(target=target) + facility = self.get_facility_class()() + facility.set_user(self.request.user) + facility_context = facility.get_facility_context_data(target=target) context.update(facility_context) return context @@ -240,7 +241,8 @@ def get_form_class(self): observation_type = self.request.GET.get('observation_type') elif self.request.method == 'POST': observation_type = self.request.POST.get('observation_type') - facility = self.get_facility_class()(user=self.request.user) + facility = self.get_facility_class()() + facility.set_user(self.request.user) form_class = type(f'Composite{observation_type}Form', (facility.get_form(observation_type), self.get_cadence_strategy_form()), {}) @@ -310,9 +312,10 @@ def form_valid(self, form): :type form: subclass of GenericObservationForm """ # Submit the observation - facility = self.get_facility_class() + facility = self.get_facility_class()() + facility.set_user(self.request.user) target = self.get_target() - observation_ids = facility(user=self.request.user).submit_observation(form.observation_payload()) + observation_ids = facility.submit_observation(form.observation_payload()) records = [] for observation_id in observation_ids: @@ -377,7 +380,8 @@ class ObservationRecordCancelView(LoginRequiredMixin, View): def get(self, request, *args, **kwargs): obsr_id = self.kwargs.get('pk') obsr = ObservationRecord.objects.get(id=obsr_id) - facility = get_service_class(obsr.facility)(user=request.user) + facility = get_service_class(obsr.facility)() + facility.set_user(request.user) try: success = facility.cancel_observation(obsr.observation_id) if success: @@ -501,10 +505,11 @@ def get_context_data(self, *args, **kwargs): """ context = super().get_context_data(*args, **kwargs) context['form'] = AddProductToGroupForm() - service_class = get_service_class(self.object.facility)(user=self.request.user) - context['editable'] = isinstance(service_class, BaseManualObservationFacility) - context['data_products'] = service_class.all_data_products(self.object) - context['can_be_cancelled'] = self.object.status not in service_class.get_terminal_observing_states() + facility = get_service_class(self.object.facility)() + facility.set_user(self.request.user) + context['editable'] = isinstance(facility, BaseManualObservationFacility) + context['data_products'] = facility.all_data_products(self.object) + context['can_be_cancelled'] = self.object.status not in facility.get_terminal_observing_states() newest_image = None for data_product in context['data_products']['saved']: newest_image = data_product if (not newest_image or data_product.modified > newest_image.modified) and \ @@ -608,7 +613,9 @@ def get_form_class(self): raise ValueError('Must provide a facility name') # TODO: modify this to work with all LCO forms - return get_service_class(facility_name)(user=self.request.user).get_template_form(None) + facility = get_service_class(facility_name)() + facility.set_user(self.request.user) + return facility.get_template_form(None) def get_form(self, form_class=None): form = super().get_form() @@ -638,7 +645,9 @@ def get_object(self): def get_form_class(self): self.object = self.get_object() - return get_service_class(self.object.facility)(user=self.request.user).get_template_form(None) + facility = get_service_class(self.object.facility)() + facility.set_user(self.request.user) + return facility.get_template_form(None) def get_form(self, form_class=None): form = super().get_form() From bd0bdf00a9cda99b5aa0e97cd7e6152765a65508 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 2 Nov 2023 22:48:27 -0100 Subject: [PATCH 06/56] remove unused import --- tom_observations/tests/tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tom_observations/tests/tests.py b/tom_observations/tests/tests.py index 0f7907764..d305eedaf 100644 --- a/tom_observations/tests/tests.py +++ b/tom_observations/tests/tests.py @@ -4,7 +4,6 @@ from django.contrib.auth.models import User from django.contrib.messages import get_messages -from django.utils.functional import SimpleLazyObject from django.forms import ValidationError from django.test import TestCase, override_settings from django.urls import reverse From ae0e3a341772c73746720f24f7965a1804c7042e Mon Sep 17 00:00:00 2001 From: Joseph Chatelain Date: Fri, 3 Nov 2023 14:20:37 -0700 Subject: [PATCH 07/56] remove special behavior for single target creation --- tom_alerts/tests/tests.py | 2 +- tom_alerts/views.py | 11 ++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/tom_alerts/tests/tests.py b/tom_alerts/tests/tests.py index c4408cf4e..92b0b3a37 100644 --- a/tom_alerts/tests/tests.py +++ b/tom_alerts/tests/tests.py @@ -228,7 +228,7 @@ def test_create_target(self): response = self.client.post(reverse('tom_alerts:create-target'), data=post_data) self.assertEqual(Target.objects.count(), 1) self.assertEqual(Target.objects.first().name, 'Hoth') - self.assertRedirects(response, reverse('tom_targets:update', kwargs={'pk': Target.objects.first().id})) + self.assertRedirects(response, reverse('tom_targets:list')) @override_settings(CACHES={ 'default': { diff --git a/tom_alerts/views.py b/tom_alerts/views.py index 6a769bd79..ac13e0fea 100644 --- a/tom_alerts/views.py +++ b/tom_alerts/views.py @@ -300,16 +300,9 @@ def post(self, request, *args, **kwargs): except IntegrityError: messages.warning(request, f'Unable to save {target.name}, target with that name already exists.') errors.append(target.name) - if (len(alerts) == len(errors)): + if len(alerts) == len(errors): return redirect(reverse('tom_alerts:run', kwargs={'pk': query_id})) - elif (len(alerts) == 1): - return redirect(reverse( - 'tom_targets:update', kwargs={'pk': target.id}) - ) - else: - return redirect(reverse( - 'tom_targets:list') - ) + return redirect(reverse('tom_targets:list')) class SubmitAlertUpstreamView(LoginRequiredMixin, FormMixin, ProcessFormView, View): From 58978905fa025f90037a9bfee7ad0cd307388284 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 7 Nov 2023 21:16:36 +0000 Subject: [PATCH 08/56] Bump django from 4.2.6 to 4.2.7 Bumps [django](https://github.com/django/django) from 4.2.6 to 4.2.7. - [Commits](https://github.com/django/django/compare/4.2.6...4.2.7) --- updated-dependencies: - dependency-name: django dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- poetry.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index 1ee75766c..b8ef17361 100644 --- a/poetry.lock +++ b/poetry.lock @@ -713,13 +713,13 @@ test-randomorder = ["pytest-randomly"] [[package]] name = "django" -version = "4.2.6" +version = "4.2.7" description = "A high-level Python web framework that encourages rapid development and clean, pragmatic design." optional = false python-versions = ">=3.8" files = [ - {file = "Django-4.2.6-py3-none-any.whl", hash = "sha256:a64d2487cdb00ad7461434320ccc38e60af9c404773a2f95ab0093b4453a3215"}, - {file = "Django-4.2.6.tar.gz", hash = "sha256:08f41f468b63335aea0d904c5729e0250300f6a1907bf293a65499496cdbc68f"}, + {file = "Django-4.2.7-py3-none-any.whl", hash = "sha256:e1d37c51ad26186de355cbcec16613ebdabfa9689bbade9c538835205a8abbe9"}, + {file = "Django-4.2.7.tar.gz", hash = "sha256:8e0f1c2c2786b5c0e39fe1afce24c926040fad47c8ea8ad30aaf1188df29fc41"}, ] [package.dependencies] From ff86c3b79e52e2a7a167a440c8fd79e182168963 Mon Sep 17 00:00:00 2001 From: Edward Gomez Date: Thu, 9 Nov 2023 15:50:01 +0000 Subject: [PATCH 09/56] None square FITS images; no data handling --- tom_dataproducts/models.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tom_dataproducts/models.py b/tom_dataproducts/models.py index 1aa3da09f..1f16495ea 100644 --- a/tom_dataproducts/models.py +++ b/tom_dataproducts/models.py @@ -234,8 +234,9 @@ def get_preview(self, size=THUMBNAIL_DEFAULT_SIZE, redraw=False): """ if self.thumbnail: im = Image.open(self.thumbnail) - if im.size != THUMBNAIL_DEFAULT_SIZE: + if im.size != THUMBNAIL_DEFAULT_SIZE and im.size[0] not in THUMBNAIL_DEFAULT_SIZE: redraw = True + logger.critical("Redrawing thumbnail for {0} due to size mismatch".format(im.size)) if not self.thumbnail or redraw: width, height = THUMBNAIL_DEFAULT_SIZE @@ -262,6 +263,9 @@ def create_thumbnail(self, width=None, height=None): :returns: Thumbnail file if created, None otherwise :rtype: file """ + if not self.data: + logger.error(f'Unable to create thumbnail for {self}: No data file found.') + return if is_fits_image_file(self.data.file): tmpfile = tempfile.NamedTemporaryFile(suffix='.jpg') try: From 2f7043448b33f10b7844c39a7a49c7e7fdc704c3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 15 Nov 2023 13:20:29 +0000 Subject: [PATCH 10/56] Bump responses from 0.23.3 to 0.24.1 Bumps [responses](https://github.com/getsentry/responses) from 0.23.3 to 0.24.1. - [Release notes](https://github.com/getsentry/responses/releases) - [Changelog](https://github.com/getsentry/responses/blob/master/CHANGES) - [Commits](https://github.com/getsentry/responses/compare/0.23.3...0.24.1) --- updated-dependencies: - dependency-name: responses dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- poetry.lock | 24 ++++++------------------ pyproject.toml | 2 +- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/poetry.lock b/poetry.lock index 1ee75766c..1a6093188 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1921,23 +1921,22 @@ use-chardet-on-py3 = ["chardet (>=3.0.2,<6)"] [[package]] name = "responses" -version = "0.23.3" +version = "0.24.1" description = "A utility library for mocking out the `requests` Python library." optional = false -python-versions = ">=3.7" +python-versions = ">=3.8" files = [ - {file = "responses-0.23.3-py3-none-any.whl", hash = "sha256:e6fbcf5d82172fecc0aa1860fd91e58cbfd96cee5e96da5b63fa6eb3caa10dd3"}, - {file = "responses-0.23.3.tar.gz", hash = "sha256:205029e1cb334c21cb4ec64fc7599be48b859a0fd381a42443cdd600bfe8b16a"}, + {file = "responses-0.24.1-py3-none-any.whl", hash = "sha256:a2b43f4c08bfb9c9bd242568328c65a34b318741d3fab884ac843c5ceeb543f9"}, + {file = "responses-0.24.1.tar.gz", hash = "sha256:b127c6ca3f8df0eb9cc82fd93109a3007a86acb24871834c47b77765152ecf8c"}, ] [package.dependencies] pyyaml = "*" requests = ">=2.30.0,<3.0" -types-PyYAML = "*" urllib3 = ">=1.25.10,<3.0" [package.extras] -tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "tomli", "tomli-w", "types-requests"] +tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "tomli", "tomli-w", "types-PyYAML", "types-requests"] [[package]] name = "scipy" @@ -2253,17 +2252,6 @@ files = [ [package.extras] doc = ["reno", "sphinx", "tornado (>=4.5)"] -[[package]] -name = "types-pyyaml" -version = "6.0.12.12" -description = "Typing stubs for PyYAML" -optional = false -python-versions = "*" -files = [ - {file = "types-PyYAML-6.0.12.12.tar.gz", hash = "sha256:334373d392fde0fdf95af5c3f1661885fa10c52167b14593eb856289e1855062"}, - {file = "types_PyYAML-6.0.12.12-py3-none-any.whl", hash = "sha256:c05bc6c158facb0676674b7f11fe3960db4f389718e19e62bd2b84d6205cfd24"}, -] - [[package]] name = "typing-extensions" version = "4.8.0" @@ -2457,4 +2445,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = ">=3.8.1,<3.12" -content-hash = "21f64667b3ee5e0c82022c18a15930f7082045a50280255f84ba773a0453c440" +content-hash = "b3ac8d14c25dada10538d6b315a5624d760e2e8cdb4fcb37cb37f1a40252cabe" diff --git a/pyproject.toml b/pyproject.toml index dad6f8053..69e3913d6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,7 +78,7 @@ requests = "<3" specutils = "<2" [tool.poetry.group.test.dependencies] -responses = "~0.23" +responses = ">=0.23,<0.25" factory_boy = ">3.2.1,<3.4" [tool.poetry.group.docs.dependencies] From 3529d76267740b62ca0421e34b5ba6414ceeae7e Mon Sep 17 00:00:00 2001 From: Joseph Chatelain Date: Thu, 16 Nov 2023 17:06:58 -0800 Subject: [PATCH 11/56] add app buttons to target detail page --- .../tom_targets/partials/module_buttons.html | 3 +++ .../tom_targets/partials/target_buttons.html | 3 +++ tom_targets/templatetags/targets_extras.py | 23 +++++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 tom_targets/templates/tom_targets/partials/module_buttons.html diff --git a/tom_targets/templates/tom_targets/partials/module_buttons.html b/tom_targets/templates/tom_targets/partials/module_buttons.html new file mode 100644 index 000000000..42c72d682 --- /dev/null +++ b/tom_targets/templates/tom_targets/partials/module_buttons.html @@ -0,0 +1,3 @@ +{% for button in button_list %} + {{button.text}} +{% endfor %} diff --git a/tom_targets/templates/tom_targets/partials/target_buttons.html b/tom_targets/templates/tom_targets/partials/target_buttons.html index 321bca67d..557d8d9e2 100644 --- a/tom_targets/templates/tom_targets/partials/target_buttons.html +++ b/tom_targets/templates/tom_targets/partials/target_buttons.html @@ -1,5 +1,8 @@ +{% load targets_extras %} Update {% if sharing %} Share {% endif %} Delete + +{% get_buttons target %} \ No newline at end of file diff --git a/tom_targets/templatetags/targets_extras.py b/tom_targets/templatetags/targets_extras.py index 2fe5b6d0a..c40e5a091 100644 --- a/tom_targets/templatetags/targets_extras.py +++ b/tom_targets/templatetags/targets_extras.py @@ -318,3 +318,26 @@ def target_table(targets, all_checked=False): """ return {'targets': targets, 'all_checked': all_checked} + + +@register.inclusion_tag('tom_targets/partials/module_buttons.html') +def get_buttons(target): + """ + Returns a list of buttons from imported modules to be displayed on the target detail page. + In order to add a button to the target detail page, an app must contain an integration points attribute. + The Integration Points attribute must be a dictionary with a key of 'target_detail_button': + 'target_detail_button' = {'namespace': <>, + 'title': <