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

Customization of blueprint routes on model does not work #77

Closed
OscarMeier opened this issue Jun 15, 2020 · 9 comments · Fixed by #78 or #80
Closed

Customization of blueprint routes on model does not work #77

OscarMeier opened this issue Jun 15, 2020 · 9 comments · Fixed by #78 or #80
Assignees

Comments

@OscarMeier
Copy link

OscarMeier commented Jun 15, 2020

I'm not able to customize the blueprint route documentation: Neither by adding swagger JSDoc nor by adding the swagger object to the model (api/models/Foo.js):

/**
 *  @swagger
 *
 * /find:
 *    summary: Foo
 *    description: Bar
 */
swagger: {
    actions: {
      find: {
        summary: 'Foo',
        description: 'Bar',
      },
    },
  },

I dug into your code and found the referenced line. It's the line, where the paths get generated. The merged models contain the correct documentation (summary: 'Foo', description: 'Bar') but the models are not used in the generatePaths() function to override the blueprint templates. Did I overlook something or is this feature not implemented yet?

defaults(specifications.paths, generatePaths(routes, blueprintActionTemplates, theDefaults, specifications, models));

@danielsharvey danielsharvey self-assigned this Jun 17, 2020
@danielsharvey
Copy link
Collaborator

Found it - the model per-blueprint-action overrides where not being merged correctly. Bugfix shortly.

danielsharvey added a commit that referenced this issue Jun 17, 2020
Previously, `allActions` custom documentation in model files was
correctly merged into the final output, but per-blueprint-action
documentation (in either JSDoc or `swagger` element) was missed.

Closes #77.
danielsharvey added a commit that referenced this issue Jun 17, 2020
Previously, `allActions` custom documentation in model files was
correctly merged into the final output, but per-blueprint-action
documentation (in either JSDoc or `swagger` element) was missed.

Closes #77.
@theoomoregbee
Copy link
Owner

🎉 This issue has been resolved in version 3.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@danielsharvey
Copy link
Collaborator

@OscarMeier Let us know how you get on. Thanks for finding this.

@OscarMeier
Copy link
Author

OscarMeier commented Jun 19, 2020

@danielsharvey Thank you for the quick fix! The custom blueprint route documentation works as expected for the description and the summary. But I didn't figure out how to exclude an action. I tried to use the exclude option:

 * /add:
 *    exclude: true
 *
 * /populate:
 *    exclude: true
 */

I checked your code and found out that you just omit the exclude property:


But the blueprint action documentation does not get removed, right?

I would propose to keep the exclude property in the clone and to add a condition below:

if (pathEntry.exclude) {
  return;
}

@danielsharvey
Copy link
Collaborator

@OscarMeier Let me look! It looks like I did not check the test data closely enough! (several changes had large effects and I must not have checked the test fixture closely enough).

@danielsharvey danielsharvey reopened this Jun 19, 2020
@danielsharvey
Copy link
Collaborator

Found and fixed. Merging now.

Note that the exclude property is excluded as it is included within the swagger content but not valid OpenAPI and so excluded from the generated output.

danielsharvey added a commit that referenced this issue Jun 19, 2020
Also add new test data and correct test fixture.

Closes #77.
danielsharvey added a commit that referenced this issue Jun 19, 2020
…80)

Also add new test data and correct test fixture.

Closes #77.
@theoomoregbee
Copy link
Owner

🎉 This issue has been resolved in version 3.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@danielsharvey
Copy link
Collaborator

@OscarMeier Let us know how you get on.

@OscarMeier
Copy link
Author

@danielsharvey Thank you for resolving this issue! Everything works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment