Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[change] Add user defined properties field in Node and Link #85 #91

Merged
merged 1 commit into from
Sep 18, 2020
Merged
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
29 changes: 26 additions & 3 deletions openwisp_network_topology/admin.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import swapper
from django import forms
from django.conf.urls import url
from django.contrib import admin, messages
from django.contrib.admin import ModelAdmin
from django.db.models import Q
from django.template.response import TemplateResponse
from django.templatetags.static import static
from django.urls import reverse
from django.utils.safestring import mark_safe
from django.utils.translation import ugettext_lazy as _
from flat_json_widget.widgets import FlatJsonWidget
Copy link
Member

Choose a reason for hiding this comment

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

this module needs to be added here: https://github.com/openwisp/openwisp-network-topology/blob/master/requirements.txt

openwisp-controller is not a direct dependency. The build is not failing because openwisp-controller is being installed with an additional command to test the integration module.

Copy link
Contributor Author

@NoumbissiValere NoumbissiValere Sep 17, 2020

Choose a reason for hiding this comment

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

@nemesisdesign Thank you for this catch. I forgot to add it 😄 . But the build isn't failing because of this. the Build is failing because modify_settings is called. I tried some many fix but they didn't work. I don't think the fix should be done in this module. if we fix it here, we might be hiding a potential bug which will certainly surface again one day. My master branch is equally failing with the same error.


from openwisp_users.multitenancy import (
MultitenantAdminMixin,
Expand Down Expand Up @@ -226,6 +229,13 @@ def visualize_view(self, request, pk):
return TemplateResponse(request, 'admin/topology/visualize.html', context)


class UserPropertiesForm(forms.ModelForm):
class Meta:
widgets = {
'user_properties': FlatJsonWidget,
}


class NodeLinkMixin(MultitenantAdminMixin):
"""
Hides organization in add form
Expand All @@ -234,7 +244,7 @@ class NodeLinkMixin(MultitenantAdminMixin):
other extensions to override the admin queryset
"""

readonly_fields = ['organization']
readonly_fields = ['organization', 'readonly_properties']

def get_fields(self, request, obj):
fields = super().get_fields(request, obj)
Expand All @@ -247,10 +257,20 @@ def get_fields(self, request, obj):
def get_queryset(self, request):
return self.model.get_queryset(super().get_queryset(request))

def readonly_properties(self, obj):
output = ''
for key, value in obj.properties.items():
key = key.replace('_', ' ').capitalize()
output += f'<p><strong>{key}</strong>: {value}</p>'
return mark_safe(output)

readonly_properties.short_description = _('Properties')


@admin.register(Node)
class NodeAdmin(NodeLinkMixin, BaseAdmin):
model = Node
form = UserPropertiesForm
change_form_template = 'admin/topology/node/change_form.html'
list_display = ['get_name', 'organization', 'topology', 'addresses']
search_fields = ['addresses', 'label', 'properties']
Expand All @@ -264,7 +284,8 @@ class NodeAdmin(NodeLinkMixin, BaseAdmin):
'organization',
'label',
'addresses',
'properties',
'readonly_properties',
'user_properties',
'created',
'modified',
]
Expand All @@ -287,6 +308,7 @@ def change_view(self, request, object_id, form_url='', extra_context=None):
@admin.register(Link)
class LinkAdmin(NodeLinkMixin, BaseAdmin):
model = Link
form = UserPropertiesForm
raw_id_fields = ['source', 'target']
search_fields = [
'source__label',
Expand Down Expand Up @@ -317,7 +339,8 @@ class LinkAdmin(NodeLinkMixin, BaseAdmin):
'target',
'cost',
'cost_text',
'properties',
'readonly_properties',
'user_properties',
'created',
'modified',
]
9 changes: 9 additions & 0 deletions openwisp_network_topology/base/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ class AbstractLink(OrgMixin, TimeStampedEditableModel):
load_kwargs={'object_pairs_hook': OrderedDict},
dump_kwargs={'indent': 4, 'cls': JSONEncoder},
)
user_properties = JSONField(
verbose_name=_('user defined properties'),
help_text=_('If you need to add additional data to this link use this field'),
default=dict,
blank=True,
load_kwargs={'object_pairs_hook': OrderedDict},
dump_kwargs={'indent': 4, 'cls': JSONEncoder},
)
status_changed = models.DateTimeField(auto_now=True)

class Meta:
Expand Down Expand Up @@ -102,6 +110,7 @@ def json(self, dict=False, original=False, **kwargs):
if self.properties:
properties.update(self.properties)
if not original:
properties.update(self.user_properties)
properties['status'] = self.status
properties['status_changed'] = self.status_changed
properties['created'] = self.created
Expand Down
10 changes: 10 additions & 0 deletions openwisp_network_topology/base/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.db import models
from django.utils.functional import cached_property
from django.utils.timezone import now
from django.utils.translation import ugettext_lazy as _
from jsonfield import JSONField
from rest_framework.utils.encoders import JSONEncoder

Expand Down Expand Up @@ -34,6 +35,14 @@ class AbstractNode(OrgMixin, TimeStampedEditableModel):
load_kwargs={'object_pairs_hook': OrderedDict},
dump_kwargs={'indent': 4, 'cls': JSONEncoder},
)
user_properties = JSONField(
verbose_name=_('user defined properties'),
help_text=_('If you need to add additional data to this node use this field'),
default=dict,
blank=True,
load_kwargs={'object_pairs_hook': OrderedDict},
dump_kwargs={'indent': 4, 'cls': JSONEncoder},
)

class Meta:
abstract = True
Expand Down Expand Up @@ -91,6 +100,7 @@ def json(self, dict=False, original=False, **kwargs):
if value or attr == 'properties':
netjson[attr] = deepcopy(value)
if not original:
netjson['properties'].update(deepcopy(self.user_properties))
netjson['properties']['created'] = JSONEncoder().default(self.created)
netjson['properties']['modified'] = JSONEncoder().default(self.modified)
if dict:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
import swapper
from django.db import migrations


def create_relations_0001(apps, schema_editor, app='topology'):
Node = swapper.load_model('topology', 'Node')
NoumbissiValere marked this conversation as resolved.
Show resolved Hide resolved
DeviceNode = swapper.load_model('topology_device', 'DeviceNode')
queryset = Node.objects.select_related('topology').filter(
topology__parser='netdiff.OpenvpnParser'
)
for node in queryset.iterator():
DeviceNode.auto_create(node)
from . import create_relations_0001


class Migration(migrations.Migration):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import swapper


def get_model(apps, app_name, model):
model_name = swapper.get_model_name(app_name, model)
model_label = swapper.split(model_name)[0]
return apps.get_model(model_label, model)


def create_relations_0001(apps, schema_editor):
Node = get_model(apps, 'topology', 'Node')
DeviceNode = swapper.load_model('topology_device', 'DeviceNode')
queryset = Node.objects.select_related('topology').filter(
topology__parser='netdiff.OpenvpnParser'
)
for node in queryset.iterator():
DeviceNode.auto_create(node)
1 change: 1 addition & 0 deletions openwisp_network_topology/integrations/device/overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def topology_get_nodes_queryset(self):
'label',
'addresses',
'properties',
'user_properties',
'created',
'modified',
'devicenode__device__name',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import swapper
from django.db import migrations


def fix_link_properties(apps, schema_editor):
NoumbissiValere marked this conversation as resolved.
Show resolved Hide resolved
Link = swapper.load_model('topology', 'Link')
for link in Link.objects.all():
link.full_clean()
link.save()
from . import fix_link_properties


class Migration(migrations.Migration):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Generated by Django 3.0.9 on 2020-09-01 14:57

import collections

import jsonfield.fields
import rest_framework.utils.encoders
from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('topology', '0012_update_openvpn_netjson_ids'),
]

operations = [
migrations.AddField(
model_name='link',
name='user_properties',
field=jsonfield.fields.JSONField(
blank=True,
default=dict,
dump_kwargs={
'cls': rest_framework.utils.encoders.JSONEncoder,
'indent': 4,
},
help_text='If you need to add additional data to this link use this field',
load_kwargs={'object_pairs_hook': collections.OrderedDict},
verbose_name='user defined properties',
),
),
migrations.AddField(
model_name='node',
name='user_properties',
field=jsonfield.fields.JSONField(
blank=True,
default=dict,
dump_kwargs={
'cls': rest_framework.utils.encoders.JSONEncoder,
'indent': 4,
},
help_text='If you need to add additional data to this node use this field',
load_kwargs={'object_pairs_hook': collections.OrderedDict},
verbose_name='user defined properties',
),
),
]
21 changes: 17 additions & 4 deletions openwisp_network_topology/migrations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@
import swapper


def migrate_addresses(apps, schema_editor, app='topology'):
Node = apps.get_model(app, 'Node')
def get_model(apps, app_name, model):
model_name = swapper.get_model_name(app_name, model)
model_label = swapper.split(model_name)[0]
return apps.get_model(model_label, model)


def migrate_addresses(apps, schema_editor):
Node = get_model(apps, 'topology', 'Node')
for node in Node.objects.iterator():
addresses = node.addresses_old.replace(' ', '')
if addresses.startswith(';'):
Expand All @@ -13,10 +19,17 @@ def migrate_addresses(apps, schema_editor, app='topology'):
node.save()


def migrate_openvpn_ids_0012(apps, schema_editor, app='topology'):
Node = swapper.load_model('topology', 'Node')
def migrate_openvpn_ids_0012(apps, schema_editor):
Node = get_model(apps, 'topology', 'Node')
queryset = Node.objects.filter(topology__parser='netdiff.OpenvpnParser')
for node in queryset.iterator():
node.addresses[0] = node.label
node.full_clean()
node.save()


def fix_link_properties(apps, schema_editor):
Link = get_model(apps, 'topology', 'Link')
for link in Link.objects.all():
link.full_clean()
link.save()
41 changes: 41 additions & 0 deletions openwisp_network_topology/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,47 @@ def test_update_selected_receive_topology(self):
self.assertEqual('warning', message.tags)
self.assertIn('1 topology was ignored', message.message)

def _test_properties_field(self, model, obj):
with self.subTest('test old properties readonly'):
content = 'readonly_properties">'
r = self.client.get(reverse(f'{self.prefix}_{model}_add'))
self.assertEqual(r.status_code, 200)
self.assertContains(r, content)

with self.subTest('test old properties display in list'):
path = reverse(f'{self.prefix}_{model}_change', args=[obj.pk])
r = self.client.get(path)
self.assertEqual(r.status_code, 200)
content = '<p><strong>Gateway</strong>: False</p>'
self.assertContains(r, content)

with self.subTest('test user properties diplay in flat json widgets'):
r = self.client.get(reverse(f'{self.prefix}_{model}_add'))
self.assertEqual(r.status_code, 200)
self.assertContains(r, 'flat-json-user_properties')

def test_node_user_properties_field(self):
t = Topology.objects.first()
n = self._create_node(label='node1org1', topology=t)
n.properties = {
'gateway': False,
}
n.full_clean()
n.save()
self._test_properties_field('node', n)

def test_link_user_properties_field(self):
t = Topology.objects.first()
n1 = self._create_node(label='node1org1', topology=t)
n2 = self._create_node(label='node2org1', topology=t)
li = self._create_link(topology=t, source=n1, target=n2)
li.properties = {
'gateway': False,
}
li.full_clean()
li.save()
self._test_properties_field('link', li)


class TestMultitenantAdmin(
CreateGraphObjectsMixin, TestMultitenantAdminMixin, TestOrganizationMixin, TestCase
Expand Down
25 changes: 25 additions & 0 deletions openwisp_network_topology/tests/test_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,28 @@ def test_link_auto_org(self):
)
link.full_clean()
self.assertEqual(link.organization, t.organization)

def test_user_properties_in_json(self):
t = self.topology_model.objects.first()
node1, node2 = self._get_nodes()
link = self.link_model(
source=node1, target=node2, cost=1.0, cost_text='100mbit/s', topology=t
)
link.properties = {
'wired': True,
}
link.user_properties = {
'user_property': True,
}
link.full_clean()
link.save()

with self.subTest('view json when original is False'):
data = link.json(dict=True)
self.assertIn('wired', data['properties'])
self.assertIn('user_property', data['properties'])

with self.subTest('view json when original is True'):
data = link.json(dict=True, original=True)
self.assertIn('wired', data['properties'])
self.assertNotIn('user_property', data['properties'])
22 changes: 22 additions & 0 deletions openwisp_network_topology/tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,25 @@ def test_node_auto_org(self):
)
n.full_clean()
self.assertEqual(n.organization, t.organization)

def test_user_properties_in_json(self):
t = self.topology_model.objects.first()
n = t._create_node(
addresses=['192.168.0.1'], label='test node', properties=None
)
n.properties = {
'gateway': True,
}
n.user_properties = {'user_property': True}
n.full_clean()
n.save()

with self.subTest('view json with original False'):
data = n.json(dict=True)
self.assertIn('gateway', data['properties'])
self.assertIn('user_property', data['properties'])

with self.subTest('view json with original True'):
data = n.json(dict=True, original=True)
self.assertIn('gateway', data['properties'])
self.assertNotIn('user_property', data['properties'])
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ djangorestframework>=3.3,<3.12
django-model-utils~=4.0.0
netdiff~=0.9.0
jsonfield>=3.1.0,<4.0.0
django-flat-json-widget~=0.1.1
Loading