Skip to content

Commit

Permalink
Change default validation level to strict and honor validation_level
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
doits committed Nov 27, 2020
1 parent 6426971 commit 7dd86bc
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 15 deletions.
10 changes: 6 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,20 @@ If there are configurations you'd like change:
- render errors: defaults to `true` (errors raised)
- minify: defaults to `false` (not minified)
- beautify: defaults to `true` (beautified)
- validation_level: defaults to `soft` (MJML syntax validation)
- validation_level: defaults to `strict` (abort on any template error, see [MJML validation documentation](https://github.com/mjmlio/mjml/tree/master/packages/mjml-validator#validating-mjml) for possible values)

```ruby
# config/initializers/mjml.rb
Mjml.setup do |config|
# set to `false` to ignore errors silently
config.raise_render_exception = true
# ignore errors silently
config.raise_render_exception = false

# optimize the size of your email
config.beautify = false
config.minify = true
config.validation_level = "strict"

# render illformed MJML templates, not recommended
config.validation_level = "soft"
end
```

Expand Down
2 changes: 1 addition & 1 deletion lib/mjml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module Mjml
@@mjml_binary_error_string = "Couldn't find the MJML #{Mjml.mjml_binary_version_supported} binary.. have you run $ npm install mjml?"
@@beautify = true
@@minify = false
@@validation_level = "soft"
@@validation_level = "strict"

def self.check_version(bin)
stdout, _, status = run_mjml('--version', mjml_bin: bin)
Expand Down
7 changes: 4 additions & 3 deletions lib/mjml/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ def render
# Exec mjml command
#
# @return [String] The result as string
def run(in_tmp_file, beautify=true, minify=false, validation_level="soft")
def run(in_tmp_file, beautify=true, minify=false, validation_level="strict")
Tempfile.create(["out", ".html"]) do |out_tmp_file|
command = "-r #{in_tmp_file} -o #{out_tmp_file.path} --config.beautify #{beautify} --config.minify #{minify} --config.validationLevel #{validation_level}"
_, stderr, _ = Mjml.run_mjml(command)
raise ParseError.new(stderr.chomp) unless stderr.blank?
_, stderr, status = Mjml.run_mjml(command)
raise ParseError.new(stderr.chomp) unless status.success?
Mjml.logger.warn(stderr.chomp) unless stderr.blank?
out_tmp_file.read
end
end
Expand Down
15 changes: 12 additions & 3 deletions test/mjml_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,18 @@ class NotifierMailerTest < ActiveSupport::TestCase
assert email.text_part.body.match(%r{Please visit https://www.example.com})
end

test "Invalid template raises error correctly" do
email = NotifierMailer.invalid_template("user@example.com")
assert_raise(ActionView::Template::Error) { email.html_part.to_s }
test "Invalid template raises error with validation level strict" do
with_settings(validation_level: 'strict') do
email = NotifierMailer.invalid_template("user@example.com")
assert_raise(ActionView::Template::Error) { email.html_part.to_s }
end
end

test "Invalid template gets compiled with validation level soft" do
with_settings(validation_level: 'soft') do
email = NotifierMailer.invalid_template("user@example.com")
assert email.html_part.to_s.blank?
end
end
end

Expand Down
8 changes: 4 additions & 4 deletions test/parser_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,24 @@
it 'uses defaults if no config is set' do
expect(Mjml.beautify).must_equal(true)
expect(Mjml.minify).must_equal(false)
expect(Mjml.validation_level).must_equal('soft')
expect(Mjml.validation_level).must_equal('strict')
end

it 'uses setup config' do
Mjml.setup do |config|
config.beautify = false
config.minify = true
config.validation_level = 'strict'
config.validation_level = 'soft'
end

expect(Mjml.beautify).must_equal(false)
expect(Mjml.minify).must_equal(true)
expect(Mjml.validation_level).must_equal('strict')
expect(Mjml.validation_level).must_equal('soft')

Mjml.setup do |config|
config.beautify = true
config.minify = false
config.validation_level = 'soft'
config.validation_level = 'strict'
end
end
end
Expand Down

0 comments on commit 7dd86bc

Please sign in to comment.