From 78937ff57e80a9b1fcebc5f0bde66b6e19e26023 Mon Sep 17 00:00:00 2001 From: Stephen Ierodiaconou Date: Fri, 16 Aug 2024 17:13:40 +0200 Subject: [PATCH] Fix `belongs_to` and ID field linking to child resources if configured (#3109) * Fix such that belongs_to fields respect `link_to_child_resource` on resource or on field options * Fix automatic linking of id fields to child resources if requested * Small refactor to not shadow method * Update app/components/avo/base_component.rb --------- Co-authored-by: Paul Bob <69730720+Paul-Bob@users.noreply.github.com> --- app/components/avo/base_component.rb | 6 +- lib/avo/fields/belongs_to_field.rb | 6 +- lib/avo/fields/has_base_field.rb | 3 +- spec/dummy/app/avo/resources/person.rb | 3 +- spec/dummy/app/models/person.rb | 1 + .../avo/link_to_child_resource_spec.rb | 66 +++++++++++++++++++ 6 files changed, 80 insertions(+), 5 deletions(-) diff --git a/app/components/avo/base_component.rb b/app/components/avo/base_component.rb index cd65f5515e..a8ed488130 100644 --- a/app/components/avo/base_component.rb +++ b/app/components/avo/base_component.rb @@ -57,12 +57,14 @@ def parent_or_child_resource end def link_to_child_resource_is_enabled? - return field_linked_to_child_resource? if @parent_resource + enabled = field_linked_to_child_resource? if @parent_resource + return enabled unless enabled.nil? @resource.link_to_child_resource end def field_linked_to_child_resource? - field.present? && field.respond_to?(:link_to_child_resource) && field.link_to_child_resource + resolved_field = @reflection ? @parent_resource.get_field(params[:for_attribute] || @reflection.name) : field + resolved_field.link_to_child_resource if resolved_field.respond_to?(:link_to_child_resource) end end diff --git a/lib/avo/fields/belongs_to_field.rb b/lib/avo/fields/belongs_to_field.rb index 049e27d0dc..d5d94d9602 100644 --- a/lib/avo/fields/belongs_to_field.rb +++ b/lib/avo/fields/belongs_to_field.rb @@ -69,6 +69,7 @@ class BelongsToField < BaseField attr_reader :attach_scope attr_reader :polymorphic_help attr_reader :link_to_record + attr_reader :link_to_child_resource def initialize(id, **args, &block) args[:placeholder] ||= I18n.t("avo.choose_an_option") @@ -86,6 +87,7 @@ def initialize(id, **args, &block) @use_resource = args[:use_resource] || nil @can_create = args[:can_create].nil? ? true : args[:can_create] @link_to_record = args[:link_to_record].present? ? args[:link_to_record] : false + @link_to_child_resource = args[:link_to_child_resource] end def value @@ -251,7 +253,9 @@ def target_resource(record: @record, polymorphic_model_class: value&.class) reflection_object = record.class.reflect_on_association(reflection_key) - if reflection_object.klass.present? + if (link_to_child_resource || @resource&.link_to_child_resource) && value.present? + get_resource_by_model_class(value.class.to_s) + elsif reflection_object.klass.present? get_resource_by_model_class(reflection_object.klass.to_s) elsif reflection_object.options[:class_name].present? get_resource_by_model_class(reflection_object.options[:class_name]) diff --git a/lib/avo/fields/has_base_field.rb b/lib/avo/fields/has_base_field.rb index 018fa9096b..7e45b0d07d 100644 --- a/lib/avo/fields/has_base_field.rb +++ b/lib/avo/fields/has_base_field.rb @@ -25,7 +25,8 @@ def initialize(id, **args, &block) @description = args[:description] @use_resource = args[:use_resource] || nil @discreet_pagination = args[:discreet_pagination] || false - @link_to_child_resource = args[:link_to_child_resource] || false + # Defaults to nil so that if not set falls back to `link_to_child_resource` defined in the resource + @link_to_child_resource = args[:link_to_child_resource] @reloadable = args[:reloadable].present? ? args[:reloadable] : false @linkable = args[:linkable].present? ? args[:linkable] : false @attach_fields = args[:attach_fields] diff --git a/spec/dummy/app/avo/resources/person.rb b/spec/dummy/app/avo/resources/person.rb index 8564d9db65..2fe4638dd8 100644 --- a/spec/dummy/app/avo/resources/person.rb +++ b/spec/dummy/app/avo/resources/person.rb @@ -13,6 +13,8 @@ def fields field :type, as: ::Pluggy::Fields::RadioField, name: "Type", options: {Spouse: "Spouse", Sibling: "Sibling"}, include_blank: true, filterable: true field :link, as: :text, as_html: true field :spouses, as: :has_many, hide_search_input: true + field :person, as: :belongs_to + field :another_person, as: :belongs_to, link_to_child_resource: true field :relatives, as: :has_many, hide_search_input: true, @@ -21,7 +23,6 @@ def fields field :peoples, as: :has_many, hide_search_input: true, - link_to_child_resource: false, description: "Default behaviour with link_to_child_resource disabled" end end diff --git a/spec/dummy/app/models/person.rb b/spec/dummy/app/models/person.rb index 55f8c16f5a..07b8fff704 100644 --- a/spec/dummy/app/models/person.rb +++ b/spec/dummy/app/models/person.rb @@ -13,6 +13,7 @@ class Person < ApplicationRecord belongs_to :user, optional: true belongs_to :person, foreign_key: :person_id, optional: true + belongs_to :another_person, foreign_key: :person_id, optional: true, class_name: "Person" has_many :spouses, foreign_key: :person_id, inverse_of: :person has_many :relatives, foreign_key: :person_id, class_name: "Person", inverse_of: :person has_many :peoples, foreign_key: :person_id, class_name: "Person", inverse_of: :person diff --git a/spec/features/avo/link_to_child_resource_spec.rb b/spec/features/avo/link_to_child_resource_spec.rb index 0a07a0cb44..c5ae3c0298 100644 --- a/spec/features/avo/link_to_child_resource_spec.rb +++ b/spec/features/avo/link_to_child_resource_spec.rb @@ -3,6 +3,14 @@ require "rails_helper" RSpec.describe "LinkToChildResource", type: :feature do + around do |example| + prev_state = Avo::Resources::Person.link_to_child_resource + # This test expects the fallback to be false + Avo::Resources::Person.link_to_child_resource = false + example.run + Avo::Resources::Person.link_to_child_resource = prev_state + end + describe "link_to_child_resource " do context "Resource linking to child resources" do let!(:sibling) { create :sibling, name: "sibling" } @@ -19,6 +27,36 @@ end end + context "Resource show page links to associated resources" do + let(:paul) { create :sibling, name: "paul" } + let(:john) { create :person, name: "john", person: paul } + it "display belongs_to association linked to the child resource if set at resource level" do + Avo::Resources::Person.link_to_child_resource = true + visit "/admin/resources/people/#{john.id}" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/siblings/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + + it "display belongs_to association linked to the parent class resource if set at resource level" do + Avo::Resources::Person.link_to_child_resource = false + visit "/admin/resources/people/#{john.id}" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/people/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + + it "display belongs_to association linked to the child resource if set at field level" do + visit "/admin/resources/people/#{john.id}" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/siblings/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + + it "display belongs_to association linked to the parent class resource if set at field level" do + visit "/admin/resources/people/#{john.id}" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/people/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + end + context "Field linking to child resources" do let(:paul) { create :sibling, name: "paul" } let(:lisa) { create :spouse, name: "lisa" } @@ -42,6 +80,34 @@ expect(page).to have_link(href: "/admin/resources/people/#{paul.id}/edit?via_record_id=#{john.to_param}&via_resource_class=Avo%3A%3AResources%3A%3APerson") expect(page).to have_link(href: "/admin/resources/people/#{lisa.id}/edit?via_record_id=#{john.to_param}&via_resource_class=Avo%3A%3AResources%3A%3APerson") end + + context "id field" do + it "links to the parent class resource if link_to_child_resource false at the resource level" do + Avo::Resources::Person.link_to_child_resource = false + visit "/admin/resources/people/#{john.id}/peoples?turbo_frame=has_many_field_show_relatives" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/people/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + + it "links to the child class resource if link_to_child_resource true at the resource level" do + Avo::Resources::Person.link_to_child_resource = true + visit "/admin/resources/people/#{john.id}/peoples?turbo_frame=has_many_field_show_relatives" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/siblings/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + + it "links to the parent class resource if link_to_child_resource false at the field level" do + visit "/admin/resources/people/#{john.id}/peoples?turbo_frame=has_many_field_show_relatives" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/people/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + + it "links to the child class resource if link_to_child_resource true at the field level" do + visit "/admin/resources/people/#{john.id}/relatives?turbo_frame=has_many_field_show_relatives" + wait_for_loaded + expect(page).to have_link(paul.name, href: "/admin/resources/siblings/#{paul.id}?via_record_id=#{john.id}&via_resource_class=Avo%3A%3AResources%3A%3APerson") + end + end end end end