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

add after_cast method to call ActiveType cast #179

Closed

Conversation

MaximilianoGarciaRoe
Copy link
Contributor

Hi there!

Apologies for the delay; I've been quite busy lately. However, I'm now sending the solution that addresses the issue I raised some time ago in the pull request.

#174

The idea is to enable the Active Type Record to have a base method for adding logic after executing the cast. This way, users can easily add business logic as needed.

I hope my contribution will be considered, and I genuinely appreciate your time and attention to this matter. Thank you!

@@ -0,0 +1,9 @@
module ActiveType
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it's monkey-patching ActiveRecord::Base, but it's embedded under a module for some reason.

I would remove this module and rather check whether the casted object responds to after_cast

@@ -59,6 +60,9 @@ def cast_record(record, klass, force: false)
if !force
make_record_unusable(record)
end

casted.after_cast(record)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
casted.after_cast(record)
casted.after_cast(record) if casted.respond_to?(:after_cast)

@kratob
Copy link
Member

kratob commented Dec 22, 2023

Sorry for the long delay. I've rebased and adjusted this a bit; #after_cast is now part of ActiveType 2.4.0.

@kratob kratob closed this Dec 22, 2023
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.

3 participants