fix: more flexible polymorphic types lookup#1434
Merged
Conversation
bf4
commented
Jan 22, 2024
| next unless Module === klass | ||
| if ActiveRecord::Base > klass | ||
| is_active_record_inspectable = ActiveRecord::Base > klass | ||
| is_active_record_inspectable &&= klass.respond_to?(:reflect_on_all_associations, true) |
Collaborator
Author
There was a problem hiding this comment.
without this we get errors in our test suite upgrading from 0.9 like
undefined method `reflect_on_all_associations' for #<refinement:ActiveRecord::Base@TestProf::Ext::ActiveRecordRefind>\n" +
klass.reflect_on_all_associations(:has_many).select { |r| r.options[:as] }.each do |reflection|\n" +
^^^^^^^^^^^^^^^^^^^^^^^^^^^^",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:21:in `block (2 levels) in build_polymorphic_types_lookup'",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:18:in `each_object'",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:18:in `block in build_polymorphic_types_lookup'",
and
undefined method `underscore' for nil:NilClass\n" +
(hash[reflection.options[:as]] ||= []) << klass.name.underscore\n" +
^^^^^^^^^^^",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:22:in `block (3 levels) in build_polymorphic_types_lookup'",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:21:in `each'",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:21:in `block (2 levels) in build_polymorphic_types_lookup'",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:18:in `each_object'",
"/app/.bundle/vendor/ruby/3.1.0/bundler/gems/jsonapi-resources-76ef777fd012/lib/jsonapi/utils/polymorphic_types_lookup.rb:18:in `block in build_polymorphic_types_lookup'",
where klass.inspect is #<#<Class:TenderJobScheduleShift(. Interestingly, klass.superclass seems to work here
Collaborator
Author
There was a problem hiding this comment.
notably https://github.com/cerebris/jsonapi-resources/pull/1045 there was a change to inferring of class_name which I noted in 0.10
I haven't confirmed yet if it helps in 0.11
I wrote this override in our 0.10 test branch
# since https://github.com/cerebris/jsonapi-resources/pull/1045/files
def define_relationship_methods(relationship_name, relationship_klass, options)
if !options.key?(:class_name)
# Initialize from an ActiveRecord model's properties
if _model_class && _model_class.ancestors.collect { |ancestor| ancestor.name }.include?("ActiveRecord::Base")
model_association = _model_class.reflect_on_association(relationship_name)
if model_association
reflected_class_name = model_association.class_name
if reflected_class_name.underscore.downcase.singularize != relationship_name.to_s.singularize
options[:class_name] = reflected_class_name
# warn "[JR-TEST] Added resource #{name} relationship #{relationship_name} option class_name=#{options[:class_name]}"
end
end
end
end
super(relationship_name, relationship_klass, options)
endaf00515 to
30445f1
Compare
bf4
commented
Jan 24, 2024
| # # klass.base_class may be nil | ||
| # warn "[POLYMORPHIC TYPE] #{__callee__} #{klass} #{ex.inspect}" | ||
| # nil | ||
| # end |
bf4
commented
Jan 24, 2024
| # is_active_record_inspectable = true | ||
| # is_active_record_inspectable &&= klass.respond_to?(:reflect_on_all_associations, true) | ||
| # is_active_record_inspectable &&= format_polymorphic_klass_type(klass).present? | ||
| # return unless is_active_record_inspectable |
bf4
commented
Jan 24, 2024
| polymorphic_types_lookup[name.to_sym] | ||
| singleton_class.attr_accessor :build_polymorphic_types_lookup_strategy | ||
| self.build_polymorphic_types_lookup_strategy = | ||
| :build_polymorphic_types_lookup_from_object_space |
bf4
commented
Jan 24, 2024
| end | ||
|
|
||
| initializer "jsonapi_resources.initialize", after: :initialize do | ||
| JSONAPI::Utils::PolymorphicTypesLookup.polymorphic_types_lookup_clear! |
bf4
commented
Jan 24, 2024
| types = self.class.polymorphic_types(_relation_name) | ||
| unless types.present? | ||
| warn "No polymorphic types found for #{parent_resource.name} #{_relation_name}" | ||
| warn "[POLYMORPHIC TYPE] No polymorphic types found for #{parent_resource.name} #{_relation_name}" |
bf4
commented
Jan 24, 2024
|
|
||
| def handle_polymorphic_type_name_found | ||
| @handle_polymorphic_type_name_found ||= lambda do |name| | ||
| warn "[POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for #{name}" |
they pass on v-11-dev I'm going to look into the existing lookup warnings now ``` [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for fileable [POLYMORPHIC TYPE] No polymorphic types found for FilePropertiesResource fileable [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for respondent [POLYMORPHIC TYPE] No polymorphic types found for QuestionResource respondent [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for respondent [POLYMORPHIC TYPE] No polymorphic types found for AnswerResource respondent [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for keepable [POLYMORPHIC TYPE] No polymorphic types found for KeeperResource keepable ```
This reverts commit 0979a7243b6bc816dd2327d3ff23f70209c52dce.
30445f1 to
93ac314
Compare
9 tasks
lgebhardt
pushed a commit
that referenced
this pull request
Apr 18, 2024
* fix: more flexible polymorphic types lookup * test: add polymorphic lookup tests they pass on v-11-dev I'm going to look into the existing lookup warnings now ``` [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for fileable [POLYMORPHIC TYPE] No polymorphic types found for FilePropertiesResource fileable [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for respondent [POLYMORPHIC TYPE] No polymorphic types found for QuestionResource respondent [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for respondent [POLYMORPHIC TYPE] No polymorphic types found for AnswerResource respondent [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for keepable [POLYMORPHIC TYPE] No polymorphic types found for KeeperResource keepable ``` * Revert "test: add polymorphic lookup tests" This reverts commit 0979a7243b6bc816dd2327d3ff23f70209c52dce. * feat: easily clear the lookup * feat: add a descendents strategy * test: polymorphic type lookup * feat: make polymorphic type lookup configurable * feat: clear polymorphic lookup after initialize
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
they pass on v-11-dev
I'm going to look into the existing lookup warnings
now
All Submissions:
New Feature Submissions:
Bug fixes and Changes to Core Features:
Test Plan:
Reviewer Checklist: