Skip to content

An/multi create fix#256

Open
AshotN wants to merge 5 commits into
feathersjs-ecosystem:masterfrom
AshotN:an/multiCreateFix
Open

An/multi create fix#256
AshotN wants to merge 5 commits into
feathersjs-ecosystem:masterfrom
AshotN:an/multiCreateFix

Conversation

@AshotN

@AshotN AshotN commented Dec 25, 2023

Copy link
Copy Markdown
Contributor

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

Mairu commented Dec 26, 2023

Copy link
Copy Markdown
Collaborator

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.

feathers-swagger/lib/v3/generator.js

Lines 218 to 241 in 0bfe44b

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AshotN @Mairu