-
Notifications
You must be signed in to change notification settings - Fork 83
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
More descriptive error messages #108
More descriptive error messages #108
Conversation
it "raises Discard::RecordNotUndiscarded" do | ||
expect(post).to receive(:do_before_undiscard) { abort_callback } | ||
expect { | ||
post.undiscard! | ||
}.to raise_error(Discard::RecordNotUndiscarded) | ||
}.to raise_error(Discard::RecordNotUndiscarded, "Failed to undiscard the record") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also noticed this test had the wrong name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this seems like a good change. I just need that typo fixed.
lib/discard/model.rb
Outdated
raise ::Discard::RecordNotUndiscarded.new(undiscarded_fail_message, self) | ||
end | ||
|
||
def disacrded_fail_message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in this method name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -677,11 +677,11 @@ def abort_callback | |||
end | |||
|
|||
describe '#undiscard!' do | |||
it "raises Discard::RecordNotDiscarded" do | |||
it "raises Discard::RecordNotUndiscarded" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
Looks good! I will merge this once I've figured out what's going on with CI. |
Thank you! Have a lovely day/afternoon/evening 😄 |
Thank you! |
While working with the discard gem I noticed that you can't discard already discarded records. There wasn't any mention of this in the docs and the error message wasn't very helpful. It cost me some time debugging.
Hopefully this saves others a bit of time :)