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

[feature] Added support for device deactivation #560 #607

Merged
merged 14 commits into from
Nov 19, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
pip install -r requirements-test.txt
pip install -U -I -e .
pip uninstall -y django
pip install ${{ matrix.django-version }}
pip install -U ${{ matrix.django-version }}
sudo npm install -g jshint stylelint

# start influxdb
Expand Down
5 changes: 5 additions & 0 deletions docs/user/device-health-status.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,8 @@ been crossed).

Example: ping is by default a critical metric which is expected to be
always 1 (reachable).

``DEACTIVATED``
---------------

The device is deactivated. All active and passive checks are disabled.
2 changes: 1 addition & 1 deletion docs/user/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ By default, if devices are not reachable by pings they are flagged as
============ ==========================================================
**type**: ``dict``
**default**: ``{'unknown': 'unknown', 'ok': 'ok', 'problem': 'problem',
'critical': 'critical'}``
'critical': 'critical', 'deactivated': 'deactivated'}``
============ ==========================================================

This setting allows to change the health status labels, for example, if we
Expand Down
3 changes: 3 additions & 0 deletions openwisp_monitoring/check/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ def check_instance(self):
def perform_check(self, store=True):
"""Initializes check instance and calls the check method."""
if (
hasattr(self.content_object, 'is_deactivated')
and self.content_object.is_deactivated()
) or (
hasattr(self.content_object, 'organization_id')
and self.content_object.organization.is_active is False
):
Expand Down
12 changes: 11 additions & 1 deletion openwisp_monitoring/check/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def test_device_deleted(self):
self.assertEqual(Check.objects.count(), 0)
d = self._create_device(organization=self._create_org())
self.assertEqual(Check.objects.count(), 3)
d.delete()
d.delete(check_deactivated=False)
self.assertEqual(Check.objects.count(), 0)

def test_config_modified_device_problem(self):
Expand Down Expand Up @@ -290,6 +290,16 @@ def test_device_organization_disabled_check_not_performed(self):
check.perform_check()
mocked_check.assert_not_called()

def test_deactivated_device_check_not_performed(self):
config = self._create_config(status='modified', organization=self._create_org())
self.assertEqual(Check.objects.filter(is_active=True).count(), 3)
config.device.deactivate()
config.set_status_deactivated()
check = Check.objects.filter(check_type=self._CONFIG_APPLIED).first()
with patch(f'{self._CONFIG_APPLIED}.check') as mocked_check:
check.perform_check()
mocked_check.assert_not_called()

def test_config_check_problem_with_interval(self):
self._create_admin()
d = self._create_device(organization=self._create_org())
Expand Down
63 changes: 52 additions & 11 deletions openwisp_monitoring/device/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
)
from swapper import load_model

from openwisp_controller.config.admin import DeactivatedDeviceReadOnlyMixin
from openwisp_controller.config.admin import DeviceAdmin as BaseDeviceAdmin
from openwisp_users.multitenancy import MultitenantAdminMixin
from openwisp_utils.admin import ReadOnlyAdmin
Expand Down Expand Up @@ -58,30 +59,68 @@ def full_clean(self):


class InlinePermissionMixin:
"""
Manages permissions for the inline objects.

This mixin class handles the permissions for the inline objects.
It checks if the user has permission to modify the main object
(e.g., view_model, add_model, change_model, or delete_model),
and if not, it checks if the user has permission to modify the
inline object (e.g., view_model_inline, add_model_inline,
change_model_inline, or delete_model_inline).
"""

def has_add_permission(self, request, obj=None):
# User will be able to add objects from inline even
# if it only has permission to add a model object
return super().has_add_permission(request, obj) or request.user.has_perm(
return super(admin.options.InlineModelAdmin, self).has_add_permission(
request
) or request.user.has_perm(
f'{self.model._meta.app_label}.add_{self.inline_permission_suffix}'
)

def has_change_permission(self, request, obj=None):
return super().has_change_permission(request, obj) or request.user.has_perm(
return super(admin.options.InlineModelAdmin, self).has_change_permission(
request, obj
) or request.user.has_perm(
f'{self.model._meta.app_label}.change_{self.inline_permission_suffix}'
)

def has_view_permission(self, request, obj=None):
return super().has_view_permission(request, obj) or request.user.has_perm(
return super(admin.options.InlineModelAdmin, self).has_view_permission(
request, obj
) or request.user.has_perm(
f'{self.model._meta.app_label}.view_{self.inline_permission_suffix}'
)

def has_delete_permission(self, request, obj=None):
return super().has_delete_permission(request, obj) or request.user.has_perm(
return super(admin.options.InlineModelAdmin, self).has_delete_permission(
request, obj
) or request.user.has_perm(
f'{self.model._meta.app_label}.delete_{self.inline_permission_suffix}'
)


class CheckInline(InlinePermissionMixin, GenericStackedInline):
class DeactivatedDeviceReadOnlyInlinePermissionMixin(
DeactivatedDeviceReadOnlyMixin,
InlinePermissionMixin,
):
def has_add_permission(self, request, obj=None):
perm = super().has_add_permission(request, obj)
return self._has_permission(request, obj, perm)

def has_change_permission(self, request, obj=None):
perm = super().has_change_permission(request, obj)
return self._has_permission(request, obj, perm)

def has_view_permission(self, request, obj=None):
perm = super().has_view_permission(request, obj)
return self._has_permission(request, obj, perm)

def has_delete_permission(self, request, obj=None):
perm = super().has_delete_permission(request, obj)
return self._has_permission(request, obj, perm)


class CheckInline(DeactivatedDeviceReadOnlyInlinePermissionMixin, GenericStackedInline):
model = Check
extra = 0
formset = CheckInlineFormSet
Expand Down Expand Up @@ -152,7 +191,9 @@ def get_queryset(self, request):
return super().get_queryset(request).order_by('created')


class MetricInline(InlinePermissionMixin, NestedGenericStackedInline):
class MetricInline(
DeactivatedDeviceReadOnlyInlinePermissionMixin, NestedGenericStackedInline
):
model = Metric
extra = 0
inlines = [AlertSettingsInline]
Expand Down Expand Up @@ -205,7 +246,7 @@ def has_delete_permission(self, request, obj=None):


class DeviceAdmin(BaseDeviceAdmin, NestedModelAdmin):
change_form_template = 'admin/config/device/change_form.html'
change_form_template = 'admin/monitoring/device/change_form.html'
list_filter = ['monitoring__status'] + BaseDeviceAdmin.list_filter
list_select_related = ['monitoring'] + list(BaseDeviceAdmin.list_select_related)
list_display = list(BaseDeviceAdmin.list_display)
Expand Down Expand Up @@ -302,7 +343,7 @@ def get_fields(self, request, obj=None):
fields = list(super().get_fields(request, obj))
if obj and not obj._state.adding:
fields.insert(fields.index('last_ip'), 'health_status')
if not obj or obj.monitoring.status in ['ok', 'unknown']:
if not obj or obj.monitoring.status in ['ok', 'unknown', 'deactivated']:
return fields
fields.insert(fields.index('health_status') + 1, 'health_checks')
return fields
Expand Down Expand Up @@ -415,7 +456,7 @@ class WiFiSessionInline(WifiSessionAdminHelperMixin, admin.TabularInline):
readonly_fields = fields
can_delete = False
extra = 0
template = 'admin/config/device/wifisession_tabular.html'
template = 'admin/monitoring/device/wifisession_tabular.html'

class Media:
css = {'all': ('monitoring/css/wifi-sessions.css',)}
Expand Down
7 changes: 7 additions & 0 deletions openwisp_monitoring/device/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core.exceptions import ValidationError
from django.db.models import Count, Q
from django.db.models.functions import Round
from django.http import Http404
from django.utils.timezone import now
from django.utils.translation import gettext_lazy as _
from django_filters.rest_framework import DjangoFilterBackend
Expand Down Expand Up @@ -113,6 +114,7 @@ class DeviceMetricView(
queryset = (
DeviceData.objects.filter(organization__is_active=True)
.only(
'_is_deactivated',
'id',
'key',
)
Expand Down Expand Up @@ -173,6 +175,11 @@ def get_object(self, pk):

def post(self, request, pk):
self.instance = self.get_object(pk)
if self.instance._is_deactivated:
# If the device is deactivated, do not accept data.
# We don't use "Device.is_deactivated()" to avoid
# generating query for the related config.
raise Http404
self.instance.data = request.data
# validate incoming data
try:
Expand Down
30 changes: 29 additions & 1 deletion openwisp_monitoring/device/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
from django.utils.translation import gettext_lazy as _
from swapper import get_model_name, load_model

from openwisp_controller.config.signals import checksum_requested, config_status_changed
from openwisp_controller.config.signals import (
checksum_requested,
config_status_changed,
device_activated,
device_deactivated,
)
from openwisp_controller.connection import settings as connection_settings
from openwisp_controller.connection.signals import is_working_changed
from openwisp_utils.admin_theme import (
Expand Down Expand Up @@ -74,6 +79,7 @@ def connect_device_signals(self):

Device = load_model('config', 'Device')
DeviceData = load_model('device_monitoring', 'DeviceData')
DeviceMonitoring = load_model('device_monitoring', 'DeviceMonitoring')
DeviceLocation = load_model('geo', 'DeviceLocation')
Metric = load_model('monitoring', 'Metric')
Chart = load_model('monitoring', 'Chart')
Expand Down Expand Up @@ -145,6 +151,26 @@ def connect_device_signals(self):
sender=Organization,
dispatch_uid='post_save_organization_disabled_monitoring',
)
device_deactivated.connect(
DeviceMonitoring.handle_deactivated_device,
sender=Device,
dispatch_uid='device_deactivated_update_devicemonitoring',
)
device_deactivated.connect(
DeviceMetricView.invalidate_get_device_cache,
sender=Device,
dispatch_uid=('device_deactivated_invalidate_view_device_cache'),
)
device_activated.connect(
DeviceMonitoring.handle_activated_device,
sender=Device,
dispatch_uid='device_activated_update_devicemonitoring',
)
device_activated.connect(
DeviceMetricView.invalidate_get_device_cache,
sender=Device,
dispatch_uid=('device_activated_invalidate_view_device_cache'),
)

@classmethod
def device_post_save_receiver(cls, instance, created, **kwargs):
Expand Down Expand Up @@ -330,12 +356,14 @@ def register_dashboard_items(self):
'problem': '#ffb442',
'critical': '#a72d1d',
'unknown': '#353c44',
'deactivated': '#000',
},
'labels': {
'ok': app_settings.HEALTH_STATUS_LABELS['ok'],
'problem': app_settings.HEALTH_STATUS_LABELS['problem'],
'critical': app_settings.HEALTH_STATUS_LABELS['critical'],
'unknown': app_settings.HEALTH_STATUS_LABELS['unknown'],
'deactivated': app_settings.HEALTH_STATUS_LABELS['deactivated'],
},
},
)
Expand Down
31 changes: 30 additions & 1 deletion openwisp_monitoring/device/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ class AbstractDeviceMonitoring(TimeStampedEditableModel):
('ok', _(app_settings.HEALTH_STATUS_LABELS['ok'])),
('problem', _(app_settings.HEALTH_STATUS_LABELS['problem'])),
('critical', _(app_settings.HEALTH_STATUS_LABELS['critical'])),
('deactivated', _(app_settings.HEALTH_STATUS_LABELS['deactivated'])),
)
status = StatusField(
_('health status'),
Expand All @@ -346,12 +347,14 @@ class AbstractDeviceMonitoring(TimeStampedEditableModel):
'"{0}" means the device has been recently added; \n'
'"{1}" means the device is operating normally; \n'
'"{2}" means the device is having issues but it\'s still reachable; \n'
'"{3}" means the device is not reachable or in critical conditions;'
'"{3}" means the device is not reachable or in critical conditions;\n'
'"{4}" means the device is deactivated;'
).format(
app_settings.HEALTH_STATUS_LABELS['unknown'],
app_settings.HEALTH_STATUS_LABELS['ok'],
app_settings.HEALTH_STATUS_LABELS['problem'],
app_settings.HEALTH_STATUS_LABELS['critical'],
app_settings.HEALTH_STATUS_LABELS['deactivated'],
),
)

Expand Down Expand Up @@ -444,6 +447,32 @@ def handle_disabled_organization(cls, organization_id):
status='unknown'
)

@classmethod
def handle_deactivated_device(cls, instance, **kwargs):
"""Handles the deactivation of a device

Sets the device's monitoring status to 'deactivated'

Parameters: - instance (Device): The device object
which is deactivated

Returns: - None
"""
cls.objects.filter(device_id=instance.id).update(status='deactivated')

@classmethod
def handle_activated_device(cls, instance, **kwargs):
"""Handles the activation of a deactivated device

Sets the device's monitoring status to 'unknown'

Parameters: - instance (Device): The device object
which is deactivated

Returns: - None
"""
cls.objects.filter(device_id=instance.id).update(status='unknown')

@classmethod
def _get_critical_metric_keys(cls):
return [metric['key'] for metric in get_critical_device_metrics()]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ class Migration(migrations.Migration):
'critical',
_(app_settings.HEALTH_STATUS_LABELS['critical']),
),
(
'deactivated',
_(app_settings.HEALTH_STATUS_LABELS['deactivated']),
),
],
default='unknown',
db_index=True,
Expand Down
2 changes: 2 additions & 0 deletions openwisp_monitoring/device/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ def get_health_status_labels():
'ok': 'ok',
'problem': 'problem',
'critical': 'critical',
'deactivated': 'deactivated',
},
)
try:
assert 'unknown' in labels
assert 'ok' in labels
assert 'problem' in labels
assert 'critical' in labels
assert 'deactivated' in labels
except AssertionError as e: # pragma: no cover
raise ImproperlyConfigured(
'OPENWISP_MONITORING_HEALTH_STATUS_LABELS must contain the following '
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
.health-deactivated {
color: #000;
}
.health-unknown {
color: grey;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ td.field-health_status,
font-weight: bold;
text-transform: uppercase;
}
.health-deactivated {
color: #000;
}
.health-unknown {
color: grey;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
const loadingOverlay = $('#device-map-container .ow-loading-spinner');
const localStorageKey = 'ow-map-shown';
const mapContainer = $('#device-map-container');
const statuses = ['critical', 'problem', 'ok', 'unknown'];
const statuses = ['critical', 'problem', 'ok', 'unknown', 'deactivated'];
const colors = {
ok: '#267126',
problem: '#ffb442',
critical: '#a72d1d',
unknown: '#353c44',
deactivated: '#0000',
};
const getLocationDeviceUrl = function (pk) {
return window._owGeoMapConfig.locationDeviceUrl.replace('000', pk);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{% extends "admin/config/change_form.html" %}
{% extends "admin/config/device/change_form.html" %}
{% load i18n static %}
{% block after_field_sets %}
{% if not add and device_data %}
Expand Down
Loading