openwisp / openwisp-radius

Administration web interface and REST API for freeradius 3 build in django & python. Supports captive portal authentication, WPA Enerprise (802.1x), freeradius rlm_rest, social login, Hotspot 2.0 / 802.11u, importing users from CSV, registration of new users and more.
https://openwisp.io/docs/dev/radius/
GNU General Public License v3.0
360 stars 176 forks source link

[change] Download batch CSV URL is inconsistent and not showing up in API docs #373

Open nemesifier opened 2 years ago

nemesifier commented 2 years ago

The URL generated with radius:serve_private_file is not consistent with the rest of the API URLs:

./manage.py show_urls | grep batch | grep api
/api/v1/radius/batch/   openwisp_radius.api.views.BatchView radius:batch
/api/v1/radius/organization/<slug:slug>/batch/<uuid:pk>/pdf/    openwisp_radius.api.views.DownloadRadiusBatchPdfView    radius:download_rad_batch_pdf
/api/v1/radiusbatch/csv/<path:csvfile>  openwisp_radius.private_storage.views.RadiusBatchCsvDownloadView    radius:serve_private_file

Shouldn't this CSV url be as follows?

/api/v1/radius/organization/<slug:slug>/batch/<uuid:pk>/csv/.

We have an inconsistency here, I will create a new issue for this, another problem is that this URL does not show up in the swagger API docs (/api/v1/docs/), let's find way to show it there too.

This is a follow up of https://github.com/openwisp/openwisp-radius/pull/366#pullrequestreview-868237372.

Further follow up: https://github.com/openwisp/openwisp-radius/commit/0e764626e80e19c0b9682ed07d61cfcccc368835 fixes the inconsistencies, but the URL is not showing up in the API docs, it would be great it we can add it in some way, for consistency, so I will leave this open.

dishantsethi commented 2 years ago

Hi, I want to start contributing and work on this issue. Can you assign me this?

nemesifier commented 2 years ago

Hi @dishantsethi,

Thanks for your interest in contributing! Please read our Contribution Guidelines carefully:

You don’t need to wait for the issue to be assigned to you. Just check if there is anyone else actively working on it (eg: an open pull request with recent activity). If nobody else is actively working on it, just announce your intention to work on it by leaving a comment in the issue.

Read also the rest and it will save everyone's time.

nemesifier commented 2 years ago

Follow up: https://github.com/openwisp/openwisp-radius/commit/0e764626e80e19c0b9682ed07d61cfcccc368835 fixes the inconsistencies, but the URL is not showing up in the API docs, it would be great it we can add it in some way, for consistency, so I will leave this open.