[2] fix(2051): fix function that does not exist #29
Conversation
index.js
Outdated
| @@ -181,7 +181,7 @@ class EmailNotifier extends NotificationBase { | |||
|
|
|||
| // Validate the settings email object | |||
| static validateConfig(config) { | |||
| return Joi.validate(config, SCHEMA_EMAIL); | |||
| return SCHEMA_EMAIL.validate(config); | |||
There was a problem hiding this comment.
SCHEMA_EMAIL cannot be validate correctly.
In the case of the above, you have not validate the following
test:
settings:
email: ←Not validate.
It is better to use SCHEMA_BUILD_SETTINGS.
There was a problem hiding this comment.
If we want to call validate only if it has settings.email, I guess we can't use SCHEMA_BUILD_SETTINGS.
https://github.com/screwdriver-cd/config-parser/pull/105/files
There was a problem hiding this comment.
The role of the validator in this plugin is to return whether the email setting is valid.
If the following yaml may not work properly.
email:
image: node12
requires: [ ~commit, ~pr ]
settings:
emaill ←typo:
addresses:
- [email protected]
statuses:
- SUCCESS
- FAILURE
When actually used, it works by simply passing the job settings.
There was a problem hiding this comment.
If emaill is made available by Custom Notifications, it is not a typo.
https://docs.screwdriver.cd/cluster-management/configure-api#notifications-plugin
I don't think there's a good way to handle it at the moment, we'll just have to verify the contents of the settings.email.
There was a problem hiding this comment.
Ahh, I was confusing SCHEMA_BUILD_SETTINGS and SCHEMA_BUILD_DATA. Probably it should use SCHEMA_BUILD_SETTINGS.
| @@ -31,19 +31,19 @@ const SCHEMA_ADDRESSES = Joi.array() | |||
| .min(1); | |||
| const SCHEMA_STATUSES = Joi.array() | |||
| .items(schema.plugins.notifications.schemaStatus) | |||
| .min(1); | |||
| .min(0); | |||
There was a problem hiding this comment.
I think 0 statuses acceptable
| const SCHEMA_EMAIL = Joi.alternatives().try( | ||
| Joi.object().keys({ addresses: SCHEMA_ADDRESSES, statuses: SCHEMA_STATUSES }), | ||
| SCHEMA_ADDRESS, SCHEMA_ADDRESSES | ||
| ); | ||
| const SCHEMA_BUILD_SETTINGS = Joi.object() | ||
| const SCHEMA_EMAIL_SETTINGS = Joi.object() |
There was a problem hiding this comment.
fix confusing value name
yoshwata
changed the title
Context
Joi.validateis not exist today.We shold use schema.validate() instead.
Objective
This PR enables
validateConfigfunction by fixingjoi.validate.References
screwdriver-cd/screwdriver#2051
License
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.