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

Apply all validation rules to empty strings #46167

Merged
merged 1 commit into from
Sep 25, 2019

Conversation

joshdover
Copy link
Contributor

Summary

This fixes a bug with the @kbn/config-schema package where empty strings would skip any other validations defined in the schema.

For example, these examples would not throw a validation error prior to this change:

schema.string({ minLength: 5 }).validate('');

schema.string({
  validate: value => {
    if (!/^[a-z]+$/.test(value)) {
      return 'not all lowercase letters!'
    }
  }
}).validate('');

cc @legrego

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Sep 19, 2019
@joshdover joshdover requested a review from azasypkin September 19, 2019 17:21
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the config-schema/fix-empty-strings branch from ae3a67c to 3434adf Compare September 19, 2019 19:05
@joshdover joshdover added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.5.0 release_note:skip Skip the PR/issue when compiling release notes and removed release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Sep 19, 2019
@joshdover joshdover force-pushed the config-schema/fix-empty-strings branch 2 times, most recently from db2c50f to 1e05d80 Compare September 19, 2019 19:42
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover requested a review from a team September 23, 2019 15:08
@joshdover joshdover force-pushed the config-schema/fix-empty-strings branch from 1e05d80 to 23e2b3a Compare September 23, 2019 15:08
@legrego legrego mentioned this pull request Sep 23, 2019
7 tasks
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover force-pushed the config-schema/fix-empty-strings branch from 23e2b3a to 1728642 Compare September 24, 2019 22:35
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover merged commit 7949f34 into elastic:master Sep 25, 2019
@joshdover joshdover deleted the config-schema/fix-empty-strings branch September 25, 2019 15:23
joshdover added a commit to joshdover/kibana that referenced this pull request Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants