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

Feature - additional filter options: operationIds & method #71

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

thim81
Copy link
Contributor

@thim81 thim81 commented Jan 9, 2021

  • Filter by OperationIds
  • Filter by method

As mentioned #70, I have added 2 options:

  • to filter by "method" (GET/POST/PUT/DELETE/....) which will remove all operations under any path that matches
  • to filter by "operationId" (operationId: addPet/ operationId: updatePet / ...) which will remove the specific operations under that matches the defined operationId

Tim added 2 commits January 9, 2021 23:15
- Filter by OperationIds
- Filter by method
@thim81
Copy link
Contributor Author

thim81 commented Jan 9, 2021

@MikeRalphson

I'm doing something wrong with the tests, but not sure how to correct it.
If I do a local diff the input & output show the result as expected.

@thim81
Copy link
Contributor Author

thim81 commented Jan 9, 2021

Question:

When combining it with the "inverse":true it removes really everything except the "paths" with the "method".

Example

"methods": ["post"],
  "inverse": true

How can I limit the "inverse" to the "path" level?

@MikeRalphson
Copy link
Contributor

How can I limit the "inverse" to the "path" level?

The original intent of the --inverse option was to see exactly what would be filtered / omitted. Over time, for people who want a kind of reverse filtering, the --valid option was added - this may be what you need in combination.

Copy link
Contributor

@MikeRalphson MikeRalphson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good - though I haven't pulled and tested yet. Just a few initial comments.

thim81 and others added 3 commits January 10, 2021 10:00
Co-authored-by: Mike Ralphson <[email protected]>
Co-authored-by: Mike Ralphson <[email protected]>
Co-authored-by: Mike Ralphson <[email protected]>
@thim81
Copy link
Contributor Author

thim81 commented Jan 10, 2021 via email

@MikeRalphson
Copy link
Contributor

state.parent always holds the parent object (i.e. the one which has obj in it) identified by state.pkey which is the "parent key". So you can delete during the recursion, or hold references and delete afterwards if need be.

@thim81
Copy link
Contributor Author

thim81 commented Jan 23, 2021

@MikeRalphson Let me know if you have more remarks or comments, that I can implement. I think it is good to go.

@yjose
Copy link

yjose commented May 12, 2021

hey @thim81 @MikeRalphson , hope you are doing great, any updates on this feature, would be a great addition to the package.
how can I help?

@thim81
Copy link
Contributor Author

thim81 commented May 13, 2021

@yjose I think that everything is ready. I cant see the change request that was asked, which seems more a Github issue on my end. I'm using this PR since Jan 2021 and I haven't ran into any issues.

@MikeRalphson
Copy link
Contributor

Thanks for the prod - I will test locally and check there's nothing outstanding before merging.

@yjose
Copy link

yjose commented May 13, 2021

by the way, i tried to filter opertionIds using using the following config :

let options = {
  flags: ['operationId'],
  flagValues: operations, // array of strings 
  inverse: true,
  strip: true,
  valid: true,
  info: true,
}; 

and look like its working but with missed parameters for paths with global parameters like /path-example/{id}/

@MikeRalphson
Copy link
Contributor

and look like its working but with missed parameters for paths with global parameters like /path-example/{id}/

@yjose could you create a failing testcase to show what is currently happening and what the output should be?

@yjose
Copy link

yjose commented May 15, 2021

@MikeRalphson #93

@mattadamson
Copy link

I'm very interested in this feature. I'm not clear what if anything is holding up the approval and merge now?

@thim81
Copy link
Contributor Author

thim81 commented Sep 27, 2023

@mattadamson
Since then, I have created a separate NPM package which supports the filtering => https://www.npmjs.com/package/openapi-format

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

Successfully merging this pull request may close these issues.

5 participants