openwisp-radius icon indicating copy to clipboard operation
openwisp-radius copied to clipboard

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

Open nemesifier opened this issue 2 years ago • 3 comments

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.

nemesifier avatar Jan 31 '22 18:01 nemesifier

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

dishantsethi avatar Jan 31 '22 18:01 dishantsethi

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 avatar Feb 01 '22 12:02 nemesifier

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.

nemesifier avatar Mar 08 '22 19:03 nemesifier