octobox icon indicating copy to clipboard operation
octobox copied to clipboard

Add openapi.yaml to the repository.

Open rickwysocki opened this issue 1 year ago • 12 comments

I created an openapi.yaml spec file for Octobox. I relied both on existing documentation and my own testing of the endpoints in a Docker version of the application. I have a points/questions to review:

  • I don't think I was able to get the sync/syncing endpoints, in the notifications controller, to work in my testing. Sync (/api/notifications/sync.json) might have worked since, based on the existing documentation, there wouldn't be a response, but I couldn't find any evidence of it working. Syncing (/api/notifications/syncing.json) only ever returned full HTML pages in Terminal. I may have misinterpreted elements of the existing documentation here.
  • I couldn't seem to post or patch pinned searches. Both returned HTML pages in Terminal. Additionally, the response examples in the existing documentation confused me, so I don't think the responses are currently correct. I'd love to work more on this if you have any thoughts.
  • I've left a few other in-text comments for smaller points, such as info I wasn't sure of or didn't have.

This was a lot of fun! I'd love to keep working on this with your feedback on the current version.

EDIT: One last point I forgot to mention: I was unsure where the openapi.yaml file should go, so it is in the root directory.

rickwysocki avatar Mar 02 '24 17:03 rickwysocki

Looking good, thanks for tackling this, I've just merged an update that fixes running octobox via docker-compose, so that may solve some of the issues you were running into.

andrew avatar Mar 03 '24 19:03 andrew

Thanks for the update! I verified that Swagger's curl command works for api/notifications/sync.json, but I'm still getting responses of very large HTML/Ruby files when using the commands produced for syncing, posting pinned searches, and updating pinned searches. There don't seem to be any changes made to the local version of Octobox I'm running using Docker, either. I'll take another look tomorrow, but any advice would be helpful.

I'm happy to work on this as long as is useful, by the way, as I'm trying to get my API documentation chops together.

rickwysocki avatar Mar 03 '24 21:03 rickwysocki

It could well be broken, although all the tests are passing which makes me think it’s a config issue, I’ll also give them a try tomorrow.

This is really helpful, thanks for contributing

andrew avatar Mar 03 '24 22:03 andrew

Hi there: Just an update that I tried those commands using my live Octobox URL rather than Docker. While this didn't return HTML in terminal, I still wasn't able to post or patch pinned notifications using CURL. Additionally, I'm also still not sure whether syncing feature is working or not.

rickwysocki avatar Mar 06 '24 21:03 rickwysocki

Hi there! Just following up to ask if you'd had a chance to look at those endpoints and/or any instructions for to continuing working on the API documentation.

rickwysocki avatar Mar 14 '24 17:03 rickwysocki

Sorry for the delay, I’ll review this later tonight

andrew avatar Mar 14 '24 18:03 andrew

I've added some inline comments, it's looking good so far, the schemas could do with a bit of tweaking to seperate the different objects and use the $ref feature of openapi.

Example: https://github.com/ecosyste-ms/packages/blob/main/openapi/api/v1/openapi.yaml#L1103

I could get all of the api endpoints working, a couple return 200 instead of 204, the creating and updating of pinned searches is a bit confusing as it follows the rails convention for parameters, I left examples in the comments above.

andrew avatar Mar 14 '24 21:03 andrew

I also opened up the file in https://editor.swagger.io, all working and no errors!

Screenshot 2024-03-14 at 21-33-44 Swagger Editor

andrew avatar Mar 14 '24 21:03 andrew

Thanks for this feedback! It's really helpful on my end. I'll start working on your recommendations and check in once I've finished the revisions.

rickwysocki avatar Mar 16 '24 17:03 rickwysocki

I've made some revisions to the openapi.yaml file based on your helpful recommendations. The only things I still seem to have issues with are the two sync-related endpoints.

  1. For the api/notifications/sync.json endpoint, I get a 200 response with no body in Swagger. In Terminal, I seem to be getting a 400 error when I run a verbose CURL request produced by Swagger.
  2. For the api/notifications/syncing.json endpoint, I get a 200 response in both Swagger and Terminal, but it comes with an "error": null.

I've attached screenshots showing those errors on my end. Everything else seems to be working for me, and I've made your requested changes.


image
image

rickwysocki avatar Mar 23 '24 14:03 rickwysocki

Hi there: Just touching base to see whether there is more work I can do on this. No rush on my end!

rickwysocki avatar Apr 09 '24 19:04 rickwysocki

@rickwysocki nothing more needed from your right now, I'm travelling for work at the moment, hope to get back to this in about a week

andrew avatar Apr 12 '24 09:04 andrew