Invoke on_assignment callback after assignment is stored in database; add global on_assignment callback

[frostmark]

MVP for #367

• Default callback configurations for on_assignment and after_assignment

Vanity.configure do |config|
  config.on_assignment = proc do |...params|
    # does something useful 
  end
end

• Added after_assignment callback and specs
• Minor fixes

Edit: final version of this PR does not include an after_assignment callback. Instead, the on_assignment callback's call has been moved after the assignment is preserved in the database.

[frostmark]

@bensheldon could you also take a look at this? 👀

[bensheldon]

Just to summarize what I'm seeing here.

• Adds new global Vanity Configuration accessors to assign on_assignment and after_assignment callbacks
• Adds a new Vanity Experiment accessor to assign an anafter_assignment callback. This callback is invoked after the assignment data is saved to the database (whereas the on_assignment callback is invoked before that save takes place)
• The global Vanity Configuration callbacks are invoked in addition to the Experiment callbacks. My intuition of reading "Default" is that an Experiment's callbacks would override the global Configuration's callbacks.
  • What do you think of re-describing this as "Global assignment callbacks"? or...
  • Change the behavior such that the Experiment Callback overrides the global Configuration callback and only one is called?

Reading the original code, I want to suggest a completely different direction too:

• Keep the global on_assignment (I think having either a default or additional callback sounds useful)
• Don't create a separate after_assignment callback and instead we simply move the place where on_assigment is invoked till after it's saved in the database. Is there a functional/behavioral reason to have both callbacks? I did a little digging and didn't really see an explanation for why it should take place before save (i.e. it's not cancelable, there isn't anything subsequent that mutates the value).

[frostmark]

@bensheldon

Change the behavior such that the Experiment Callback overrides the global Configuration callback and only one is called?

Yep, it's already working that way. Default callback can be overridden in Experiment

Keep the global on_assignment (I think having either a default or additional callback sounds useful)

Don't create a separate after_assignment callback and instead we simply move the place where on_assigment is invoked till after it's saved in the database. Is there a functional/behavioral reason to have both callbacks? I did a little digging and didn't really see an explanation for why it should take place before save (i.e. it's not cancelable, there isn't anything subsequent that mutates the value).

Yep, I agree that we may don't add after_assignment. I just didn't want to change the current behavior of on_assignment, but I agree that there is no need to add a new type of callback and we can move on_assigment

[bensheldon]

@frostmark this looks good. Let me know when I should review.

[frostmark]

@bensheldon this is ready for review

[bensheldon]

👍 Looks good. Thanks for the discussion on this.

[bensheldon]

I spent some time looking at this, because this guard clause was lifted up out of the #call_on_assignment_if_available method. I think it's safe and an improvement because it ensures re-assigning the same assignment.

[bensheldon]

Released in v3.1.0