[bug] Fix API prefix add error #132

[atb00ker]

Closes #132

[coveralls]

Pull Request Test Coverage Report for Build 527

• 42 of 42 (100.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.003%) to 99.259%

---

Totals
Change from base Build 517:	0.003%
Covered Lines:	2010
Relevant Lines:	2025

---

💛 - Coveralls

[nemesifier]

the implementation of this view is really weird.
All that code executed in the if I think shouldn't be done there but handled in the serializer, and then serializer.save() should be called.

However, I'm not sure if that's the cause of the issue.
Can you tell me how to replicate the issue so I'll try to do so?

[atb00ker]

To reproduce the issue, just make a request to batchview API for generating users, example:

curl -X POST -H "Content-type: application/json" \
     -H "Authorization: Bearer <key>" \
     -d '{
          "name": "sample2",
          "strategy": "prefix",
          "prefix": "sample2",
          "number_of_users": 2,
          "expiration_date": "2021-02-05"
     }' 'http://127.0.0.1:8000/api/v1/default/radiusbatch/'

Without batch.save() it should give an IntegrityError and with it, it should work fine.

[atb00ker]

I have not found the reason for the original problem. To reproduce the same please, please see the comment above and create that request in the master branch! 😄

However, I have moved the radius batch creation to serializer where I create the radiusbatch which takes care of the save() problem and solves the bug.

[nemesifier]

looking better, so you weren't able to reproduce this bug in tests at all?

[nemesifier]

Forgot to flag as more work needed.

[atb00ker]

Think this should work, contains 2 commits, one of them has a seperate PR open here #144

[atb00ker]

While debugging on the web API interface, I noticed another issue that I didn't think about before, so I have added a model check with some modification. 😄

[nemesifier]

I tried again to create a radius batch from the form of the web UI.

POST data:

csrfmiddlewaretoken: pHv9NP6y4jR3cXnFJrGCfs6o0kEy8hoeZlW68Q5gx4fTcrCJQH8Q9SZTVt9Ac9Xl
organization_slug: staging
prefix: gsoc20-
csvfile: (binary)
number_of_users: 2
strategy: prefix
name: gsoc20
expiration_date: 

Result:

Traceback (most recent call last):
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/rest_framework/views.py", line 505, in dispatch
    response = self.handle_exception(exc)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/rest_framework/views.py", line 465, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception
    raise exc
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/rest_framework/views.py", line 502, in dispatch
    response = handler(request, *args, **kwargs)
  File "/home/nemesis/Code/openwisp/openwisp-radius/openwisp_radius/api/views.py", line 357, in post
    if serializer.is_valid():
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/rest_framework/serializers.py", line 234, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/rest_framework/serializers.py", line 436, in run_validation
    value = self.validate(value)
  File "/home/nemesis/Code/openwisp/openwisp-radius/openwisp_radius/api/serializers.py", line 149, in validate
    batch_data.update(organization=Organization.objects.get(slug=slug))
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.7/site-packages/django/db/models/query.py", line 417, in get
    self.model._meta.object_name

Exception Type: DoesNotExist at /api/v1/radiusbatch/
Exception Value: Organization matching query does not exist.

[nemesifier]

Git log:

744dd61 - (HEAD -> atb00ker-issues/132-batch-add-user) [bug] Fix API prefix add error #132 (5 hours ago) <Ajay Tripathi>
211c30c - [change] Converted cron management commands to celery beat periodic tasks #117 (8 hours ago) <Ajay Tripathi>
7c42457 - (origin/master, origin/HEAD) [fix] Fixed sample-app migration problems (8 hours ago) <Ajay Tripathi>
07ae291 - (master) [minor] Improves sample-radius migrations #143 (25 hours ago) <Ajay Tripathi>

[nemesifier]

With ipdb:

-> batch_data.update(organization=Organization.objects.get(slug=org_slug))
(Pdb) org_slug
<Organization: FVGwifi demo>

org_slug is an organization instance instead of a string.

[atb00ker]

Yes! Thank you for the information, I am able to reproduce it now.. on it! 😄

[atb00ker]

Fixed the problem and made changes in tests to ensure such error is captured by automated tests.
However, because the changes in the tests was not 100% relevant to this PR, hence I have made another commit for it, let me know if I should move these changes to another PR.

[nemesifier]

Thanks!