openapi: document operations for admin ui feedback #4226
Conversation
Sonatype Lift is retiringSonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| responses: { 200: createResponseSchema('feedbackSchema') }, | ||
| responses: { | ||
| 200: createResponseSchema('feedbackSchema'), | ||
| ...getStandardResponses(400, 401, 415), |
There was a problem hiding this comment.
Can we get a 404 if the id does not exist?
There was a problem hiding this comment.
Good question! Let me find out!
There was a problem hiding this comment.
You should! Right now you might get a 500 😳
There was a problem hiding this comment.
I found similar situations on other update endpoints!
There was a problem hiding this comment.
Same here. I try to turn those 5xx into client 4xx errors whenever I test some endpoint. As as general rule of thumb if the endpoint has id or some other query/path param we may need to test for 400 vs 500 handling
There was a problem hiding this comment.
No, it does not. If it doesn't exist, we create that feedback id..
There was a problem hiding this comment.
Update: this only returns 500 if you use an admin token (and thus have no identifiable user). @kwasniew , I think we might have discussed cases like this before? I don't know if it's worth adding the handling for this in this one specific place now, but I can definitely add it to the description.
There was a problem hiding this comment.
I remember us having this conversation but I don't remember how we solved it. @sjaanus do you remember what we did for the admin tokens that had no user id?
There was a problem hiding this comment.
I think we never solved it, but the consensus was that are moving away from admin tokens and endpoints that require user as foreign key, will not work for admin tokens.
There was a problem hiding this comment.
In that case, I'm going to merge it for now and we can update that later if we want to.
| app = await setupAppWithCustomAuth(stores, preHook, { | ||
| experimental: { | ||
| flags: { | ||
| strictSchemaValidation: true, |
|
@gastonfournier The 500 wasn't because the ID was not found, but because I used an admin token. You need an identifiable user to post and put these. |
This PR updates the endpoint documentation and schemas related to Admin UI feedback.