Skip to content

add accessor and regenerate tests#16

Merged
phoet merged 3 commits intomasterfrom
add_problems_accessor
Feb 9, 2021
Merged

add accessor and regenerate tests#16
phoet merged 3 commits intomasterfrom
add_problems_accessor

Conversation

@phoet
Copy link
Copy Markdown
Contributor

@phoet phoet commented Feb 2, 2021

#14

@vfonic i think that this approach is the more rubyesque way to solve this issue. you would need to handle errors and maybe even have your own error-class to show improved error-messages or validation hints.

Copy link
Copy Markdown
Contributor

@vfonic vfonic left a comment

Choose a reason for hiding this comment

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

This pretty-much works. The difference is that, in order to see the detailed explanation of the error (problems), you'll have to catch the error and then do error.problems. Otherwise, in the terminal console, you'll only see Validation Error with no explanation.

So I'm not sure if adding the full error immediately is better.

Comment thread test/webflow_test.rb
field_with_validation: "sh\nrt",
}
begin
client.create_item(COLLECTION_ID, data)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this line doesn't raise, this (and preview test) will pass, without actually testing anything.

Comment thread test/webflow_test.rb
items.each do |item|
result = client.delete_item(item)
assert_equal({"deleted"=>1}, result)
assert_equal({"deleted"=>{}}, result)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is strange. We also have this test when testing our after_destroy callback: https://github.com/vfonic/webflow_sync/blob/master/spec/jobs/webflow_sync/destroy_item_job_spec.rb#L38

@vfonic
Copy link
Copy Markdown
Contributor

vfonic commented Feb 2, 2021

Here's how rails does it:

[1] pry(main)> doc = Document.new;
[2] pry(main)> doc.save!
ActiveRecord::RecordInvalid: Validation failed: Entity must exist, Municipality must exist, File url can't be blank, Title can't be blank, Type is not included in the list
from /Users/viktor/.rvm/gems/ruby-2.6.6/gems/activerecord-6.0.3.4/lib/active_record/validations.rb:80:in `raise_validation_error'

It returns all of the validation errors joined together in a single string:
https://github.com/rails/rails/blob/main/activerecord/lib/active_record/validations.rb#L22

@phoet phoet force-pushed the add_problems_accessor branch from 06f32cb to 5d35ca9 Compare February 9, 2021 21:01
@phoet phoet merged commit e6c0e68 into master Feb 9, 2021
@phoet phoet deleted the add_problems_accessor branch February 9, 2021 21:52
@phoet phoet mentioned this pull request Feb 9, 2021
@vfonic
Copy link
Copy Markdown
Contributor

vfonic commented Feb 10, 2021

Thank you, @phoet! 🚀

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.

2 participants