From b12812e202dcacabb1156aff277f43194a213b23 Mon Sep 17 00:00:00 2001 From: Elisabeth Heinrich-Josties Date: Sat, 7 Sep 2019 00:16:23 +0000 Subject: [PATCH 1/3] Enable multiple instrument configs --- observation_portal/requestgroups/serializers.py | 7 ------- observation_portal/requestgroups/test/test_api.py | 13 ------------- static/js/components/instrumentconfig.vue | 2 +- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/observation_portal/requestgroups/serializers.py b/observation_portal/requestgroups/serializers.py index d431b060..afb76164 100644 --- a/observation_portal/requestgroups/serializers.py +++ b/observation_portal/requestgroups/serializers.py @@ -250,13 +250,6 @@ def validate(self, data): default_modes = configdb.get_default_modes_by_type(instrument_type) guiding_config = data['guiding_config'] - # Validate the number of instrument configs if this is not a SOAR instrument - # TODO: remove this check once we support multiple instrument configs - if len(data['instrument_configs']) != 1 and 'SOAR' not in instrument_type.upper(): - raise serializers.ValidationError(_( - 'Currently only a single instrument_config is supported. This restriction will be lifted in the future.' - )) - # Validate the guide mode guide_validation_helper = ModeValidationHelper('guiding', instrument_type, default_modes, modes) if guide_validation_helper.mode_is_not_set(guiding_config): diff --git a/observation_portal/requestgroups/test/test_api.py b/observation_portal/requestgroups/test/test_api.py index 7a1164c0..28e372bd 100644 --- a/observation_portal/requestgroups/test/test_api.py +++ b/observation_portal/requestgroups/test/test_api.py @@ -1554,7 +1554,6 @@ def test_configurations_automatically_have_priority_set(self): for i, configuration in enumerate(rg['requests'][0]['configurations']): self.assertEqual(configuration['priority'], i + 1) - @skip("Requires multiple instrument configs") def test_fill_window_on_more_than_one_instrument_config_fails(self): bad_data = self.generic_payload.copy() bad_data['requests'][0]['configurations'][0]['instrument_configs'].append( @@ -1576,7 +1575,6 @@ def test_fill_window_one_configuration_fills_the_window(self): initial_exposure_count) self.assertEqual(response.status_code, 201) - @skip("Requires multiple instrument configs") def test_fill_window_two_instrument_configs_one_false_fills_the_window(self): good_data = self.generic_payload.copy() good_data['requests'][0]['configurations'][0]['instrument_configs'].append( @@ -1591,7 +1589,6 @@ def test_fill_window_two_instrument_configs_one_false_fills_the_window(self): initial_exposure_count) self.assertEqual(response.status_code, 201) - @skip("Requires multiple instrument configs") def test_fill_window_two_instrument_configs_one_blank_fills_the_window(self): good_data = self.generic_payload.copy() good_data['requests'][0]['configurations'][0]['instrument_configs'].append( @@ -1604,7 +1601,6 @@ def test_fill_window_two_instrument_configs_one_blank_fills_the_window(self): self.assertGreater(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], initial_exposure_count) self.assertEqual(response.status_code, 201) - @skip("Requires multiple instrument configs") def test_fill_window_two_instrument_configs_first_fills_the_window(self): good_data = self.generic_payload.copy() good_data['requests'][0]['configurations'][0]['instrument_configs'].append( @@ -1761,15 +1757,6 @@ def test_empty_roi_rejected(self): self.assertEqual(response.status_code, 400) self.assertIn('Must submit at least one bound for a region of interest', str(response.content)) - def test_multiple_instrument_configs_currently_rejected(self): - bad_data = self.generic_payload.copy() - bad_data['requests'][0]['configurations'][0]['instrument_configs'].append( - bad_data['requests'][0]['configurations'][0]['instrument_configs'][0].copy() - ) - response = self.client.post(reverse('api:request_groups-list'), data=bad_data) - self.assertEqual(response.status_code, 400) - self.assertIn('Currently only a single instrument_config', str(response.content)) - def test_multiple_same_targets_and_constraints_accepted(self): good_data = self.generic_payload.copy() good_data['requests'][0]['configurations'].append( diff --git a/static/js/components/instrumentconfig.vue b/static/js/components/instrumentconfig.vue index 628e6668..7b9f4c87 100644 --- a/static/js/components/instrumentconfig.vue +++ b/static/js/components/instrumentconfig.vue @@ -6,7 +6,7 @@ :index="index" :errors="errors" :canremove="this.index > 0" - :cancopy="false" + :cancopy="true" @remove="$emit('remove')" @copy="$emit('copy')" @show="show = $event" From bc404fe904467ecbd5bcd257cb3826bd8f09e80a Mon Sep 17 00:00:00 2001 From: Elisabeth Heinrich-Josties Date: Mon, 4 Nov 2019 21:03:20 +0000 Subject: [PATCH 2/3] Add some restrictions for submitting multiple instrument configs --- .../common/test_data/configdb.json | 15 +++-- .../requestgroups/serializers.py | 6 +- .../requestgroups/test/test_api.py | 59 +++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/observation_portal/common/test_data/configdb.json b/observation_portal/common/test_data/configdb.json index 09ff3eaa..1b697f47 100644 --- a/observation_portal/common/test_data/configdb.json +++ b/observation_portal/common/test_data/configdb.json @@ -48,7 +48,8 @@ "EXPOSE", "BIAS", "DARK", - "SCRIPT" + "SCRIPT", + "SKY_FLAT" ], "default_acceptability_threshold": 90, "pixels_x": 1000, @@ -172,7 +173,8 @@ "EXPOSE", "BIAS", "DARK", - "SCRIPT" + "SCRIPT", + "SKY_FLAT" ], "default_acceptability_threshold": 90, "pixels_x": 1000, @@ -296,7 +298,8 @@ "EXPOSE", "BIAS", "DARK", - "SCRIPT" + "SCRIPT", + "SKY_FLAT" ], "default_acceptability_threshold": 90, "pixels_x": 1000, @@ -598,7 +601,8 @@ "EXPOSE", "BIAS", "DARK", - "SCRIPT" + "SCRIPT", + "SKY_FLAT" ], "default_acceptability_threshold": 100, "pixels_x": 1000, @@ -1008,7 +1012,8 @@ "EXPOSE", "BIAS", "DARK", - "SCRIPT" + "SCRIPT", + "SKY_FLAT" ], "default_acceptability_threshold": 100, "pixels_x": 1000, diff --git a/observation_portal/requestgroups/serializers.py b/observation_portal/requestgroups/serializers.py index 41636ced..006fe462 100644 --- a/observation_portal/requestgroups/serializers.py +++ b/observation_portal/requestgroups/serializers.py @@ -228,6 +228,8 @@ class Meta: def validate_instrument_configs(self, value): if [instrument_config.get('fill_window', False) for instrument_config in value].count(True) > 1: raise serializers.ValidationError(_('Only one instrument_config can have `fill_window` set')) + if len(set([instrument_config.get('rotator_mode', '') for instrument_config in value])) > 1: + raise serializers.ValidationError(_('Rotator modes within the same configuration must be the same')) return value def validate_instrument_type(self, value): @@ -250,6 +252,9 @@ def validate(self, data): default_modes = configdb.get_default_modes_by_type(instrument_type) guiding_config = data['guiding_config'] + if len(data['instrument_configs']) > 1 and data['type'] in ['SCRIPT', 'SKY_FLAT']: + raise serializers.ValidationError(_(f'Multiple instrument configs are not allowed for type {data["type"]}')) + # Validate the guide mode guide_validation_helper = ModeValidationHelper('guiding', instrument_type, default_modes, modes) if guide_validation_helper.mode_is_not_set(guiding_config): @@ -365,7 +370,6 @@ def validate(self, data): else: instrument_config['optical_elements'][oe_type] = available_elements[value.lower()] - # Also check that any optical element group in configdb is specified in the request unless we are a BIAS or # DARK or SCRIPT type observation observation_types_without_oe = ['BIAS', 'DARK', 'SCRIPT'] diff --git a/observation_portal/requestgroups/test/test_api.py b/observation_portal/requestgroups/test/test_api.py index f8c1bf1d..091f6a37 100644 --- a/observation_portal/requestgroups/test/test_api.py +++ b/observation_portal/requestgroups/test/test_api.py @@ -1685,6 +1685,65 @@ def test_fill_window_when_exposures_fit_exactly(self): n_exposures_fit_exactly - 1 ) + def test_multiple_instrument_configs_with_different_rotator_modes_fails(self): + bad_data = self.generic_payload.copy() + bad_data['requests'][0]['configurations'][0]['instrument_configs'].append( + self.extra_configuration['instrument_configs'][0].copy() + ) + bad_data['requests'][0]['configurations'][0]['instrument_type'] = '2M0-FLOYDS-SCICAM' + bad_data['requests'][0]['location']['telescope_class'] = '2m0' + bad_data['requests'][0]['configurations'][0]['type'] = 'SPECTRUM' + del bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['optical_elements']['filter'] + del bad_data['requests'][0]['configurations'][0]['instrument_configs'][1]['optical_elements']['filter'] + bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['optical_elements']['slit'] = 'slit_1.6as' + bad_data['requests'][0]['configurations'][0]['instrument_configs'][1]['optical_elements']['slit'] = 'slit_1.6as' + bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['rotator_mode'] = 'SKY' + bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['extra_params'] = {} + bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['extra_params']['rotator_angle'] = '1' + bad_data['requests'][0]['configurations'][0]['instrument_configs'][1]['rotator_mode'] = 'VFLOAT' + response = self.client.post(reverse('api:request_groups-list'), data=bad_data) + self.assertEqual(response.status_code, 400) + self.assertIn('within the same configuration must be the same', str(response.content)) + + def test_multiple_instrument_configs_with_same_rotator_modes_succeeds(self): + good_data = self.generic_payload.copy() + good_data['requests'][0]['configurations'][0]['instrument_configs'].append( + self.extra_configuration['instrument_configs'][0].copy() + ) + good_data['requests'][0]['configurations'][0]['instrument_type'] = '2M0-FLOYDS-SCICAM' + good_data['requests'][0]['location']['telescope_class'] = '2m0' + good_data['requests'][0]['configurations'][0]['type'] = 'SPECTRUM' + del good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['optical_elements']['filter'] + del good_data['requests'][0]['configurations'][0]['instrument_configs'][1]['optical_elements']['filter'] + good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['optical_elements']['slit'] = 'slit_1.6as' + good_data['requests'][0]['configurations'][0]['instrument_configs'][1]['optical_elements']['slit'] = 'slit_1.6as' + good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['rotator_mode'] = 'VFLOAT' + good_data['requests'][0]['configurations'][0]['instrument_configs'][1]['rotator_mode'] = 'VFLOAT' + response = self.client.post(reverse('api:request_groups-list'), data=good_data) + self.assertEqual(response.status_code, 201) + + def test_script_type_not_allowed_for_multiple_instrument_configs(self): + bad_data = self.generic_payload.copy() + bad_data['requests'][0]['configurations'][0]['instrument_configs'].append( + self.extra_configuration['instrument_configs'][0].copy() + ) + bad_data['requests'][0]['configurations'][0]['type'] = 'SCRIPT' + bad_data['requests'][0]['configurations'][0]['extra_params'] = {} + bad_data['requests'][0]['configurations'][0]['extra_params']['script_name'] = 'my_cool_thing' + response = self.client.post(reverse('api:request_groups-list'), data=bad_data) + self.assertEqual(response.status_code, 400) + self.assertIn('are not allowed for type SCRIPT', str(response.content)) + + def test_skyflat_not_allowed_for_multiple_instrument_configs(self): + bad_data = self.generic_payload.copy() + bad_data['requests'][0]['configurations'][0]['instrument_configs'].append( + self.extra_configuration['instrument_configs'][0].copy() + ) + bad_data['requests'][0]['configurations'][0]['type'] = 'SKY_FLAT' + response = self.client.post(reverse('api:request_groups-list'), data=bad_data) + self.assertEqual(response.status_code, 400) + self.assertIn('are not allowed for type SKY_FLAT', str(response.content)) + def test_configuration_type_matches_instrument(self): bad_data = self.generic_payload.copy() bad_data['requests'][0]['configurations'][0]['type'] = 'SPECTRUM' From f95908ab528ed5f376640774ba7654ced62770fd Mon Sep 17 00:00:00 2001 From: Elisabeth Heinrich-Josties Date: Mon, 4 Nov 2019 21:10:38 +0000 Subject: [PATCH 3/3] Display failed addition top level validation errors --- static/js/components/instrumentconfig.vue | 12 ++++++++++-- static/js/components/window.vue | 9 +++++++-- static/js/utils.js | 17 ++++++++++++++++- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/static/js/components/instrumentconfig.vue b/static/js/components/instrumentconfig.vue index 7b9f4c87..de9d9cc0 100644 --- a/static/js/components/instrumentconfig.vue +++ b/static/js/components/instrumentconfig.vue @@ -12,7 +12,7 @@ @show="show = $event" > import _ from 'lodash'; -import { collapseMixin, lampFlatDefaultExposureTime, arcDefaultExposureTime } from '../utils.js'; +import { + collapseMixin, + lampFlatDefaultExposureTime, + arcDefaultExposureTime, + extractTopLevelErrors +} from '../utils.js'; import customfield from './util/customfield.vue'; import customselect from './util/customselect.vue'; import panel from './util/panel.vue'; @@ -185,6 +190,9 @@ export default { } }, computed: { + topLevelErrors: function() { + return extractTopLevelErrors(this.errors); + }, instrumentHasRotatorModes: function() { return this.available_instruments[this.selectedinstrument] && 'rotator' in this.available_instruments[this.selectedinstrument].modes; }, diff --git a/static/js/components/window.vue b/static/js/components/window.vue index af53e059..ccd6bb89 100644 --- a/static/js/components/window.vue +++ b/static/js/components/window.vue @@ -12,7 +12,7 @@ @show="show = $event" >