From c73ee882382fe36f528fb8db0f7c29dbbbe65b15 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 8 May 2024 21:35:32 +0530 Subject: [PATCH] [change] Use device.organization_id for RADIUS metric if SHARED_ACCOUNTING is enabled --- openwisp_radius/integrations/monitoring/tasks.py | 16 ++++++++++++---- .../monitoring/tests/test_metrics.py | 6 ++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/openwisp_radius/integrations/monitoring/tasks.py b/openwisp_radius/integrations/monitoring/tasks.py index 0d6c1927..b6f271e0 100644 --- a/openwisp_radius/integrations/monitoring/tasks.py +++ b/openwisp_radius/integrations/monitoring/tasks.py @@ -194,14 +194,13 @@ def post_save_radiusaccounting( else: registration_method = clean_registration_method(registration_method) device_lookup = Q(mac_address__iexact=called_station_id.replace('-', ':')) - if app_settings.SHARED_ACCOUNTING: - organization_id = None - else: + # Do not use organization_id for device lookup if shared accounting is enabled + if not app_settings.SHARED_ACCOUNTING: device_lookup &= Q(organization_id=organization_id) try: device = ( Device.objects.select_related('devicelocation') - .only('id', 'devicelocation__location_id') + .only('id', 'organization_id', 'devicelocation__location_id') .get(device_lookup) ) except Device.DoesNotExist: @@ -213,6 +212,8 @@ def post_save_radiusaccounting( object_id = None content_type = None location_id = None + if app_settings.SHARED_ACCOUNTING: + organization_id = None else: object_id = str(device.id) content_type = ContentType.objects.get_for_model(Device) @@ -220,6 +221,13 @@ def post_save_radiusaccounting( location_id = str(device.devicelocation.location_id) else: location_id = None + # Give preference to the organization_id of the device + # over RadiusAccounting object. + # This would also handle the case when SHARED_ACCOUNTING is enabled, + # and write the metric with the same organization_id as the device. + # If SHARED_ACCOUNTING is disabled, the organization_id of + # Device and RadiusAccounting would be same. + organization_id = str(device.organization_id) metric, created = Metric._get_or_create( configuration='radius_acc', diff --git a/openwisp_radius/integrations/monitoring/tests/test_metrics.py b/openwisp_radius/integrations/monitoring/tests/test_metrics.py index b05b751d..c2317af6 100644 --- a/openwisp_radius/integrations/monitoring/tests/test_metrics.py +++ b/openwisp_radius/integrations/monitoring/tests/test_metrics.py @@ -135,7 +135,7 @@ def test_post_save_radius_accounting_shared_accounting(self, mocked_logger): 'calling_station_id': sha1_hash('00:00:00:00:00:00'), 'location_id': str(device_loc.location.id), 'method': reg_user.method, - 'organization_id': None, + 'organization_id': str(device.organization_id), }, ) @@ -174,7 +174,9 @@ def test_post_save_radius_accounting_shared_accounting(self, mocked_logger): 1, ) metric = device_metric_qs.first() - self.assertEqual(metric.extra_tags['organization_id'], None) + self.assertEqual( + metric.extra_tags['organization_id'], str(device.organization_id) + ) traffic_chart = metric.chart_set.get(configuration='radius_traffic') points = traffic_chart.read() self.assertEqual(points['traces'][0][0], 'download')