-
Notifications
You must be signed in to change notification settings - Fork 533
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 STI relationship option #1359
base: release-0-10
Are you sure you want to change the base?
Conversation
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.
I think this is what @JamesGlover was referring to
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, primary_key)} AS \"#{linkage_table_alias}_#{primary_key}\"") | ||
|
||
if linkage_relationship.sti? | ||
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, 'type')} AS \"#{linkage_table_alias}_type\"") |
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.
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, 'type')} AS \"#{linkage_table_alias}_type\"") | |
pluck_fields << Arel.sql("#{concat_table_field(linkage_table_alias, klass.inheritance_column)} AS \"#{linkage_table_alias}_type\"") |
right?
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.
I think at this point klass is the resource, not the ActiveRecord::Base class, so you'd also need to define an inheritance_column
method on JSONAPI::BasicResource
.
Although in practice you'd probably want to use _inheritance_column
instead, to be consistent with _primary_key
.
# lib/jsonapi/basic_resource.rb
def inheritance_column(key)
@_inheritance_column = key.to_sym
end
def _inheritance_column
@_inheritance_column ||= _default_inheritance_column
end
def _default_inheritance_column
@_default_inheritance_column ||=_model_class.respond_to?(:inheritance_column) ? _model_class.inheritance_column : :type
end
@@ -413,6 +436,10 @@ def find_related_monomorphic_fragments(source_rids, relationship, options, conne | |||
Arel.sql("#{concat_table_field(resource_table_alias, resource_klass._primary_key)} AS #{resource_table_alias}_#{resource_klass._primary_key}") | |||
] | |||
|
|||
if relationship.sti? | |||
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, 'type')} AS #{resource_table_alias}_type") |
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.
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, 'type')} AS #{resource_table_alias}_type") | |
pluck_fields << Arel.sql("#{concat_table_field(resource_table_alias, klass.inheritance_column)} AS #{resource_table_alias}_type") |
right?
This fixes a few issues I found with STI
All Submissions:
New Feature Submissions:
Bug fixes and Changes to Core Features:
Test Plan:
Reviewer Checklist: