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

An/multi create fix #256

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AshotN
Copy link
Contributor

@AshotN AshotN commented Dec 25, 2023

Summary

feathers-swagger would handle multi: ['create'] by using the base schema type. Which would be not ideal if you have a more narrow create schema. This PR is my attempt at correcting that.

Other Information

Would appreciate letting me know if anything here could use fixes.

My other PR is in here as well, I can remove it if needed. Ideally both can be merged together here.

@Mairu
Copy link
Collaborator

Mairu commented Dec 26, 2023

Hi @AshotN,

the error you are describing needs not to be fixed in the generator class, there I think all is correct.
The problem is, that the createSwaggerServiceOptions helper function does not add the schema refs for the multi operations.

The reason that there is no createMulti function in the generator is, that in the OpenAPI spec, there is no difference in the path for creating a single vs creating multiple, so the multiple case is also handled in the create handler.

create ({ tags, security, securities, refs, multiOperations }) {
const multi = multiOperations.includes('create');
return {
tags,
description: 'Creates a new resource with data.',
requestBody: {
required: true,
content: multi ? jsonSchemaRef(refs.createMultiRequest) : jsonSchemaRef(refs.createRequest)
},
responses: {
201: {
description: 'created',
content: multi ? jsonSchemaRef(refs.createMultiResponse) : jsonSchemaRef(refs.createResponse)
},
401: {
description: 'not authenticated'
},
500: {
description: 'general error'
}
},
security: utils.security('create', securities, security)
};
},

To fix the problem you have, the ref for multiCreateRequest would need to be added within the createSwaggerServiceOptions helper function. It would then be picked up by the generator.

I think a multi parameter should be added to the createSwaggerServiceOptions function, to be able to create the needed refs for multi operations. The option for multi should probably then also be set.

Give me a hint if you want to work on it, otherwise I could also look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants