-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix inheritance sample code in CONCEPTS.md #35
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #35 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 271 271
Branches 48 48
=========================================
Hits 271 271 Continue to review full report at Codecov.
|
dependency :preference, :profile | ||
computed def display_name | ||
"#{preference.title} #{profile.name}" | ||
included 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.
This is not an intended usage. Is there a problem with computed_model itself? If it's pb-serializer's problem, please leave it as-is and write a notice somewhere else.
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.
Although include do ... end
may work as well, this is unrelated to computed_model's inheritance feature, so the example won't fit here.
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.
This seems to be a problem on the computed model side.
Here is the code that reproduces it. If you do not use included, you will get an error.
module UserConcern
extend ActiveSupport::Concern
include ComputedModel::Model
dependency raw_user: [:first_name, :last_name]
computed def name
[raw_user.first_name, raw_user.last_name].map(&:presence).compact.join(' ')
end
end
# NoMethodError: undefined method `dependency' for UserConcern:Module
# from <main>:5:in `<module:UserConcern>'
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, reproduced! It's strange though. Kinda ActiveSupport::Concern
's corner case? The following modifications work.
module UserConcern
# extend ActiveSupport::Concern
include ComputedModel::Model
dependency raw_user: [:first_name, :last_name]
computed def name
[raw_user.first_name, raw_user.last_name].map(&:presence).compact.join(' ')
end
end
module UserConcern
extend ActiveSupport::Concern
include ComputedModel::Model
extend ComputedModel::Model::ClassMethods
dependency raw_user: [:first_name, :last_name]
computed def name
[raw_user.first_name, raw_user.last_name].map(&:presence).compact.join(' ')
end
end
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.
Anyway include do ... end
is not an intended fix. Let's investigate it in #36
No description provided.