Adding conditional callbacks

[jwoertink]

Fixes #483

This PR does 2 things.

• It removes the skip_* macros that I just put in the other day
• It adds in the ability to conditionally run callbacks

You can read through the issue for more info, but a quick TL;DR: You can still skip callbacks, it's just done by override a method yourself instead of a macro doing it for you.

This allows us to now do things like this:

abstract class SaveTransaction < Transaction::SaveOperation
  before_save :validate_card_number, if: :new_transaction?
  def validate_card_number
    card_number.value == 42
  end
end

class CreateTransaction < SaveTransaction
  permit_columns :card_number, :exp_date
  private def new_transaction?
    true
  end
end

class UpdateTransaction < SaveTransaction
  permit_columns :exp_date
  private def new_transaction?
    false
  end
end

[matthewmcgarvey]

Just missing the error for an invalid unless, otherwise it looks good and I'm glad we can remove those skips.

[paulcsmith]

So stoked for this!

[jwoertink]

/sparkle @matthewmcgarvey for the review

[jwoertink]

I'm moving these compile errors to look like this:

[jwoertink]

weird... no clue why ameba failed randomly... Probably a fluke though.

[jwoertink]

Ok, turns out it's doing this locally as well

[matthewmcgarvey]

@jwoertink it seems to be the Callbacks module. Here is the most minimal I'm able to make the file and still cause that error

module Avram::Callbacks
  macro conditional_error_for_inline_callbacks
    \{%
      raise ""
    %}
  end

  macro before_save(if _if = nil)
  end
end

[jwoertink]

It seems I've unearthed some ameba bugs... crystal-ameba/ameba#169 😂 oops..

[veelenga]

@jwoertink the buggy rule is disabled for a while. I think this work is not blocked anymore by Ameba if you pull the latest master. Thanks for reporting.

[jwoertink]

@veelenga cool, thanks! I was mainly keeping it open so we could reference it and use it for a "test ground" if we needed.

[veelenga]

@jwoertink could you please elaborate on why the leading slash is needed here? I think this is causing the compiler to report the incorrect location.

As I can see from the doc it is used to prevent evaluating a macro the outer macro, which is not the case here.

https://crystal-lang.org/reference/syntax_and_semantics/macros.html#nested-macros

[jwoertink]

I have the macro interpolation in this, and without the leading slashes, the compile-time error produces the curly brace literals.

# with the leading slash, I get
You must pass a Symbol to `if`

# without the leading slash, I get
You must pass a Symbol to `{{ condition.id }}`

[matthewmcgarvey]

Woops! didn't realize I was still blocking

[jwoertink]

Sweet! Thanks for the update @matthewmcgarvey. Now that it's using Ameba master, it's all passing. Also looks like the crystal issue was fixed in master, so next release of those will be good again 👍