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

Allow overriding column setter #226

Open
mloughran opened this issue May 12, 2022 · 3 comments
Open

Allow overriding column setter #226

mloughran opened this issue May 12, 2022 · 3 comments

Comments

@mloughran
Copy link

Apologies if this is possible and I've overlooked.

I'm struggling to understand why the custom Account#email= method in the following does not get called:

require "spec"
require "clear"

class Account
  include Clear::Model

  column email : String

  def email=(e : String)
    super(email.downcase.strip)
  end
end

describe Account do
  it "overrides email setter" do
    account = Account.new
    account.email = "FOO@example.com "
    account.email.should eq("foo@example.com")
  end
end

Is there a better way of achieving this? Thanks!

@Vici37
Copy link
Contributor

Vici37 commented Jul 7, 2022

I think clear implements a lot of defs in a finished macro, which would be defined after your own email= def, overwriting whatever you had with its own. You should be able to overwrite its own def with yours by wrapping your method with a finished macro:

macro finished
  def email=(e : String)
    previous_def(email.downcase.strip)
  end
end

No idea if you still have this problem, since it was posted so long ago 😬

@Vici37
Copy link
Contributor

Vici37 commented Jul 7, 2022

And thinking about it, you might want to replace super with previous_def, since you don't want to call the parent classes' method of email=, which doesn't exist for Object, only the previous definition of it.

@mloughran
Copy link
Author

@Vici37 thanks – I can confirm that using macro finished and previous_def does indeed allow overriding the setter method. As a newcomer to crystal/clear I would say this was not particularly obvious!

I ended up writing a before :validate hook method to perform this cleanup. This would seem to be the way to go since column= methods are not called when mass assigning values (unlike in sequel from which I was porting some code).

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

No branches or pull requests

2 participants