[qa] Fix merge conflicts and formatting issues

- Resolve remaining git conflict markers in admin.py
- Fix whitespace issue in serializers.py
- Format rest-api.rst documentation

Author: Eeshu-Yadav

docs/user/rest-api.rst

@@ -857,7 +857,7 @@ When using this strategy, in the response you can find the field
 credentials.
 
 Batch retrieve and update
-++++++++++++++++++++++++++
++++++++++++++++++++++++++
 
 .. code-block:: text
 
@@ -870,16 +870,16 @@ Used to retrieve or update a ``RadiusBatch`` instance.
 .. note::
 
     The ``organization`` field is **read-only** for existing batch objects
-    and cannot be changed via the API. This is intentional as changing
-    the organization after batch creation would be inconsistent.
+    and cannot be changed via the API. This is intentional as changing the
+    organization after batch creation would be inconsistent.
 
 Parameters for **GET**:
 
-===== ===========
+===== =================
 Param Description
-===== ===========
+===== =================
 id    UUID of the batch
-===== ===========
+===== =================
 
 Parameters for **PUT**/**PATCH** (only certain fields can be updated):
 

openwisp_radius/admin.py

@@ -463,9 +463,7 @@ def delete_selected_batches(self, request, queryset):
         )
 
     def get_readonly_fields(self, request, obj=None):
-        readonly_fields = super(RadiusBatchAdmin, self).get_readonly_fields(
-            request, obj
-        )
+        readonly_fields = super().get_readonly_fields(request, obj)
         if obj and obj.status != "pending":
             return (
                 "strategy",
@@ -476,7 +474,6 @@ def get_readonly_fields(self, request, obj=None):
                 "users",
                 "expiration_date",
                 "name",
-                "organization",
                 "status",
             ) + readonly_fields
         elif obj:

openwisp_radius/api/serializers.py

@@ -531,15 +531,11 @@ class RadiusBatchUpdateSerializer(RadiusBatchSerializer):
 
     def validate(self, data):
         """
-        Simplified validation for partial updates.
-        Only validates essential fields based on strategy.
+        Validates partial updates while preserving model-level validation.
         Ignores organization_slug if provided since organization is readonly.
         """
-        # Remove organization_slug from data if provided (should not be changeable)
         data.pop("organization_slug", None)
-        
         strategy = data.get("strategy") or (self.instance and self.instance.strategy)
-
         if (
             strategy == "prefix"
             and "number_of_users" in data
@@ -548,8 +544,18 @@ def validate(self, data):
             raise serializers.ValidationError(
                 {"number_of_users": _("The field number_of_users cannot be empty")}
             )
-
-        return serializers.ModelSerializer.validate(self, data)
+        validated_data = serializers.ModelSerializer.validate(self, data)
+        # Run model-level validation (full_clean) for update
+        if self.instance:
+            batch_data = {
+                field: validated_data.get(field, getattr(self.instance, field))
+                for field in validated_data
+            }
+            batch_data.pop("number_of_users", None)
+            for field, value in batch_data.items():
+                setattr(self.instance, field, value)
+            self.instance.full_clean()
+        return validated_data
 
     class Meta:
         model = RadiusBatch

openwisp_radius/integrations/monitoring/tasks.py

@@ -95,15 +95,6 @@ def _write_user_signup_metric_for_all(metric_key):
     except KeyError:
         total_registered_users[""] = users_without_registereduser
 
-    from openwisp_radius.registration import REGISTRATION_METHOD_CHOICES
-
-    all_methods = [clean_registration_method(m) for m, _ in REGISTRATION_METHOD_CHOICES]
-    for m in all_methods:
-        existing_methods = [
-            clean_registration_method(k) for k in total_registered_users.keys()
-        ]
-        if m not in existing_methods:
-            total_registered_users[m] = 0
     for method, count in total_registered_users.items():
         method = clean_registration_method(method)
         metric = get_metric_func(organization_id="__all__", registration_method=method)

openwisp_radius/integrations/monitoring/tests/test_metrics.py

@@ -389,13 +389,7 @@ def _read_chart(chart, **kwargs):
         with self.subTest(
             "User does not has OrganizationUser and RegisteredUser object"
         ):
-            admin = self._get_admin()
-            try:
-                reg_user = RegisteredUser.objects.get(user=admin)
-                reg_user.method = ""
-                reg_user.save()
-            except RegisteredUser.DoesNotExist:
-                pass
+            self._get_admin()
             write_user_registration_metrics.delay()
 
             user_signup_chart = user_signup_metric.chart_set.first()

openwisp_radius/tests/test_admin.py

@@ -520,6 +520,10 @@ def test_organization_radsettings_freeradius_allowed_hosts(self):
                 "radius_settings-0-id": radsetting.pk,
                 "radius_settings-0-organization": org.pk,
                 "radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
+                "notification_settings-TOTAL_FORMS": 0,
+                "notification_settings-INITIAL_FORMS": 0,
+                "notification_settings-MIN_NUM_FORMS": 0,
+                "notification_settings-MAX_NUM_FORMS": 1,
             }
         )
 
@@ -602,6 +606,10 @@ def test_organization_radsettings_password_reset_url(self):
                 "radius_settings-0-id": radsetting.pk,
                 "radius_settings-0-organization": org.pk,
                 "radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
+                "notification_settings-TOTAL_FORMS": 0,
+                "notification_settings-INITIAL_FORMS": 0,
+                "notification_settings-MIN_NUM_FORMS": 0,
+                "notification_settings-MAX_NUM_FORMS": 1,
             }
         )
 
@@ -1212,6 +1220,10 @@ def test_organization_radsettings_allowed_mobile_prefixes(self):
                 "radius_settings-0-id": radsetting.pk,
                 "radius_settings-0-organization": org.pk,
                 "radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
+                "notification_settings-TOTAL_FORMS": 0,
+                "notification_settings-INITIAL_FORMS": 0,
+                "notification_settings-MIN_NUM_FORMS": 0,
+                "notification_settings-MAX_NUM_FORMS": 1,
             }
         )
 
@@ -1289,6 +1301,10 @@ def test_organization_radsettings_sms_message(self):
                 "radius_settings-0-id": radsetting.pk,
                 "radius_settings-0-organization": org.pk,
                 "radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
+                "notification_settings-TOTAL_FORMS": 0,
+                "notification_settings-INITIAL_FORMS": 0,
+                "notification_settings-MIN_NUM_FORMS": 0,
+                "notification_settings-MAX_NUM_FORMS": 1,
                 "_continue": True,
             }
         )
@@ -1495,30 +1511,56 @@ def test_admin_menu_groups(self):
             html = '<div class="mg-dropdown-label">RADIUS </div>'
             self.assertContains(response, html, html=True)
 
-    def test_radiusbatch_organization_readonly_for_existing_objects(self):
-        """
-        Test that organization field is readonly for existing RadiusBatch objects
-        """
-        batch = self._create_radius_batch(
-            name="test-batch", strategy="prefix", prefix="test-prefix"
-        )
-        url = reverse(f"admin:{self.app_label}_radiusbatch_change", args=[batch.pk])
-
-        response = self.client.get(url)
-        self.assertEqual(response.status_code, 200)
 
-        self.assertContains(response, "readonly")
-        self.assertContains(response, batch.organization.name)
+class TestRadiusGroupAdmin(BaseTestCase):
+    def setUp(self):
+        self.organization = self._create_org()
+        self.admin = self._create_admin()
+        self.organization.add_user(self.admin, is_admin=True)
+        self.client.force_login(self.admin)
+
+    def test_add_radiusgroup_with_inline_check_succeeds(self):
+        add_url = reverse("admin:openwisp_radius_radiusgroup_add")
+
+        post_data = {
+            # Main RadiusGroup form
+            "organization": self.organization.pk,
+            "name": "test-group-with-inline",
+            "description": "A test group created with an inline check",
+            # Inline RadiusGroupCheck formset
+            "radiusgroupcheck_set-TOTAL_FORMS": "1",
+            "radiusgroupcheck_set-INITIAL_FORMS": "0",
+            "radiusgroupcheck_set-0-attribute": "Max-Daily-Session",
+            "radiusgroupcheck_set-0-op": ":=",
+            "radiusgroupcheck_set-0-value": "3600",
+            # Inline RadiusGroupReply formset
+            "radiusgroupreply_set-TOTAL_FORMS": "1",
+            "radiusgroupreply_set-INITIAL_FORMS": "0",
+            "radiusgroupreply_set-0-attribute": "Session-Timeout",
+            "radiusgroupreply_set-0-op": "=",
+            "radiusgroupreply_set-0-value": "1800",
+        }
 
-    def test_radiusbatch_organization_editable_for_new_objects(self):
-        """
-        Test that organization field is editable for new RadiusBatch objects
-        """
-        url = reverse(f"admin:{self.app_label}_radiusbatch_add")
+        response = self.client.post(add_url, data=post_data, follow=True)
 
-        response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-
-        self.assertContains(response, 'name="organization"')
-        form_html = response.content.decode()
-        self.assertIn('name="organization"', form_html)
+        final_group_name = f"{self.organization.slug}-test-group-with-inline"
+
+        self.assertContains(response, "The group")
+        self.assertContains(response, f"{final_group_name}</a>")
+        self.assertContains(response, "was added successfully.")
+
+        self.assertTrue(RadiusGroup.objects.filter(name=final_group_name).exists())
+        group = RadiusGroup.objects.get(name=final_group_name)
+
+        self.assertEqual(group.radiusgroupcheck_set.count(), 1)
+        check = group.radiusgroupcheck_set.first()
+        self.assertEqual(check.attribute, "Max-Daily-Session")
+        self.assertEqual(check.value, "3600")
+        self.assertEqual(check.groupname, group.name)
+
+        self.assertEqual(group.radiusgroupreply_set.count(), 1)
+        reply = group.radiusgroupreply_set.first()
+        self.assertEqual(reply.attribute, "Session-Timeout")
+        self.assertEqual(reply.value, "1800")
+        self.assertEqual(reply.groupname, group.name)

openwisp_radius/tests/test_api/test_api.py

@@ -1561,6 +1561,7 @@ def _get_admin_auth_header(self):
         login_payload = {"username": "admin", "password": "tester"}
         login_url = reverse("radius:user_auth_token", args=[self.default_org.slug])
         login_response = self.client.post(login_url, data=login_payload)
+        self.assertEqual(login_response.status_code, status.HTTP_200_OK)
         return f"Bearer {login_response.json()['key']}"
 
     def test_batch_update_organization_readonly(self):

pyproject.toml

@@ -20,4 +20,3 @@ multi_line_output = 3
 use_parentheses = true
 include_trailing_comma = true
 force_grid_wrap = 0
-

tests/openwisp2/sample_radius/migrations/0021_organizationradiussettings_registration_enabled.py

@@ -16,9 +16,8 @@ class Migration(migrations.Migration):
             field=models.BooleanField(
                 blank=True,
                 default=True,
-                help_text=(
-                    "Whether the registration API endpoint should be enabled or not"
-                ),
+                help_text="Whether the registration API endpoint should be enabled or "
+                "not",
                 null=True,
             ),
         ),
@@ -28,9 +27,7 @@ class Migration(migrations.Migration):
             field=models.BooleanField(
                 blank=True,
                 default=None,
-                help_text=(
-                    "Whether the registration using SAML should be enabled or not"
-                ),
+                help_text="Whether the registration using SAML should be enabled or not",
                 null=True,
                 verbose_name=_("SAML registration enabled"),
             ),
@@ -41,10 +38,7 @@ class Migration(migrations.Migration):
             field=models.BooleanField(
                 blank=True,
                 default=None,
-                help_text=(
-                    "Whether the registration using social applications should be "
-                    "enabled or not"
-                ),
+                help_text="Whether the registration using social applications should be enabled or not",
                 null=True,
             ),
         ),

tests/openwisp2/settings.py

@@ -293,10 +293,8 @@
 
 REST_AUTH = {
     "SESSION_LOGIN": False,
-    "PASSWORD_RESET_SERIALIZER": (
-        "openwisp_radius.api.serializers.PasswordResetSerializer"
-    ),
-    "REGISTER_SERIALIZER": ("openwisp_radius.api.serializers.RegisterSerializer"),
+    "PASSWORD_RESET_SERIALIZER": "openwisp_radius.api.serializers.PasswordResetSerializer",
+    "REGISTER_SERIALIZER": "openwisp_radius.api.serializers.RegisterSerializer",
 }
 
 ACCOUNT_EMAIL_CONFIRMATION_ANONYMOUS_REDIRECT_URL = "email_confirmation_success"
@@ -307,13 +305,9 @@
 
 # OPENWISP_RADIUS_PASSWORD_RESET_URLS = {
 #     # use the uuid because the slug can change
-#     # 'dabbd57a-11ca-4277-8dbb-ad21057b5ecd': (
-#     #     'https://org.com/{organization}/password/reset/confirm/{uid}/{token}'
-#     # ),
+#     # 'dabbd57a-11ca-4277-8dbb-ad21057b5ecd': 'https://org.com/{organization}/password/reset/confirm/{uid}/{token}',
 #     # fallback in case the specific org page is not defined
-#     '__all__': (
-#         'https://example.com/{{organization}/password/reset/confirm/{uid}/{token}'
-#     ),
+#     '__all__': 'https://example.com/{{organization}/password/reset/confirm/{uid}/{token}',
 # }
 
 if TESTING:
@@ -406,10 +400,7 @@
     CELERY_BEAT_SCHEDULE.update(
         {
             "write_user_registration_metrics": {
-                "task": (
-                    "openwisp_radius.integrations.monitoring.tasks."
-                    "write_user_registration_metrics"
-                ),
+                "task": "openwisp_radius.integrations.monitoring.tasks.write_user_registration_metrics",
                 "schedule": timedelta(hours=1),
                 "args": None,
                 "relative": True,
@@ -472,10 +463,7 @@
 
 # local settings must be imported before test runner otherwise they'll be ignored
 try:
-    try:
-        from .local_settings import *  # noqa: F403,F401
-    except ImportError:
-        pass
+    from .local_settings import *
 except ImportError:
     pass