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

fix ContentTransformIntentValidator #165

Merged
merged 4 commits into from
Jan 9, 2019

Conversation

jarkkosyrjala
Copy link
Contributor

Summary

shouldPublish should accept 'preserve' in addition to boolean values

Description

Add 'preserve' as a valid option to validations for shouldPublish field

Motivation and Context

#143 (comment)

shouldPublish should accept 'preserve' in addition to boolean values
TimBeyer
TimBeyer previously approved these changes Jan 4, 2019
@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 4, 2019

I guess we also need to add this for EntryTransformToTypeIntentValidator then?

@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 4, 2019

And actually EntryTransformToTypeValidator needs to check for isEntryTransformToType instead of isEntryDerive.

Must have missed this during the review.

@jarkkosyrjala
Copy link
Contributor Author

@TimBeyer Also, this makes the content-transform.spec.ts test to fail, because the error message from the validation is now different at

message: '"string" is not a valid type for the content transformation property "shouldPublish". Expected "boolean".',

@@ -157,7 +157,7 @@ describe('Content transformation', function () {
type: 'contentType/transformEntries'
}
},
message: '"string" is not a valid type for the content transformation property "shouldPublish". Expected "boolean".',
message: '"undefined" is not a valid type for the content transformation property "shouldPublish". Expected "alternatives".',
Copy link
Contributor

Choose a reason for hiding this comment

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

This new error message isn't really useful anymore for people.
Maybe we could figure out in

whether we are dealing with Joi.alternatives and figure out the different types that are allowed.
Either that, or somehow change the message for the alternatives validation so that it doesn't say Expected "alternatives".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TimBeyer error message fixed with tests

@Khaledgarbaya Khaledgarbaya requested a review from TimBeyer January 9, 2019 13:06
@TimBeyer
Copy link
Contributor

TimBeyer commented Jan 9, 2019

👏

@Khaledgarbaya Khaledgarbaya merged commit 1e0a82c into contentful:master Jan 9, 2019
@phoebeschmidt
Copy link
Contributor

🎉 This PR is included in version 0.16.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

4 participants