diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 544b76188..3c56d320f 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -463,6 +463,13 @@ def get_fields(self, request, obj): fields = super().get_fields(request, obj) return self._error_reason_field_conditional(obj, fields) + def formfield_for_manytomany(self, db_field, request, **kwargs): + # setting queryset none for all requests except POST as queryset + # is required for the form to be valid + if db_field.name == "templates" and request.method != "POST": + kwargs["queryset"] = Template.objects.none() + return super().formfield_for_manytomany(db_field, request, **kwargs) + class ChangeDeviceGroupForm(forms.Form): device_group = forms.ModelChoiceField( @@ -1319,6 +1326,13 @@ def get_extra_context(self, pk=None): } return ctx + def formfield_for_manytomany(self, db_field, request, **kwargs): + # setting queryset none for all requests except POST as queryset + # is required for the form to be valid + if db_field.name == "templates" and request.method != "POST": + kwargs["queryset"] = Template.objects.none() + return super().formfield_for_manytomany(db_field, request, **kwargs) + admin.site.register(Device, DeviceAdminExportable) admin.site.register(Template, TemplateAdmin) diff --git a/openwisp_controller/config/static/config/js/relevant_templates.js b/openwisp_controller/config/static/config/js/relevant_templates.js index 9684d409c..e8aeef746 100644 --- a/openwisp_controller/config/static/config/js/relevant_templates.js +++ b/openwisp_controller/config/static/config/js/relevant_templates.js @@ -1,14 +1,19 @@ "use strict"; django.jQuery(function ($) { - var firstRun = true, + var pageLoading = true, backendFieldSelector = "#id_config-0-backend", orgFieldSelector = "#id_organization", isDeviceGroup = function () { - return window._deviceGroup; + return window._deviceGroupId !== undefined; }, templatesFieldName = function () { return isDeviceGroup() ? "templates" : "config-0-templates"; }, + isAddingNewObject = function () { + return isDeviceGroup() + ? !$(".add-form").length + : $('input[name="config-0-id"]').val().length === 0; + }, getTemplateOptionElement = function ( index, templateId, @@ -33,7 +38,8 @@ django.jQuery(function ($) { if (templateConfig.required) { inputField.prop("disabled", true); } - if (isSelected || templateConfig.required) { + // mark the template as selected if it is required or if it is enabled for the current device or group + if (isSelected || templateConfig.required || templateConfig.selected) { inputField.prop("checked", true); } return element; @@ -41,15 +47,6 @@ django.jQuery(function ($) { resetTemplateOptions = function () { $("ul.sortedm2m-items").empty(); }, - updateTemplateSelection = function (selectedTemplates) { - // Marks currently applied templates from database as selected - // Only executed at page load. - selectedTemplates.forEach(function (templateId) { - $( - `li.sortedm2m-item input[type="checkbox"][value="${templateId}"]:first`, - ).prop("checked", true); - }); - }, updateTemplateHelpText = function () { var helpText = "Choose items and order by drag & drop."; if ($("li.sortedm2m-item:first").length === 0) { @@ -73,11 +70,32 @@ django.jQuery(function ($) { showRelevantTemplates(); }, updateConfigTemplateField = function (templates) { - $(`input[name="${templatesFieldName()}"]`).attr( - "value", - templates.join(","), - ); - $("input.sortedm2m:first").trigger("change"); + var value = templates.join(","), + templateField = templatesFieldName(), + updateInitialValue = false; + $(`input[name="${templateField}"]`).attr("value", value); + if ( + pageLoading || + // Handle cases where the AJAX request finishes after initial page load. + // If we're editing an existing object and the initial value hasn't been set, + // assign it now to avoid false positives in the unsaved changes warning. + (!isAddingNewObject() && + django._owcInitialValues[templateField] === undefined) + ) { + django._owcInitialValues[templateField] = value; + updateInitialValue = true; + } + $("input.sortedm2m:first").trigger("change", { + updateInitialValue: updateInitialValue, + }); + }, + getSelectedTemplates = function () { + // Returns the selected templates from the sortedm2m input + var selectedTemplates = {}; + $("input.sortedm2m:checked").each(function (index, element) { + selectedTemplates[$(element).val()] = $(element).prop("checked"); + }); + return selectedTemplates; }, parseSelectedTemplates = function (selectedTemplates) { if (selectedTemplates !== undefined) { @@ -88,85 +106,57 @@ django.jQuery(function ($) { } } }, + getRelevantTemplateUrl = function (orgID, backend) { + // Returns the URL to fetch relevant templates + var baseUrl = window._relevantTemplateUrl.replace("org_id", orgID); + var url = new URL(baseUrl, window.location.origin); + + // Get relevant templates of selected org and backend + if (backend) { + url.searchParams.set("backend", backend); + } + if (isDeviceGroup() && !$(".add-form").length) { + url.searchParams.set("group_id", window._deviceGroupId); + } else if ($('input[name="config-0-id"]').length) { + url.searchParams.set("config_id", $('input[name="config-0-id"]').val()); + } + return url.toString(); + }, showRelevantTemplates = function () { var orgID = $(orgFieldSelector).val(), backend = isDeviceGroup() ? "" : $(backendFieldSelector).val(), - selectedTemplates; + currentSelection = getSelectedTemplates(); // Hide templates if no organization or backend is selected - if (orgID.length === 0 || (!isDeviceGroup() && backend.length === 0)) { + if (!orgID || (!isDeviceGroup() && backend.length === 0)) { resetTemplateOptions(); updateTemplateHelpText(); return; } - if (firstRun) { - // selectedTemplates will be undefined on device add page or - // when the user has changed any of organization or backend field. - // selectedTemplates will be an empty string if no template is selected - // ''.split(',') returns [''] hence, this case requires special handling - selectedTemplates = isDeviceGroup() - ? parseSelectedTemplates($("#id_templates").val()) - : parseSelectedTemplates( - django._owcInitialValues[templatesFieldName()], - ); - } - - var url = window._relevantTemplateUrl.replace("org_id", orgID); - // Get relevant templates of selected org and backend - url = url + "?backend=" + backend; + var url = getRelevantTemplateUrl(orgID, backend); $.get(url).done(function (data) { resetTemplateOptions(); var enabledTemplates = [], sortedm2mUl = $("ul.sortedm2m-items:first"), sortedm2mPrefixUl = $("ul.sortedm2m-items:last"); - // Adds "li" elements for templates that are already selected - // in the database. Select these templates and remove their key from "data" - // This maintains the order of the templates and keep - // enabled templates on the top - if (selectedTemplates !== undefined) { - selectedTemplates.forEach(function (templateId, index) { - // corner case in which backend of template does not match - if (!data[templateId]) { - return; - } - var element = getTemplateOptionElement( - index, - templateId, - data[templateId], - true, - false, - ), - prefixElement = getTemplateOptionElement( - index, - templateId, - data[templateId], - true, - true, - ); - sortedm2mUl.append(element); - if (!isDeviceGroup()) { - sortedm2mPrefixUl.append(prefixElement); - } - delete data[templateId]; - }); - } - - // Adds "li" elements for templates that are not selected - // in the database. - var counter = - selectedTemplates !== undefined ? selectedTemplates.length : 0; + // Adds "li" elements for templates Object.keys(data).forEach(function (templateId, index) { - // corner case in which backend of template does not match - if (!data[templateId]) { - return; - } - index = index + counter; var isSelected = - data[templateId].default && - selectedTemplates === undefined && - !data[templateId].required, + // Template is selected in the database + data[templateId].selected || + // Shared template which was already selected + (currentSelection[templateId] !== undefined && + currentSelection[templateId]) || + // Default template should be selected when: + // 1. A new object is created. + // 2. Organization or backend field has changed. + // (when the fields are changed, the currentSelection will be non-empty) + (data[templateId].default && + (pageLoading || + isAddingNewObject() || + Object.keys(currentSelection).length > 0)), element = getTemplateOptionElement( index, templateId, @@ -180,9 +170,6 @@ django.jQuery(function ($) { isSelected, true, ); - // Default templates should only be enabled for new - // device or when user has changed any of organization - // or backend field if (isSelected === true) { enabledTemplates.push(templateId); } @@ -191,14 +178,20 @@ django.jQuery(function ($) { sortedm2mPrefixUl.append(prefixElement); } }); - if (firstRun === true && selectedTemplates !== undefined) { - updateTemplateSelection(selectedTemplates); - } updateTemplateHelpText(); updateConfigTemplateField(enabledTemplates); }); }, + initTemplateField = function () { + // sortedm2m generates a hidden input dynamically using rendered input checkbox elements, + // but because the queryset is set to None in the Django admin, the input is created + // without a name attribute. This workaround assigns the correct name to the hidden input. + $('.sortedm2m-container input[type="hidden"][id="undefined"]') + .first() + .attr("name", templatesFieldName()); + }, bindDefaultTemplateLoading = function () { + initTemplateField(); var backendField = $(backendFieldSelector); $(orgFieldSelector).change(function () { // Only fetch templates when backend field is present @@ -211,16 +204,18 @@ django.jQuery(function ($) { addChangeEventHandlerToBackendField(); } else if (isDeviceGroup()) { // Initially request data to get templates + initTemplateField(); showRelevantTemplates(); } else { // Add view: backendField is added when user adds configuration $("#config-group > fieldset.module").ready(function () { $("div.add-row > a").one("click", function () { + initTemplateField(); addChangeEventHandlerToBackendField(); }); }); } - firstRun = false; + pageLoading = false; $("#content-main form").submit(function () { $( 'ul.sortedm2m-items:first input[type="checkbox"][data-required="true"]', diff --git a/openwisp_controller/config/static/config/js/widget.js b/openwisp_controller/config/static/config/js/widget.js index 9bcbe457a..4a689a168 100644 --- a/openwisp_controller/config/static/config/js/widget.js +++ b/openwisp_controller/config/static/config/js/widget.js @@ -494,8 +494,8 @@ getDefaultValues(true); }); } - $(".sortedm2m-items").on("change", function () { - getDefaultValues(); + $(".sortedm2m-items").on("change", function (event, data) { + getDefaultValues(data && data.updateInitialValue === true); }); $(".sortedm2m-items").on("sortstop", function () { getDefaultValues(); diff --git a/openwisp_controller/config/templates/admin/device_group/change_form.html b/openwisp_controller/config/templates/admin/device_group/change_form.html index 71df46ead..ae51524a1 100644 --- a/openwisp_controller/config/templates/admin/device_group/change_form.html +++ b/openwisp_controller/config/templates/admin/device_group/change_form.html @@ -12,7 +12,7 @@ (function ($) { $(document).ready( function () { window._relevantTemplateUrl = "{{ relevant_template_url | safe }}"; - window._deviceGroup = true; + window._deviceGroupId = "{{ original.pk }}"; window.bindDefaultTemplateLoading(); }) }) (django.jQuery); diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index 9ef195cdc..49152293c 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -4,6 +4,7 @@ from unittest.mock import patch from uuid import uuid4 +import django from django.contrib.admin.models import LogEntry from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError @@ -479,15 +480,6 @@ def test_device_organization_fk_autocomplete_view(self): hidden=[data["org2"].name, data["inactive"].name], ) - def test_device_templates_m2m_queryset(self): - data = self._create_multitenancy_test_env() - t_shared = self._create_template(name="t-shared", organization=None) - self._test_multitenant_admin( - url=reverse(f"admin:{self.app_label}_device_add"), - visible=[str(data["t1"]), str(t_shared)], - hidden=[str(data["t2"]), str(data["t3_inactive"])], - ) - def test_template_queryset(self): data = self._create_multitenancy_test_env() self._test_multitenant_admin( @@ -796,56 +788,6 @@ def test_template_not_contains_default_templates_js(self): response = self.client.get(path) self.assertNotContains(response, "// enable default templates") - def test_configuration_templates_removed(self): - def _update_template(templates): - params.update( - { - "config-0-templates": ",".join( - [str(template.pk) for template in templates] - ) - } - ) - response = self.client.post(path, data=params, follow=True) - self.assertEqual(response.status_code, 200) - for template in templates: - self.assertContains( - response, f'class="sortedm2m" checked> {template.name}' - ) - return response - - template = self._create_template() - - # Add a new device - path = reverse(f"admin:{self.app_label}_device_add") - params = self._get_device_params(org=self._get_org()) - response = self.client.post(path, data=params, follow=True) - self.assertEqual(response.status_code, 200) - - config = Device.objects.get(name=params["name"]).config - path = reverse(f"admin:{self.app_label}_device_change", args=[config.device_id]) - params.update( - { - "config-0-id": str(config.pk), - "config-0-device": str(config.device_id), - "config-INITIAL_FORMS": 1, - "_continue": True, - } - ) - - # Add template to the device - _update_template(templates=[template]) - config.refresh_from_db() - self.assertEqual(config.templates.count(), 1) - self.assertEqual(config.status, "modified") - config.set_status_applied() - self.assertEqual(config.status, "applied") - - # Remove template from the device - _update_template(templates=[]) - config.refresh_from_db() - self.assertEqual(config.templates.count(), 0) - self.assertEqual(config.status, "modified") - def test_vpn_not_contains_default_templates_js(self): vpn = self._create_vpn() path = reverse(f"admin:{self.app_label}_vpn_change", args=[vpn.pk]) @@ -1637,10 +1579,6 @@ def _update_template(templates): ) response = self.client.post(path, data=params, follow=True) self.assertEqual(response.status_code, 200) - for template in templates: - self.assertContains( - response, f'class="sortedm2m" checked> {template.name}' - ) return response vpn = self._create_vpn() @@ -2160,6 +2098,41 @@ def test_vpn_template_switch(self): ) self.assertEqual(config.vpnclient_set.count(), 1) + # helper for asserting queries executed during template fetch for a device + def _verify_template_queries(self, config, count): + path = reverse(f"admin:{self.app_label}_device_change", args=[config.device.pk]) + for i in range(count): + self._create_template(name=f"template-{i}") + expected_count = 24 + if django.VERSION < (5, 2): + # In django version < 5.2, there is an extra SAVEPOINT query + # leading to extra RELEASE SAVEPOINT query, thus 2 extra queries + expected_count += 2 + with self.assertNumQueries(expected_count): + # contains 22 queries for fetching normal device data + response = self.client.get(path) + # contains 2 queries, 1 for fetching organization + # and 1 for fetching templates + response = self.client.get( + reverse( + "admin:get_relevant_templates", args=[config.device.organization.pk] + ) + ) + self.assertEqual(response.status_code, 200) + + # ensuring queries are consistent for different number of templates + def test_templates_fetch_queries_1(self): + config = self._create_config(organization=self._get_org()) + self._verify_template_queries(config, 1) + + def test_templates_fetch_queries_5(self): + config = self._create_config(organization=self._get_org()) + self._verify_template_queries(config, 5) + + def test_templates_fetch_queries_10(self): + config = self._create_config(organization=self._get_org()) + self._verify_template_queries(config, 10) + class TestTransactionAdmin( CreateConfigTemplateMixin, diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index f6d489472..ae9061d39 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -5,26 +5,80 @@ from django.urls.base import reverse from selenium.common.exceptions import TimeoutException from selenium.webdriver.common.action_chains import ActionChains -from selenium.webdriver.common.alert import Alert from selenium.webdriver.common.by import By from selenium.webdriver.common.keys import Keys from selenium.webdriver.support import expected_conditions as EC from selenium.webdriver.support.ui import Select, WebDriverWait from swapper import load_model -from openwisp_utils.tests import SeleniumTestMixin +from openwisp_utils.tests import SeleniumTestMixin as BaseSeleniumTestMixin -from .utils import CreateConfigTemplateMixin, TestWireguardVpnMixin +from .utils import CreateConfigTemplateMixin, TestVpnX509Mixin, TestWireguardVpnMixin Device = load_model("config", "Device") +DeviceGroup = load_model("config", "DeviceGroup") +Cert = load_model("django_x509", "Cert") + + +class SeleniumTestMixin(BaseSeleniumTestMixin): + def _select_organization(self, org): + self.find_element( + by=By.CSS_SELECTOR, value="#select2-id_organization-container" + ).click() + self.wait_for_invisibility( + By.CSS_SELECTOR, ".select2-results__option.loading-results" + ) + self.find_element(by=By.CLASS_NAME, value="select2-search__field").send_keys( + org.name + ) + self.wait_for_invisibility( + By.CSS_SELECTOR, ".select2-results__option.loading-results" + ) + self.find_element(by=By.CLASS_NAME, value="select2-results__option").click() + + def _verify_templates_visibility(self, hidden=None, visible=None): + hidden = hidden or [] + visible = visible or [] + for template in hidden: + self.wait_for_invisibility(By.XPATH, f'//*[@value="{template.id}"]') + for template in visible: + self.wait_for_visibility(By.XPATH, f'//*[@value="{template.id}"]') @tag("selenium_tests") class TestDeviceAdmin( SeleniumTestMixin, CreateConfigTemplateMixin, + TestVpnX509Mixin, StaticLiveServerTestCase, ): + # helper function for adding/removing templates + def _update_template(self, device_id, templates, is_enabled=False): + self.open( + reverse("admin:config_device_change", args=[device_id]) + "#config-group" + ) + self.wait_for_presence(By.CSS_SELECTOR, 'input[name="config-0-templates"]') + + # if not is_enabled: + self.hide_loading_overlay() + for template in templates: + template_element = self.find_element( + By.XPATH, f'//*[@value="{template.id}"][@type="checkbox"]' + ) + # if enabled by default, assert that the checkbox is selected and enabled + if is_enabled: + self.assertEqual(template_element.is_enabled(), True) + self.assertEqual(template_element.is_selected(), True) + # enable/disable the checkbox + template_element.click() + + # Hide user tools because it covers the save button + self.web_driver.execute_script( + 'document.querySelector("#ow-user-tools").style.display="none"' + ) + self.find_element(by=By.NAME, value="_save").click() + self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5) + def test_create_new_device(self): required_template = self._create_template(name="Required", required=True) default_template = self._create_template(name="Default", default=True) @@ -86,7 +140,7 @@ def test_create_new_device(self): 'document.querySelector("#ow-user-tools").style.display="none"' ) self.find_element(by=By.NAME, value="_save").click() - self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success") + self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5) self.assertEqual( self.find_elements(by=By.CLASS_NAME, value="success")[0].text, "The Device “11:22:33:44:55:66” was added successfully.", @@ -113,9 +167,7 @@ def test_device_preview_keyboard_shortcuts(self): self.wait_for_invisibility(By.CSS_SELECTOR, ".djnjc-overlay:not(.loading)") def test_multiple_organization_templates(self): - shared_required_template = self._create_template( - name="shared required", organization=None - ) + shared_template = self._create_template(name="shared", organization=None) org1 = self._create_org(name="org1", slug="org1") org1_required_template = self._create_template( @@ -133,30 +185,69 @@ def test_multiple_organization_templates(self): name="org2 default", organization=org2, default=True ) - org1_device = self._create_config( + device = self._create_config( device=self._create_device(organization=org1) ).device self.login() self.open( - reverse("admin:config_device_change", args=[org1_device.id]) - + "#config-group" + reverse("admin:config_device_change", args=[device.id]) + "#config-group" ) self.hide_loading_overlay() - # org2 templates should not be visible - self.wait_for_invisibility( - By.XPATH, f'//*[@value="{org2_required_template.id}"]' - ) - self.wait_for_invisibility( - By.XPATH, f'//*[@value="{org2_default_template.id}"]' - ) - # org1 and shared templates should be visible - self.wait_for_visibility(By.XPATH, f'//*[@value="{org1_required_template.id}"]') - self.wait_for_visibility(By.XPATH, f'//*[@value="{org1_default_template.id}"]') - self.wait_for_visibility( - By.XPATH, f'//*[@value="{shared_required_template.id}"]' - ) + with self.subTest("only org1 and shared templates should be visible"): + self._verify_templates_visibility( + hidden=[org2_required_template, org2_default_template], + visible=[ + org1_required_template, + org1_default_template, + shared_template, + ], + ) + + # Select shared template + self.find_element( + by=By.XPATH, value=f'//*[@value="{shared_template.id}"]' + ).click() + + with self.subTest("changing org should update templates"): + self.find_element( + By.CSS_SELECTOR, value='a[href="#overview-group"]' + ).click() + self._select_organization(org2) + self.find_element( + by=By.CSS_SELECTOR, value='a[href="#config-group"]' + ).click() + self._verify_templates_visibility( + hidden=[ + org1_required_template, + org1_default_template, + ], + visible=[ + org2_required_template, + org2_default_template, + shared_template, + ], + ) + # Verify that shared template is selected + self.assertEqual( + self.find_element( + by=By.CSS_SELECTOR, + value=f'input[type="checkbox"][value="{shared_template.id}"]', + ).is_selected(), + True, + ) + self.find_element( + by=By.CSS_SELECTOR, value='input[name="_continue"]' + ).click() + self._wait_until_page_ready() + device.refresh_from_db() + device.config.refresh_from_db() + self.assertEqual(device.organization, org2) + self.assertEqual(device.config.templates.count(), 3) + self.assertIn(org2_required_template, device.config.templates.all()) + self.assertIn(org2_default_template, device.config.templates.all()) + self.assertIn(shared_template, device.config.templates.all()) def test_change_config_backend(self): device = self._create_config(organization=self._get_org()).device @@ -175,43 +266,6 @@ def test_change_config_backend(self): config_backend_select.select_by_visible_text("OpenWISP Firmware 1.x") self.wait_for_invisibility(By.XPATH, f'//*[@value="{template.id}"]') - def test_template_context_variables(self): - self._create_template( - name="Template1", default_values={"vni": "1"}, required=True - ) - self._create_template( - name="Template2", default_values={"vni": "2"}, required=True - ) - device = self._create_config(organization=self._get_org()).device - self.login() - self.open( - reverse("admin:config_device_change", args=[device.id]) + "#config-group" - ) - self.hide_loading_overlay() - try: - WebDriverWait(self.web_driver, 2).until( - EC.text_to_be_present_in_element_value( - ( - By.XPATH, - '//*[@id="flat-json-config-0-context"]/div[2]/div/div/input[1]', - ), - "vni", - ) - ) - except TimeoutException: - self.fail("Timed out wating for configuration variabled to get loaded") - self.find_element( - by=By.XPATH, value='//*[@id="main-content"]/div[2]/a[3]' - ).click() - try: - WebDriverWait(self.web_driver, 2).until(EC.alert_is_present()) - except TimeoutException: - pass - else: - alert = Alert(self.web_driver) - alert.accept() - self.fail("Unsaved changes alert displayed without any change") - def test_force_delete_device_with_deactivating_config(self): self._create_template(default=True) config = self._create_config(organization=self._get_org()) @@ -296,6 +350,132 @@ def test_force_delete_multiple_devices_with_deactivating_config(self): delete_confirm.click() self.assertEqual(Device.objects.count(), 0) + def test_add_remove_templates(self): + template = self._create_template(organization=self._get_org()) + config = self._create_config(organization=self._get_org()) + device = config.device + self.login() + # some times the url fetching in js gives unauthorized error + # so we add a wait to allow login to complete + time.sleep(2) + + with self.subTest("Template should be added"): + self._update_template(device.id, templates=[template]) + config.refresh_from_db() + self.assertEqual(config.templates.count(), 1) + self.assertEqual(config.status, "modified") + config.set_status_applied() + self.assertEqual(config.status, "applied") + + with self.subTest("Template should be removed"): + self._update_template(device.id, templates=[template], is_enabled=True) + config.refresh_from_db() + self.assertEqual(config.templates.count(), 0) + self.assertEqual(config.status, "modified") + + +@tag("selenium_tests") +class TestDeviceGroupAdmin( + SeleniumTestMixin, + CreateConfigTemplateMixin, + StaticLiveServerTestCase, +): + def test_show_relevant_templates(self): + org1 = self._create_org(name="org1", slug="org1") + org2 = self._create_org(name="org2", slug="org2") + shared_template = self._create_template(name="shared template") + org1_template = self._create_template(name="org1 template", organization=org1) + org1_required_template = self._create_template( + name="org1 required", organization=org1, required=True + ) + org1_default_template = self._create_template( + name="org1 default", organization=org1, default=True + ) + org2_template = self._create_template(name="org2 template", organization=org2) + org2_required_template = self._create_template( + name="org2 required", organization=org2, required=True + ) + org2_default_template = self._create_template( + name="org2 default", organization=org2, default=True + ) + + self.login() + self.open(reverse("admin:config_devicegroup_add")) + self.assertEqual( + self.wait_for_visibility( + By.CSS_SELECTOR, ".sortedm2m-container .help" + ).text, + "No Template available", + ) + self.find_element(by=By.CSS_SELECTOR, value='input[name="name"]').send_keys( + "Test Device Group" + ) + self._select_organization(org1) + self._verify_templates_visibility( + hidden=[ + org1_default_template, + org1_required_template, + org2_template, + org2_default_template, + org2_required_template, + ], + visible=[shared_template, org1_template], + ) + # Select org1 template + self.find_element( + by=By.XPATH, value=f'//*[@value="{org1_template.id}"]' + ).click() + self.web_driver.execute_script( + "window.scrollTo(0, document.body.scrollHeight);" + ) + self.find_element(by=By.CSS_SELECTOR, value='input[name="_continue"]').click() + self._wait_until_page_ready() + device_group = DeviceGroup.objects.first() + self.assertEqual(device_group.name, "Test Device Group") + self.assertIn(org1_template, device_group.templates.all()) + self.assertEqual( + self.find_element( + by=By.CSS_SELECTOR, + value=f'input[type="checkbox"][value="{org1_template.id}"]', + ).is_selected(), + True, + ) + + with self.subTest("Change organization to org2"): + self._select_organization(org2) + self._verify_templates_visibility( + hidden=[ + org1_template, + org1_default_template, + org1_required_template, + org2_required_template, + org2_default_template, + ], + visible=[ + shared_template, + org2_template, + ], + ) + self.assertEqual( + self.find_element( + by=By.CSS_SELECTOR, + value=f'input[type="checkbox"][value="{org2_template.id}"]', + ).is_selected(), + False, + ) + self.find_element( + by=By.CSS_SELECTOR, value='input[name="_continue"]' + ).click() + self._wait_until_page_ready() + self.assertEqual(device_group.templates.count(), 0) + self.assertEqual( + self.find_element( + by=By.CSS_SELECTOR, + value=f'input[type="checkbox"][value="{org2_template.id}"]', + ).is_selected(), + False, + ) + @tag("selenium_tests") class TestDeviceAdminUnsavedChanges( @@ -305,6 +485,21 @@ class TestDeviceAdminUnsavedChanges( ): browser = "chrome" + def _is_unsaved_changes_alert_present(self): + for entry in self.get_browser_logs(): + if ( + entry["level"] == "WARNING" + and "You haven't saved your changes yet!" in entry["message"] + ): + return True + return False + + def _override_unsaved_changes_alert(self): + self.web_driver.execute_script( + 'django.jQuery(window).on("beforeunload", function(e) {' + " console.warn(e.returnValue); });" + ) + def test_unsaved_changes(self): """ Execute this test using Chrome instead of Firefox. @@ -312,17 +507,17 @@ def test_unsaved_changes(self): impossible to test the unsaved changes alert. """ self.login() + self._create_template(default=True, default_values={"ssid": "default"}) device = self._create_config(organization=self._get_org()).device path = reverse("admin:config_device_change", args=[device.id]) with self.subTest("Alert should not be displayed without any change"): self.open(path) self.hide_loading_overlay() - try: - WebDriverWait(self.web_driver, 1).until(EC.alert_is_present()) - except TimeoutException: - pass - else: + self._override_unsaved_changes_alert() + # Simulate navigating away from the page + self.open(reverse("admin:index")) + if self._is_unsaved_changes_alert_present(): self.fail("Unsaved changes alert displayed without any change") with self.subTest("Alert should be displayed after making changes"): @@ -332,10 +527,9 @@ def test_unsaved_changes(self): # # our own JS code sets e.returnValue when triggered # so we just need to ensure it's set as expected - self.web_driver.execute_script( - 'django.jQuery(window).on("beforeunload", function(e) {' - " console.warn(e.returnValue); });" - ) + self.open(path) + self.hide_loading_overlay() + self._override_unsaved_changes_alert() # simulate hand gestures self.find_element(by=By.TAG_NAME, value="body").click() self.find_element(by=By.NAME, value="name").click() @@ -344,15 +538,55 @@ def test_unsaved_changes(self): # simulate hand gestures self.find_element(by=By.TAG_NAME, value="body").click() self.web_driver.refresh() - for entry in self.get_browser_logs(): - if ( - entry["level"] == "WARNING" - and "You haven't saved your changes yet!" in entry["message"] - ): - break - else: + if not self._is_unsaved_changes_alert_present(): self.fail("Unsaved changes code was not executed.") + def test_template_context_variables(self): + self._create_template( + name="Template1", default_values={"vni": "1"}, required=True + ) + self._create_template( + name="Template2", default_values={"vni": "2"}, required=True + ) + device = self._create_config(organization=self._get_org()).device + self.login() + self.open( + reverse("admin:config_device_change", args=[device.id]) + "#config-group" + ) + self.hide_loading_overlay() + try: + WebDriverWait(self.web_driver, 2).until( + EC.text_to_be_present_in_element_value( + ( + By.XPATH, + '//*[@id="flat-json-config-0-context"]/div[2]/div/div/input[1]', + ), + "vni", + ) + ) + except TimeoutException: + self.fail("Timed out waiting for configuration variables to get loaded") + + with self.subTest("Navigating away from the page should not show alert"): + self._override_unsaved_changes_alert() + # Simulate navigating away from the page + self.find_element( + by=By.XPATH, value='//*[@id="main-content"]/div[2]/a[3]' + ).click() + if self._is_unsaved_changes_alert_present(): + self.fail("Unsaved changes alert displayed without any change") + + with self.subTest("Saving the objects should not save context variables"): + self.open(reverse("admin:config_device_change", args=[device.id])) + self.web_driver.execute_script( + "window.scrollTo(0, document.body.scrollHeight);" + ) + self.find_element( + by=By.CSS_SELECTOR, value='input[name="_continue"]' + ).click() + device.refresh_from_db() + self.assertEqual(device.config.context, {}) + @tag("selenium_tests") class TestVpnAdmin( diff --git a/openwisp_controller/config/tests/test_views.py b/openwisp_controller/config/tests/test_views.py index ba6fa0696..49c525171 100644 --- a/openwisp_controller/config/tests/test_views.py +++ b/openwisp_controller/config/tests/test_views.py @@ -107,6 +107,7 @@ def test_get_relevant_templates_without_backend_filter(self): "required": t4.required, "default": t4.default, "name": t4.name, + "selected": False, } }, ) @@ -144,6 +145,7 @@ def test_get_relevant_templates_with_backend_filtering(self): "required": t1.required, "default": t1.default, "name": t1.name, + "selected": False, } }, ) @@ -163,6 +165,7 @@ def test_get_relevant_templates_with_backend_filtering(self): "required": t2.required, "default": t2.default, "name": t2.name, + "selected": False, } }, ) @@ -240,7 +243,7 @@ def test_get_relevant_templates_400(self): response = self.client.get( reverse("admin:get_relevant_templates", args=["wrong"]) ) - self.assertEqual(response.status_code, 404) + self.assertEqual(response.status_code, 400) def test_get_default_values_authorization(self): org1 = self._get_org() diff --git a/openwisp_controller/config/views.py b/openwisp_controller/config/views.py index 2cc185718..8f8f3cd61 100644 --- a/openwisp_controller/config/views.py +++ b/openwisp_controller/config/views.py @@ -1,9 +1,10 @@ import json +from collections import OrderedDict from copy import deepcopy from uuid import UUID from django.db.models import Q -from django.http import HttpResponse, JsonResponse +from django.http import Http404, HttpResponse, HttpResponseBadRequest, JsonResponse from django.utils import timezone from django.utils.module_loading import import_string from django.utils.translation import gettext as _ @@ -11,41 +12,106 @@ from swapper import load_model from .settings import BACKENDS, VPN_BACKENDS -from .utils import get_object_or_404 Organization = load_model("openwisp_users", "Organization") Template = load_model("config", "Template") +Config = load_model("config", "Config") DeviceGroup = load_model("config", "DeviceGroup") OrganizationConfigSettings = load_model("config", "OrganizationConfigSettings") +def _get_relevant_templates_dict(queryset, selected=False): + relevant_templates = OrderedDict() + for template in queryset: + relevant_templates[str(template.pk)] = dict( + name=template.name, + backend=template.get_backend_display(), + default=template.default, + required=template.required, + selected=selected, + ) + return relevant_templates + + def get_relevant_templates(request, organization_id): """ returns default templates of specified organization """ backend = request.GET.get("backend", None) + config_id = request.GET.get("config_id", None) + group_id = request.GET.get("group_id", None) user = request.user - if not user.is_superuser and not user.is_manager(organization_id): + # organization_id is passed as 'null' for add device + organization_id = None if organization_id == "null" else organization_id + if ( + not user.is_superuser + and organization_id + and not user.is_manager(organization_id) + ): return HttpResponse(status=403) - org = get_object_or_404(Organization, pk=organization_id, is_active=True) + + if organization_id: + # return 400 if organization_id is not a valid UUID + try: + organization_id = UUID(organization_id, version=4) + except ValueError: + return HttpResponseBadRequest(_(f"{organization_id} is not a valid UUID.")) + if not Organization.objects.filter(pk=organization_id, is_active=True).exists(): + raise Http404(_("Organization does not exist.")) + org_filters = Q(organization_id=organization_id) + # if the user is superuser then we need to fetch all the templates + elif user.is_superuser: + org_filters = Q(organization_id__isnull=False) + # else fetch templates of organizations managed by the user + else: + org_filters = Q(organization_id__in=user.organizations_managed) + + # this filter is for shared templates + org_filters |= Q(organization_id=None) + filter_options = {} if backend: filter_options.update(backend=backend) else: filter_options.update(required=False, default=False) + queryset = ( Template.objects.filter(**filter_options) - .filter(Q(organization_id=org.pk) | Q(organization_id=None)) + .filter(org_filters) + .order_by("-required", "-default") .only("id", "name", "backend", "default", "required") ) - relevant_templates = {} - for template in queryset: - relevant_templates[str(template.pk)] = dict( - name=template.name, - backend=template.get_backend_display(), - default=template.default, - required=template.required, + selected_templates = [] + if config_id: + try: + selected_templates = ( + Config.objects.prefetch_related("templates") + .only("templates") + .get(pk=config_id) + .templates.filter(org_filters) + .filter(**filter_options) + ) + except (Config.DoesNotExist, ValueError): + pass + + if group_id: + try: + selected_templates = ( + DeviceGroup.objects.prefetch_related("templates") + .only("templates") + .get(pk=group_id) + .templates.filter(org_filters) + .filter(**filter_options) + ) + except (DeviceGroup.DoesNotExist, ValueError): + pass + + relevant_templates = _get_relevant_templates_dict(selected_templates, selected=True) + relevant_templates.update( + _get_relevant_templates_dict( + queryset.exclude(pk__in=relevant_templates.keys()), selected=False ) + ) return JsonResponse(relevant_templates)