Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions openwisp_controller/config/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

from openwisp_controller.config.views import (
get_default_templates,
get_relevant_templates,
get_template_default_values,
)
from openwisp_users.models import Organization
Expand Down Expand Up @@ -478,13 +479,29 @@ def _get_default_template_urls(self):
urls[str(org.pk)] = reverse('admin:get_default_templates', args=[org.pk])
return json.dumps(urls)

def _get_relevant_template_urls(self):
"""
returns URLs to get relevant templates
used in change_form.html template
"""
organizations = Organization.active.all()
urls = {}
for org in organizations:
urls[str(org.pk)] = reverse('admin:get_relevant_templates', args=[org.pk])
return json.dumps(urls)

def get_urls(self):
return [
url(
r'^config/get-default-templates/(?P<organization_id>[^/]+)/$',
get_default_templates,
name='get_default_templates',
),
url(
r'^config/get-relevant-templates/(?P<organization_id>[^/]+)/$',
get_relevant_templates,
name='get_relevant_templates',
),
url(
r'^get-template-default-values/$',
get_template_default_values,
Expand All @@ -495,6 +512,7 @@ def get_urls(self):
def get_extra_context(self, pk=None):
ctx = super().get_extra_context(pk)
ctx.update({'default_template_urls': self._get_default_template_urls()})
ctx.update({'relevant_template_urls': self._get_relevant_template_urls()})
return ctx

def add_view(self, request, form_url='', extra_context=None):
Expand Down
75 changes: 67 additions & 8 deletions openwisp_controller/config/static/config/js/default_templates.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
'use strict';
django.jQuery(function ($) {
var firstRun = true,
addChangeEventToBackend = function (urls) {
addChangeEventToBackend = function (default_urls, relevant_urls) {
$('#id_config-0-backend').change(function () {
setTimeout(function () {
// ensures getDefaultTemplates execute only after other
// onChange event handlers attached this field has been
// executed.
getDefaultTemplates(urls);
getRelevantTemplates(relevant_urls);
getDefaultTemplates(default_urls);
});
});
},
Expand All @@ -27,7 +28,7 @@ django.jQuery(function ($) {
return;
}
var url = urls[orgID],
isNew = $('#id_config-0-id').length == 0;
isNew = $('#id_config-0-id').length === 0;
// if device is not new, do not execute on page load
if (!isNew && firstRun) {
return;
Expand All @@ -46,23 +47,81 @@ django.jQuery(function ($) {
});
});
},
bindDefaultTemplateLoading = function (urls) {
getRelevantTemplates = function (urls) {
var orgID = $('#id_organization').val(),
backend = $('#id_config-0-backend').val();
if (!orgID || !backend) {
return;
}
// proceed only if an organization and a backend have been selected
if (orgID.length === 0 || backend.length === 0) {
return;
}

var url = urls[orgID];
// get relevant templates of selected org and backend
url = url + '?backend=' + backend;
$.get(url).done(function (data) {
var relevantTemplates = {};
$.each(data.templates, function (i, uuid) {
relevantTemplates[uuid] = uuid;
});
var disabledTemplates =
localStorage.getItem('config-disabled-templates', null);
disabledTemplates = JSON.parse(disabledTemplates) || {};
$('input.sortedm2m').each(function () {
if (($(this).val() in relevantTemplates) === false) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use do relevantTemplates[$(this).val()] and put everything in try catch instead? The "in" operator will take more time if number of entries is more in relevantTemplates.

// hide templates which are not relevant
// also removed disabled attribute and uncheck it
if ($(this).prop('disabled')) {
disabledTemplates[$(this).val()] = $(this).val();
$(this).prop('disabled', false);
$(this).prop('checked', false);
}
$(this).parent().parent().hide();
} else {
// show templates which are relevant
if ($(this).val() in disabledTemplates) {
$(this).prop('disabled', true);
$(this).prop('checked', true);
delete disabledTemplates[$(this).val()];
}
$(this).parent().parent().show();
}
});
disabledTemplates = JSON.stringify(disabledTemplates);
localStorage.setItem('config-disabled-templates', disabledTemplates);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just keep it in the memory instead of using localStorage? How making disabledTemplates persistent helpful?

// change help text if no relevant template
var msg = 'Choose items and order by drag & drop.';
if (Object.keys(relevantTemplates).length === 0) {
msg = 'No Template available';
}
if (gettext) {
msg = gettext(msg);
}
$('.sortedm2m-container > .help').text(msg);
});
},
bindDefaultTemplateLoading = function (default_urls, relevant_urls) {
var backendField = $('#id_config-0-backend');
$('#id_organization').change(function () {
if ($('#id_config-0-backend').length > 0) {
getDefaultTemplates(urls);
getRelevantTemplates(relevant_urls);
getDefaultTemplates(default_urls);
}
});
if (backendField.length > 0) {
addChangeEventToBackend(urls);
addChangeEventToBackend(default_urls, relevant_urls);
} else {
$('#config-group > fieldset.module').ready(function () {
$('div.add-row > a').click(function () {
addChangeEventToBackend(urls);
getDefaultTemplates(urls);
addChangeEventToBackend(default_urls, relevant_urls);
getRelevantTemplates(relevant_urls);
getDefaultTemplates(default_urls);
});
});
}
getRelevantTemplates(relevant_urls);
firstRun = false;
};
window.bindDefaultTemplateLoading = bindDefaultTemplateLoading;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@
// enable default templates - do not remove this comment
(function ($) {
$(document).ready( function () {
window.bindDefaultTemplateLoading({{ default_template_urls| safe }});
window.bindDefaultTemplateLoading(
{{ default_template_urls| safe }},
{{relevant_template_urls| safe}}
Comment on lines +65 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{ default_template_urls| safe }},
{{relevant_template_urls| safe}}
{{ default_template_urls | safe }},
{{ relevant_template_urls | safe }}

);
})
}) (django.jQuery);
</script>
Expand Down
40 changes: 40 additions & 0 deletions openwisp_controller/config/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,43 @@ def test_get_default_templates_400(self):
reverse('admin:get_default_templates', args=['wrong'])
)
self.assertEqual(response.status_code, 404)

def test_get_relevant_templates(self):
org1 = self._create_org(name='org1')
org2 = self._create_org(name='org2')
t1 = self._create_template(
name='t1', organization=org1, backend='netjsonconfig.OpenWrt'
)
_ = self._create_template(
name='t2', organization=org2, backend='netjsonconfig.OpenWrt'
)
# shared template
t3 = self._create_template(
organization=None, name='t3', default=True, backend='netjsonconfig.OpenWrt'
)
self._login()

r = self.client.get(
reverse('admin:get_relevant_templates', args=[org1.pk]),
{'backend': 'netjsonconfig.OpenWrt'},
)
templates = r.json()['templates']
self.assertEqual(len(templates), 2)
self.assertIn(str(t1.pk), templates)
self.assertIn(str(t3.pk), templates)

def test_get_relevant_templates_403(self):
org1 = self._create_org(name='org1')
response = self.client.get(
reverse('admin:get_relevant_templates', args=[org1.pk]),
{'backend': 'netjsonconfig.OpenWrt'},
)
self.assertEqual(response.status_code, 403)

def test_get_relevant_templates_404(self):
self._login()
response = self.client.get(
reverse('admin:get_relevant_templates', args=['wrong']),
{'backend': 'netjsonconfig.OpenWrt'},
)
self.assertEqual(response.status_code, 404)
15 changes: 15 additions & 0 deletions openwisp_controller/config/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,18 @@ def get_default_templates_queryset(
if backend:
queryset = queryset.filter(backend=backend)
return queryset


def get_relevant_templates_queryset(organization_id, backend, model=None):
"""
It filters template as:
* Shared templates (organization==None), belonging to the
same config backend of the device.
* Templates belonging to the same organization and config backend
of the device
"""
queryset = model.objects.filter(backend=backend)
queryset = queryset.filter(
Q(organization_id=organization_id) | Q(organization_id=None)
)
return queryset
24 changes: 23 additions & 1 deletion openwisp_controller/config/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@
from openwisp_users.models import Organization

from .settings import BACKENDS, VPN_BACKENDS
from .utils import get_default_templates_queryset, get_object_or_404
from .utils import (
get_default_templates_queryset,
get_object_or_404,
get_relevant_templates_queryset,
)

Template = load_model('config', 'Template')

Expand All @@ -34,6 +38,24 @@ def get_default_templates(request, organization_id):
return JsonResponse({'default_templates': uuids})


def get_relevant_templates(request, organization_id):
"""
returns relevant templates of specified organization
and config backend
"""
backend = request.GET.get("backend", None)
user = request.user
authenticated = user.is_authenticated
if not authenticated and not user.is_staff:
return HttpResponse(status=403)
org = get_object_or_404(Organization, pk=organization_id, is_active=True)
templates = get_relevant_templates_queryset(org.pk, backend, model=Template).only(
'id'
)
uuids = [str(t.pk) for t in templates]
return JsonResponse({'templates': uuids})
Comment on lines +41 to +56

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to do a check like if organization_id in user.organization_managed or user.is_superuser? It will make sure that templates can be only be used by users of the same organization.

Please also add a test for this.

@pandafy pandafy Apr 6, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this is a problem with get_default_templates view also. I will open a separate issue for this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #420

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

damn 🤦



ALL_BACKENDS = BACKENDS + VPN_BACKENDS

# ``available_schemas`` and ``available_schemas_json``
Expand Down