Change default validation level to strict and honor validation_level
#72
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
doits
force-pushed
the
strict_by_default
branch
from
fa173a2
to
9cdb107
Compare
`with_settings` can be used to change MJML-Rails settings only for the duration of the block. It is passed a hash with settings to change. ```ruby puts Mjml.validation_level # => 'soft' with_settings(validation_level: 'strict') do puts Mjml.validation_level # => 'strict' end puts Mjml.validation_level # => 'soft' ```
Before this commit MJML-Rails *always* raised an exception if there is any template validation error. In MJML there are two validation levels though (besides disabling validation at all): - `strict`: Raise an error if there are any problems, abort - `soft`: Do your best and compile what you can, but do not raise any error. Output errors on stderr. Right now it does not matter what validation level is used in MJML-Rails: It always raises an error if something is wrong with the template, which actually makes the setting `MJML.validation_level` superfluous. With this commit, MJML-Rails follows the MJML convention to only abort if the validation level is `strict`. If it is `soft`, it lets MJML do what it can and only logs the errors, like MJML does. To prevent apps using the default configuration from breaking, the default validation level is changed to `strict`, too. This means it raises an error on an invalid template like it did before.
doits
force-pushed
the
strict_by_default
branch
from
9cdb107
to
7dd86bc
Compare
sighmon
added a commit
that referenced
this pull request
Nov 29, 2020
|
@doits Looks good, thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: This is based on top of #71. Please first look at #71, so hopefully #71 can be merged and then there will be fewer changes here. Then you can take a look at this one.
Second note: If this is merged and released, there should be a major version bump (I'd say
4.5) with a changelog entry, because it is a possible backwards incompatible change if somebody has:soft(e.g. is not using the default config) andBefore this commit MJML-Rails always raised an exception if there is any
template validation error.
In MJML there are two validation levels though (besides disabling
validation completely):
strict: Raise an error if there are any problems, abortsoft: Do your best and compile what you can, but do not raise anyerror. Output errors on stderr.
Right now it does not matter what validation level is used in
MJML-Rails: It always raises an error if something is wrong with the
template, which actually makes the setting
MJML.validation_levelsuperfluous.
With this commit, MJML-Rails follows the MJML convention to only abort
if the validation level is
strict. If it issoft, it lets MJML dowhat it can and only logs the errors, like MJML does.
To prevent apps using the default configuration from breaking, the
default validation level is changed to
strict, too. This means itraises an error on an invalid template like it did before.