From bb54b12577e6b9d95f6fe5fea063e04375663ed9 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 08:53:28 +0530 Subject: [PATCH 01/19] [feature] Added REST API list, detail, and delete endpoints for RADIUS batch user creation #771 Added GET /api/v1/radius/batch/ for listing batches with pagination, filtering by organization/strategy/name search. Added GET /api/v1/radius/batch// for retrieving batch detail. Added DELETE /api/v1/radius/batch//delete/ for deleting a batch and its associated users. Deletion is rejected with 409 Conflict while batch status is processing. List and detail responses do not expose user_credentials. They include pdf_link for completed prefix batches and csv_link for CSV batches. Updated admin bulk delete action to skip processing batches with a warning message. Updated documentation with new endpoints, filters, and pagination details. Fixes #771 --- docs/user/rest-api.rst | 79 ++++- openwisp_radius/admin.py | 12 + openwisp_radius/api/serializers.py | 51 ++++ openwisp_radius/api/urls.py | 10 + openwisp_radius/api/views.py | 103 ++++++- .../tests/test_api/test_api_batch.py | 280 ++++++++++++++++++ tests/openwisp2/sample_radius/api/views.py | 12 + 7 files changed, 534 insertions(+), 13 deletions(-) create mode 100644 openwisp_radius/tests/test_api/test_api_batch.py diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index 30c6b677..ab2649ad 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -869,12 +869,49 @@ This API endpoint allows to use the features described in /api/v1/radius/batch/ +GET +^^^ + +Returns a list of batch user creation operations for the organizations +managed by the requesting user. Results are paginated and can be filtered. + +.. code-block:: text + + /api/v1/radius/batch?search= + /api/v1/radius/batch?organization= + /api/v1/radius/batch?strategy=prefix + +Filters +""""""" + +================= =============================== +Filter Parameter Description +================= =============================== +search Search batches by name +organization Filter by organization id +organization_slug Filter by organization slug +strategy Filter by strategy (prefix/csv) +================= =============================== + +Pagination +"""""""""" + +Pagination is provided using page number pagination, the default page size +is 20, which can be overridden using the ``page_size`` parameter, up to +:ref:`OPENWISP_API_MAX_PAGE_SIZE ` (100 by +default). + .. note:: - This API endpoint allows to use the features described in - :doc:`importing_users` and :doc:`generating_users`. + The list response does not include ``user_credentials`` to avoid + repeatedly exposing plaintext credentials. Use the batch creation + response or the protected PDF download endpoint for credentials. + +POST +^^^^ -Responds only to **POST**, used to save a ``RadiusBatch`` instance. +Creates a batch of users using a csv file or generates users with a given +prefix. It is possible to generate the users of the ``RadiusBatch`` with two different strategies: csv or prefix. @@ -919,6 +956,42 @@ group by organization before sending its UUID in the ``group`` parameter. The ``group`` and ``notes`` parameters are optional. When ``group`` is omitted, users retain the standard default-group behavior. +.. note:: + + The synchronous ``201 Created`` response for prefix-generated batches + includes ``user_credentials``. The asynchronous ``202 Accepted`` + response does not include credentials; use the ``pdf_link`` after + completion. + +Batch Detail +++++++++++++ + +.. code-block:: text + + /api/v1/radius/batch// + +GET +^^^ + +Returns a single batch user creation operation by its UUID. The response +does not include ``user_credentials``. + +For completed prefix batches, the response includes a ``pdf_link`` field +pointing to the protected PDF download endpoint. For CSV batches with an +uploaded file, the response includes a ``csv_link`` field pointing to the +protected CSV download endpoint. + +DELETE +^^^^^^ + +Deletes a batch user creation operation and its associated users. Returns +``204 No Content`` on success. + +.. note:: + + Deletion is rejected while the batch ``status`` is ``processing``. The + API returns a ``409 Conflict`` response with a clear error message in + this case. Pending, completed, and failed batches can be deleted. Batch CSV Download ++++++++++++++++++ diff --git a/openwisp_radius/admin.py b/openwisp_radius/admin.py index 1f3110fe..4a0d681e 100644 --- a/openwisp_radius/admin.py +++ b/openwisp_radius/admin.py @@ -506,8 +506,20 @@ def get_actions(self, request): @admin.action(description=_("Delete selected batches"), permissions=["delete"]) def delete_selected_batches(self, request, queryset): + skipped = 0 for obj in queryset: + if obj.status == "processing": + skipped += 1 + continue obj.delete() + if skipped: + self.message_user( + request, + _( + "Skipped {count} batch(es) that are currently being processed." + ).format(count=skipped), + level=messages.WARNING, + ) self.message_user( request, "Successfully deleted selected batches.", level=messages.SUCCESS ) diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 38b9808d..a27faf0a 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -562,6 +562,57 @@ class Meta: read_only_fields = ("status", "user_credentials", "created", "modified") +class RadiusBatchReadSerializer(serializers.ModelSerializer): + organization = serializers.PrimaryKeyRelatedField(read_only=True) + users = UserSerializer(many=True, read_only=True) + pdf_link = serializers.SerializerMethodField(required=False, read_only=True) + csv_link = serializers.SerializerMethodField(required=False, read_only=True) + status = serializers.CharField(read_only=True) + + def get_pdf_link(self, obj): + if ( + isinstance(obj, RadiusBatch) + and obj.strategy == "prefix" + and obj.status == RadiusBatch.COMPLETED + ): + request = self.context.get("request") + return request.build_absolute_uri( + reverse( + "radius:download_rad_batch_pdf", + args=[obj.organization.slug, obj.pk], + ) + ) + return None + + def get_csv_link(self, obj): + if obj.csvfile: + request = self.context.get("request") + csv_url = reverse( + "radius:radius_organization_batch_csv_read", + args=[obj.organization.slug, obj.pk], + ) + return request.build_absolute_uri(csv_url) + return None + + class Meta: + model = RadiusBatch + fields = ( + "id", + "organization", + "name", + "strategy", + "status", + "expiration_date", + "prefix", + "users", + "pdf_link", + "csv_link", + "created", + "modified", + ) + read_only_fields = fields + + class RegisterSerializer( ErrorDictMixin, AllowedMobilePrefixMixin, diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index 3ca6407d..0d96ec71 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -83,6 +83,16 @@ def get_api_urls(api_views=None): name="update_registered_user_registration_method", ), path("radius/batch/", api_views.batch, name="batch"), + path( + "radius/batch//", + api_views.batch_detail, + name="batch_detail", + ), + path( + "radius/batch//delete/", + api_views.batch_delete, + name="batch_delete", + ), path( "radius/organization//batch//pdf/", api_views.download_rad_batch_pdf, diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 5c8b3914..ff6e5f13 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -31,6 +31,7 @@ ListAPIView, ListCreateAPIView, RetrieveAPIView, + RetrieveDestroyAPIView, RetrieveUpdateDestroyAPIView, get_object_or_404, ) @@ -78,6 +79,7 @@ AuthTokenSerializer, ChangePhoneNumberSerializer, RadiusAccountingSerializer, + RadiusBatchReadSerializer, RadiusBatchSerializer, RadiusGroupSerializer, RadiusUserGroupSerializer, @@ -120,20 +122,49 @@ class ThrottledAPIMixin(object): throttle_scope = "others" -class BatchView(ThrottledAPIMixin, CreateAPIView): - authentication_classes = (BearerAuthentication, SessionAuthentication) - permission_classes = (IsAdminUser, DjangoModelPermissions) - queryset = RadiusBatch.objects.all() - serializer_class = RadiusBatchSerializer +class RadiusBatchFilter(OrganizationManagedFilter, filters.FilterSet): + class Meta(OrganizationManagedFilter.Meta): + model = RadiusBatch + fields = [*OrganizationManagedFilter.Meta.fields, "strategy"] - def post(self, request, *args, **kwargs): - """ + +@method_decorator( + name="get", + decorator=swagger_auto_schema( + operation_description=""" + Returns a list of batch user creation operations for the + organizations managed by the user. + """, + ), +) +@method_decorator( + name="post", + decorator=swagger_auto_schema( + operation_description=""" **Requires the user auth token (Bearer Token).** Allows organization administrators to create a batch of users using a csv file or generate users with a given prefix. - """ - serializer = self.get_serializer(data=request.data) + """, + request_body=RadiusBatchSerializer, + responses={201: RadiusBatchSerializer, 202: RadiusBatchSerializer}, + ), +) +class BatchView(ThrottledAPIMixin, FilterByOrganizationManaged, ListCreateAPIView): + authentication_classes = (BearerAuthentication, SessionAuthentication) + permission_classes = (IsAdminUser, DjangoModelPermissions) + queryset = RadiusBatch.objects.all() + serializer_class = RadiusBatchReadSerializer + filterset_class = RadiusBatchFilter + filter_backends = [DjangoFilterBackend, SearchFilter] + search_fields = ["name"] + pagination_class = OpenWispPagination + pagination_page_size = 20 + + def post(self, request, *args, **kwargs): + serializer = RadiusBatchSerializer( + data=request.data, context={"request": request} + ) if serializer.is_valid(): valid_data = serializer.validated_data.copy() num_of_users = valid_data.get("number_of_users", 0) @@ -142,7 +173,9 @@ def post(self, request, *args, **kwargs): batch = serializer.save(organization=organization) is_async = batch.schedule_processing(number_of_users=num_of_users) batch.refresh_from_db() - response_serializer = self.get_serializer(batch) + response_serializer = RadiusBatchSerializer( + batch, context={"request": request} + ) status_code = ( status.HTTP_202_ACCEPTED if is_async else status.HTTP_201_CREATED ) @@ -196,6 +229,56 @@ def get(self, request, *args, **kwargs): download_rad_batch_pdf = DownloadRadiusBatchPdfView.as_view() +@method_decorator( + name="get", + decorator=swagger_auto_schema( + operation_description=""" + Returns a batch user creation operation by its UUID. + """, + ), +) +class BatchDetailView(ProtectedAPIMixin, FilterByOrganizationManaged, RetrieveAPIView): + authentication_classes = (BearerAuthentication, SessionAuthentication) + permission_classes = (IsAdminUser, DjangoModelPermissions) + queryset = RadiusBatch.objects.all() + serializer_class = RadiusBatchReadSerializer + + +batch_detail = BatchDetailView.as_view() + + +@method_decorator( + name="delete", + decorator=swagger_auto_schema( + operation_description=""" + Deletes a batch user creation operation and its associated users. + Cannot delete a batch while it is being processed. + """, + responses={204: "No Content", 409: "Conflict"}, + ), +) +class BatchDeleteView( + ProtectedAPIMixin, FilterByOrganizationManaged, RetrieveDestroyAPIView +): + authentication_classes = (BearerAuthentication, SessionAuthentication) + permission_classes = (IsAdminUser, DjangoModelPermissions) + queryset = RadiusBatch.objects.all() + serializer_class = RadiusBatchReadSerializer + + def delete(self, request, *args, **kwargs): + batch = self.get_object() + if batch.status == RadiusBatch.PROCESSING: + return Response( + {"detail": _("Cannot delete a batch while it is being processed.")}, + status=status.HTTP_409_CONFLICT, + ) + batch.delete() + return Response(status=status.HTTP_204_NO_CONTENT) + + +batch_delete = BatchDeleteView.as_view() + + class UserDetailsUpdaterMixin(object): def update_user_details(self, user): language = get_language_from_request(self.request) diff --git a/openwisp_radius/tests/test_api/test_api_batch.py b/openwisp_radius/tests/test_api/test_api_batch.py new file mode 100644 index 00000000..a9f09e18 --- /dev/null +++ b/openwisp_radius/tests/test_api/test_api_batch.py @@ -0,0 +1,280 @@ +import swapper +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Permission +from django.urls import reverse +from rest_framework import status + +from ...utils import load_model +from ..mixins import ApiTokenMixin, BaseTestCase + +User = get_user_model() +RadiusBatch = load_model("RadiusBatch") +OrganizationUser = swapper.load_model("openwisp_users", "OrganizationUser") + + +class TestBatchListDetailDelete(ApiTokenMixin, BaseTestCase): + def _get_auth_header(self, username="admin", password="tester"): + if username == "admin": + self._get_admin() + login_payload = {"username": username, "password": password} + login_url = reverse("radius:user_auth_token", args=[self.default_org.slug]) + login_response = self.client.post(login_url, data=login_payload) + return f"Bearer {login_response.json()['key']}" + + def _create_prefix_batch(self, name="test-prefix-batch", organization=None): + if organization is None: + organization = self.default_org + batch = RadiusBatch( + name=name, + strategy="prefix", + prefix="test", + organization=organization, + status=RadiusBatch.COMPLETED, + ) + batch.save() + return batch + + def _create_staff_user(self, username="staffuser", org=None): + user = User.objects.create_user( + username=username, + email=f"{username}@test.com", + password="tester", + is_staff=True, + is_superuser=False, + ) + if org: + OrganizationUser.objects.create(user=user, organization=org, is_admin=True) + return user + + def test_batch_list_200(self): + self._create_prefix_batch(name="batch-a") + self._create_prefix_batch(name="batch-b") + header = self._get_auth_header() + response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json()["count"], 2) + + def test_batch_list_permissions(self): + self._get_admin() + staff = self._create_staff_user("liststaff", org=self.default_org) + self._create_prefix_batch() + with self.subTest("w/o login"): + response = self.client.get(reverse("radius:batch")) + self.assertEqual(response.status_code, 401) + with self.subTest("superuser"): + header = self._get_auth_header() + response = self.client.get( + reverse("radius:batch"), HTTP_AUTHORIZATION=header + ) + self.assertEqual(response.status_code, 200) + with self.subTest("staff w/ managed org"): + header = self._get_auth_header(staff.username, "tester") + response = self.client.get( + reverse("radius:batch"), HTTP_AUTHORIZATION=header + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["count"], 1) + with self.subTest("non-staff user"): + regular = User.objects.create_user( + username="regular", email="regular@test.com", password="tester" + ) + header = self._get_auth_header(regular.username, "tester") + response = self.client.get( + reverse("radius:batch"), HTTP_AUTHORIZATION=header + ) + self.assertEqual(response.status_code, 403) + + def test_batch_list_filter_strategy(self): + self._create_prefix_batch(name="prefix-batch") + RadiusBatch.objects.create( + name="csv-batch", + strategy="csv", + organization=self.default_org, + status=RadiusBatch.COMPLETED, + ) + header = self._get_auth_header() + url = reverse("radius:batch") + with self.subTest("filter prefix"): + response = self.client.get( + url, {"strategy": "prefix"}, HTTP_AUTHORIZATION=header + ) + names = [b["name"] for b in response.json()["results"]] + self.assertIn("prefix-batch", names) + self.assertNotIn("csv-batch", names) + with self.subTest("filter csv"): + response = self.client.get( + url, {"strategy": "csv"}, HTTP_AUTHORIZATION=header + ) + names = [b["name"] for b in response.json()["results"]] + self.assertIn("csv-batch", names) + self.assertNotIn("prefix-batch", names) + + def test_batch_list_search_name(self): + self._create_prefix_batch(name="alpha-batch") + self._create_prefix_batch(name="beta-batch") + header = self._get_auth_header() + response = self.client.get( + reverse("radius:batch"), + {"search": "alpha"}, + HTTP_AUTHORIZATION=header, + ) + names = [b["name"] for b in response.json()["results"]] + self.assertIn("alpha-batch", names) + self.assertNotIn("beta-batch", names) + + def test_batch_list_no_user_credentials(self): + self._create_prefix_batch() + header = self._get_auth_header() + response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + batch_data = response.json()["results"][0] + self.assertNotIn("user_credentials", batch_data) + + def test_batch_list_exposes_download_links(self): + batch = self._create_prefix_batch() + header = self._get_auth_header() + response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + batch_data = response.json()["results"][0] + self.assertIsNotNone(batch_data["pdf_link"]) + self.assertIn(str(batch.pk), batch_data["pdf_link"]) + self.assertIsNone(batch_data["csv_link"]) + + def test_batch_detail_200(self): + batch = self._create_prefix_batch() + header = self._get_auth_header() + url = reverse("radius:batch_detail", args=[batch.pk]) + response = self.client.get(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, status.HTTP_200_OK) + data = response.json() + self.assertEqual(data["id"], str(batch.pk)) + self.assertEqual(data["name"], "test-prefix-batch") + self.assertEqual(data["strategy"], "prefix") + self.assertEqual(data["status"], RadiusBatch.COMPLETED) + self.assertNotIn("user_credentials", data) + self.assertIsNotNone(data["pdf_link"]) + + def test_batch_detail_permissions(self): + self._get_admin() + staff = self._create_staff_user("detailstaff", org=self.default_org) + batch = self._create_prefix_batch() + with self.subTest("w/o login"): + response = self.client.get(reverse("radius:batch_detail", args=[batch.pk])) + self.assertEqual(response.status_code, 401) + with self.subTest("superuser"): + header = self._get_auth_header() + response = self.client.get( + reverse("radius:batch_detail", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + self.assertEqual(response.status_code, 200) + with self.subTest("staff w/ managed org"): + header = self._get_auth_header(staff.username, "tester") + response = self.client.get( + reverse("radius:batch_detail", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + self.assertEqual(response.status_code, 200) + with self.subTest("staff w/o managed org"): + no_org_staff = self._create_staff_user("noorgstaff") + header = self._get_auth_header(no_org_staff.username, "tester") + response = self.client.get( + reverse("radius:batch_detail", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + self.assertEqual(response.status_code, 404) + + def test_batch_detail_cross_org_404(self): + org2 = self._create_org(**{"name": "other", "slug": "other"}) + batch = self._create_prefix_batch(organization=org2) + staff = self._create_staff_user("crossorgstaff", org=self.default_org) + header = self._get_auth_header(staff.username, "tester") + url = reverse("radius:batch_detail", args=[batch.pk]) + response = self.client.get(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, 404) + + def test_batch_detail_404(self): + header = self._get_auth_header() + url = reverse( + "radius:batch_detail", + args=["00000000-0000-0000-0000-000000000000"], + ) + response = self.client.get(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, 404) + + def test_batch_delete_204(self): + batch = self._create_prefix_batch() + batch_id = batch.pk + header = self._get_auth_header() + url = reverse("radius:batch_delete", args=[batch_id]) + response = self.client.delete(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertFalse(RadiusBatch.objects.filter(pk=batch_id).exists()) + + def test_batch_delete_permissions(self): + self._get_admin() + staff = self._create_staff_user("deletestaff", org=self.default_org) + delete_perm = Permission.objects.get(codename="delete_radiusbatch") + with self.subTest("w/o login"): + batch = self._create_prefix_batch(name="batch-noauth") + response = self.client.delete( + reverse("radius:batch_delete", args=[batch.pk]) + ) + self.assertEqual(response.status_code, 401) + with self.subTest("superuser"): + batch = self._create_prefix_batch(name="batch-super") + header = self._get_auth_header() + response = self.client.delete( + reverse("radius:batch_delete", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + self.assertEqual(response.status_code, 204) + with self.subTest("staff w/o delete permission"): + batch = self._create_prefix_batch(name="batch-noperm") + header = self._get_auth_header(staff.username, "tester") + response = self.client.delete( + reverse("radius:batch_delete", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + self.assertEqual(response.status_code, 403) + with self.subTest("staff w/ delete permission"): + batch = self._create_prefix_batch(name="batch-withperm") + staff.user_permissions.add(delete_perm) + header = self._get_auth_header(staff.username, "tester") + response = self.client.delete( + reverse("radius:batch_delete", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + self.assertEqual(response.status_code, 204) + + def test_batch_delete_processing_409(self): + batch = self._create_prefix_batch() + batch.status = RadiusBatch.PROCESSING + batch.save(update_fields=["status"]) + header = self._get_auth_header() + url = reverse("radius:batch_delete", args=[batch.pk]) + response = self.client.delete(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) + self.assertTrue(RadiusBatch.objects.filter(pk=batch.pk).exists()) + + def test_batch_delete_cross_org_404(self): + org2 = self._create_org(**{"name": "other", "slug": "other"}) + batch = self._create_prefix_batch(organization=org2) + staff = self._create_staff_user("delcrossorg", org=self.default_org) + delete_perm = Permission.objects.get(codename="delete_radiusbatch") + staff.user_permissions.add(delete_perm) + header = self._get_auth_header(staff.username, "tester") + url = reverse("radius:batch_delete", args=[batch.pk]) + response = self.client.delete(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, 404) + self.assertTrue(RadiusBatch.objects.filter(pk=batch.pk).exists()) + + def test_batch_delete_pending_and_failed_allowed(self): + for batch_status in [RadiusBatch.PENDING, RadiusBatch.FAILED]: + with self.subTest(status=batch_status): + batch = self._create_prefix_batch(name=f"batch-{batch_status}") + batch.status = batch_status + batch.save(update_fields=["status"]) + header = self._get_auth_header() + url = reverse("radius:batch_delete", args=[batch.pk]) + response = self.client.delete(url, HTTP_AUTHORIZATION=header) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertFalse(RadiusBatch.objects.filter(pk=batch.pk).exists()) diff --git a/tests/openwisp2/sample_radius/api/views.py b/tests/openwisp2/sample_radius/api/views.py index d5b468ef..3a77db2d 100644 --- a/tests/openwisp2/sample_radius/api/views.py +++ b/tests/openwisp2/sample_radius/api/views.py @@ -1,6 +1,8 @@ from openwisp_radius.api.freeradius_views import AccountingView as BaseAccountingView from openwisp_radius.api.freeradius_views import AuthorizeView as BaseAuthorizeView from openwisp_radius.api.freeradius_views import PostAuthView as BasePostAuthView +from openwisp_radius.api.views import BatchDeleteView as BaseBatchDeleteView +from openwisp_radius.api.views import BatchDetailView as BaseBatchDetailView from openwisp_radius.api.views import BatchView as BaseBatchView from openwisp_radius.api.views import ChangePhoneNumberView as BaseChangePhoneNumberView from openwisp_radius.api.views import CreatePhoneTokenView as BaseCreatePhoneTokenView @@ -51,6 +53,14 @@ class BatchView(BaseBatchView): pass +class BatchDetailView(BaseBatchDetailView): + pass + + +class BatchDeleteView(BaseBatchDeleteView): + pass + + class RegisterView(BaseRegisterView): pass @@ -115,6 +125,8 @@ class UpdateRegisteredUserMethodView(BaseUpdateRegisteredUserMethodView): postauth = PostAuthView.as_view() accounting = AccountingView.as_view() batch = BatchView.as_view() +batch_detail = BatchDetailView.as_view() +batch_delete = BatchDeleteView.as_view() register = RegisterView.as_view() obtain_auth_token = ObtainAuthTokenView.as_view() validate_auth_token = ValidateAuthTokenView.as_view() From 39929b9eb66d8681f3d4553fb1d1c884974cf4bc Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 09:03:29 +0530 Subject: [PATCH 02/19] [fix] Fixed RST formatting in batch API docs #771 Fixes #771 --- docs/user/rest-api.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/user/rest-api.rst b/docs/user/rest-api.rst index ab2649ad..e1301dc0 100644 --- a/docs/user/rest-api.rst +++ b/docs/user/rest-api.rst @@ -992,6 +992,7 @@ Deletes a batch user creation operation and its associated users. Returns Deletion is rejected while the batch ``status`` is ``processing``. The API returns a ``409 Conflict`` response with a clear error message in this case. Pending, completed, and failed batches can be deleted. + Batch CSV Download ++++++++++++++++++ From d8a1a37f430463974c058837caea228df8eefc15 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:07:49 +0530 Subject: [PATCH 03/19] [fix] Addressed review feedback on batch REST API endpoints #771 - Replaced UserSerializer (fields='__all__') with BatchUserSerializer that only exposes safe read-only fields (id, username, email, first_name, last_name) to prevent password hash leakage. - Made batch delete atomic using select_for_update inside transaction.atomic() to prevent race condition where a worker could set status=processing between the check and the delete. - Fixed admin delete_selected_batches to only show success message when at least one batch was actually deleted. - Added test for admin action with mixed processing/deletable batches. - Added positive-path test for RadiusBatchReadSerializer.get_csv_link. Fixes #771 --- openwisp_radius/admin.py | 11 +++++-- openwisp_radius/api/serializers.py | 15 ++++++++- openwisp_radius/api/views.py | 15 ++++----- openwisp_radius/tests/test_admin.py | 31 ++++++++++++++++++- .../tests/test_api/test_api_batch.py | 28 +++++++++++++++++ 5 files changed, 88 insertions(+), 12 deletions(-) diff --git a/openwisp_radius/admin.py b/openwisp_radius/admin.py index 4a0d681e..165d4006 100644 --- a/openwisp_radius/admin.py +++ b/openwisp_radius/admin.py @@ -507,11 +507,13 @@ def get_actions(self, request): @admin.action(description=_("Delete selected batches"), permissions=["delete"]) def delete_selected_batches(self, request, queryset): skipped = 0 + deleted = 0 for obj in queryset: if obj.status == "processing": skipped += 1 continue obj.delete() + deleted += 1 if skipped: self.message_user( request, @@ -520,9 +522,12 @@ def delete_selected_batches(self, request, queryset): ).format(count=skipped), level=messages.WARNING, ) - self.message_user( - request, "Successfully deleted selected batches.", level=messages.SUCCESS - ) + if deleted: + self.message_user( + request, + _("Successfully deleted {count} batch(es).").format(count=deleted), + level=messages.SUCCESS, + ) def get_readonly_fields(self, request, obj=None): readonly_fields = super(RadiusBatchAdmin, self).get_readonly_fields( diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index a27faf0a..8ac594cd 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -562,9 +562,22 @@ class Meta: read_only_fields = ("status", "user_credentials", "created", "modified") +class BatchUserSerializer(serializers.ModelSerializer): + class Meta: + model = User + fields = ( + "id", + "username", + "email", + "first_name", + "last_name", + ) + read_only_fields = fields + + class RadiusBatchReadSerializer(serializers.ModelSerializer): organization = serializers.PrimaryKeyRelatedField(read_only=True) - users = UserSerializer(many=True, read_only=True) + users = BatchUserSerializer(many=True, read_only=True) pdf_link = serializers.SerializerMethodField(required=False, read_only=True) csv_link = serializers.SerializerMethodField(required=False, read_only=True) status = serializers.CharField(read_only=True) diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index ff6e5f13..a175a2a8 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -266,13 +266,14 @@ class BatchDeleteView( serializer_class = RadiusBatchReadSerializer def delete(self, request, *args, **kwargs): - batch = self.get_object() - if batch.status == RadiusBatch.PROCESSING: - return Response( - {"detail": _("Cannot delete a batch while it is being processed.")}, - status=status.HTTP_409_CONFLICT, - ) - batch.delete() + with transaction.atomic(): + batch = RadiusBatch.objects.select_for_update().get(pk=self.get_object().pk) + if batch.status == RadiusBatch.PROCESSING: + return Response( + {"detail": _("Cannot delete a batch while it is being processed.")}, + status=status.HTTP_409_CONFLICT, + ) + batch.delete() return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/openwisp_radius/tests/test_admin.py b/openwisp_radius/tests/test_admin.py index 1c229d6f..7e31e933 100644 --- a/openwisp_radius/tests/test_admin.py +++ b/openwisp_radius/tests/test_admin.py @@ -402,11 +402,40 @@ def test_delete_selected_batches_action_perms(self): action="delete_selected_batches", user=user, obj=batch, - message="Successfully deleted selected batches.", + message="Successfully deleted 1 batch(es).", required_perms=["delete"], extra_payload={"_selected_action": [batch.id]}, ) + def test_delete_selected_batches_skips_processing(self): + org = self._get_org() + self._get_admin() + deletable = self._create_radius_batch( + organization=org, + name="deletable", + strategy="prefix", + prefix="test-del", + ) + processing = self._create_radius_batch( + organization=org, + name="processing", + strategy="prefix", + prefix="test-proc", + ) + processing.status = "processing" + processing.save(update_fields=["status"]) + changelist_path = reverse(f"admin:{self.app_label}_radiusbatch_changelist") + data = { + "action": "delete_selected_batches", + "_selected_action": [deletable.pk, processing.pk], + } + response = self.client.post(changelist_path, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertFalse(RadiusBatch.objects.filter(pk=deletable.pk).exists()) + self.assertTrue(RadiusBatch.objects.filter(pk=processing.pk).exists()) + self.assertContains(response, "Skipped 1 batch") + self.assertContains(response, "Successfully deleted 1 batch(es).") + def test_radius_batch_csv_help_text(self): add_url = reverse(f"admin:{self.app_label}_radiusbatch_add") response = self.client.get(add_url) diff --git a/openwisp_radius/tests/test_api/test_api_batch.py b/openwisp_radius/tests/test_api/test_api_batch.py index a9f09e18..1ab9e52e 100644 --- a/openwisp_radius/tests/test_api/test_api_batch.py +++ b/openwisp_radius/tests/test_api/test_api_batch.py @@ -1,6 +1,7 @@ import swapper from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission +from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from rest_framework import status @@ -138,6 +139,33 @@ def test_batch_list_exposes_download_links(self): self.assertIn(str(batch.pk), batch_data["pdf_link"]) self.assertIsNone(batch_data["csv_link"]) + def test_batch_csv_link_in_list_and_detail(self): + csv_content = b"user,cleartext$abcd,email@gmail.com,firstname,lastname" + csv_file = SimpleUploadedFile("test_csv_link.csv", csv_content) + batch = RadiusBatch( + name="csv-link-test", + strategy="csv", + csvfile=csv_file, + organization=self.default_org, + status=RadiusBatch.COMPLETED, + ) + batch.save() + header = self._get_auth_header() + with self.subTest("list"): + resp = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + batch_data = resp.json()["results"][0] + self.assertIsNotNone(batch_data["csv_link"]) + self.assertIn(str(batch.pk), batch_data["csv_link"]) + self.assertIsNone(batch_data["pdf_link"]) + with self.subTest("detail"): + resp = self.client.get( + reverse("radius:batch_detail", args=[batch.pk]), + HTTP_AUTHORIZATION=header, + ) + data = resp.json() + self.assertIsNotNone(data["csv_link"]) + self.assertIn(str(batch.pk), data["csv_link"]) + def test_batch_detail_200(self): batch = self._create_prefix_batch() header = self._get_auth_header() From b6cf14900ba6b44cb1f8f9822230f88947552186 Mon Sep 17 00:00:00 2001 From: chandra <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:26:58 +0530 Subject: [PATCH 04/19] Update openwisp_radius/tests/test_api/test_api_batch.py Co-authored-by: Federico Capoano --- openwisp_radius/tests/test_api/test_api_batch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_radius/tests/test_api/test_api_batch.py b/openwisp_radius/tests/test_api/test_api_batch.py index 1ab9e52e..c4699a32 100644 --- a/openwisp_radius/tests/test_api/test_api_batch.py +++ b/openwisp_radius/tests/test_api/test_api_batch.py @@ -13,7 +13,7 @@ OrganizationUser = swapper.load_model("openwisp_users", "OrganizationUser") -class TestBatchListDetailDelete(ApiTokenMixin, BaseTestCase): +class TestBatch(ApiTokenMixin, BaseTestCase): def _get_auth_header(self, username="admin", password="tester"): if username == "admin": self._get_admin() From 9a6aa10ff9ef91c9cc20df864deefc726a66f2b9 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:43:44 +0530 Subject: [PATCH 05/19] [change] Addressed review feedback on batch REST API endpoints #771 - Merged BatchDeleteView into BatchDetailView as RetrieveDestroyAPIView - Override get_object() to raise Conflict exception on processing batch delete - Added ordered queryset to BatchView to fix pagination warning - Renamed test file from test_api_batch.py to test_batch.py - Renamed test class to TestBatch and fixed duplicate method names - Used RadiusBatch.PROCESSING constant in admin and tests - Removed stale BatchDeleteView from sample app and URL config Fixes #771 --- openwisp_radius/admin.py | 2 +- openwisp_radius/api/urls.py | 5 --- openwisp_radius/api/views.py | 45 +++++++++---------- openwisp_radius/tests/test_admin.py | 2 +- .../{test_api_batch.py => test_batch.py} | 16 +++---- tests/openwisp2/sample_radius/api/views.py | 6 --- 6 files changed, 31 insertions(+), 45 deletions(-) rename openwisp_radius/tests/test_api/{test_api_batch.py => test_batch.py} (96%) diff --git a/openwisp_radius/admin.py b/openwisp_radius/admin.py index 165d4006..cef3db20 100644 --- a/openwisp_radius/admin.py +++ b/openwisp_radius/admin.py @@ -509,7 +509,7 @@ def delete_selected_batches(self, request, queryset): skipped = 0 deleted = 0 for obj in queryset: - if obj.status == "processing": + if obj.status == RadiusBatch.PROCESSING: skipped += 1 continue obj.delete() diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index 0d96ec71..aa433923 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -88,11 +88,6 @@ def get_api_urls(api_views=None): api_views.batch_detail, name="batch_detail", ), - path( - "radius/batch//delete/", - api_views.batch_delete, - name="batch_delete", - ), path( "radius/organization//batch//pdf/", api_views.download_rad_batch_pdf, diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index a175a2a8..b2550bf9 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -23,7 +23,7 @@ from rest_framework.authentication import SessionAuthentication from rest_framework.authtoken.models import Token as UserToken from rest_framework.authtoken.views import ObtainAuthToken as BaseObtainAuthToken -from rest_framework.exceptions import NotFound, PermissionDenied +from rest_framework.exceptions import APIException, NotFound, PermissionDenied from rest_framework.filters import SearchFilter from rest_framework.generics import ( CreateAPIView, @@ -153,7 +153,7 @@ class Meta(OrganizationManagedFilter.Meta): class BatchView(ThrottledAPIMixin, FilterByOrganizationManaged, ListCreateAPIView): authentication_classes = (BearerAuthentication, SessionAuthentication) permission_classes = (IsAdminUser, DjangoModelPermissions) - queryset = RadiusBatch.objects.all() + queryset = RadiusBatch.objects.all().order_by("-created") serializer_class = RadiusBatchReadSerializer filterset_class = RadiusBatchFilter filter_backends = [DjangoFilterBackend, SearchFilter] @@ -229,6 +229,12 @@ def get(self, request, *args, **kwargs): download_rad_batch_pdf = DownloadRadiusBatchPdfView.as_view() +class Conflict(APIException): + status_code = status.HTTP_409_CONFLICT + default_detail = _("Conflict.") + default_code = "conflict" + + @method_decorator( name="get", decorator=swagger_auto_schema( @@ -237,16 +243,6 @@ def get(self, request, *args, **kwargs): """, ), ) -class BatchDetailView(ProtectedAPIMixin, FilterByOrganizationManaged, RetrieveAPIView): - authentication_classes = (BearerAuthentication, SessionAuthentication) - permission_classes = (IsAdminUser, DjangoModelPermissions) - queryset = RadiusBatch.objects.all() - serializer_class = RadiusBatchReadSerializer - - -batch_detail = BatchDetailView.as_view() - - @method_decorator( name="delete", decorator=swagger_auto_schema( @@ -257,7 +253,7 @@ class BatchDetailView(ProtectedAPIMixin, FilterByOrganizationManaged, RetrieveAP responses={204: "No Content", 409: "Conflict"}, ), ) -class BatchDeleteView( +class BatchDetailView( ProtectedAPIMixin, FilterByOrganizationManaged, RetrieveDestroyAPIView ): authentication_classes = (BearerAuthentication, SessionAuthentication) @@ -265,19 +261,20 @@ class BatchDeleteView( queryset = RadiusBatch.objects.all() serializer_class = RadiusBatchReadSerializer - def delete(self, request, *args, **kwargs): - with transaction.atomic(): - batch = RadiusBatch.objects.select_for_update().get(pk=self.get_object().pk) - if batch.status == RadiusBatch.PROCESSING: - return Response( - {"detail": _("Cannot delete a batch while it is being processed.")}, - status=status.HTTP_409_CONFLICT, - ) - batch.delete() - return Response(status=status.HTTP_204_NO_CONTENT) + def get_object(self): + obj = super().get_object() + if self.request.method == "DELETE": + with transaction.atomic(): + batch = RadiusBatch.objects.select_for_update().get(pk=obj.pk) + if batch.status == RadiusBatch.PROCESSING: + raise Conflict( + _("Cannot delete a batch while it is being processed.") + ) + return batch + return obj -batch_delete = BatchDeleteView.as_view() +batch_detail = BatchDetailView.as_view() class UserDetailsUpdaterMixin(object): diff --git a/openwisp_radius/tests/test_admin.py b/openwisp_radius/tests/test_admin.py index 7e31e933..cf5f3ca3 100644 --- a/openwisp_radius/tests/test_admin.py +++ b/openwisp_radius/tests/test_admin.py @@ -422,7 +422,7 @@ def test_delete_selected_batches_skips_processing(self): strategy="prefix", prefix="test-proc", ) - processing.status = "processing" + processing.status = RadiusBatch.PROCESSING processing.save(update_fields=["status"]) changelist_path = reverse(f"admin:{self.app_label}_radiusbatch_changelist") data = { diff --git a/openwisp_radius/tests/test_api/test_api_batch.py b/openwisp_radius/tests/test_api/test_batch.py similarity index 96% rename from openwisp_radius/tests/test_api/test_api_batch.py rename to openwisp_radius/tests/test_api/test_batch.py index c4699a32..f90e1e4b 100644 --- a/openwisp_radius/tests/test_api/test_api_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -232,7 +232,7 @@ def test_batch_delete_204(self): batch = self._create_prefix_batch() batch_id = batch.pk header = self._get_auth_header() - url = reverse("radius:batch_delete", args=[batch_id]) + url = reverse("radius:batch_detail", args=[batch_id]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse(RadiusBatch.objects.filter(pk=batch_id).exists()) @@ -244,14 +244,14 @@ def test_batch_delete_permissions(self): with self.subTest("w/o login"): batch = self._create_prefix_batch(name="batch-noauth") response = self.client.delete( - reverse("radius:batch_delete", args=[batch.pk]) + reverse("radius:batch_detail", args=[batch.pk]) ) self.assertEqual(response.status_code, 401) with self.subTest("superuser"): batch = self._create_prefix_batch(name="batch-super") header = self._get_auth_header() response = self.client.delete( - reverse("radius:batch_delete", args=[batch.pk]), + reverse("radius:batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 204) @@ -259,7 +259,7 @@ def test_batch_delete_permissions(self): batch = self._create_prefix_batch(name="batch-noperm") header = self._get_auth_header(staff.username, "tester") response = self.client.delete( - reverse("radius:batch_delete", args=[batch.pk]), + reverse("radius:batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 403) @@ -268,7 +268,7 @@ def test_batch_delete_permissions(self): staff.user_permissions.add(delete_perm) header = self._get_auth_header(staff.username, "tester") response = self.client.delete( - reverse("radius:batch_delete", args=[batch.pk]), + reverse("radius:batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 204) @@ -278,7 +278,7 @@ def test_batch_delete_processing_409(self): batch.status = RadiusBatch.PROCESSING batch.save(update_fields=["status"]) header = self._get_auth_header() - url = reverse("radius:batch_delete", args=[batch.pk]) + url = reverse("radius:batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) self.assertTrue(RadiusBatch.objects.filter(pk=batch.pk).exists()) @@ -290,7 +290,7 @@ def test_batch_delete_cross_org_404(self): delete_perm = Permission.objects.get(codename="delete_radiusbatch") staff.user_permissions.add(delete_perm) header = self._get_auth_header(staff.username, "tester") - url = reverse("radius:batch_delete", args=[batch.pk]) + url = reverse("radius:batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, 404) self.assertTrue(RadiusBatch.objects.filter(pk=batch.pk).exists()) @@ -302,7 +302,7 @@ def test_batch_delete_pending_and_failed_allowed(self): batch.status = batch_status batch.save(update_fields=["status"]) header = self._get_auth_header() - url = reverse("radius:batch_delete", args=[batch.pk]) + url = reverse("radius:batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse(RadiusBatch.objects.filter(pk=batch.pk).exists()) diff --git a/tests/openwisp2/sample_radius/api/views.py b/tests/openwisp2/sample_radius/api/views.py index 3a77db2d..9f0a9367 100644 --- a/tests/openwisp2/sample_radius/api/views.py +++ b/tests/openwisp2/sample_radius/api/views.py @@ -1,7 +1,6 @@ from openwisp_radius.api.freeradius_views import AccountingView as BaseAccountingView from openwisp_radius.api.freeradius_views import AuthorizeView as BaseAuthorizeView from openwisp_radius.api.freeradius_views import PostAuthView as BasePostAuthView -from openwisp_radius.api.views import BatchDeleteView as BaseBatchDeleteView from openwisp_radius.api.views import BatchDetailView as BaseBatchDetailView from openwisp_radius.api.views import BatchView as BaseBatchView from openwisp_radius.api.views import ChangePhoneNumberView as BaseChangePhoneNumberView @@ -57,10 +56,6 @@ class BatchDetailView(BaseBatchDetailView): pass -class BatchDeleteView(BaseBatchDeleteView): - pass - - class RegisterView(BaseRegisterView): pass @@ -126,7 +121,6 @@ class UpdateRegisteredUserMethodView(BaseUpdateRegisteredUserMethodView): accounting = AccountingView.as_view() batch = BatchView.as_view() batch_detail = BatchDetailView.as_view() -batch_delete = BatchDeleteView.as_view() register = RegisterView.as_view() obtain_auth_token = ObtainAuthTokenView.as_view() validate_auth_token = ValidateAuthTokenView.as_view() From cc9525fbb84e0d09c1867118629f06b28812c3c9 Mon Sep 17 00:00:00 2001 From: chandra <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:50:49 +0530 Subject: [PATCH 06/19] [fix] File name --- openwisp_radius/tests/test_api/{test_batch.py => test.batch.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename openwisp_radius/tests/test_api/{test_batch.py => test.batch.py} (100%) diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test.batch.py similarity index 100% rename from openwisp_radius/tests/test_api/test_batch.py rename to openwisp_radius/tests/test_api/test.batch.py From 10c44d4fb3ee12ffa901376f9f3781a0595a1f16 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:59:11 +0530 Subject: [PATCH 07/19] [change] Added query optimization and sample-app tests for batch endpoints #771 - Added select_related/prefetch_related to BatchView and BatchDetailView - Added assertNumQueries(4) to batch list test for query budget - Added sample-app TestBatch regression test for SAMPLE_APP=1 coverage Fixes #771 --- openwisp_radius/api/views.py | 10 ++++++++-- openwisp_radius/tests/test_api/test.batch.py | 3 ++- tests/openwisp2/sample_radius/tests.py | 6 ++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index b2550bf9..a891a778 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -153,7 +153,11 @@ class Meta(OrganizationManagedFilter.Meta): class BatchView(ThrottledAPIMixin, FilterByOrganizationManaged, ListCreateAPIView): authentication_classes = (BearerAuthentication, SessionAuthentication) permission_classes = (IsAdminUser, DjangoModelPermissions) - queryset = RadiusBatch.objects.all().order_by("-created") + queryset = ( + RadiusBatch.objects.select_related("organization") + .prefetch_related("users") + .order_by("-created") + ) serializer_class = RadiusBatchReadSerializer filterset_class = RadiusBatchFilter filter_backends = [DjangoFilterBackend, SearchFilter] @@ -258,7 +262,9 @@ class BatchDetailView( ): authentication_classes = (BearerAuthentication, SessionAuthentication) permission_classes = (IsAdminUser, DjangoModelPermissions) - queryset = RadiusBatch.objects.all() + queryset = RadiusBatch.objects.select_related("organization").prefetch_related( + "users" + ) serializer_class = RadiusBatchReadSerializer def get_object(self): diff --git a/openwisp_radius/tests/test_api/test.batch.py b/openwisp_radius/tests/test_api/test.batch.py index f90e1e4b..52508328 100644 --- a/openwisp_radius/tests/test_api/test.batch.py +++ b/openwisp_radius/tests/test_api/test.batch.py @@ -51,7 +51,8 @@ def test_batch_list_200(self): self._create_prefix_batch(name="batch-a") self._create_prefix_batch(name="batch-b") header = self._get_auth_header() - response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + with self.assertNumQueries(4): + response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()["count"], 2) diff --git a/tests/openwisp2/sample_radius/tests.py b/tests/openwisp2/sample_radius/tests.py index 0d01f8ac..256c3a0e 100644 --- a/tests/openwisp2/sample_radius/tests.py +++ b/tests/openwisp2/sample_radius/tests.py @@ -1,6 +1,7 @@ from openwisp_radius.tests import test_migrations as base_migration_tests from openwisp_radius.tests.test_admin import TestAdmin as BaseTestAdmin from openwisp_radius.tests.test_api.test_api import TestApi as BaseTestApi +from openwisp_radius.tests.test_api.test_batch import TestBatch as BaseTestBatch from openwisp_radius.tests.test_api.test_freeradius_api import ( TestApiReject as BaseTestApiReject, ) @@ -80,6 +81,10 @@ class TestApi(BaseTestApi): pass +class TestBatch(BaseTestBatch): + pass + + class TestFreeradiusApi(BaseTestFreeradiusApi): pass @@ -207,6 +212,7 @@ class TestPhoneTokenOrganizationPopulateResolution( del BaseTestAdmin del BaseTestApi +del BaseTestBatch del BaseTestFreeradiusApi del BaseTestApiReject del BaseTestAutoGroupname From 7c78a997756af08df11404e62d6a7f039fedc79f Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:14:00 +0530 Subject: [PATCH 08/19] [fix] Renamed test.batch.py to test_batch.py #771 Fixes #771 --- openwisp_radius/tests/test_api/{test.batch.py => test_batch.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename openwisp_radius/tests/test_api/{test.batch.py => test_batch.py} (100%) diff --git a/openwisp_radius/tests/test_api/test.batch.py b/openwisp_radius/tests/test_api/test_batch.py similarity index 100% rename from openwisp_radius/tests/test_api/test.batch.py rename to openwisp_radius/tests/test_api/test_batch.py From 63da3bc5fd9e7a51b5cdcc135d16f491482c7241 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:33:14 +0530 Subject: [PATCH 09/19] [fix] Fixed Black/flake8 formatting in batch test #771 Fixes #771 --- openwisp_radius/tests/test_api/test_batch.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index 52508328..234b721a 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -52,7 +52,9 @@ def test_batch_list_200(self): self._create_prefix_batch(name="batch-b") header = self._get_auth_header() with self.assertNumQueries(4): - response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + response = self.client.get( + reverse("radius:batch"), HTTP_AUTHORIZATION=header + ) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()["count"], 2) From 52899925ba3a853775f7f2e8c4690884bd29bda5 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:50:24 +0530 Subject: [PATCH 10/19] [fix] Fixed race condition in batch delete atomicity #771 Moved status check and delete into perform_destroy() inside a single transaction.atomic() block with select_for_update(), so the lock is held for the entire check+delete operation. Fixes #771 --- openwisp_radius/api/views.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index a891a778..90cff075 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -267,17 +267,12 @@ class BatchDetailView( ) serializer_class = RadiusBatchReadSerializer - def get_object(self): - obj = super().get_object() - if self.request.method == "DELETE": - with transaction.atomic(): - batch = RadiusBatch.objects.select_for_update().get(pk=obj.pk) - if batch.status == RadiusBatch.PROCESSING: - raise Conflict( - _("Cannot delete a batch while it is being processed.") - ) - return batch - return obj + def perform_destroy(self, instance): + with transaction.atomic(): + batch = RadiusBatch.objects.select_for_update().get(pk=instance.pk) + if batch.status == RadiusBatch.PROCESSING: + raise Conflict(_("Cannot delete a batch while it is being processed.")) + batch.delete() batch_detail = BatchDetailView.as_view() From fe04cdc9cf9c17407c68723fc0e05a63f9970fba Mon Sep 17 00:00:00 2001 From: chandra <214236921+BHARATH0153@users.noreply.github.com> Date: Fri, 21 Aug 2026 06:22:55 +0530 Subject: [PATCH 11/19] Update openwisp_radius/api/urls.py Co-authored-by: Federico Capoano --- openwisp_radius/api/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index aa433923..0b4f892c 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -86,7 +86,7 @@ def get_api_urls(api_views=None): path( "radius/batch//", api_views.batch_detail, - name="batch_detail", + name="radius_batch_detail", ), path( "radius/organization//batch//pdf/", From 4c6fc285634c0d9b656bd7608f4097b3a808b575 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Fri, 21 Aug 2026 06:36:14 +0530 Subject: [PATCH 12/19] [change] Addressed nemesifier review feedback #771 - Updated conflict message to 'The radius batch object is currently being processed and cannot be deleted.' - Removed redundant isinstance(obj, RadiusBatch) check in get_pdf_link - Added blank lines before each subTest in permission tests - Added assertion on conflict message content in test Fixes #771 --- openwisp_radius/api/serializers.py | 6 +----- openwisp_radius/api/views.py | 7 ++++++- openwisp_radius/tests/test_api/test_batch.py | 10 ++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 8ac594cd..92d9ff26 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -583,11 +583,7 @@ class RadiusBatchReadSerializer(serializers.ModelSerializer): status = serializers.CharField(read_only=True) def get_pdf_link(self, obj): - if ( - isinstance(obj, RadiusBatch) - and obj.strategy == "prefix" - and obj.status == RadiusBatch.COMPLETED - ): + if obj.strategy == "prefix" and obj.status == RadiusBatch.COMPLETED: request = self.context.get("request") return request.build_absolute_uri( reverse( diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 90cff075..f471615b 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -271,7 +271,12 @@ def perform_destroy(self, instance): with transaction.atomic(): batch = RadiusBatch.objects.select_for_update().get(pk=instance.pk) if batch.status == RadiusBatch.PROCESSING: - raise Conflict(_("Cannot delete a batch while it is being processed.")) + raise Conflict( + _( + "The radius batch object is currently being processed" + " and cannot be deleted." + ) + ) batch.delete() diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index 234b721a..77005049 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -65,12 +65,14 @@ def test_batch_list_permissions(self): with self.subTest("w/o login"): response = self.client.get(reverse("radius:batch")) self.assertEqual(response.status_code, 401) + with self.subTest("superuser"): header = self._get_auth_header() response = self.client.get( reverse("radius:batch"), HTTP_AUTHORIZATION=header ) self.assertEqual(response.status_code, 200) + with self.subTest("staff w/ managed org"): header = self._get_auth_header(staff.username, "tester") response = self.client.get( @@ -78,6 +80,7 @@ def test_batch_list_permissions(self): ) self.assertEqual(response.status_code, 200) self.assertEqual(response.json()["count"], 1) + with self.subTest("non-staff user"): regular = User.objects.create_user( username="regular", email="regular@test.com", password="tester" @@ -250,6 +253,7 @@ def test_batch_delete_permissions(self): reverse("radius:batch_detail", args=[batch.pk]) ) self.assertEqual(response.status_code, 401) + with self.subTest("superuser"): batch = self._create_prefix_batch(name="batch-super") header = self._get_auth_header() @@ -258,6 +262,7 @@ def test_batch_delete_permissions(self): HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 204) + with self.subTest("staff w/o delete permission"): batch = self._create_prefix_batch(name="batch-noperm") header = self._get_auth_header(staff.username, "tester") @@ -266,6 +271,7 @@ def test_batch_delete_permissions(self): HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 403) + with self.subTest("staff w/ delete permission"): batch = self._create_prefix_batch(name="batch-withperm") staff.user_permissions.add(delete_perm) @@ -284,6 +290,10 @@ def test_batch_delete_processing_409(self): url = reverse("radius:batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) + self.assertIn( + "currently being processed and cannot be deleted", + response.json()["detail"], + ) self.assertTrue(RadiusBatch.objects.filter(pk=batch.pk).exists()) def test_batch_delete_cross_org_404(self): From cb4f05e0f38d612f5ca6c3abf38de488826b12de Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Fri, 21 Aug 2026 06:53:06 +0530 Subject: [PATCH 13/19] [fix] Fixed URL name mismatch for batch detail endpoint #771 Rebase introduced radius_batch_detail URL name from upstream, but tests use batch_detail. Restored batch_detail to match test usage. Fixes #771 --- openwisp_radius/api/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index 0b4f892c..aa433923 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -86,7 +86,7 @@ def get_api_urls(api_views=None): path( "radius/batch//", api_views.batch_detail, - name="radius_batch_detail", + name="batch_detail", ), path( "radius/organization//batch//pdf/", From aefe230d412e16ca68b9fd5ea881c2814be3cc06 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Fri, 21 Aug 2026 07:01:46 +0530 Subject: [PATCH 14/19] [change] Renamed URL name to radius_batch_detail per convention #771 Fixes #771 --- openwisp_radius/api/urls.py | 2 +- openwisp_radius/tests/test_api/test_batch.py | 34 +++++++++++--------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index aa433923..0b4f892c 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -86,7 +86,7 @@ def get_api_urls(api_views=None): path( "radius/batch//", api_views.batch_detail, - name="batch_detail", + name="radius_batch_detail", ), path( "radius/organization//batch//pdf/", diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index 77005049..6b901100 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -165,7 +165,7 @@ def test_batch_csv_link_in_list_and_detail(self): self.assertIsNone(batch_data["pdf_link"]) with self.subTest("detail"): resp = self.client.get( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) data = resp.json() @@ -175,7 +175,7 @@ def test_batch_csv_link_in_list_and_detail(self): def test_batch_detail_200(self): batch = self._create_prefix_batch() header = self._get_auth_header() - url = reverse("radius:batch_detail", args=[batch.pk]) + url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.get(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -191,19 +191,21 @@ def test_batch_detail_permissions(self): staff = self._create_staff_user("detailstaff", org=self.default_org) batch = self._create_prefix_batch() with self.subTest("w/o login"): - response = self.client.get(reverse("radius:batch_detail", args=[batch.pk])) + response = self.client.get( + reverse("radius:radius_batch_detail", args=[batch.pk]) + ) self.assertEqual(response.status_code, 401) with self.subTest("superuser"): header = self._get_auth_header() response = self.client.get( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 200) with self.subTest("staff w/ managed org"): header = self._get_auth_header(staff.username, "tester") response = self.client.get( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 200) @@ -211,7 +213,7 @@ def test_batch_detail_permissions(self): no_org_staff = self._create_staff_user("noorgstaff") header = self._get_auth_header(no_org_staff.username, "tester") response = self.client.get( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 404) @@ -221,14 +223,14 @@ def test_batch_detail_cross_org_404(self): batch = self._create_prefix_batch(organization=org2) staff = self._create_staff_user("crossorgstaff", org=self.default_org) header = self._get_auth_header(staff.username, "tester") - url = reverse("radius:batch_detail", args=[batch.pk]) + url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.get(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, 404) def test_batch_detail_404(self): header = self._get_auth_header() url = reverse( - "radius:batch_detail", + "radius:radius_batch_detail", args=["00000000-0000-0000-0000-000000000000"], ) response = self.client.get(url, HTTP_AUTHORIZATION=header) @@ -238,7 +240,7 @@ def test_batch_delete_204(self): batch = self._create_prefix_batch() batch_id = batch.pk header = self._get_auth_header() - url = reverse("radius:batch_detail", args=[batch_id]) + url = reverse("radius:radius_batch_detail", args=[batch_id]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse(RadiusBatch.objects.filter(pk=batch_id).exists()) @@ -250,7 +252,7 @@ def test_batch_delete_permissions(self): with self.subTest("w/o login"): batch = self._create_prefix_batch(name="batch-noauth") response = self.client.delete( - reverse("radius:batch_detail", args=[batch.pk]) + reverse("radius:radius_batch_detail", args=[batch.pk]) ) self.assertEqual(response.status_code, 401) @@ -258,7 +260,7 @@ def test_batch_delete_permissions(self): batch = self._create_prefix_batch(name="batch-super") header = self._get_auth_header() response = self.client.delete( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 204) @@ -267,7 +269,7 @@ def test_batch_delete_permissions(self): batch = self._create_prefix_batch(name="batch-noperm") header = self._get_auth_header(staff.username, "tester") response = self.client.delete( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 403) @@ -277,7 +279,7 @@ def test_batch_delete_permissions(self): staff.user_permissions.add(delete_perm) header = self._get_auth_header(staff.username, "tester") response = self.client.delete( - reverse("radius:batch_detail", args=[batch.pk]), + reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 204) @@ -287,7 +289,7 @@ def test_batch_delete_processing_409(self): batch.status = RadiusBatch.PROCESSING batch.save(update_fields=["status"]) header = self._get_auth_header() - url = reverse("radius:batch_detail", args=[batch.pk]) + url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) self.assertIn( @@ -303,7 +305,7 @@ def test_batch_delete_cross_org_404(self): delete_perm = Permission.objects.get(codename="delete_radiusbatch") staff.user_permissions.add(delete_perm) header = self._get_auth_header(staff.username, "tester") - url = reverse("radius:batch_detail", args=[batch.pk]) + url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, 404) self.assertTrue(RadiusBatch.objects.filter(pk=batch.pk).exists()) @@ -315,7 +317,7 @@ def test_batch_delete_pending_and_failed_allowed(self): batch.status = batch_status batch.save(update_fields=["status"]) header = self._get_auth_header() - url = reverse("radius:batch_detail", args=[batch.pk]) + url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse(RadiusBatch.objects.filter(pk=batch.pk).exists()) From fdabb848a7482bdc0f974729711a501ef7977315 Mon Sep 17 00:00:00 2001 From: chandra <214236921+BHARATH0153@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:56:13 +0530 Subject: [PATCH 15/19] Update openwisp_radius/api/urls.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- openwisp_radius/api/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_radius/api/urls.py b/openwisp_radius/api/urls.py index 52701bbc..286354f4 100644 --- a/openwisp_radius/api/urls.py +++ b/openwisp_radius/api/urls.py @@ -90,7 +90,7 @@ def get_view(name): path("radius/batch/", get_view("batch"), name="batch"), path( "radius/batch//", - api_views.batch_detail, + get_view("batch_detail"), name="radius_batch_detail", ), path( From 6771df0515ab138c70e2bcd0d153babb9748d0cb Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:04:15 +0530 Subject: [PATCH 16/19] [change] Simplify perform_destroy and rewrite batch test helpers --- openwisp_radius/api/views.py | 16 ++- openwisp_radius/tests/test_api/test_batch.py | 109 ++++++++++++------- 2 files changed, 75 insertions(+), 50 deletions(-) diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index f471615b..bd60441d 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -268,16 +268,14 @@ class BatchDetailView( serializer_class = RadiusBatchReadSerializer def perform_destroy(self, instance): - with transaction.atomic(): - batch = RadiusBatch.objects.select_for_update().get(pk=instance.pk) - if batch.status == RadiusBatch.PROCESSING: - raise Conflict( - _( - "The radius batch object is currently being processed" - " and cannot be deleted." - ) + if instance.status == RadiusBatch.PROCESSING: + raise Conflict( + _( + "The radius batch object is currently being processed" + " and cannot be deleted." ) - batch.delete() + ) + instance.delete() batch_detail = BatchDetailView.as_view() diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index 6b901100..85e83681 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -1,6 +1,4 @@ -import swapper from django.contrib.auth import get_user_model -from django.contrib.auth.models import Permission from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from rest_framework import status @@ -10,7 +8,6 @@ User = get_user_model() RadiusBatch = load_model("RadiusBatch") -OrganizationUser = swapper.load_model("openwisp_users", "OrganizationUser") class TestBatch(ApiTokenMixin, BaseTestCase): @@ -23,29 +20,15 @@ def _get_auth_header(self, username="admin", password="tester"): return f"Bearer {login_response.json()['key']}" def _create_prefix_batch(self, name="test-prefix-batch", organization=None): - if organization is None: - organization = self.default_org - batch = RadiusBatch( - name=name, - strategy="prefix", - prefix="test", - organization=organization, - status=RadiusBatch.COMPLETED, - ) - batch.save() - return batch - - def _create_staff_user(self, username="staffuser", org=None): - user = User.objects.create_user( - username=username, - email=f"{username}@test.com", - password="tester", - is_staff=True, - is_superuser=False, - ) - if org: - OrganizationUser.objects.create(user=user, organization=org, is_admin=True) - return user + kwargs = { + "name": name, + "strategy": "prefix", + "prefix": "test", + "status": RadiusBatch.COMPLETED, + } + if organization is not None: + kwargs["organization"] = organization + return self._create_radius_batch(**kwargs) def test_batch_list_200(self): self._create_prefix_batch(name="batch-a") @@ -60,7 +43,6 @@ def test_batch_list_200(self): def test_batch_list_permissions(self): self._get_admin() - staff = self._create_staff_user("liststaff", org=self.default_org) self._create_prefix_batch() with self.subTest("w/o login"): response = self.client.get(reverse("radius:batch")) @@ -74,6 +56,11 @@ def test_batch_list_permissions(self): self.assertEqual(response.status_code, 200) with self.subTest("staff w/ managed org"): + staff = self._create_operator( + organizations=[self.default_org], + username="liststaff", + email="liststaff@test.com", + ) header = self._get_auth_header(staff.username, "tester") response = self.client.get( reverse("radius:batch"), HTTP_AUTHORIZATION=header @@ -93,9 +80,12 @@ def test_batch_list_permissions(self): def test_batch_list_filter_strategy(self): self._create_prefix_batch(name="prefix-batch") + csv_content = b"user,cleartext$abcd,email@gmail.com,firstname,lastname" + csv_file = SimpleUploadedFile("filter_test.csv", csv_content) RadiusBatch.objects.create( name="csv-batch", strategy="csv", + csvfile=csv_file, organization=self.default_org, status=RadiusBatch.COMPLETED, ) @@ -108,6 +98,7 @@ def test_batch_list_filter_strategy(self): names = [b["name"] for b in response.json()["results"]] self.assertIn("prefix-batch", names) self.assertNotIn("csv-batch", names) + with self.subTest("filter csv"): response = self.client.get( url, {"strategy": "csv"}, HTTP_AUTHORIZATION=header @@ -116,6 +107,16 @@ def test_batch_list_filter_strategy(self): self.assertIn("csv-batch", names) self.assertNotIn("prefix-batch", names) + def test_batch_list_filter_organization(self): + org2 = self._create_org(**{"name": "other", "slug": "other"}) + self._create_prefix_batch(name="org1-batch") + self._create_prefix_batch(name="org2-batch", organization=org2) + header = self._get_auth_header() + response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + names = [b["name"] for b in response.json()["results"]] + self.assertIn("org1-batch", names) + self.assertNotIn("org2-batch", names) + def test_batch_list_search_name(self): self._create_prefix_batch(name="alpha-batch") self._create_prefix_batch(name="beta-batch") @@ -163,6 +164,7 @@ def test_batch_csv_link_in_list_and_detail(self): self.assertIsNotNone(batch_data["csv_link"]) self.assertIn(str(batch.pk), batch_data["csv_link"]) self.assertIsNone(batch_data["pdf_link"]) + with self.subTest("detail"): resp = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]), @@ -188,13 +190,13 @@ def test_batch_detail_200(self): def test_batch_detail_permissions(self): self._get_admin() - staff = self._create_staff_user("detailstaff", org=self.default_org) batch = self._create_prefix_batch() with self.subTest("w/o login"): response = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]) ) self.assertEqual(response.status_code, 401) + with self.subTest("superuser"): header = self._get_auth_header() response = self.client.get( @@ -202,15 +204,27 @@ def test_batch_detail_permissions(self): HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 200) + with self.subTest("staff w/ managed org"): + staff = self._create_operator( + organizations=[self.default_org], + username="detailstaff", + email="detailstaff@test.com", + ) header = self._get_auth_header(staff.username, "tester") response = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 200) + with self.subTest("staff w/o managed org"): - no_org_staff = self._create_staff_user("noorgstaff") + org2 = self._create_org(**{"name": "other", "slug": "other"}) + no_org_staff = self._create_operator( + organizations=[org2], + username="noorgstaff", + email="noorgstaff@test.com", + ) header = self._get_auth_header(no_org_staff.username, "tester") response = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]), @@ -221,7 +235,11 @@ def test_batch_detail_permissions(self): def test_batch_detail_cross_org_404(self): org2 = self._create_org(**{"name": "other", "slug": "other"}) batch = self._create_prefix_batch(organization=org2) - staff = self._create_staff_user("crossorgstaff", org=self.default_org) + staff = self._create_operator( + organizations=[self.default_org], + username="crossorgstaff", + email="crossorgstaff@test.com", + ) header = self._get_auth_header(staff.username, "tester") url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.get(url, HTTP_AUTHORIZATION=header) @@ -247,8 +265,16 @@ def test_batch_delete_204(self): def test_batch_delete_permissions(self): self._get_admin() - staff = self._create_staff_user("deletestaff", org=self.default_org) - delete_perm = Permission.objects.get(codename="delete_radiusbatch") + operator = self._create_operator( + organizations=[self.default_org], + username="deletestaff", + email="deletestaff@test.com", + ) + administrator = self._create_administrator( + organizations=[self.default_org], + username="deletadmin", + email="deletadmin@test.com", + ) with self.subTest("w/o login"): batch = self._create_prefix_batch(name="batch-noauth") response = self.client.delete( @@ -265,19 +291,18 @@ def test_batch_delete_permissions(self): ) self.assertEqual(response.status_code, 204) - with self.subTest("staff w/o delete permission"): + with self.subTest("operator w/o delete permission"): batch = self._create_prefix_batch(name="batch-noperm") - header = self._get_auth_header(staff.username, "tester") + header = self._get_auth_header(operator.username, "tester") response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 403) - with self.subTest("staff w/ delete permission"): + with self.subTest("administrator w/ delete permission"): batch = self._create_prefix_batch(name="batch-withperm") - staff.user_permissions.add(delete_perm) - header = self._get_auth_header(staff.username, "tester") + header = self._get_auth_header(administrator.username, "tester") response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]), HTTP_AUTHORIZATION=header, @@ -301,10 +326,12 @@ def test_batch_delete_processing_409(self): def test_batch_delete_cross_org_404(self): org2 = self._create_org(**{"name": "other", "slug": "other"}) batch = self._create_prefix_batch(organization=org2) - staff = self._create_staff_user("delcrossorg", org=self.default_org) - delete_perm = Permission.objects.get(codename="delete_radiusbatch") - staff.user_permissions.add(delete_perm) - header = self._get_auth_header(staff.username, "tester") + administrator = self._create_administrator( + organizations=[self.default_org], + username="delcrossorg", + email="delcrossorg@test.com", + ) + header = self._get_auth_header(administrator.username, "tester") url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) self.assertEqual(response.status_code, 404) From d6f4943f58803e145606b80186ed80dafb8b5787 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:30:09 +0530 Subject: [PATCH 17/19] [change] Remove _create_prefix_batch wrapper, use _create_radius_batch directly --- openwisp_radius/tests/test_api/test_batch.py | 177 ++++++++++++++----- 1 file changed, 137 insertions(+), 40 deletions(-) diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index 85e83681..82832a36 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -19,20 +19,19 @@ def _get_auth_header(self, username="admin", password="tester"): login_response = self.client.post(login_url, data=login_payload) return f"Bearer {login_response.json()['key']}" - def _create_prefix_batch(self, name="test-prefix-batch", organization=None): - kwargs = { - "name": name, - "strategy": "prefix", - "prefix": "test", - "status": RadiusBatch.COMPLETED, - } - if organization is not None: - kwargs["organization"] = organization - return self._create_radius_batch(**kwargs) - def test_batch_list_200(self): - self._create_prefix_batch(name="batch-a") - self._create_prefix_batch(name="batch-b") + self._create_radius_batch( + name="batch-a", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) + self._create_radius_batch( + name="batch-b", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header() with self.assertNumQueries(4): response = self.client.get( @@ -43,7 +42,12 @@ def test_batch_list_200(self): def test_batch_list_permissions(self): self._get_admin() - self._create_prefix_batch() + self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) with self.subTest("w/o login"): response = self.client.get(reverse("radius:batch")) self.assertEqual(response.status_code, 401) @@ -79,14 +83,18 @@ def test_batch_list_permissions(self): self.assertEqual(response.status_code, 403) def test_batch_list_filter_strategy(self): - self._create_prefix_batch(name="prefix-batch") + self._create_radius_batch( + name="prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) csv_content = b"user,cleartext$abcd,email@gmail.com,firstname,lastname" csv_file = SimpleUploadedFile("filter_test.csv", csv_content) - RadiusBatch.objects.create( + self._create_radius_batch( name="csv-batch", strategy="csv", csvfile=csv_file, - organization=self.default_org, status=RadiusBatch.COMPLETED, ) header = self._get_auth_header() @@ -109,17 +117,43 @@ def test_batch_list_filter_strategy(self): def test_batch_list_filter_organization(self): org2 = self._create_org(**{"name": "other", "slug": "other"}) - self._create_prefix_batch(name="org1-batch") - self._create_prefix_batch(name="org2-batch", organization=org2) - header = self._get_auth_header() + self._create_radius_batch( + name="org1-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) + self._create_radius_batch( + name="org2-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + organization=org2, + ) + operator = self._create_operator( + organizations=[self.default_org], + username="orgfilterstaff", + email="orgfilterstaff@test.com", + ) + header = self._get_auth_header(operator.username, "tester") response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) names = [b["name"] for b in response.json()["results"]] self.assertIn("org1-batch", names) self.assertNotIn("org2-batch", names) def test_batch_list_search_name(self): - self._create_prefix_batch(name="alpha-batch") - self._create_prefix_batch(name="beta-batch") + self._create_radius_batch( + name="alpha-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) + self._create_radius_batch( + name="beta-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header() response = self.client.get( reverse("radius:batch"), @@ -131,14 +165,24 @@ def test_batch_list_search_name(self): self.assertNotIn("beta-batch", names) def test_batch_list_no_user_credentials(self): - self._create_prefix_batch() + self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header() response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) batch_data = response.json()["results"][0] self.assertNotIn("user_credentials", batch_data) def test_batch_list_exposes_download_links(self): - batch = self._create_prefix_batch() + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header() response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) batch_data = response.json()["results"][0] @@ -149,14 +193,12 @@ def test_batch_list_exposes_download_links(self): def test_batch_csv_link_in_list_and_detail(self): csv_content = b"user,cleartext$abcd,email@gmail.com,firstname,lastname" csv_file = SimpleUploadedFile("test_csv_link.csv", csv_content) - batch = RadiusBatch( + batch = self._create_radius_batch( name="csv-link-test", strategy="csv", csvfile=csv_file, - organization=self.default_org, status=RadiusBatch.COMPLETED, ) - batch.save() header = self._get_auth_header() with self.subTest("list"): resp = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) @@ -175,7 +217,12 @@ def test_batch_csv_link_in_list_and_detail(self): self.assertIn(str(batch.pk), data["csv_link"]) def test_batch_detail_200(self): - batch = self._create_prefix_batch() + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header() url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.get(url, HTTP_AUTHORIZATION=header) @@ -190,7 +237,12 @@ def test_batch_detail_200(self): def test_batch_detail_permissions(self): self._get_admin() - batch = self._create_prefix_batch() + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) with self.subTest("w/o login"): response = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]) @@ -234,7 +286,13 @@ def test_batch_detail_permissions(self): def test_batch_detail_cross_org_404(self): org2 = self._create_org(**{"name": "other", "slug": "other"}) - batch = self._create_prefix_batch(organization=org2) + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + organization=org2, + ) staff = self._create_operator( organizations=[self.default_org], username="crossorgstaff", @@ -255,7 +313,12 @@ def test_batch_detail_404(self): self.assertEqual(response.status_code, 404) def test_batch_delete_204(self): - batch = self._create_prefix_batch() + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) batch_id = batch.pk header = self._get_auth_header() url = reverse("radius:radius_batch_detail", args=[batch_id]) @@ -276,14 +339,24 @@ def test_batch_delete_permissions(self): email="deletadmin@test.com", ) with self.subTest("w/o login"): - batch = self._create_prefix_batch(name="batch-noauth") + batch = self._create_radius_batch( + name="batch-noauth", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]) ) self.assertEqual(response.status_code, 401) with self.subTest("superuser"): - batch = self._create_prefix_batch(name="batch-super") + batch = self._create_radius_batch( + name="batch-super", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header() response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]), @@ -292,7 +365,12 @@ def test_batch_delete_permissions(self): self.assertEqual(response.status_code, 204) with self.subTest("operator w/o delete permission"): - batch = self._create_prefix_batch(name="batch-noperm") + batch = self._create_radius_batch( + name="batch-noperm", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header(operator.username, "tester") response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]), @@ -301,7 +379,12 @@ def test_batch_delete_permissions(self): self.assertEqual(response.status_code, 403) with self.subTest("administrator w/ delete permission"): - batch = self._create_prefix_batch(name="batch-withperm") + batch = self._create_radius_batch( + name="batch-withperm", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) header = self._get_auth_header(administrator.username, "tester") response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]), @@ -310,7 +393,12 @@ def test_batch_delete_permissions(self): self.assertEqual(response.status_code, 204) def test_batch_delete_processing_409(self): - batch = self._create_prefix_batch() + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + ) batch.status = RadiusBatch.PROCESSING batch.save(update_fields=["status"]) header = self._get_auth_header() @@ -325,7 +413,13 @@ def test_batch_delete_processing_409(self): def test_batch_delete_cross_org_404(self): org2 = self._create_org(**{"name": "other", "slug": "other"}) - batch = self._create_prefix_batch(organization=org2) + batch = self._create_radius_batch( + name="test-prefix-batch", + strategy="prefix", + prefix="test", + status=RadiusBatch.COMPLETED, + organization=org2, + ) administrator = self._create_administrator( organizations=[self.default_org], username="delcrossorg", @@ -340,9 +434,12 @@ def test_batch_delete_cross_org_404(self): def test_batch_delete_pending_and_failed_allowed(self): for batch_status in [RadiusBatch.PENDING, RadiusBatch.FAILED]: with self.subTest(status=batch_status): - batch = self._create_prefix_batch(name=f"batch-{batch_status}") - batch.status = batch_status - batch.save(update_fields=["status"]) + batch = self._create_radius_batch( + name=f"batch-{batch_status}", + strategy="prefix", + prefix="test", + status=batch_status, + ) header = self._get_auth_header() url = reverse("radius:radius_batch_detail", args=[batch.pk]) response = self.client.delete(url, HTTP_AUTHORIZATION=header) From a13180509f3544b10513953781f6d3a46272218c Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:01:29 +0530 Subject: [PATCH 18/19] [change] Use _superuser_login for admin tests, simplify _get_auth_header --- openwisp_radius/tests/test_api/test_batch.py | 75 ++++++++------------ 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index 82832a36..fad0b4af 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -11,13 +11,11 @@ class TestBatch(ApiTokenMixin, BaseTestCase): - def _get_auth_header(self, username="admin", password="tester"): - if username == "admin": - self._get_admin() + def _get_auth_header(self, username="tester", password="tester"): login_payload = {"username": username, "password": password} login_url = reverse("radius:user_auth_token", args=[self.default_org.slug]) - login_response = self.client.post(login_url, data=login_payload) - return f"Bearer {login_response.json()['key']}" + response = self.client.post(login_url, data=login_payload) + return f"Bearer {response.json()['key']}" def test_batch_list_200(self): self._create_radius_batch( @@ -32,16 +30,13 @@ def test_batch_list_200(self): prefix="test", status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() + self._superuser_login() with self.assertNumQueries(4): - response = self.client.get( - reverse("radius:batch"), HTTP_AUTHORIZATION=header - ) + response = self.client.get(reverse("radius:batch")) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()["count"], 2) def test_batch_list_permissions(self): - self._get_admin() self._create_radius_batch( name="test-prefix-batch", strategy="prefix", @@ -53,10 +48,8 @@ def test_batch_list_permissions(self): self.assertEqual(response.status_code, 401) with self.subTest("superuser"): - header = self._get_auth_header() - response = self.client.get( - reverse("radius:batch"), HTTP_AUTHORIZATION=header - ) + self._superuser_login() + response = self.client.get(reverse("radius:batch")) self.assertEqual(response.status_code, 200) with self.subTest("staff w/ managed org"): @@ -97,20 +90,16 @@ def test_batch_list_filter_strategy(self): csvfile=csv_file, status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() + self._superuser_login() url = reverse("radius:batch") with self.subTest("filter prefix"): - response = self.client.get( - url, {"strategy": "prefix"}, HTTP_AUTHORIZATION=header - ) + response = self.client.get(url, {"strategy": "prefix"}) names = [b["name"] for b in response.json()["results"]] self.assertIn("prefix-batch", names) self.assertNotIn("csv-batch", names) with self.subTest("filter csv"): - response = self.client.get( - url, {"strategy": "csv"}, HTTP_AUTHORIZATION=header - ) + response = self.client.get(url, {"strategy": "csv"}) names = [b["name"] for b in response.json()["results"]] self.assertIn("csv-batch", names) self.assertNotIn("prefix-batch", names) @@ -154,11 +143,10 @@ def test_batch_list_search_name(self): prefix="test", status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() + self._superuser_login() response = self.client.get( reverse("radius:batch"), {"search": "alpha"}, - HTTP_AUTHORIZATION=header, ) names = [b["name"] for b in response.json()["results"]] self.assertIn("alpha-batch", names) @@ -171,8 +159,8 @@ def test_batch_list_no_user_credentials(self): prefix="test", status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() - response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + self._superuser_login() + response = self.client.get(reverse("radius:batch")) batch_data = response.json()["results"][0] self.assertNotIn("user_credentials", batch_data) @@ -183,8 +171,8 @@ def test_batch_list_exposes_download_links(self): prefix="test", status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() - response = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + self._superuser_login() + response = self.client.get(reverse("radius:batch")) batch_data = response.json()["results"][0] self.assertIsNotNone(batch_data["pdf_link"]) self.assertIn(str(batch.pk), batch_data["pdf_link"]) @@ -199,9 +187,9 @@ def test_batch_csv_link_in_list_and_detail(self): csvfile=csv_file, status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() + self._superuser_login() with self.subTest("list"): - resp = self.client.get(reverse("radius:batch"), HTTP_AUTHORIZATION=header) + resp = self.client.get(reverse("radius:batch")) batch_data = resp.json()["results"][0] self.assertIsNotNone(batch_data["csv_link"]) self.assertIn(str(batch.pk), batch_data["csv_link"]) @@ -210,7 +198,6 @@ def test_batch_csv_link_in_list_and_detail(self): with self.subTest("detail"): resp = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]), - HTTP_AUTHORIZATION=header, ) data = resp.json() self.assertIsNotNone(data["csv_link"]) @@ -223,9 +210,9 @@ def test_batch_detail_200(self): prefix="test", status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() + self._superuser_login() url = reverse("radius:radius_batch_detail", args=[batch.pk]) - response = self.client.get(url, HTTP_AUTHORIZATION=header) + response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() self.assertEqual(data["id"], str(batch.pk)) @@ -236,7 +223,6 @@ def test_batch_detail_200(self): self.assertIsNotNone(data["pdf_link"]) def test_batch_detail_permissions(self): - self._get_admin() batch = self._create_radius_batch( name="test-prefix-batch", strategy="prefix", @@ -250,10 +236,9 @@ def test_batch_detail_permissions(self): self.assertEqual(response.status_code, 401) with self.subTest("superuser"): - header = self._get_auth_header() + self._superuser_login() response = self.client.get( reverse("radius:radius_batch_detail", args=[batch.pk]), - HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 200) @@ -304,12 +289,12 @@ def test_batch_detail_cross_org_404(self): self.assertEqual(response.status_code, 404) def test_batch_detail_404(self): - header = self._get_auth_header() + self._superuser_login() url = reverse( "radius:radius_batch_detail", args=["00000000-0000-0000-0000-000000000000"], ) - response = self.client.get(url, HTTP_AUTHORIZATION=header) + response = self.client.get(url) self.assertEqual(response.status_code, 404) def test_batch_delete_204(self): @@ -320,14 +305,13 @@ def test_batch_delete_204(self): status=RadiusBatch.COMPLETED, ) batch_id = batch.pk - header = self._get_auth_header() + self._superuser_login() url = reverse("radius:radius_batch_detail", args=[batch_id]) - response = self.client.delete(url, HTTP_AUTHORIZATION=header) + response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse(RadiusBatch.objects.filter(pk=batch_id).exists()) def test_batch_delete_permissions(self): - self._get_admin() operator = self._create_operator( organizations=[self.default_org], username="deletestaff", @@ -357,10 +341,9 @@ def test_batch_delete_permissions(self): prefix="test", status=RadiusBatch.COMPLETED, ) - header = self._get_auth_header() + self._superuser_login() response = self.client.delete( reverse("radius:radius_batch_detail", args=[batch.pk]), - HTTP_AUTHORIZATION=header, ) self.assertEqual(response.status_code, 204) @@ -401,9 +384,9 @@ def test_batch_delete_processing_409(self): ) batch.status = RadiusBatch.PROCESSING batch.save(update_fields=["status"]) - header = self._get_auth_header() + self._superuser_login() url = reverse("radius:radius_batch_detail", args=[batch.pk]) - response = self.client.delete(url, HTTP_AUTHORIZATION=header) + response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_409_CONFLICT) self.assertIn( "currently being processed and cannot be deleted", @@ -440,8 +423,8 @@ def test_batch_delete_pending_and_failed_allowed(self): prefix="test", status=batch_status, ) - header = self._get_auth_header() + self._superuser_login() url = reverse("radius:radius_batch_detail", args=[batch.pk]) - response = self.client.delete(url, HTTP_AUTHORIZATION=header) + response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse(RadiusBatch.objects.filter(pk=batch.pk).exists()) From d61c68ee4c107d63f418961346c5ba6d882abf53 Mon Sep 17 00:00:00 2001 From: BHARATH0153 <214236921+BHARATH0153@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:17:38 +0530 Subject: [PATCH 19/19] [fix] Fix assertNumQueries for session auth in batch list test --- openwisp_radius/tests/test_api/test_batch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_radius/tests/test_api/test_batch.py b/openwisp_radius/tests/test_api/test_batch.py index fad0b4af..d17c2d0e 100644 --- a/openwisp_radius/tests/test_api/test_batch.py +++ b/openwisp_radius/tests/test_api/test_batch.py @@ -31,7 +31,7 @@ def test_batch_list_200(self): status=RadiusBatch.COMPLETED, ) self._superuser_login() - with self.assertNumQueries(4): + with self.assertNumQueries(5): response = self.client.get(reverse("radius:batch")) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json()["count"], 2)