Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nemesifier opened this issue Jan 31, 2022 · 3 comments

Comments

@nemesifier
Copy link
Member

nemesifier commented Jan 31, 2022

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 #366 (review).

Further follow up: 0e76462 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 nemesifier added the enhancement New feature or request label Jan 31, 2022
@nemesifier nemesifier added this to To do (general) in OpenWISP Contributor's Board via automation Jan 31, 2022
@nemesifier nemesifier moved this from To do (general) to To do (Python & Django) in OpenWISP Contributor's Board Jan 31, 2022
@nemesifier nemesifier added this to Backlog in OpenWISP Priorities for next releases via automation Jan 31, 2022
@nemesifier nemesifier moved this from Backlog to To do in OpenWISP Priorities for next releases Jan 31, 2022
@dishantsethi
Copy link
Contributor

dishantsethi commented Jan 31, 2022

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

@nemesifier
Copy link
Member Author

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.

dishantsethi pushed a commit to dishantsethi/openwisp-radius that referenced this issue Feb 10, 2022
URL generated with radius:serve_private_file was not consistent with the rest of the API URLs
Changed Download batch CSV API endpoint from radiusbatch/csv/<uuid>/filename.csv to radius/organization/<slug:slug>/batch/<uuid:pk>/csv/filename.csv and added the same in API documentation as well. Fixes issue openwisp#373
dishantsethi pushed a commit to dishantsethi/openwisp-radius that referenced this issue Feb 10, 2022
URL generated with radius:serve_private_file was not consistent with the rest of the API URLs
Changed Download batch CSV API endpoint from radiusbatch/csv/<uuid>/filename.csv to radius/organization/<slug:slug>/batch/<uuid:pk>/csv/filename.csv and added the same in API documentation as well. Fixes issue openwisp#373
dishantsethi pushed a commit to dishantsethi/openwisp-radius that referenced this issue Feb 10, 2022
* URL generated with radius:serve_private_file was not consistent with the rest of the API URLs.
* Changed Download batch CSV API endpoint from radiusbatch/csv/<uuid>/filename.csv to radius/organization/<slug:slug>/batch/<uuid:pk>/csv/filename.csv and added the same in API documentation as well.

Fixes openwisp#373
dishantsethi pushed a commit to dishantsethi/openwisp-radius that referenced this issue Feb 10, 2022
* URL generated with radius:serve_private_file was not consistent with the rest of the API URLs.
* Changed Download batch CSV API endpoint from radiusbatch/csv/<uuid>/filename.csv to radius/organization/<slug:slug>/batch/<uuid:pk>/csv/filename.csv and added the same in API documentation as well.

Closes openwisp#373
nemesifier added a commit that referenced this issue Mar 8, 2022
* URL generated with radius:serve_private_file was not consistent with the rest of the API URLs.
* Changed Download batch CSV API endpoint from radiusbatch/csv/<uuid>/filename.csv to radius/organization/<slug:slug>/batch/<uuid:pk>/csv/filename.csv and added the same in API documentation as well.

Related to #373

Co-authored-by: Dishant Sethi <dishantsethi@Dishants-MacBook-Air.local>
Co-authored-by: Federico Capoano <f.capoano@openwisp.io>
@nemesifier
Copy link
Member Author

Follow up: 0e76462 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 nemesifier moved this from To do (Python & Django) to To do (ansible/docker) in OpenWISP Contributor's Board Jan 23, 2023
@nemesifier nemesifier moved this from To do (ansible/docker) to To do (Python & Django) in OpenWISP Contributor's Board Jan 23, 2023
@nemesifier nemesifier moved this from To do to Backlog in OpenWISP Priorities for next releases May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
OpenWISP Contributor's Board
  
To do (Python & Django)
Development

No branches or pull requests

2 participants