From 617ead62d09f3f8c04a7438c5fc7cc461f5fcee8 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Wed, 11 Dec 2024 10:32:57 -0800 Subject: [PATCH 01/17] feature: add automatic field detection in resources --- lib/avo/concerns/has_field_discovery.rb | 183 ++++++++++++++++++ lib/avo/configuration.rb | 4 + lib/avo/resources/base.rb | 1 + lib/avo/resources/items/sidebar.rb | 2 + spec/dummy/app/avo/resources/compact_user.rb | 10 +- .../app/avo/resources/field_discovery_user.rb | 35 ++++ .../avo/field_discovery_users_controller.rb | 4 + spec/dummy/config/initializers/avo.rb | 4 + spec/system/avo/has_field_discovery_spec.rb | 83 ++++++++ 9 files changed, 319 insertions(+), 7 deletions(-) create mode 100644 lib/avo/concerns/has_field_discovery.rb create mode 100644 spec/dummy/app/avo/resources/field_discovery_user.rb create mode 100644 spec/dummy/app/controllers/avo/field_discovery_users_controller.rb create mode 100644 spec/system/avo/has_field_discovery_spec.rb diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb new file mode 100644 index 0000000000..2890ac4bd4 --- /dev/null +++ b/lib/avo/concerns/has_field_discovery.rb @@ -0,0 +1,183 @@ +module Avo + module Concerns + # This concern facilitates field discovery for models in Avo, mapping database columns and associations to Avo fields. + # It supports: + # - Automatic detection of fields based on column names, types, and associations. + # - Customization via `only`, `except`, and global configuration overrides. + # - Handling of special associations like rich text, attachments, and tags. + module HasFieldDiscovery + extend ActiveSupport::Concern + + DEFAULT_COLUMN_NAMES_MAPPING = { + id: { field: "id" }, + description: { field: "textarea" }, + gravatar: { field: "gravatar" }, + email: { field: "text" }, + password: { field: "password" }, + password_confirmation: { field: "password" }, + created_at: { field: "date_time" }, + updated_at: { field: "date_time" }, + stage: { field: "select" }, + budget: { field: "currency" }, + money: { field: "currency" }, + country: { field: "country" }, + }.freeze + + DEFAULT_COLUMN_TYPES_MAPPING = { + primary_key: { field: "id" }, + string: { field: "text" }, + text: { field: "textarea" }, + integer: { field: "number" }, + float: { field: "number" }, + decimal: { field: "number" }, + datetime: { field: "date_time" }, + timestamp: { field: "date_time" }, + time: { field: "date_time" }, + date: { field: "date" }, + binary: { field: "number" }, + boolean: { field: "boolean" }, + references: { field: "belongs_to" }, + json: { field: "code" }, + }.freeze + + COLUMN_NAMES_TO_IGNORE = %i[ + encrypted_password reset_password_token reset_password_sent_at remember_created_at password_digest + ].freeze + + class_methods do + def column_names_mapping + @column_names_mapping ||= DEFAULT_COLUMN_NAMES_MAPPING.dup + .except(*COLUMN_NAMES_TO_IGNORE) + .merge(Avo.configuration.column_names_mapping || {}) + end + + def column_types_mapping + @column_types_mapping ||= DEFAULT_COLUMN_TYPES_MAPPING.dup + .merge(Avo.configuration.column_types_mapping || {}) + end + end + + # Returns database columns for the model, excluding ignored columns + def model_db_columns + @model_db_columns ||= safe_model_class.columns_hash.symbolize_keys.except(*COLUMN_NAMES_TO_IGNORE) + end + + # Discovers and configures database columns as fields + def discover_columns(only: nil, except: nil, **field_options) + @only, @except, @field_options = only, except, field_options + return unless safe_model_class.respond_to?(:columns_hash) + + model_db_columns.each do |column_name, column| + next unless column_in_scope?(column_name) + next if reflections.key?(column_name) || rich_texts.key?("rich_text_#{column_name}") + + field_config = determine_field_config(column_name, column) + next unless field_config + + field_options = build_field_options(field_config, column) + field column_name, **field_options, **@field_options + end + end + + # Discovers and configures associations as fields + def discover_associations(only: nil, except: nil, **field_options) + @only, @except, @field_options = only, except, field_options + return unless safe_model_class.respond_to?(:reflections) + + discover_by_type(tags, :tags) { |name| name.split("_").pop.join("_").pluralize } + discover_by_type(rich_texts, :trix) { |name| name.delete_prefix("rich_text_") } + discover_attachments + discover_basic_associations + end + + private + + # Fetches the model class, falling back to the items_holder parent record in certain instances (e.g. in the context of the sidebar) + def safe_model_class + respond_to?(:model_class) ? model_class : @items_holder.parent.record.class + rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished + nil + end + + # Determines if a column is included in the discovery scope. + # A column is in scope if it's included in `only` and not in `except`. + def column_in_scope?(column_name) + (!@only || @only.include?(column_name)) && (!@except || !@except.include?(column_name)) + end + + def determine_field_config(attribute, column) + if safe_model_class.respond_to?(:defined_enums) && safe_model_class.defined_enums[attribute.to_s] + return { field: "select", enum: "::#{safe_model_class.name}.#{attribute.to_s.pluralize}" } + end + + self.class.column_names_mapping[attribute] || self.class.column_types_mapping[column.type] + end + + def build_field_options(field_config, column) + { as: field_config[:field].to_sym, required: !column.null }.merge(field_config.except(:field)) + end + + def discover_by_type(associations, as_type) + associations.each_key do |association_name| + next unless column_in_scope?(association_name) + + field association_name, as: as_type, **@field_options.merge(yield(association_name)) + end + end + + def discover_attachments + attachment_associations.each do |association_name, reflection| + next unless column_in_scope?(association_name) + + field_type = reflection.options[:as] == :has_one_attached ? :file : :files + field association_name, as: field_type, **@field_options + end + end + + def discover_basic_associations + associations.each do |association_name, reflection| + next unless column_in_scope?(association_name) + + options = { as: reflection.macro, searchable: true, sortable: true } + options.merge!(polymorphic_options(reflection)) if reflection.options[:polymorphic] + + field association_name, **options, **@field_options + end + end + + def polymorphic_options(reflection) + { polymorphic_as: reflection.name, types: detect_polymorphic_types(reflection) } + end + + def detect_polymorphic_types(reflection) + ApplicationRecord.descendants.select { |klass| klass.reflections[reflection.plural_name] } + end + + def reflections + @reflections ||= safe_model_class.reflections.symbolize_keys.reject do |name, _| + ignore_reflection?(name.to_s) + end + end + + def attachment_associations + @attachment_associations ||= reflections.select { |_, r| r.options[:class_name] == "ActiveStorage::Attachment" } + end + + def rich_texts + @rich_texts ||= reflections.select { |_, r| r.options[:class_name] == "ActionText::RichText" } + end + + def tags + @tags ||= reflections.select { |_, r| r.options[:as] == :taggable } + end + + def associations + @associations ||= reflections.reject { |key| attachment_associations.key?(key) || tags.key?(key) || rich_texts.key?(key) } + end + + def ignore_reflection?(name) + %w[blob blobs tags].include?(name.split("_").pop) || name.to_sym == :taggings + end + end + end +end diff --git a/lib/avo/configuration.rb b/lib/avo/configuration.rb index 5155957bb2..a0a9f4f185 100644 --- a/lib/avo/configuration.rb +++ b/lib/avo/configuration.rb @@ -58,6 +58,8 @@ class Configuration attr_accessor :search_results_count attr_accessor :first_sorting_option attr_accessor :associations_lookup_list_limit + attr_accessor :column_names_mapping + attr_accessor :column_types_mapping def initialize @root_path = "/avo" @@ -123,6 +125,8 @@ def initialize @first_sorting_option = :desc # :desc or :asc @associations_lookup_list_limit = 1000 @exclude_from_status = [] + @column_names_mapping = {} + @column_types_mapping = {} end # Authorization is enabled when: diff --git a/lib/avo/resources/base.rb b/lib/avo/resources/base.rb index b8b8cae16e..2f4672a362 100644 --- a/lib/avo/resources/base.rb +++ b/lib/avo/resources/base.rb @@ -4,6 +4,7 @@ class Base extend ActiveSupport::DescendantsTracker include ActionView::Helpers::UrlHelper + include Avo::Concerns::HasFieldDiscovery include Avo::Concerns::HasItems include Avo::Concerns::CanReplaceItems include Avo::Concerns::HasControls diff --git a/lib/avo/resources/items/sidebar.rb b/lib/avo/resources/items/sidebar.rb index 0e894cfd88..e1927e3d2a 100644 --- a/lib/avo/resources/items/sidebar.rb +++ b/lib/avo/resources/items/sidebar.rb @@ -1,6 +1,7 @@ class Avo::Resources::Items::Sidebar prepend Avo::Concerns::IsResourceItem + include Avo::Concerns::HasFieldDiscovery include Avo::Concerns::HasItems include Avo::Concerns::HasItemType include Avo::Concerns::IsVisible @@ -26,6 +27,7 @@ def panel_wrapper? class Builder include Avo::Concerns::BorrowItemsHolder + include Avo::Concerns::HasFieldDiscovery delegate :field, to: :items_holder delegate :tool, to: :items_holder diff --git a/spec/dummy/app/avo/resources/compact_user.rb b/spec/dummy/app/avo/resources/compact_user.rb index fcb7995efe..619abdda4a 100644 --- a/spec/dummy/app/avo/resources/compact_user.rb +++ b/spec/dummy/app/avo/resources/compact_user.rb @@ -6,15 +6,11 @@ class Avo::Resources::CompactUser < Avo::BaseResource def fields field :personal_information, as: :heading - - field :first_name, as: :text - field :last_name, as: :text - field :birthday, as: :date + discover_columns only: [:first_name, :last_name, :birthday] field :heading, as: :heading, label: "Contact" + discover_columns only: [:email] - field :email, as: :text - - field :posts, as: :has_many + discover_associations only: [:posts] end end diff --git a/spec/dummy/app/avo/resources/field_discovery_user.rb b/spec/dummy/app/avo/resources/field_discovery_user.rb new file mode 100644 index 0000000000..a6bf9b16b9 --- /dev/null +++ b/spec/dummy/app/avo/resources/field_discovery_user.rb @@ -0,0 +1,35 @@ +class Avo::Resources::FieldDiscoveryUser < Avo::BaseResource + self.model_class = ::User + self.description = 'This is a resource with discovered fields. It will show fields and associations as defined in the model.' + self.find_record_method = -> { + query.friendly.find id + } + + def fields + main_panel do + discover_columns except: %i[email active is_admin? birthday is_writer outside_link custom_css] + discover_associations only: %i[cv_attachment] + + sidebar do + with_options only_on: :show do + discover_columns only: %i[email], as: :gravatar, link_to_record: true, as_avatar: :circle + field :heading, as: :heading, label: "" + discover_columns only: %i[active], name: "Is active" + end + + discover_columns only: %i[birthday] + + field :password, as: :password, name: "User Password", required: false, only_on: :forms, help: 'You may verify the password strength here.' + field :password_confirmation, as: :password, name: "Password confirmation", required: false, revealable: true + + with_options only_on: :forms do + field :dev, as: :heading, label: '
DEV
', as_html: true + discover_columns only: %i[custom_css] + end + end + end + + discover_associations only: %i[posts] + discover_associations except: %i[posts post cv_attachment] + end +end diff --git a/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb b/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb new file mode 100644 index 0000000000..4ef6e31b11 --- /dev/null +++ b/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb @@ -0,0 +1,4 @@ +# This controller has been generated to enable Rails' resource routes. +# More information on https://docs.avohq.io/3.0/controllers.html +class Avo::FieldDiscoveryUsersController < Avo::ResourcesController +end diff --git a/spec/dummy/config/initializers/avo.rb b/spec/dummy/config/initializers/avo.rb index f9bd5c6370..98180de559 100644 --- a/spec/dummy/config/initializers/avo.rb +++ b/spec/dummy/config/initializers/avo.rb @@ -102,6 +102,10 @@ # type: :countless # } # end + + config.column_names_mapping = { + custom_css: { field: "code" }, + } end if defined?(Avo::DynamicFilters) diff --git a/spec/system/avo/has_field_discovery_spec.rb b/spec/system/avo/has_field_discovery_spec.rb new file mode 100644 index 0000000000..ea9968d3da --- /dev/null +++ b/spec/system/avo/has_field_discovery_spec.rb @@ -0,0 +1,83 @@ +require "rails_helper" + +RSpec.describe Avo::Concerns::HasFieldDiscovery, type: :system do + let!(:user) { create :user, first_name: "John", last_name: "Doe", birthday: "1990-01-01", email: "john.doe@example.com" } + let!(:post) { create :post, user: user, name: "Sample Post" } + + describe "Show Page" do + let(:url) { "/admin/resources/field_discovery_users/#{user.slug}" } + + before { visit url } + + it "displays discovered columns correctly" do + wait_for_loaded + + # Verify discovered columns + expect(page).to have_text "FIRST NAME" + expect(page).to have_text "John" + expect(page).to have_text "LAST NAME" + expect(page).to have_text "Doe" + expect(page).to have_text "BIRTHDAY" + expect(page).to have_text "1990-01-01" + + # Verify excluded fields are not displayed + expect(page).not_to have_text "IS ADMIN?" + expect(page).not_to have_text "CUSTOM CSS" + end + + it "displays the email as a gravatar field with a link to the record" do + within(".resource-sidebar-component") do + expect(page).to have_css("img") # Check for avatar + end + end + + it "displays discovered associations correctly" do + wait_for_loaded + + # Verify `posts` association + expect(page).to have_text "Posts" + expect(page).to have_text "Sample Post" + expect(page).to have_link "Sample Post", href: "/admin/resources/posts/#{post.slug}?via_record_id=#{user.slug}&via_resource_class=Avo%3A%3AResources%3A%3AFieldDiscoveryUser" + + # Verify `cv_attachment` association is present + expect(page).to have_text "CV ATTACHMENT" + end + end + + describe "Index Page" do + let(:url) { "/admin/resources/field_discovery_users" } + + before { visit url } + + it "lists discovered fields in the index view" do + wait_for_loaded + + within("table") do + expect(page).to have_text "John" + expect(page).to have_text "Doe" + expect(page).to have_text user.slug + end + end + end + + describe "Form Page" do + let(:url) { "/admin/resources/field_discovery_users/#{user.id}/edit" } + + before { visit url } + + it "displays form-specific fields" do + wait_for_loaded + + # Verify form-only fields + expect(page).to have_field "User Password" + expect(page).to have_field "Password confirmation" + + # Verify custom CSS field is displayed + expect(page).to have_text "CUSTOM CSS" + + # Verify password fields allow input + fill_in "User Password", with: "new_password" + fill_in "Password confirmation", with: "new_password" + end + end +end From 3e55c88b2815928942235187044314192a190a6a Mon Sep 17 00:00:00 2001 From: Keoni Garner Date: Thu, 9 Jan 2025 15:35:06 -0800 Subject: [PATCH 02/17] Apply suggestions from code review Co-authored-by: Paul Bob <69730720+Paul-Bob@users.noreply.github.com> --- lib/avo/concerns/has_field_discovery.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 2890ac4bd4..593c858b0b 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -47,7 +47,6 @@ module HasFieldDiscovery class_methods do def column_names_mapping @column_names_mapping ||= DEFAULT_COLUMN_NAMES_MAPPING.dup - .except(*COLUMN_NAMES_TO_IGNORE) .merge(Avo.configuration.column_names_mapping || {}) end @@ -94,7 +93,7 @@ def discover_associations(only: nil, except: nil, **field_options) # Fetches the model class, falling back to the items_holder parent record in certain instances (e.g. in the context of the sidebar) def safe_model_class - respond_to?(:model_class) ? model_class : @items_holder.parent.record.class + respond_to?(:model_class) ? model_class : @items_holder.parent.model_class rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished nil end @@ -114,7 +113,7 @@ def determine_field_config(attribute, column) end def build_field_options(field_config, column) - { as: field_config[:field].to_sym, required: !column.null }.merge(field_config.except(:field)) + { as: field_config.delete(:field).to_sym }.merge(field_config) end def discover_by_type(associations, as_type) From f6e8b238f11d8dc0e882c86bb37f870fb4a88c2e Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 15:37:28 -0800 Subject: [PATCH 03/17] Optimize model enum check --- lib/avo/concerns/has_field_discovery.rb | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 593c858b0b..30a452aa08 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -98,6 +98,16 @@ def safe_model_class nil end + def model_enums + @model_enums ||= if safe_model_class.respond_to?(:defined_enums) + safe_model_class.defined_enums.transform_values do |enum_values| + { field: "select", enum: "::#{safe_model_class.name}.#{enum_values.keys.first.pluralize}" } + end + else + {} + end.with_indifferent_access + end + # Determines if a column is included in the discovery scope. # A column is in scope if it's included in `only` and not in `except`. def column_in_scope?(column_name) @@ -105,11 +115,9 @@ def column_in_scope?(column_name) end def determine_field_config(attribute, column) - if safe_model_class.respond_to?(:defined_enums) && safe_model_class.defined_enums[attribute.to_s] - return { field: "select", enum: "::#{safe_model_class.name}.#{attribute.to_s.pluralize}" } - end - - self.class.column_names_mapping[attribute] || self.class.column_types_mapping[column.type] + model_enums[attribute.to_s] || + self.class.column_names_mapping[attribute] || + self.class.column_types_mapping[column.type] end def build_field_options(field_config, column) From ac6625833ef084dbbe5ac7ccdffdedd6c197e2e0 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 17:16:05 -0800 Subject: [PATCH 04/17] Rubocop / Refactor for readability + solving problems with select inputs --- lib/avo/concerns/has_field_discovery.rb | 184 +++++++++++++++--------- 1 file changed, 120 insertions(+), 64 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 30a452aa08..6abb80f82c 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -1,6 +1,11 @@ +# frozen_string_literal: true + +# TODO: Refactor this concern to be more readable and maintainable +# rubocop:disable Metrics/ModuleLength module Avo module Concerns - # This concern facilitates field discovery for models in Avo, mapping database columns and associations to Avo fields. + # This concern facilitates field discovery for models in Avo, + # mapping database columns and associations to Avo fields. # It supports: # - Automatic detection of fields based on column names, types, and associations. # - Customization via `only`, `except`, and global configuration overrides. @@ -9,35 +14,35 @@ module HasFieldDiscovery extend ActiveSupport::Concern DEFAULT_COLUMN_NAMES_MAPPING = { - id: { field: "id" }, - description: { field: "textarea" }, - gravatar: { field: "gravatar" }, - email: { field: "text" }, - password: { field: "password" }, - password_confirmation: { field: "password" }, - created_at: { field: "date_time" }, - updated_at: { field: "date_time" }, - stage: { field: "select" }, - budget: { field: "currency" }, - money: { field: "currency" }, - country: { field: "country" }, + id: { field: 'id' }, + description: { field: 'textarea' }, + gravatar: { field: 'gravatar' }, + email: { field: 'text' }, + password: { field: 'password' }, + password_confirmation: { field: 'password' }, + created_at: { field: 'date_time' }, + updated_at: { field: 'date_time' }, + stage: { field: 'select' }, + budget: { field: 'currency' }, + money: { field: 'currency' }, + country: { field: 'country' } }.freeze DEFAULT_COLUMN_TYPES_MAPPING = { - primary_key: { field: "id" }, - string: { field: "text" }, - text: { field: "textarea" }, - integer: { field: "number" }, - float: { field: "number" }, - decimal: { field: "number" }, - datetime: { field: "date_time" }, - timestamp: { field: "date_time" }, - time: { field: "date_time" }, - date: { field: "date" }, - binary: { field: "number" }, - boolean: { field: "boolean" }, - references: { field: "belongs_to" }, - json: { field: "code" }, + primary_key: { field: 'id' }, + string: { field: 'text' }, + text: { field: 'textarea' }, + integer: { field: 'number' }, + float: { field: 'number' }, + decimal: { field: 'number' }, + datetime: { field: 'date_time' }, + timestamp: { field: 'date_time' }, + time: { field: 'date_time' }, + date: { field: 'date' }, + binary: { field: 'number' }, + boolean: { field: 'boolean' }, + references: { field: 'belongs_to' }, + json: { field: 'code' } }.freeze COLUMN_NAMES_TO_IGNORE = %i[ @@ -47,12 +52,12 @@ module HasFieldDiscovery class_methods do def column_names_mapping @column_names_mapping ||= DEFAULT_COLUMN_NAMES_MAPPING.dup - .merge(Avo.configuration.column_names_mapping || {}) + .merge(Avo.configuration.column_names_mapping || {}) end def column_types_mapping @column_types_mapping ||= DEFAULT_COLUMN_TYPES_MAPPING.dup - .merge(Avo.configuration.column_types_mapping || {}) + .merge(Avo.configuration.column_types_mapping || {}) end end @@ -63,35 +68,88 @@ def model_db_columns # Discovers and configures database columns as fields def discover_columns(only: nil, except: nil, **field_options) - @only, @except, @field_options = only, except, field_options + setup_discovery_options(only, except, field_options) return unless safe_model_class.respond_to?(:columns_hash) - model_db_columns.each do |column_name, column| - next unless column_in_scope?(column_name) - next if reflections.key?(column_name) || rich_texts.key?("rich_text_#{column_name}") - - field_config = determine_field_config(column_name, column) - next unless field_config - - field_options = build_field_options(field_config, column) - field column_name, **field_options, **@field_options + discoverable_columns.each do |column_name, column| + process_column(column_name, column) end + + discover_by_type(tags, :tags) { |name| name.split('_').pop.join('_').pluralize } + discover_by_type(rich_texts, :trix) { |name| name.to_s.delete_prefix('rich_text_') } end # Discovers and configures associations as fields def discover_associations(only: nil, except: nil, **field_options) - @only, @except, @field_options = only, except, field_options + setup_discovery_options(only, except, field_options) return unless safe_model_class.respond_to?(:reflections) - discover_by_type(tags, :tags) { |name| name.split("_").pop.join("_").pluralize } - discover_by_type(rich_texts, :trix) { |name| name.delete_prefix("rich_text_") } discover_attachments discover_basic_associations end private - # Fetches the model class, falling back to the items_holder parent record in certain instances (e.g. in the context of the sidebar) + def setup_discovery_options(only, except, field_options) + @only = only + @except = except + @field_options = field_options + end + + def discoverable_columns + model_db_columns.reject do |column_name, _| + skip_column?(column_name) + end + end + + def skip_column?(column_name) + !column_in_scope?(column_name) || + reflections.key?(column_name) || + rich_text_column?(column_name) + end + + def rich_text_column?(column_name) + rich_texts.key?("rich_text_#{column_name}") + end + + def process_column(column_name, column) + field_config = determine_field_config(column_name, column) + return unless field_config + + create_field(column_name, field_config) + end + + def create_field(column_name, field_config) + field_options = { as: field_config.dup.delete(:field).to_sym }.merge(field_config) + field(column_name, **field_options.symbolize_keys, **@field_options.symbolize_keys) + end + + def create_attachment_field(association_name, reflection) + field_type = determine_attachment_field_type(reflection) + field(association_name, as: field_type, **@field_options) + end + + def determine_attachment_field_type(reflection) + reflection.options[:as] == :has_one_attached ? :file : :files + end + + def create_association_field(association_name, reflection) + options = base_association_options(reflection) + options.merge!(polymorphic_options(reflection)) if reflection.options[:polymorphic] + + field(association_name, **options, **@field_options) + end + + def base_association_options(reflection) + { + as: reflection.macro, + searchable: true, + sortable: true + } + end + + # Fetches the model class, falling back to the items_holder parent record in certain instances + # (e.g. in the context of the sidebar) def safe_model_class respond_to?(:model_class) ? model_class : @items_holder.parent.model_class rescue ActiveRecord::NoDatabaseError, ActiveRecord::ConnectionNotEstablished @@ -100,12 +158,15 @@ def safe_model_class def model_enums @model_enums ||= if safe_model_class.respond_to?(:defined_enums) - safe_model_class.defined_enums.transform_values do |enum_values| - { field: "select", enum: "::#{safe_model_class.name}.#{enum_values.keys.first.pluralize}" } - end - else - {} - end.with_indifferent_access + safe_model_class.defined_enums.transform_values do |options| + { + field: :select, + options: + } + end + else + {} + end.with_indifferent_access end # Determines if a column is included in the discovery scope. @@ -120,15 +181,11 @@ def determine_field_config(attribute, column) self.class.column_types_mapping[column.type] end - def build_field_options(field_config, column) - { as: field_config.delete(:field).to_sym }.merge(field_config) - end - def discover_by_type(associations, as_type) associations.each_key do |association_name| next unless column_in_scope?(association_name) - field association_name, as: as_type, **@field_options.merge(yield(association_name)) + field association_name, as: as_type, **@field_options.merge(name: yield(association_name)) end end @@ -136,8 +193,7 @@ def discover_attachments attachment_associations.each do |association_name, reflection| next unless column_in_scope?(association_name) - field_type = reflection.options[:as] == :has_one_attached ? :file : :files - field association_name, as: field_type, **@field_options + create_attachment_field(association_name, reflection) end end @@ -145,10 +201,7 @@ def discover_basic_associations associations.each do |association_name, reflection| next unless column_in_scope?(association_name) - options = { as: reflection.macro, searchable: true, sortable: true } - options.merge!(polymorphic_options(reflection)) if reflection.options[:polymorphic] - - field association_name, **options, **@field_options + create_association_field(association_name, reflection) end end @@ -167,11 +220,11 @@ def reflections end def attachment_associations - @attachment_associations ||= reflections.select { |_, r| r.options[:class_name] == "ActiveStorage::Attachment" } + @attachment_associations ||= reflections.select { |_, r| r.options[:class_name] == 'ActiveStorage::Attachment' } end def rich_texts - @rich_texts ||= reflections.select { |_, r| r.options[:class_name] == "ActionText::RichText" } + @rich_texts ||= reflections.select { |_, r| r.options[:class_name] == 'ActionText::RichText' } end def tags @@ -179,12 +232,15 @@ def tags end def associations - @associations ||= reflections.reject { |key| attachment_associations.key?(key) || tags.key?(key) || rich_texts.key?(key) } + @associations ||= reflections.reject do |key| + attachment_associations.key?(key) || tags.key?(key) || rich_texts.key?(key) + end end def ignore_reflection?(name) - %w[blob blobs tags].include?(name.split("_").pop) || name.to_sym == :taggings + %w[blob blobs tags].include?(name.split('_').pop) || name.to_sym == :taggings end end end end +# rubocop:enable Metrics/ModuleLength From b5f0c6fd82e6443f3d26a3ac77c58835426201a1 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 17:34:24 -0800 Subject: [PATCH 05/17] Fix up tags and rich texts a bit --- lib/avo/concerns/has_field_discovery.rb | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 6abb80f82c..1dc7823465 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -75,8 +75,8 @@ def discover_columns(only: nil, except: nil, **field_options) process_column(column_name, column) end - discover_by_type(tags, :tags) { |name| name.split('_').pop.join('_').pluralize } - discover_by_type(rich_texts, :trix) { |name| name.to_s.delete_prefix('rich_text_') } + discover_tags + discover_rich_texts end # Discovers and configures associations as fields @@ -189,6 +189,24 @@ def discover_by_type(associations, as_type) end end + def discover_rich_texts + rich_texts.each do |association_name, reflection| + next unless column_in_scope?(association_name) + + field_name = association_name&.to_s&.delete_prefix('rich_text_').to_sym || association_name + field field_name, as: :trix, **@field_options + end + end + + def discover_tags + tags.each do |association_name, reflection| + next unless column_in_scope?(association_name) + + field_name = association_name&.to_s&.delete_suffix('_taggings').pluralize.to_sym || association_name + field field_name, as: :tags, **@field_options.merge(acts_as_taggable_on: field_name) + end + end + def discover_attachments attachment_associations.each do |association_name, reflection| next unless column_in_scope?(association_name) From 657c2da528e2947e2d2a551c4c48510cc66715b8 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 17:36:18 -0800 Subject: [PATCH 06/17] Rubocop --- lib/avo/concerns/has_field_discovery.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 1dc7823465..780a0250f3 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -190,23 +190,26 @@ def discover_by_type(associations, as_type) end def discover_rich_texts - rich_texts.each do |association_name, reflection| + rich_texts.each_key do |association_name| next unless column_in_scope?(association_name) - field_name = association_name&.to_s&.delete_prefix('rich_text_').to_sym || association_name + field_name = association_name&.to_s&.delete_prefix('rich_text_')&.to_sym || association_name field field_name, as: :trix, **@field_options end end def discover_tags - tags.each do |association_name, reflection| + tags.each_key do |association_name| next unless column_in_scope?(association_name) - field_name = association_name&.to_s&.delete_suffix('_taggings').pluralize.to_sym || association_name - field field_name, as: :tags, **@field_options.merge(acts_as_taggable_on: field_name) + field tag_field_name(association_name), as: :tags, **@field_options.merge(acts_as_taggable_on: field_name) end end + def tag_field_name(association_name) + association_name&.to_s&.delete_suffix('_taggings')&.pluralize&.to_sym || association_name + end + def discover_attachments attachment_associations.each do |association_name, reflection| next unless column_in_scope?(association_name) From c76f03496103422598e60e2cf277e2ca1717b901 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 17:51:58 -0800 Subject: [PATCH 07/17] Oops - use `standardrb` instead of `rubocop` --- lib/avo/concerns/has_field_discovery.rb | 70 ++++++++++++------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 780a0250f3..478d53e513 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -14,35 +14,35 @@ module HasFieldDiscovery extend ActiveSupport::Concern DEFAULT_COLUMN_NAMES_MAPPING = { - id: { field: 'id' }, - description: { field: 'textarea' }, - gravatar: { field: 'gravatar' }, - email: { field: 'text' }, - password: { field: 'password' }, - password_confirmation: { field: 'password' }, - created_at: { field: 'date_time' }, - updated_at: { field: 'date_time' }, - stage: { field: 'select' }, - budget: { field: 'currency' }, - money: { field: 'currency' }, - country: { field: 'country' } + id: {field: "id"}, + description: {field: "textarea"}, + gravatar: {field: "gravatar"}, + email: {field: "text"}, + password: {field: "password"}, + password_confirmation: {field: "password"}, + created_at: {field: "date_time"}, + updated_at: {field: "date_time"}, + stage: {field: "select"}, + budget: {field: "currency"}, + money: {field: "currency"}, + country: {field: "country"} }.freeze DEFAULT_COLUMN_TYPES_MAPPING = { - primary_key: { field: 'id' }, - string: { field: 'text' }, - text: { field: 'textarea' }, - integer: { field: 'number' }, - float: { field: 'number' }, - decimal: { field: 'number' }, - datetime: { field: 'date_time' }, - timestamp: { field: 'date_time' }, - time: { field: 'date_time' }, - date: { field: 'date' }, - binary: { field: 'number' }, - boolean: { field: 'boolean' }, - references: { field: 'belongs_to' }, - json: { field: 'code' } + primary_key: {field: "id"}, + string: {field: "text"}, + text: {field: "textarea"}, + integer: {field: "number"}, + float: {field: "number"}, + decimal: {field: "number"}, + datetime: {field: "date_time"}, + timestamp: {field: "date_time"}, + time: {field: "date_time"}, + date: {field: "date"}, + binary: {field: "number"}, + boolean: {field: "boolean"}, + references: {field: "belongs_to"}, + json: {field: "code"} }.freeze COLUMN_NAMES_TO_IGNORE = %i[ @@ -120,7 +120,7 @@ def process_column(column_name, column) end def create_field(column_name, field_config) - field_options = { as: field_config.dup.delete(:field).to_sym }.merge(field_config) + field_options = {as: field_config.dup.delete(:field).to_sym}.merge(field_config) field(column_name, **field_options.symbolize_keys, **@field_options.symbolize_keys) end @@ -130,7 +130,7 @@ def create_attachment_field(association_name, reflection) end def determine_attachment_field_type(reflection) - reflection.options[:as] == :has_one_attached ? :file : :files + (reflection.options[:as] == :has_one_attached) ? :file : :files end def create_association_field(association_name, reflection) @@ -166,7 +166,7 @@ def model_enums end else {} - end.with_indifferent_access + end.with_indifferent_access end # Determines if a column is included in the discovery scope. @@ -193,7 +193,7 @@ def discover_rich_texts rich_texts.each_key do |association_name| next unless column_in_scope?(association_name) - field_name = association_name&.to_s&.delete_prefix('rich_text_')&.to_sym || association_name + field_name = association_name&.to_s&.delete_prefix("rich_text_")&.to_sym || association_name field field_name, as: :trix, **@field_options end end @@ -207,7 +207,7 @@ def discover_tags end def tag_field_name(association_name) - association_name&.to_s&.delete_suffix('_taggings')&.pluralize&.to_sym || association_name + association_name&.to_s&.delete_suffix("_taggings")&.pluralize&.to_sym || association_name end def discover_attachments @@ -227,7 +227,7 @@ def discover_basic_associations end def polymorphic_options(reflection) - { polymorphic_as: reflection.name, types: detect_polymorphic_types(reflection) } + {polymorphic_as: reflection.name, types: detect_polymorphic_types(reflection)} end def detect_polymorphic_types(reflection) @@ -241,11 +241,11 @@ def reflections end def attachment_associations - @attachment_associations ||= reflections.select { |_, r| r.options[:class_name] == 'ActiveStorage::Attachment' } + @attachment_associations ||= reflections.select { |_, r| r.options[:class_name] == "ActiveStorage::Attachment" } end def rich_texts - @rich_texts ||= reflections.select { |_, r| r.options[:class_name] == 'ActionText::RichText' } + @rich_texts ||= reflections.select { |_, r| r.options[:class_name] == "ActionText::RichText" } end def tags @@ -259,7 +259,7 @@ def associations end def ignore_reflection?(name) - %w[blob blobs tags].include?(name.split('_').pop) || name.to_sym == :taggings + %w[blob blobs tags].include?(name.split("_").pop) || name.to_sym == :taggings end end end From 3faf2dacc6ed7491e9153fc78804aad6910c0973 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 17:57:06 -0800 Subject: [PATCH 08/17] Few more lint fixes --- spec/dummy/app/avo/resources/field_discovery_user.rb | 2 +- spec/dummy/config/initializers/avo.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/dummy/app/avo/resources/field_discovery_user.rb b/spec/dummy/app/avo/resources/field_discovery_user.rb index a6bf9b16b9..ec05c17daf 100644 --- a/spec/dummy/app/avo/resources/field_discovery_user.rb +++ b/spec/dummy/app/avo/resources/field_discovery_user.rb @@ -1,6 +1,6 @@ class Avo::Resources::FieldDiscoveryUser < Avo::BaseResource self.model_class = ::User - self.description = 'This is a resource with discovered fields. It will show fields and associations as defined in the model.' + self.description = "This is a resource with discovered fields. It will show fields and associations as defined in the model." self.find_record_method = -> { query.friendly.find id } diff --git a/spec/dummy/config/initializers/avo.rb b/spec/dummy/config/initializers/avo.rb index 98180de559..0b05121fb4 100644 --- a/spec/dummy/config/initializers/avo.rb +++ b/spec/dummy/config/initializers/avo.rb @@ -104,7 +104,7 @@ # end config.column_names_mapping = { - custom_css: { field: "code" }, + custom_css: {field: "code"} } end From 7bc8c901ab65966f0c43dd625b77df172403be00 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 17:57:22 -0800 Subject: [PATCH 09/17] Couple more --- lib/avo/concerns/has_field_discovery.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 478d53e513..38d9355c34 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -52,12 +52,12 @@ module HasFieldDiscovery class_methods do def column_names_mapping @column_names_mapping ||= DEFAULT_COLUMN_NAMES_MAPPING.dup - .merge(Avo.configuration.column_names_mapping || {}) + .merge(Avo.configuration.column_names_mapping || {}) end def column_types_mapping @column_types_mapping ||= DEFAULT_COLUMN_TYPES_MAPPING.dup - .merge(Avo.configuration.column_types_mapping || {}) + .merge(Avo.configuration.column_types_mapping || {}) end end @@ -158,14 +158,14 @@ def safe_model_class def model_enums @model_enums ||= if safe_model_class.respond_to?(:defined_enums) - safe_model_class.defined_enums.transform_values do |options| - { - field: :select, - options: - } - end - else - {} + safe_model_class.defined_enums.transform_values do |options| + { + field: :select, + options: + } + end + else + {} end.with_indifferent_access end From 9f1bc4cf4c1da26350ea12e42b3b0cb201407464 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Thu, 9 Jan 2025 18:00:31 -0800 Subject: [PATCH 10/17] Indentation --- lib/avo/concerns/has_field_discovery.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 38d9355c34..0e09649aa6 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -52,12 +52,12 @@ module HasFieldDiscovery class_methods do def column_names_mapping @column_names_mapping ||= DEFAULT_COLUMN_NAMES_MAPPING.dup - .merge(Avo.configuration.column_names_mapping || {}) + .merge(Avo.configuration.column_names_mapping || {}) end def column_types_mapping @column_types_mapping ||= DEFAULT_COLUMN_TYPES_MAPPING.dup - .merge(Avo.configuration.column_types_mapping || {}) + .merge(Avo.configuration.column_types_mapping || {}) end end From 4b36f1fc20a13f8aaff9ef833abdd10c37eb86fa Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Fri, 10 Jan 2025 10:51:18 -0800 Subject: [PATCH 11/17] PR suggestions --- lib/avo/concerns/has_field_discovery.rb | 16 +- spec/system/avo/has_field_discovery_spec.rb | 153 +++++++++++++++++++- 2 files changed, 164 insertions(+), 5 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 0e09649aa6..d071e4610f 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -109,7 +109,7 @@ def skip_column?(column_name) end def rich_text_column?(column_name) - rich_texts.key?("rich_text_#{column_name}") + rich_texts.key?(:"rich_text_#{column_name}") end def process_column(column_name, column) @@ -125,12 +125,16 @@ def create_field(column_name, field_config) end def create_attachment_field(association_name, reflection) + field_name = association_name&.to_s&.delete_suffix("_attachment")&.to_sym || association_name field_type = determine_attachment_field_type(reflection) - field(association_name, as: field_type, **@field_options) + field(field_name, as: field_type, **@field_options) end def determine_attachment_field_type(reflection) - (reflection.options[:as] == :has_one_attached) ? :file : :files + ( + reflection.is_a?(ActiveRecord::Reflection::HasOneReflection) || + reflection.is_a?(ActiveStorage::Reflection::HasOneAttachedReflection) + ) ? :file : :files end def create_association_field(association_name, reflection) @@ -202,7 +206,11 @@ def discover_tags tags.each_key do |association_name| next unless column_in_scope?(association_name) - field tag_field_name(association_name), as: :tags, **@field_options.merge(acts_as_taggable_on: field_name) + field( + tag_field_name(association_name), as: :tags, + acts_as_taggable_on: tag_field_name(association_name), + **@field_options + ) end end diff --git a/spec/system/avo/has_field_discovery_spec.rb b/spec/system/avo/has_field_discovery_spec.rb index ea9968d3da..63ca893b09 100644 --- a/spec/system/avo/has_field_discovery_spec.rb +++ b/spec/system/avo/has_field_discovery_spec.rb @@ -40,7 +40,36 @@ expect(page).to have_link "Sample Post", href: "/admin/resources/posts/#{post.slug}?via_record_id=#{user.slug}&via_resource_class=Avo%3A%3AResources%3A%3AFieldDiscoveryUser" # Verify `cv_attachment` association is present - expect(page).to have_text "CV ATTACHMENT" + expect(page).to have_text "CV" + end + + it "renders each field exactly once" do + wait_for_loaded + + within("[data-panel-id='main']") do + # Basic fields + ## Main Panel + expect(page).to have_text("FIRST NAME", count: 1) + expect(page).to have_text("LAST NAME", count: 1) + expect(page).to have_text("ROLES", count: 1) + expect(page).to have_text("TEAM ID", count: 1) + expect(page).to have_text("CREATED AT", count: 1) + expect(page).to have_text("UPDATED AT", count: 1) + expect(page).to have_text("SLUG", count: 1) + + # Sidebar + expect(page).to have_text("AVATAR", count: 1) + expect(page).to have_text("IS ACTIVE", count: 1) + expect(page).to have_text("BIRTHDAY", count: 1) + + + # Single file uploads + expect(page).to have_text("CV", count: 1) + expect(page).not_to have_text("CV ATTACHMENT") + end + + # Associations + expect(page).to have_text("Posts", count: 1) end end @@ -79,5 +108,127 @@ fill_in "User Password", with: "new_password" fill_in "Password confirmation", with: "new_password" end + + it "renders each input field exactly once" do + wait_for_loaded + + # Form fields + expect(page).to have_text("FIRST NAME", count: 1) + expect(page).to have_text("LAST NAME", count: 1) + expect(page).to have_text("ROLES", count: 1) + expect(page).to have_text("TEAM ID", count: 1) + expect(page).to have_text("CREATED AT", count: 1) + expect(page).to have_text("UPDATED AT", count: 1) + expect(page).to have_text("SLUG", count: 1) + + # File upload fields + expect(page).to have_text("CV", count: 1) + + # Password fields + expect(page).to have_text("USER PASSWORD", count: 1) + expect(page).to have_text("PASSWORD CONFIRMATION", count: 1) + end + end + + describe "Has One Attachment" do + let(:url) { "/admin/resources/field_discovery_users/#{user.id}/edit" } + + before { visit url } + + it "displays single file upload correctly for has_one_attached" do + wait_for_loaded + + within('[data-field-id="cv"]') do + # Verify it shows "Choose File" instead of "Choose Files" + expect(page).to have_css('input[type="file"]:not([multiple])') + end + end + end + + describe "Trix Editor" do + let(:event) { create :event } + let(:url) { "/admin/resources/events/#{event.id}/edit" } + + before do + Avo::Resources::Event.with_temporary_items do + discover_columns + end + visit url + end + + it "renders Trix editor only once" do + wait_for_loaded + + # Verify only one Trix editor instance is present + expect(page).to have_css('trix-editor', count: 1) + end + end + + describe "Tags" do + let(:post) { create :post } + let(:url) { "/admin/resources/posts/#{post.id}" } + + before do + Avo::Resources::Post.with_temporary_items do + discover_columns + end + visit url + end + + it "renders tags correctly" do + wait_for_loaded + + # Verify only one Trix editor instance is present + expect(page).to have_text('TAGS', count: 1) + expect(page).to have_css('[data-target="tag-component"]') + end + end + + describe "Enum Fields" do + before { visit "/admin/resources/posts/#{post.id}/edit" } + + it "displays enum fields as select boxes" do + wait_for_loaded + + within('[data-field-id="status"]') do + expect(page).to have_css('select') + expect(page).to have_select(options: ["draft", "published", "archived"]) + expect(page).to have_select(selected: "draft") + end + end + end + + describe "Polymorphic Associations" do + let(:post) { create :post } + let(:comment) { create :comment, commentable: post } + before do + Avo::Resources::Comment.with_temporary_items do + discover_associations + end + visit "/admin/resources/comments/#{comment.id}" + end + + it "displays polymorphic association correctly" do + wait_for_loaded + + within("[data-panel-id='main']") do + expect(page).to have_text("COMMENTABLE") + expect(page).to have_link(post.name, href: /\/admin\/resources\/posts\//) + end + end + end + + describe "Ignored Fields" do + before { visit "/admin/resources/field_discovery_users/#{user.slug}" } + + it "does not display sensitive fields" do + wait_for_loaded + + within("[data-panel-id='main']") do + expect(page).not_to have_text("ENCRYPTED_PASSWORD") + expect(page).not_to have_text("RESET_PASSWORD_TOKEN") + expect(page).not_to have_text("REMEMBER_CREATED_AT") + end + end end end From 329d0c8f2dc775f2f3252d882df20811f65e9993 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Fri, 10 Jan 2025 11:02:55 -0800 Subject: [PATCH 12/17] Lint spec file --- spec/system/avo/has_field_discovery_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/system/avo/has_field_discovery_spec.rb b/spec/system/avo/has_field_discovery_spec.rb index 63ca893b09..a5271bc5bf 100644 --- a/spec/system/avo/has_field_discovery_spec.rb +++ b/spec/system/avo/has_field_discovery_spec.rb @@ -62,7 +62,6 @@ expect(page).to have_text("IS ACTIVE", count: 1) expect(page).to have_text("BIRTHDAY", count: 1) - # Single file uploads expect(page).to have_text("CV", count: 1) expect(page).not_to have_text("CV ATTACHMENT") @@ -160,7 +159,7 @@ wait_for_loaded # Verify only one Trix editor instance is present - expect(page).to have_css('trix-editor', count: 1) + expect(page).to have_css("trix-editor", count: 1) end end @@ -179,7 +178,7 @@ wait_for_loaded # Verify only one Trix editor instance is present - expect(page).to have_text('TAGS', count: 1) + expect(page).to have_text("TAGS", count: 1) expect(page).to have_css('[data-target="tag-component"]') end end @@ -191,7 +190,7 @@ wait_for_loaded within('[data-field-id="status"]') do - expect(page).to have_css('select') + expect(page).to have_css("select") expect(page).to have_select(options: ["draft", "published", "archived"]) expect(page).to have_select(selected: "draft") end From d7dd291ad1f47c08ce68ddb5cc9c834a857a8ec4 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Fri, 10 Jan 2025 11:26:07 -0800 Subject: [PATCH 13/17] Remove custom resource in favor of using temporary items --- .../app/avo/resources/field_discovery_user.rb | 35 ---------------- .../avo/field_discovery_users_controller.rb | 4 -- spec/system/avo/has_field_discovery_spec.rb | 42 ++++++++++++++++--- 3 files changed, 36 insertions(+), 45 deletions(-) delete mode 100644 spec/dummy/app/avo/resources/field_discovery_user.rb delete mode 100644 spec/dummy/app/controllers/avo/field_discovery_users_controller.rb diff --git a/spec/dummy/app/avo/resources/field_discovery_user.rb b/spec/dummy/app/avo/resources/field_discovery_user.rb deleted file mode 100644 index ec05c17daf..0000000000 --- a/spec/dummy/app/avo/resources/field_discovery_user.rb +++ /dev/null @@ -1,35 +0,0 @@ -class Avo::Resources::FieldDiscoveryUser < Avo::BaseResource - self.model_class = ::User - self.description = "This is a resource with discovered fields. It will show fields and associations as defined in the model." - self.find_record_method = -> { - query.friendly.find id - } - - def fields - main_panel do - discover_columns except: %i[email active is_admin? birthday is_writer outside_link custom_css] - discover_associations only: %i[cv_attachment] - - sidebar do - with_options only_on: :show do - discover_columns only: %i[email], as: :gravatar, link_to_record: true, as_avatar: :circle - field :heading, as: :heading, label: "" - discover_columns only: %i[active], name: "Is active" - end - - discover_columns only: %i[birthday] - - field :password, as: :password, name: "User Password", required: false, only_on: :forms, help: 'You may verify the password strength here.' - field :password_confirmation, as: :password, name: "Password confirmation", required: false, revealable: true - - with_options only_on: :forms do - field :dev, as: :heading, label: '
DEV
', as_html: true - discover_columns only: %i[custom_css] - end - end - end - - discover_associations only: %i[posts] - discover_associations except: %i[posts post cv_attachment] - end -end diff --git a/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb b/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb deleted file mode 100644 index 4ef6e31b11..0000000000 --- a/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb +++ /dev/null @@ -1,4 +0,0 @@ -# This controller has been generated to enable Rails' resource routes. -# More information on https://docs.avohq.io/3.0/controllers.html -class Avo::FieldDiscoveryUsersController < Avo::ResourcesController -end diff --git a/spec/system/avo/has_field_discovery_spec.rb b/spec/system/avo/has_field_discovery_spec.rb index a5271bc5bf..cc99d8fac2 100644 --- a/spec/system/avo/has_field_discovery_spec.rb +++ b/spec/system/avo/has_field_discovery_spec.rb @@ -4,8 +4,38 @@ let!(:user) { create :user, first_name: "John", last_name: "Doe", birthday: "1990-01-01", email: "john.doe@example.com" } let!(:post) { create :post, user: user, name: "Sample Post" } + before do + Avo::Resources::User.with_temporary_items do + main_panel do + discover_columns except: %i[email active is_admin? birthday is_writer outside_link custom_css] + discover_associations only: %i[cv_attachment] + + sidebar do + with_options only_on: :show do + discover_columns only: %i[email], as: :gravatar, link_to_record: true, as_avatar: :circle + field :heading, as: :heading, label: "" + discover_columns only: %i[active], name: "Is active" + end + + discover_columns only: %i[birthday] + + field :password, as: :password, name: "User Password", required: false, only_on: :forms, help: 'You may verify the password strength here.' + field :password_confirmation, as: :password, name: "Password confirmation", required: false, revealable: true + + with_options only_on: :forms do + field :dev, as: :heading, label: '
DEV
', as_html: true + discover_columns only: %i[custom_css] + end + end + end + + discover_associations only: %i[posts] + discover_associations except: %i[posts post cv_attachment] + end + end + describe "Show Page" do - let(:url) { "/admin/resources/field_discovery_users/#{user.slug}" } + let(:url) { "/admin/resources/users/#{user.slug}" } before { visit url } @@ -37,7 +67,7 @@ # Verify `posts` association expect(page).to have_text "Posts" expect(page).to have_text "Sample Post" - expect(page).to have_link "Sample Post", href: "/admin/resources/posts/#{post.slug}?via_record_id=#{user.slug}&via_resource_class=Avo%3A%3AResources%3A%3AFieldDiscoveryUser" + expect(page).to have_link "Sample Post", href: "/admin/resources/posts/#{post.slug}?via_record_id=#{user.slug}&via_resource_class=Avo%3A%3AResources%3A%3AUser" # Verify `cv_attachment` association is present expect(page).to have_text "CV" @@ -73,7 +103,7 @@ end describe "Index Page" do - let(:url) { "/admin/resources/field_discovery_users" } + let(:url) { "/admin/resources/users" } before { visit url } @@ -89,7 +119,7 @@ end describe "Form Page" do - let(:url) { "/admin/resources/field_discovery_users/#{user.id}/edit" } + let(:url) { "/admin/resources/users/#{user.id}/edit" } before { visit url } @@ -130,7 +160,7 @@ end describe "Has One Attachment" do - let(:url) { "/admin/resources/field_discovery_users/#{user.id}/edit" } + let(:url) { "/admin/resources/users/#{user.id}/edit" } before { visit url } @@ -218,7 +248,7 @@ end describe "Ignored Fields" do - before { visit "/admin/resources/field_discovery_users/#{user.slug}" } + before { visit "/admin/resources/users/#{user.slug}" } it "does not display sensitive fields" do wait_for_loaded From 2ffac80328dbed0433358c546826e384894029ec Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Mon, 13 Jan 2025 10:09:16 -0800 Subject: [PATCH 14/17] Add after blocks for cleanup --- lib/avo/concerns/has_field_discovery.rb | 6 +++--- spec/system/avo/has_field_discovery_spec.rb | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index d071e4610f..818fa9dec1 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -13,7 +13,7 @@ module Concerns module HasFieldDiscovery extend ActiveSupport::Concern - DEFAULT_COLUMN_NAMES_MAPPING = { + DEFAULT_COLUMN_NAMES_MAPPING ||= { id: {field: "id"}, description: {field: "textarea"}, gravatar: {field: "gravatar"}, @@ -28,7 +28,7 @@ module HasFieldDiscovery country: {field: "country"} }.freeze - DEFAULT_COLUMN_TYPES_MAPPING = { + DEFAULT_COLUMN_TYPES_MAPPING ||= { primary_key: {field: "id"}, string: {field: "text"}, text: {field: "textarea"}, @@ -45,7 +45,7 @@ module HasFieldDiscovery json: {field: "code"} }.freeze - COLUMN_NAMES_TO_IGNORE = %i[ + COLUMN_NAMES_TO_IGNORE ||= %i[ encrypted_password reset_password_token reset_password_sent_at remember_created_at password_digest ].freeze diff --git a/spec/system/avo/has_field_discovery_spec.rb b/spec/system/avo/has_field_discovery_spec.rb index cc99d8fac2..4f144e2b0a 100644 --- a/spec/system/avo/has_field_discovery_spec.rb +++ b/spec/system/avo/has_field_discovery_spec.rb @@ -34,6 +34,10 @@ end end + after do + Avo::Resources::User.restore_items_from_backup + end + describe "Show Page" do let(:url) { "/admin/resources/users/#{user.slug}" } @@ -178,6 +182,10 @@ let(:event) { create :event } let(:url) { "/admin/resources/events/#{event.id}/edit" } + after do + Avo::Resources::Event.restore_items_from_backup + end + before do Avo::Resources::Event.with_temporary_items do discover_columns @@ -197,6 +205,10 @@ let(:post) { create :post } let(:url) { "/admin/resources/posts/#{post.id}" } + after do + Avo::Resources::Post.restore_items_from_backup + end + before do Avo::Resources::Post.with_temporary_items do discover_columns @@ -222,7 +234,7 @@ within('[data-field-id="status"]') do expect(page).to have_css("select") expect(page).to have_select(options: ["draft", "published", "archived"]) - expect(page).to have_select(selected: "draft") + expect(page).to have_select(selected: post.status) end end end @@ -230,6 +242,11 @@ describe "Polymorphic Associations" do let(:post) { create :post } let(:comment) { create :comment, commentable: post } + + after do + Avo::Resources::Comment.restore_items_from_backup + end + before do Avo::Resources::Comment.with_temporary_items do discover_associations From a6db2374e14512eccf1575b6e2fa87afb95ea6d8 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Mon, 13 Jan 2025 10:10:46 -0800 Subject: [PATCH 15/17] Lint --- lib/avo/concerns/has_field_discovery.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index 818fa9dec1..d071e4610f 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -13,7 +13,7 @@ module Concerns module HasFieldDiscovery extend ActiveSupport::Concern - DEFAULT_COLUMN_NAMES_MAPPING ||= { + DEFAULT_COLUMN_NAMES_MAPPING = { id: {field: "id"}, description: {field: "textarea"}, gravatar: {field: "gravatar"}, @@ -28,7 +28,7 @@ module HasFieldDiscovery country: {field: "country"} }.freeze - DEFAULT_COLUMN_TYPES_MAPPING ||= { + DEFAULT_COLUMN_TYPES_MAPPING = { primary_key: {field: "id"}, string: {field: "text"}, text: {field: "textarea"}, @@ -45,7 +45,7 @@ module HasFieldDiscovery json: {field: "code"} }.freeze - COLUMN_NAMES_TO_IGNORE ||= %i[ + COLUMN_NAMES_TO_IGNORE = %i[ encrypted_password reset_password_token reset_password_sent_at remember_created_at password_digest ].freeze From 089290897fd7a60d62cc31ad25ac7c717001ea66 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Tue, 14 Jan 2025 10:51:35 -0800 Subject: [PATCH 16/17] Add back resource with discovered fields --- .../app/avo/resources/field_discovery_user.rb | 35 +++++++++++++++++++ .../avo/field_discovery_users_controller.rb | 4 +++ 2 files changed, 39 insertions(+) create mode 100644 spec/dummy/app/avo/resources/field_discovery_user.rb create mode 100644 spec/dummy/app/controllers/avo/field_discovery_users_controller.rb diff --git a/spec/dummy/app/avo/resources/field_discovery_user.rb b/spec/dummy/app/avo/resources/field_discovery_user.rb new file mode 100644 index 0000000000..ec05c17daf --- /dev/null +++ b/spec/dummy/app/avo/resources/field_discovery_user.rb @@ -0,0 +1,35 @@ +class Avo::Resources::FieldDiscoveryUser < Avo::BaseResource + self.model_class = ::User + self.description = "This is a resource with discovered fields. It will show fields and associations as defined in the model." + self.find_record_method = -> { + query.friendly.find id + } + + def fields + main_panel do + discover_columns except: %i[email active is_admin? birthday is_writer outside_link custom_css] + discover_associations only: %i[cv_attachment] + + sidebar do + with_options only_on: :show do + discover_columns only: %i[email], as: :gravatar, link_to_record: true, as_avatar: :circle + field :heading, as: :heading, label: "" + discover_columns only: %i[active], name: "Is active" + end + + discover_columns only: %i[birthday] + + field :password, as: :password, name: "User Password", required: false, only_on: :forms, help: 'You may verify the password strength here.' + field :password_confirmation, as: :password, name: "Password confirmation", required: false, revealable: true + + with_options only_on: :forms do + field :dev, as: :heading, label: '
DEV
', as_html: true + discover_columns only: %i[custom_css] + end + end + end + + discover_associations only: %i[posts] + discover_associations except: %i[posts post cv_attachment] + end +end diff --git a/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb b/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb new file mode 100644 index 0000000000..4ef6e31b11 --- /dev/null +++ b/spec/dummy/app/controllers/avo/field_discovery_users_controller.rb @@ -0,0 +1,4 @@ +# This controller has been generated to enable Rails' resource routes. +# More information on https://docs.avohq.io/3.0/controllers.html +class Avo::FieldDiscoveryUsersController < Avo::ResourcesController +end From 82fab1da8ad0253a37b70313f02247dbf65005d7 Mon Sep 17 00:00:00 2001 From: ObiWanKeoni Date: Wed, 15 Jan 2025 19:27:18 -0800 Subject: [PATCH 17/17] Fix status issue and remedy test setup --- lib/avo/concerns/has_field_discovery.rb | 4 +- spec/system/avo/has_field_discovery_spec.rb | 60 +++++++++++++-------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/lib/avo/concerns/has_field_discovery.rb b/lib/avo/concerns/has_field_discovery.rb index d071e4610f..c8e20cd286 100644 --- a/lib/avo/concerns/has_field_discovery.rb +++ b/lib/avo/concerns/has_field_discovery.rb @@ -162,10 +162,10 @@ def safe_model_class def model_enums @model_enums ||= if safe_model_class.respond_to?(:defined_enums) - safe_model_class.defined_enums.transform_values do |options| + safe_model_class.defined_enums.transform_values do |enum| { field: :select, - options: + enum: } end else diff --git a/spec/system/avo/has_field_discovery_spec.rb b/spec/system/avo/has_field_discovery_spec.rb index 4f144e2b0a..8d97a0097b 100644 --- a/spec/system/avo/has_field_discovery_spec.rb +++ b/spec/system/avo/has_field_discovery_spec.rb @@ -80,29 +80,31 @@ it "renders each field exactly once" do wait_for_loaded - within("[data-panel-id='main']") do - # Basic fields - ## Main Panel - expect(page).to have_text("FIRST NAME", count: 1) - expect(page).to have_text("LAST NAME", count: 1) - expect(page).to have_text("ROLES", count: 1) - expect(page).to have_text("TEAM ID", count: 1) - expect(page).to have_text("CREATED AT", count: 1) - expect(page).to have_text("UPDATED AT", count: 1) - expect(page).to have_text("SLUG", count: 1) - - # Sidebar - expect(page).to have_text("AVATAR", count: 1) - expect(page).to have_text("IS ACTIVE", count: 1) - expect(page).to have_text("BIRTHDAY", count: 1) - - # Single file uploads - expect(page).to have_text("CV", count: 1) - expect(page).not_to have_text("CV ATTACHMENT") - end + within(".main-content-area") do + within("[data-panel-id='main']") do + # Basic fields + ## Main Panel + expect(page).to have_text("FIRST NAME", count: 1) + expect(page).to have_text("LAST NAME", count: 1) + expect(page).to have_text("ROLES", count: 1) + expect(page).to have_text("TEAM ID", count: 1) + expect(page).to have_text("CREATED AT", count: 1) + expect(page).to have_text("UPDATED AT", count: 1) + expect(page).to have_text("SLUG", count: 1) + + # Sidebar + expect(page).to have_text("AVATAR", count: 1) + expect(page).to have_text("IS ACTIVE", count: 1) + expect(page).to have_text("BIRTHDAY", count: 1) + + # Single file uploads + expect(page).to have_text("CV", count: 1) + expect(page).not_to have_text("CV ATTACHMENT") + end - # Associations - expect(page).to have_text("Posts", count: 1) + # Associations + expect(page).to have_text("Posts", count: 1) + end end end @@ -226,7 +228,19 @@ end describe "Enum Fields" do - before { visit "/admin/resources/posts/#{post.id}/edit" } + let(:post) { create :post } + let(:url) { "/admin/resources/posts/#{post.id}/edit" } + + after do + Avo::Resources::Post.restore_items_from_backup + end + + before do + Avo::Resources::Post.with_temporary_items do + discover_columns + end + visit url + end it "displays enum fields as select boxes" do wait_for_loaded