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

Custom global validators #1369

Merged
merged 1 commit into from
Mar 20, 2019
Merged

Custom global validators #1369

merged 1 commit into from
Mar 20, 2019

Conversation

fxisco
Copy link
Contributor

@fxisco fxisco commented Jan 23, 2019

Description

This PR adds global validators to kiln. To enable this in the site we can include validators the same way we do for components: add a validator to window.kiln.validators. The main difference relies on the validator object, is needed to include scope: metadata to identify it as a global validator.

e.g.:

Metadata validation for all pages

include scope: 'metadata' in object and on type indicate if is error or warning

'use strict';

module.exports = {
  label: 'Lorem Ipsum',
  description: 'Lorem ipsum dolor sit amet, consectetur...',
  type: 'error',
  scope: 'metadata',
  validate(meta) {
    if (meta.authors.length < 1) {
      return [{
        preview: 'Blah blah'
      }];
    }

    return [];
  }
};

Metadata validation for specific page

include scope: 'metadata' in object. NOTE: include uri: <URI>,

'use strict';

module.exports = {
  label: 'Lorem ipsum',
  description: 'Lorem ipsum dolor sit amet, consectetur...',
  type: 'error',
  scope: 'metadata',
  uri: 'cjp2sl6c50000cyv2gt73xbwt',
  validate(meta) {
    if (meta.authors.length < 1) {
      return [{
        preview: 'Blah blah'
      }];
    }

    return [];
  }
};

Approach taken

  • Filter validations using condition scope , type and uri (for specific page).
  • Running validators with a different function that fetch the metadata first and then use this info to validate all the metadata validators.

Note: we are able to create metadata warnings if necessary.

Ticket

Closes #1368

WIP

  • Missing tests
  • Missing comments

@coveralls
Copy link

coveralls commented Jan 28, 2019

Coverage Status

Coverage increased (+0.2%) to 34.49% when pulling 201ae01 on validators into f5fe40e on master.

@fxisco fxisco force-pushed the validators branch 4 times, most recently from aea9185 to 657154c Compare March 1, 2019 20:29
@fxisco fxisco changed the title [WIP]: Custom global validators Custom global validators Mar 1, 2019
@fxisco fxisco requested a review from scottnash March 1, 2019 20:29
@fxisco
Copy link
Contributor Author

fxisco commented Mar 5, 2019

@scottnash we should not add anything special. Here an example(https://github.com/nymag/sites/pull/7308/files)

if we don't set scope, it is consider a global validator, i.e. all the ones that already exist. To be able to make validations with the metadata we should specify scope: 'metadata' and what we are going to receive as argument in the validate functions will we the metadata and not the component state as normal.

Copy link
Contributor

@scottnash scottnash left a comment

Choose a reason for hiding this comment

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

Branch is out of date with master now, but other than that it looks good.

@fxisco fxisco merged commit 1684ac4 into master Mar 20, 2019
@fxisco fxisco deleted the validators branch March 20, 2019 18:45
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