Skip to content

Commit 9c476d5

Browse files
committed
[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
1 parent 486d958 commit 9c476d5

9 files changed

Lines changed: 91 additions & 49 deletions

File tree

docs/user/rest-api.rst

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -857,7 +857,7 @@ When using this strategy, in the response you can find the field
857857
credentials.
858858

859859
Batch retrieve and update
860-
++++++++++++++++++++++++++
860+
+++++++++++++++++++++++++
861861

862862
.. code-block:: text
863863
@@ -870,16 +870,16 @@ Used to retrieve or update a ``RadiusBatch`` instance.
870870
.. note::
871871

872872
The ``organization`` field is **read-only** for existing batch objects
873-
and cannot be changed via the API. This is intentional as changing
874-
the organization after batch creation would be inconsistent.
873+
and cannot be changed via the API. This is intentional as changing the
874+
organization after batch creation would be inconsistent.
875875

876876
Parameters for **GET**:
877877

878-
===== ===========
878+
===== =================
879879
Param Description
880-
===== ===========
880+
===== =================
881881
id UUID of the batch
882-
===== ===========
882+
===== =================
883883

884884
Parameters for **PUT**/**PATCH** (only certain fields can be updated):
885885

openwisp_radius/admin.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,9 +463,7 @@ def delete_selected_batches(self, request, queryset):
463463
)
464464

465465
def get_readonly_fields(self, request, obj=None):
466-
readonly_fields = super(RadiusBatchAdmin, self).get_readonly_fields(
467-
request, obj
468-
)
466+
readonly_fields = super().get_readonly_fields(request, obj)
469467
if obj and obj.status != "pending":
470468
return (
471469
"strategy",
@@ -476,7 +474,6 @@ def get_readonly_fields(self, request, obj=None):
476474
"users",
477475
"expiration_date",
478476
"name",
479-
"organization",
480477
"status",
481478
) + readonly_fields
482479
elif obj:

openwisp_radius/api/serializers.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -531,15 +531,11 @@ class RadiusBatchUpdateSerializer(RadiusBatchSerializer):
531531

532532
def validate(self, data):
533533
"""
534-
Simplified validation for partial updates.
535-
Only validates essential fields based on strategy.
534+
Validates partial updates while preserving model-level validation.
536535
Ignores organization_slug if provided since organization is readonly.
537536
"""
538-
# Remove organization_slug from data if provided (should not be changeable)
539537
data.pop("organization_slug", None)
540-
541538
strategy = data.get("strategy") or (self.instance and self.instance.strategy)
542-
543539
if (
544540
strategy == "prefix"
545541
and "number_of_users" in data
@@ -548,8 +544,18 @@ def validate(self, data):
548544
raise serializers.ValidationError(
549545
{"number_of_users": _("The field number_of_users cannot be empty")}
550546
)
551-
552-
return serializers.ModelSerializer.validate(self, data)
547+
validated_data = serializers.ModelSerializer.validate(self, data)
548+
# Run model-level validation (full_clean) for update
549+
if self.instance:
550+
batch_data = {
551+
field: validated_data.get(field, getattr(self.instance, field))
552+
for field in validated_data
553+
}
554+
batch_data.pop("number_of_users", None)
555+
for field, value in batch_data.items():
556+
setattr(self.instance, field, value)
557+
self.instance.full_clean()
558+
return validated_data
553559

554560
class Meta:
555561
model = RadiusBatch

openwisp_radius/integrations/monitoring/tasks.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ def _write_user_signup_metric_for_all(metric_key):
9898
from openwisp_radius.registration import REGISTRATION_METHOD_CHOICES
9999

100100
all_methods = [clean_registration_method(m) for m, _ in REGISTRATION_METHOD_CHOICES]
101+
existing_methods = {
102+
clean_registration_method(k) for k in total_registered_users.keys()
103+
}
101104
for m in all_methods:
102-
existing_methods = [
103-
clean_registration_method(k) for k in total_registered_users.keys()
104-
]
105105
if m not in existing_methods:
106106
total_registered_users[m] = 0
107107
for method, count in total_registered_users.items():

openwisp_radius/integrations/monitoring/tests/test_metrics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ def _read_chart(chart, **kwargs):
387387
user_signup_metric = self.metric_model.objects.get(key="user_signups")
388388
total_user_signup_metric = self.metric_model.objects.get(key="tot_user_signups")
389389
with self.subTest(
390-
"User does not has OrganizationUser and RegisteredUser object"
390+
"User does not have OrganizationUser and RegisteredUser object"
391391
):
392392
admin = self._get_admin()
393393
try:

openwisp_radius/tests/test_admin.py

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,10 @@ def test_organization_radsettings_freeradius_allowed_hosts(self):
520520
"radius_settings-0-id": radsetting.pk,
521521
"radius_settings-0-organization": org.pk,
522522
"radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
523+
"notification_settings-TOTAL_FORMS": 0,
524+
"notification_settings-INITIAL_FORMS": 0,
525+
"notification_settings-MIN_NUM_FORMS": 0,
526+
"notification_settings-MAX_NUM_FORMS": 1,
523527
}
524528
)
525529

@@ -602,6 +606,10 @@ def test_organization_radsettings_password_reset_url(self):
602606
"radius_settings-0-id": radsetting.pk,
603607
"radius_settings-0-organization": org.pk,
604608
"radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
609+
"notification_settings-TOTAL_FORMS": 0,
610+
"notification_settings-INITIAL_FORMS": 0,
611+
"notification_settings-MIN_NUM_FORMS": 0,
612+
"notification_settings-MAX_NUM_FORMS": 1,
605613
}
606614
)
607615

@@ -1212,6 +1220,10 @@ def test_organization_radsettings_allowed_mobile_prefixes(self):
12121220
"radius_settings-0-id": radsetting.pk,
12131221
"radius_settings-0-organization": org.pk,
12141222
"radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
1223+
"notification_settings-TOTAL_FORMS": 0,
1224+
"notification_settings-INITIAL_FORMS": 0,
1225+
"notification_settings-MIN_NUM_FORMS": 0,
1226+
"notification_settings-MAX_NUM_FORMS": 1,
12151227
}
12161228
)
12171229

@@ -1289,6 +1301,10 @@ def test_organization_radsettings_sms_message(self):
12891301
"radius_settings-0-id": radsetting.pk,
12901302
"radius_settings-0-organization": org.pk,
12911303
"radius_settings-0-password_reset_url": PASSWORD_RESET_URL,
1304+
"notification_settings-TOTAL_FORMS": 0,
1305+
"notification_settings-INITIAL_FORMS": 0,
1306+
"notification_settings-MIN_NUM_FORMS": 0,
1307+
"notification_settings-MAX_NUM_FORMS": 1,
12921308
"_continue": True,
12931309
}
12941310
)
@@ -1495,30 +1511,56 @@ def test_admin_menu_groups(self):
14951511
html = '<div class="mg-dropdown-label">RADIUS </div>'
14961512
self.assertContains(response, html, html=True)
14971513

1498-
def test_radiusbatch_organization_readonly_for_existing_objects(self):
1499-
"""
1500-
Test that organization field is readonly for existing RadiusBatch objects
1501-
"""
1502-
batch = self._create_radius_batch(
1503-
name="test-batch", strategy="prefix", prefix="test-prefix"
1504-
)
1505-
url = reverse(f"admin:{self.app_label}_radiusbatch_change", args=[batch.pk])
1506-
1507-
response = self.client.get(url)
1508-
self.assertEqual(response.status_code, 200)
15091514

1510-
self.assertContains(response, "readonly")
1511-
self.assertContains(response, batch.organization.name)
1515+
class TestRadiusGroupAdmin(BaseTestCase):
1516+
def setUp(self):
1517+
self.organization = self._create_org()
1518+
self.admin = self._create_admin()
1519+
self.organization.add_user(self.admin, is_admin=True)
1520+
self.client.force_login(self.admin)
1521+
1522+
def test_add_radiusgroup_with_inline_check_succeeds(self):
1523+
add_url = reverse("admin:openwisp_radius_radiusgroup_add")
1524+
1525+
post_data = {
1526+
# Main RadiusGroup form
1527+
"organization": self.organization.pk,
1528+
"name": "test-group-with-inline",
1529+
"description": "A test group created with an inline check",
1530+
# Inline RadiusGroupCheck formset
1531+
"radiusgroupcheck_set-TOTAL_FORMS": "1",
1532+
"radiusgroupcheck_set-INITIAL_FORMS": "0",
1533+
"radiusgroupcheck_set-0-attribute": "Max-Daily-Session",
1534+
"radiusgroupcheck_set-0-op": ":=",
1535+
"radiusgroupcheck_set-0-value": "3600",
1536+
# Inline RadiusGroupReply formset
1537+
"radiusgroupreply_set-TOTAL_FORMS": "1",
1538+
"radiusgroupreply_set-INITIAL_FORMS": "0",
1539+
"radiusgroupreply_set-0-attribute": "Session-Timeout",
1540+
"radiusgroupreply_set-0-op": "=",
1541+
"radiusgroupreply_set-0-value": "1800",
1542+
}
15121543

1513-
def test_radiusbatch_organization_editable_for_new_objects(self):
1514-
"""
1515-
Test that organization field is editable for new RadiusBatch objects
1516-
"""
1517-
url = reverse(f"admin:{self.app_label}_radiusbatch_add")
1544+
response = self.client.post(add_url, data=post_data, follow=True)
15181545

1519-
response = self.client.get(url)
15201546
self.assertEqual(response.status_code, 200)
1521-
1522-
self.assertContains(response, 'name="organization"')
1523-
form_html = response.content.decode()
1524-
self.assertIn('name="organization"', form_html)
1547+
final_group_name = f"{self.organization.slug}-test-group-with-inline"
1548+
1549+
self.assertContains(response, "The group")
1550+
self.assertContains(response, f"{final_group_name}</a>")
1551+
self.assertContains(response, "was added successfully.")
1552+
1553+
self.assertTrue(RadiusGroup.objects.filter(name=final_group_name).exists())
1554+
group = RadiusGroup.objects.get(name=final_group_name)
1555+
1556+
self.assertEqual(group.radiusgroupcheck_set.count(), 1)
1557+
check = group.radiusgroupcheck_set.first()
1558+
self.assertEqual(check.attribute, "Max-Daily-Session")
1559+
self.assertEqual(check.value, "3600")
1560+
self.assertEqual(check.groupname, group.name)
1561+
1562+
self.assertEqual(group.radiusgroupreply_set.count(), 1)
1563+
reply = group.radiusgroupreply_set.first()
1564+
self.assertEqual(reply.attribute, "Session-Timeout")
1565+
self.assertEqual(reply.value, "1800")
1566+
self.assertEqual(reply.groupname, group.name)

openwisp_radius/tests/test_api/test_api.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,6 +1561,7 @@ def _get_admin_auth_header(self):
15611561
login_payload = {"username": "admin", "password": "tester"}
15621562
login_url = reverse("radius:user_auth_token", args=[self.default_org.slug])
15631563
login_response = self.client.post(login_url, data=login_payload)
1564+
self.assertEqual(login_response.status_code, status.HTTP_200_OK)
15641565
return f"Bearer {login_response.json()['key']}"
15651566

15661567
def test_batch_update_organization_readonly(self):

pyproject.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,3 @@ multi_line_output = 3
2020
use_parentheses = true
2121
include_trailing_comma = true
2222
force_grid_wrap = 0
23-

tests/openwisp2/settings.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,7 @@
472472

473473
# local settings must be imported before test runner otherwise they'll be ignored
474474
try:
475-
try:
476-
from .local_settings import * # noqa: F403,F401
477-
except ImportError:
478-
pass
475+
from .local_settings import * # noqa: F403,F401
479476
except ImportError:
480477
pass
481478

0 commit comments

Comments
 (0)