Skip to content

Conversation

@DavidVujic
Copy link
Contributor

@DavidVujic DavidVujic commented Feb 24, 2018

Description

In this PR, the filter function used for filtering Tagged Operations is written as a plugin, instead of inline code in a component (operations.jsx). This will make it possible to implement a custom filter. One example is a custom filter making it possible to search by more than one phrase (example: pet, store).

Motivation and Context

One scenario (that is very common in the organisation I work in) is that users are testing an operation that requires an authenticated session or token. Usually, getting a session/token is made executing a different operation. So, there are two operations that will be tested out. A filter that accepts more than one phrase will solve the UI issues with navigating an API with a long list of operations.

How Has This Been Tested?

A unit test is added to the source code, testing the OpsFilter plugin function.

Screenshots (if appropriate):

With a plugin, a custom filter function can easily be implemented, overriding the default filter.

This is an example of a custom filter, that will make it possible to search by multiple phrases (in this case, separated by comma).

function myCustomFilter () {
    return {
      fn: {
        OpsFilter: (taggedOps, phrase) => {
          const phrases = phrase.split(/\s*,\s*/)

          return taggedOps.filter((val, key) => {
            return phrases.find((text) => key.indexOf(text.trim()) !== -1)
          })
        }
      }
    }

The custom plugin can be added as any plugin to the SwaggerUI constructor:

plugins: [
        myCustomFilter
      ]

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@heldersepu
Copy link
Contributor

A couple of edge cases in the tests could be:

Copy link
Contributor

@heldersepu heldersepu left a comment

Choose a reason for hiding this comment

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

Now on the template you can check the:

I have taken care to cover edge cases in my tests.

Copy link
Contributor

@shockey shockey left a comment

Choose a reason for hiding this comment

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

I like this! Just a couple things:

  • OpsFilter should be opsFilter, in order to be consistent with most of the other things currently under fn
  • Tests should include the System passing around opsFilter and then using it by system reference, to protect against breaking changes since we're exposing opsFilter as a plug point

@DavidVujic
Copy link
Contributor Author

DavidVujic commented Feb 27, 2018

@shockey Great! I have added a unit test that does what I think is what you meant by including the System passing around fn. There is also one commit where I have renamed the function to opsFilter.

Copy link
Contributor

@shockey shockey left a comment

Choose a reason for hiding this comment

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

LGTM

@shockey shockey merged commit e6722d8 into swagger-api:master Feb 27, 2018
@shockey
Copy link
Contributor

shockey commented Feb 27, 2018

thanks @DavidVujic!

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.

3 participants