From 7cddab1b0c71beb8c78a294ad3d9659963efb859 Mon Sep 17 00:00:00 2001 From: Pablo Brasero <36066+pablobm@users.noreply.github.com> Date: Tue, 27 Oct 2020 19:03:00 +0000 Subject: [PATCH] Add automatic associations (#1633) Currently, associative dashboard fields (`BelongsTo`, `HasMany`, `HasOne`) require explicit configuration options (`:class_name`, `:foreign_key`, `:primary_key`) for Administrate to find their associated models. There are details that Administrate can figure out automatically, but not everything, and not always reliably. This PR changes these field types so that they do not need this configuration, instead detecting associations by using ActiveRecord's reflection API. This implementation relies on `Field::Base.new` always receiving a `:resource_class` option. This is safe as it's already the case throughout the codebase. However third party gems may have incompatibilities here. From looking at the list of reverse dependencies[1]. From this list, only `administrate-field-nested_has_many` appear to have incompatibilities, which is not surprising as it adds a new associative field with some advanced features. Specifically you'll find that: * It will not work if you remove your `:class_name` options from `NestedHasMany` fields. * You can work around this by continuing to use the deprecated options (just for your `NestedHasMany` fields), although you'll get deprecation warnings. * There's a branch that will bring this plugin in sync [2]. The old behviour will still exist for a while, to help with backwards compatibility. This commit introduces a set of deprecation warnings. Finally, notes for the future: * The current code relies a lot on globals (eg: class methods) that are difficult to test and generally work with. In the future, we may want to change the interface for field types to improve the situation. * There's still some risky string-to-class metaprogramming going on, for example a `"#{associated_class_name}Dashboard".constantize.new` in `lib/administrate/field/associative.rb` (not shown in the diff as unchanged). This should be reviewed in the future as part of work in the resolved to fix issues such as namespacing bugs, which are out of scope here. [1]: https://rubygems.org/gems/administrate/reverse_dependencies [2]: https://github.com/nickcharlton/administrate-field-nested_has_many/compare/master...pablobm:automatic-associations Fixes #1597 --- docs/customizing_dashboards.md | 26 +- lib/administrate.rb | 19 ++ lib/administrate/base_dashboard.rb | 5 +- lib/administrate/field/associative.rb | 52 ++- lib/administrate/field/belongs_to.rb | 10 +- lib/administrate/field/deferred.rb | 10 +- lib/administrate/field/has_many.rb | 17 +- lib/administrate/field/has_one.rb | 25 +- lib/administrate/field/polymorphic.rb | 2 +- lib/administrate/search.rb | 17 +- .../dashboard/dashboard_generator.rb | 12 +- spec/dashboards/order_dashboard_spec.rb | 2 +- .../app/dashboards/customer_dashboard.rb | 3 - spec/generators/dashboard_generator_spec.rb | 90 ------ spec/lib/administrate/search_spec.rb | 302 ++++++++++-------- spec/lib/fields/belongs_to_spec.rb | 247 +++++++++++--- spec/lib/fields/boolean_spec.rb | 10 +- spec/lib/fields/deferred_spec.rb | 78 ++++- spec/lib/fields/has_many_spec.rb | 212 +++++++----- spec/lib/fields/has_one_spec.rb | 49 ++- spec/lib/fields/number_spec.rb | 9 +- spec/lib/fields/password_spec.rb | 9 +- spec/lib/fields/polymorphic_spec.rb | 9 +- spec/lib/fields/string_spec.rb | 9 +- spec/support/field_matchers.rb | 7 +- 25 files changed, 801 insertions(+), 430 deletions(-) diff --git a/docs/customizing_dashboards.md b/docs/customizing_dashboards.md index f6bd71ce0d..193b5f3836 100644 --- a/docs/customizing_dashboards.md +++ b/docs/customizing_dashboards.md @@ -79,17 +79,9 @@ which are specified through the `.with_options` class method: `:order` - Specifies the order of the dropdown menu, can be ordered by more than one column. e.g.: `"name, email DESC"`. -`:primary_key` - Specifies object's primary_key. Defaults to `:id`. - -`:foreign_key` - Specifies the name of the foreign key directly. -Defaults to `:#{attribute}_id`. - `:scope` - Specifies a custom scope inside a callable. Useful for preloading. Example: `.with_options(scope: -> { MyModel.includes(:rel).limit(5) })` -`:class_name` - Specifies the name of the associated class. -Defaults to `:#{attribute}.to_s.singularize.camelcase`. - `:include_blank` - Specifies if the select element to be rendered should include blank option. Default is `true`. @@ -111,6 +103,12 @@ For example: with this, you will be able to search through the column `name` from the association `belongs_to :country`, from your model. +`:primary_key` (deprecated) - Specifies the association's primary_key. + +`:foreign_key` (deprecated) - Specifies the name of the foreign key directly. + +`:class_name` (deprecated) - Specifies the name of the associated class. + **Field::HasMany** `:limit` - Set the number of resources to display in the show view. Default is @@ -120,18 +118,14 @@ association `belongs_to :country`, from your model. `:direction` - What direction the sort should be in, `:asc` (default) or `:desc`. -`:primary_key` - Specifies object's primary_key. Defaults to `:id`. +`:primary_key` (deprecated) - Specifies object's primary_key. -`:foreign_key` - Specifies the name of the foreign key directly. Defaults to `:#{attribute}_id` +`:foreign_key` (deprecated) - Specifies the name of the foreign key directly. -`:class_name` - Specifies the name of the associated class. -Defaults to `:#{attribute}.to_s.singularize.camelcase`. +`:class_name` (deprecated) - Specifies the name of the associated class. **Field::HasOne** -`:class_name` - Specifies the name of the associated class. -Defaults to `:#{attribute}.to_s.singularize.camelcase`. - `:searchable` - Specify if the attribute should be considered when searching. Default is `false`. @@ -150,6 +144,8 @@ For example: with this, you will be able to search through the column `name` from the association `has_many :cities`, from your model. +`:class_name` (deprecated) - Specifies the name of the associated class. + **Field::Number** `:searchable` - Specify if the attribute should be considered when searching. diff --git a/lib/administrate.rb b/lib/administrate.rb index a31181102a..0827f0d293 100644 --- a/lib/administrate.rb +++ b/lib/administrate.rb @@ -1,4 +1,23 @@ require "administrate/engine" module Administrate + def self.warn_of_missing_resource_class + ActiveSupport::Deprecation.warn( + "Calling Field::Base.permitted_attribute without the option " + + ":resource_class is deprecated. If you are seeing this " + + "message, you are probably using a custom field type that" + + "does this. Please make sure to update it to a version that " + + "does not use a deprecated API", + ) + end + + def self.warn_of_deprecated_option(name) + ActiveSupport::Deprecation.warn( + "The option :#{name} is deprecated. " + + "Administrate should detect it automatically. " + + "Please file an issue at " + + "https://github.com/thoughtbot/administrate/issues " + + "if you think otherwise.", + ) + end end diff --git a/lib/administrate/base_dashboard.rb b/lib/administrate/base_dashboard.rb index e63be1e441..a597149eb7 100644 --- a/lib/administrate/base_dashboard.rb +++ b/lib/administrate/base_dashboard.rb @@ -56,7 +56,10 @@ def form_attributes def permitted_attributes form_attributes.map do |attr| - attribute_types[attr].permitted_attribute(attr) + attribute_types[attr].permitted_attribute( + attr, + resource_class: self.class.model, + ) end.uniq end diff --git a/lib/administrate/field/associative.rb b/lib/administrate/field/associative.rb index b5321a6121..b24bee1054 100644 --- a/lib/administrate/field/associative.rb +++ b/lib/administrate/field/associative.rb @@ -3,12 +3,32 @@ module Administrate module Field class Associative < Base + def self.foreign_key_for(resource_class, attr) + reflection(resource_class, attr).foreign_key + end + + def self.associated_class(resource_class, attr) + reflection(resource_class, attr).klass + end + + def self.associated_class_name(resource_class, attr) + reflection(resource_class, attr).class_name + end + + def self.reflection(resource_class, attr) + resource_class.reflect_on_association(attr) + end + def display_associated_resource associated_dashboard.display_resource(data) end def associated_class - associated_class_name.constantize + if option_given?(:class_name) + associated_class_name.constantize + else + self.class.associated_class(resource.class, attribute) + end end private @@ -18,15 +38,39 @@ def associated_dashboard end def associated_class_name - options.fetch(:class_name, attribute.to_s.singularize.camelcase) + if option_given?(:class_name) + deprecated_option(:class_name) + else + self.class.associated_class_name( + resource.class, + attribute, + ) + end end def primary_key - options.fetch(:primary_key, :id) + if option_given?(:primary_key) + deprecated_option(:primary_key) + else + :id + end end def foreign_key - options.fetch(:foreign_key, :"#{attribute}_id") + if option_given?(:foreign_key) + deprecated_option(:foreign_key) + else + self.class.foreign_key_for(resource.class, attribute) + end + end + + def option_given?(name) + options.key?(name) + end + + def deprecated_option(name) + Administrate.warn_of_deprecated_option(name) + options.fetch(name) end end end diff --git a/lib/administrate/field/belongs_to.rb b/lib/administrate/field/belongs_to.rb index 6b1b096a61..cf82dfef0a 100644 --- a/lib/administrate/field/belongs_to.rb +++ b/lib/administrate/field/belongs_to.rb @@ -3,8 +3,14 @@ module Administrate module Field class BelongsTo < Associative - def self.permitted_attribute(attr, _options = nil) - :"#{attr}_id" + def self.permitted_attribute(attr, options = {}) + resource_class = options[:resource_class] + if resource_class + foreign_key_for(resource_class, attr) + else + Administrate.warn_of_missing_resource_class + :"#{attr}_id" + end end def permitted_attribute diff --git a/lib/administrate/field/deferred.rb b/lib/administrate/field/deferred.rb index 91275b5262..c0680742a5 100644 --- a/lib/administrate/field/deferred.rb +++ b/lib/administrate/field/deferred.rb @@ -44,9 +44,13 @@ def searchable_fields end end - def permitted_attribute(attr, _options = nil) - options.fetch(:foreign_key, - deferred_class.permitted_attribute(attr, options)) + def permitted_attribute(attr, opts = {}) + if options.key?(:foreign_key) + Administrate.warn_of_deprecated_option(:foreign_key) + options.fetch(:foreign_key) + else + deferred_class.permitted_attribute(attr, options.merge(opts)) + end end delegate :html_class, to: :deferred_class diff --git a/lib/administrate/field/has_many.rb b/lib/administrate/field/has_many.rb index bbb84cb503..b223ba807f 100644 --- a/lib/administrate/field/has_many.rb +++ b/lib/administrate/field/has_many.rb @@ -7,7 +7,17 @@ module Field class HasMany < Associative DEFAULT_LIMIT = 5 - def self.permitted_attribute(attr, _options = nil) + def self.permitted_attribute(attr, _options = {}) + # This may seem arbitrary, and improvable by using reflection. + # Worry not: here we do exactly what Rails does. Regardless of the name + # of the foreign key, has_many associations use the suffix `_ids` + # for this. + # + # Eg: if the associated table and primary key are `countries.code`, + # you may expect `country_codes` as attribute here, but it will + # be `country_ids` instead. + # + # See https://github.com/rails/rails/blob/b30a23f53b52e59d31358f7b80385ee5c2ba3afe/activerecord/lib/active_record/associations/builder/collection_association.rb#L48 { "#{attr.to_s.singularize}_ids".to_sym => [] } end @@ -36,7 +46,10 @@ def limit end def permitted_attribute - self.class.permitted_attribute(attribute) + self.class.permitted_attribute( + attribute, + resource_class: resource.class, + ) end def resources(page = 1, order = self.order) diff --git a/lib/administrate/field/has_one.rb b/lib/administrate/field/has_one.rb index a951d95d56..5c919f7e93 100644 --- a/lib/administrate/field/has_one.rb +++ b/lib/administrate/field/has_one.rb @@ -3,17 +3,26 @@ module Administrate module Field class HasOne < Associative - def self.permitted_attribute(attr, options = nil) - associated_class_name = - if options - options.fetch(:class_name, attr.to_s.singularize.camelcase) + def self.permitted_attribute(attr, options = {}) + resource_class = options[:resource_class] + final_associated_class_name = + if options.key?(:class_name) + Administrate.warn_of_deprecated_option(:class_name) + options.fetch(:class_name) + elsif resource_class + associated_class_name(resource_class, attr) else - attr + Administrate.warn_of_missing_resource_class + if options + attr.to_s.singularize.camelcase + else + attr + end end related_dashboard_attributes = - Administrate::ResourceResolver.new("admin/#{associated_class_name}"). + Administrate::ResourceResolver. + new("admin/#{final_associated_class_name}"). dashboard_class.new.permitted_attributes + [:id] - { "#{attr}_attributes": related_dashboard_attributes } end @@ -35,7 +44,7 @@ def nested_show def resolver @resolver ||= - Administrate::ResourceResolver.new("admin/#{associated_class_name}") + Administrate::ResourceResolver.new("admin/#{associated_class.name}") end end end diff --git a/lib/administrate/field/polymorphic.rb b/lib/administrate/field/polymorphic.rb index 002517f40f..4bf22e2461 100644 --- a/lib/administrate/field/polymorphic.rb +++ b/lib/administrate/field/polymorphic.rb @@ -3,7 +3,7 @@ module Administrate module Field class Polymorphic < BelongsTo - def self.permitted_attribute(attr, _options = nil) + def self.permitted_attribute(attr, _options = {}) { attr => %i{type value} } end diff --git a/lib/administrate/search.rb b/lib/administrate/search.rb index 07cc9acf2e..cd8ce0fb0d 100644 --- a/lib/administrate/search.rb +++ b/lib/administrate/search.rb @@ -82,8 +82,8 @@ def query_template search_attributes.map do |attr| table_name = query_table_name(attr) searchable_fields(attr).map do |field| - attr_name = column_to_query(field) - "LOWER(CAST(#{table_name}.#{attr_name} AS CHAR(256))) LIKE ?" + column_name = column_to_query(field) + "LOWER(CAST(#{table_name}.#{column_name} AS CHAR(256))) LIKE ?" end.join(" OR ") end.join(" OR ") end @@ -128,11 +128,14 @@ def attribute_types def query_table_name(attr) if association_search?(attr) provided_class_name = attribute_types[attr].options[:class_name] - if provided_class_name - provided_class_name.constantize.table_name - else - ActiveRecord::Base.connection.quote_table_name(attr.to_s.pluralize) - end + unquoted_table_name = + if provided_class_name + Administrate.warn_of_deprecated_option(:class_name) + provided_class_name.constantize.table_name + else + @scoped_resource.reflect_on_association(attr).klass.table_name + end + ActiveRecord::Base.connection.quote_table_name(unquoted_table_name) else ActiveRecord::Base.connection. quote_table_name(@scoped_resource.table_name) diff --git a/lib/generators/administrate/dashboard/dashboard_generator.rb b/lib/generators/administrate/dashboard/dashboard_generator.rb index ac91237ed4..3f2c647c3c 100644 --- a/lib/generators/administrate/dashboard/dashboard_generator.rb +++ b/lib/generators/administrate/dashboard/dashboard_generator.rb @@ -110,11 +110,11 @@ def association_type(attribute) if relationship.has_one? "Field::HasOne" elsif relationship.collection? - "Field::HasMany" + relationship_options_string(relationship) + "Field::HasMany" elsif relationship.polymorphic? "Field::Polymorphic" else - "Field::BelongsTo" + relationship_options_string(relationship) + "Field::BelongsTo" end end @@ -122,14 +122,6 @@ def klass @klass ||= Object.const_get(class_name) end - def relationship_options_string(relationship) - if relationship.class_name != relationship.name.to_s.classify - options_string(class_name: relationship.class_name) - else - "" - end - end - def options_string(options) if options.any? ".with_options(#{inspect_hash_as_ruby(options)})" diff --git a/spec/dashboards/order_dashboard_spec.rb b/spec/dashboards/order_dashboard_spec.rb index a45ec6024f..baf9b30fa4 100644 --- a/spec/dashboards/order_dashboard_spec.rb +++ b/spec/dashboards/order_dashboard_spec.rb @@ -11,7 +11,7 @@ it "returns the attribute_id name for belongs_to relationships" do dashboard = OrderDashboard.new - expect(dashboard.permitted_attributes).to include(:customer_id) + expect(dashboard.permitted_attributes).to include("customer_id") end end end diff --git a/spec/example_app/app/dashboards/customer_dashboard.rb b/spec/example_app/app/dashboards/customer_dashboard.rb index eff8bc3881..5d49bd0dd6 100644 --- a/spec/example_app/app/dashboards/customer_dashboard.rb +++ b/spec/example_app/app/dashboards/customer_dashboard.rb @@ -13,9 +13,6 @@ class CustomerDashboard < Administrate::BaseDashboard updated_at: Field::DateTime, kind: Field::Select.with_options(collection: Customer::KINDS), territory: Field::BelongsTo.with_options( - primary_key: :code, - foreign_key: :country_code, - class_name: "Country", searchable: true, searchable_fields: ["name"], include_blank: true, diff --git a/spec/generators/dashboard_generator_spec.rb b/spec/generators/dashboard_generator_spec.rb index a574de88e2..85fb61451e 100644 --- a/spec/generators/dashboard_generator_spec.rb +++ b/spec/generators/dashboard_generator_spec.rb @@ -90,20 +90,6 @@ class Foo < ApplicationRecord expect(dashboard).to contain("orders: Field::HasMany") end - it "looks for class_name options on has_many fields" do - class Customer < ApplicationRecord - reset_column_information - has_many :purchases, class_name: "Order", foreign_key: "purchase_id" - end - dashboard = file("app/dashboards/customer_dashboard.rb") - - run_generator ["customer"] - - expect(dashboard).to contain( - 'purchases: Field::HasMany.with_options(class_name: "Order")', - ) - end - it "assigns numeric fields a type of `Number`" do begin ActiveRecord::Schema.define do @@ -225,54 +211,6 @@ class Event < ApplicationRecord end end - it "determines a class_name from `through` and `source` options" do - begin - ActiveRecord::Schema.define do - create_table :people - create_table :concerts - create_table(:numbers) { |t| t.references :ticket } - - create_table :tickets do |t| - t.references :concert - t.references :attendee - end - end - - class Concert < ApplicationRecord - reset_column_information - has_many :tickets - has_many :attendees, through: :tickets, source: :person - has_many :venues, through: :tickets - has_many :numbers, through: :tickets - end - - class Ticket < ApplicationRecord - reset_column_information - belongs_to :concert - belongs_to :person - belongs_to :venue - has_many :numbers - end - - class Number; end - class Person < ApplicationRecord - reset_column_information - end - - dashboard = file("app/dashboards/concert_dashboard.rb") - - run_generator ["concert"] - - expect(dashboard).to contain( - 'attendees: Field::HasMany.with_options(class_name: "Person"),', - ) - expect(dashboard).to contain("venues: Field::HasMany,") - expect(dashboard).to contain("numbers: Field::HasMany,") - ensure - remove_constants :Concert, :Ticket, :Number, :Person - end - end - it "detects belongs_to relationships" do begin ActiveRecord::Schema.define do @@ -293,34 +231,6 @@ class Comment < ApplicationRecord end end - it "detects custom class names for belongs_to relationships" do - begin - ActiveRecord::Schema.define do - create_table :users - create_table :invitations do |t| - t.references :sender - t.references :recipient - end - end - class User < ApplicationRecord; end - class Invitation < ApplicationRecord - belongs_to :sender, class_name: "User" - belongs_to :recipient, class_name: "User" - end - - run_generator ["invitation"] - load file("app/dashboards/invitation_dashboard.rb") - attrs = InvitationDashboard::ATTRIBUTE_TYPES - - expected_field = Administrate::Field::BelongsTo. - with_options(class_name: "User") - expect(attrs[:sender]).to eq(expected_field) - expect(attrs[:recipient]).to eq(expected_field) - ensure - remove_constants :User, :Invitation, :InvitationDashboard - end - end - it "detects polymorphic belongs_to relationships" do begin ActiveRecord::Schema.define do diff --git a/spec/lib/administrate/search_spec.rb b/spec/lib/administrate/search_spec.rb index 3b111bdea2..d0bd0eb1d2 100644 --- a/spec/lib/administrate/search_spec.rb +++ b/spec/lib/administrate/search_spec.rb @@ -10,128 +10,162 @@ require "administrate/field/string" require "administrate/search" -class MockDashboard - ATTRIBUTE_TYPES = { - id: Administrate::Field::Number.with_options(searchable: true), - name: Administrate::Field::String, - email: Administrate::Field::Email, - phone: Administrate::Field::Number, - }.freeze - - COLLECTION_FILTERS = { - vip: ->(resources) { resources.where(kind: :vip) }, - }.freeze -end +describe Administrate::Search do + before :all do + module Administrate + module SearchSpecMocks + class MockRecord < ApplicationRecord + def self.table_name + name.demodulize.underscore.pluralize + end + end -class MockDashboardWithAssociation - ATTRIBUTE_TYPES = { - role: Administrate::Field::BelongsTo.with_options( - searchable: true, - searchable_field: "name", - ), - author: Administrate::Field::BelongsTo.with_options( - searchable: true, - searchable_fields: ["first_name", "last_name"], - ), - address: Administrate::Field::HasOne.with_options( - searchable: true, - searchable_fields: ["street"], - ), - }.freeze -end + class Role < MockRecord; end + class Person < MockRecord; end + class Address < MockRecord; end + + class Foo < MockRecord + belongs_to :role + belongs_to( + :author, + class_name: "Administrate::SearchSpecMocks::Person", + ) + has_one :address + end + + class UserDashboard + ATTRIBUTE_TYPES = { + id: Administrate::Field::Number.with_options(searchable: true), + name: Administrate::Field::String, + email: Administrate::Field::Email, + phone: Administrate::Field::Number, + }.freeze + + COLLECTION_FILTERS = { + vip: ->(resources) { resources.where(kind: :vip) }, + }.freeze + end + + class FooDashboard + ATTRIBUTE_TYPES = { + role: Administrate::Field::BelongsTo.with_options( + searchable: true, + searchable_field: "name", + ), + author: Administrate::Field::BelongsTo.with_options( + searchable: true, + searchable_fields: ["first_name", "last_name"], + class_name: "Administrate::SearchSpecMocks::Person", + ), + address: Administrate::Field::HasOne.with_options( + searchable: true, + searchable_fields: ["street"], + ), + }.freeze + end + end + end + end + + after :all do + Administrate.send(:remove_const, :SearchSpecMocks) + end -describe Administrate::Search do before { ActiveSupport::Deprecation.silenced = true } after { ActiveSupport::Deprecation.silenced = false } describe "#run" do it "returns all records when no search term" do - begin - class User < ApplicationRecord; end - scoped_object = User.default_scoped - search = Administrate::Search.new(scoped_object, - MockDashboard, - nil) - expect(scoped_object).to receive(:all) - - search.run - ensure - remove_constants :User - end + class User < ApplicationRecord; end + scoped_object = User.default_scoped + search = Administrate::Search.new( + scoped_object, + Administrate::SearchSpecMocks::UserDashboard, + nil, + ) + expect(scoped_object).to receive(:all) + + search.run + ensure + remove_constants :User end it "returns all records when search is empty" do - begin - class User < ApplicationRecord; end - scoped_object = User.default_scoped - search = Administrate::Search.new(scoped_object, - MockDashboard, - " ") - expect(scoped_object).to receive(:all) - - search.run - ensure - remove_constants :User - end + class User < ApplicationRecord; end + scoped_object = User.default_scoped + search = Administrate::Search.new( + scoped_object, + Administrate::SearchSpecMocks::UserDashboard, + " ", + ) + expect(scoped_object).to receive(:all) + + search.run + ensure + remove_constants :User end it "searches using LOWER + LIKE for all searchable fields" do - begin - class User < ApplicationRecord; end - scoped_object = User.default_scoped - search = Administrate::Search.new(scoped_object, - MockDashboard, - "test") - expected_query = [ - [ - 'LOWER(CAST("users"."id" AS CHAR(256))) LIKE ?', - 'LOWER(CAST("users"."name" AS CHAR(256))) LIKE ?', - 'LOWER(CAST("users"."email" AS CHAR(256))) LIKE ?', - ].join(" OR "), - "%test%", - "%test%", - "%test%", - ] - expect(scoped_object).to receive(:where).with(*expected_query) - - search.run - ensure - remove_constants :User - end + class User < ApplicationRecord; end + scoped_object = User.default_scoped + search = Administrate::Search.new( + scoped_object, + Administrate::SearchSpecMocks::UserDashboard, + "test", + ) + expected_query = [ + [ + 'LOWER(CAST("users"."id" AS CHAR(256))) LIKE ?', + 'LOWER(CAST("users"."name" AS CHAR(256))) LIKE ?', + 'LOWER(CAST("users"."email" AS CHAR(256))) LIKE ?', + ].join(" OR "), + "%test%", + "%test%", + "%test%", + ] + expect(scoped_object).to receive(:where).with(*expected_query) + + search.run + ensure + remove_constants :User end it "converts search term LOWER case for latin and cyrillic strings" do - begin - class User < ApplicationRecord; end - scoped_object = User.default_scoped - search = Administrate::Search.new(scoped_object, - MockDashboard, - "Тест Test") - expected_query = [ - [ - 'LOWER(CAST("users"."id" AS CHAR(256))) LIKE ?', - 'LOWER(CAST("users"."name" AS CHAR(256))) LIKE ?', - 'LOWER(CAST("users"."email" AS CHAR(256))) LIKE ?', - ].join(" OR "), - "%тест test%", - "%тест test%", - "%тест test%", - ] - expect(scoped_object).to receive(:where).with(*expected_query) - - search.run - ensure - remove_constants :User - end + class User < ApplicationRecord; end + scoped_object = User.default_scoped + search = Administrate::Search.new( + scoped_object, + Administrate::SearchSpecMocks::UserDashboard, + "Тест Test", + ) + expected_query = [ + [ + 'LOWER(CAST("users"."id" AS CHAR(256))) LIKE ?', + 'LOWER(CAST("users"."name" AS CHAR(256))) LIKE ?', + 'LOWER(CAST("users"."email" AS CHAR(256))) LIKE ?', + ].join(" OR "), + "%тест test%", + "%тест test%", + "%тест test%", + ] + expect(scoped_object).to receive(:where).with(*expected_query) + + search.run + ensure + remove_constants :User end context "when searching through associations" do - let(:scoped_object) { double(:scoped_object) } + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + end + + let(:scoped_object) { Administrate::SearchSpecMocks::Foo } let(:search) do Administrate::Search.new( scoped_object, - MockDashboardWithAssociation, + Administrate::SearchSpecMocks::FooDashboard, "Тест Test", ) end @@ -139,8 +173,8 @@ class User < ApplicationRecord; end let(:expected_query) do [ 'LOWER(CAST("roles"."name" AS CHAR(256))) LIKE ?'\ - ' OR LOWER(CAST("authors"."first_name" AS CHAR(256))) LIKE ?'\ - ' OR LOWER(CAST("authors"."last_name" AS CHAR(256))) LIKE ?'\ + ' OR LOWER(CAST("people"."first_name" AS CHAR(256))) LIKE ?'\ + ' OR LOWER(CAST("people"."last_name" AS CHAR(256))) LIKE ?'\ ' OR LOWER(CAST("addresses"."street" AS CHAR(256))) LIKE ?', "%тест test%", "%тест test%", @@ -151,44 +185,60 @@ class User < ApplicationRecord; end it "joins with the correct association table to query" do allow(scoped_object).to receive(:where) - - expect(scoped_object).to receive(:left_joins). - with(%i(role author address)). - and_return(scoped_object) + allow(scoped_object).to receive(:left_joins).and_return(scoped_object) search.run + + expect(scoped_object).to( + have_received(:left_joins).with(%i(role author address)), + ) end it "builds the 'where' clause using the joined tables" do - allow(scoped_object).to receive(:left_joins). - with(%i(role author address)). - and_return(scoped_object) + allow(scoped_object).to receive(:where) + allow(scoped_object).to receive(:left_joins).and_return(scoped_object) + + search.run + + expect(scoped_object).to( + have_received(:where).with(*expected_query), + ) + end - expect(scoped_object).to receive(:where).with(*expected_query) + it "triggers a deprecation warning" do + allow(scoped_object).to receive(:where) + allow(scoped_object).to( + receive(:left_joins). + with(%i(role author address)). + and_return(scoped_object), + ) search.run + + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:class_name is deprecated/) end end it "searches using a filter" do - begin - class User < ActiveRecord::Base - scope :vip, -> { where(kind: :vip) } - end - scoped_object = User.default_scoped - search = Administrate::Search.new(scoped_object, - MockDashboard, - "vip:") - expect(scoped_object).to \ - receive(:where). - with(kind: :vip). - and_return(scoped_object) - expect(scoped_object).to receive(:where).and_return(scoped_object) - - search.run - ensure - remove_constants :User + class User < ApplicationRecord + scope :vip, -> { where(kind: :vip) } end + scoped_object = User.default_scoped + search = Administrate::Search.new( + scoped_object, + Administrate::SearchSpecMocks::UserDashboard, + "vip:", + ) + expect(scoped_object).to \ + receive(:where). + with(kind: :vip). + and_return(scoped_object) + expect(scoped_object).to receive(:where).and_return(scoped_object) + + search.run + ensure + remove_constants :User end end end diff --git a/spec/lib/fields/belongs_to_spec.rb b/spec/lib/fields/belongs_to_spec.rb index 453e0aaec1..98922be2e5 100644 --- a/spec/lib/fields/belongs_to_spec.rb +++ b/spec/lib/fields/belongs_to_spec.rb @@ -6,7 +6,13 @@ describe Administrate::Field::BelongsTo do include FieldMatchers - it { should_permit_param(:foo_id, for_attribute: :foo) } + it do + should_permit_param( + :country_code, + on_model: Customer, + for_attribute: :territory, + ) + end describe "#to_partial_path" do it "returns a partial based on the page being rendered" do @@ -20,31 +26,130 @@ end end + describe "#associated_class" do + it "is automatically resolved for a conventional association" do + line_item = create(:line_item) + field = Administrate::Field::BelongsTo.new( + :product, + nil, + :show, + resource: line_item, + ) + expect(field.associated_class).to eq(Product) + end + + it "is automatically resolved for a custom association" do + customer = create(:customer) + field = Administrate::Field::BelongsTo.new( + :territory, + nil, + :show, + resource: customer, + ) + expect(field.associated_class).to eq(Country) + end + end + + describe "#display_associated_resource" do + it "renders the associated resource for a conventional association" do + product = create(:product, name: "Associated Product") + line_item = create(:line_item, product: product) + field = Administrate::Field::BelongsTo.new( + :product, + line_item.product, + :show, + resource: line_item, + ) + allow_any_instance_of(ProductDashboard).to( + receive(:display_resource) do |_, resource| + "Mock #{resource.name}" + end, + ) + expect(field.display_associated_resource).to eq("Mock Associated Product") + end + + it "renders the associated resource for a custom association" do + country = create(:country, name: "Associated Country") + customer = create(:customer) + field = Administrate::Field::BelongsTo.new( + :territory, + country, + :show, + resource: customer, + ) + allow_any_instance_of(CountryDashboard).to( + receive(:display_resource) do |_, resource| + "Mock #{resource.name}" + end, + ) + expect(field.display_associated_resource).to eq("Mock Associated Country") + end + end + describe "class_name option" do - it "determines what dashboard is used to present the association" do - begin - Foo = Class.new - allow(Foo).to receive(:all).and_return([]) + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + end - association = Administrate::Field::BelongsTo.with_options( - class_name: "Foo", - ) - field = association.new(:customers, [], :show) - candidates = field.associated_resource_options + it "determines the associated_class" do + line_item = create(:line_item) + field_class = Administrate::Field::BelongsTo.with_options( + class_name: "Customer", + ) + field = field_class.new( + :product, + line_item.product, + :show, + resource: line_item, + ) + expect(field.associated_class).to eq(Customer) + end - expect(Foo).to have_received(:all) - expect(candidates).to eq([]) - ensure - remove_constants :Foo - end + it "uses it to determine how to render the associated resource" do + product = create(:product, name: "Associated Product") + line_item = create(:line_item, product: product) + field_class = Administrate::Field::BelongsTo.with_options( + class_name: "LineItem", + ) + field = field_class.new( + :product, + line_item.product, + :show, + resource: line_item, + ) + expect(field.display_associated_resource).to match( + /^Line Item \#\d\d\d\d$/, + ) + end + + it "triggers a deprecation warning" do + line_item = create(:line_item) + field_class = Administrate::Field::BelongsTo.with_options( + class_name: "Customer", + ) + field = field_class.new( + :product, + line_item.product, + :show, + resource: line_item, + ) + field.associated_class + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:class_name is deprecated/) end end - describe "include_blank option" do + describe ":include_blank option" do context "default value as true" do it "determines if choices has blank option or not" do + customer = create(:customer, territory: nil) association = Administrate::Field::BelongsTo - field = association.new(:customers, [], :edit) + field = association.new( + :territory, + [], + :edit, + resource: customer, + ) candidates = field.associated_resource_options expect(field.include_blank_option). to eq(true) @@ -54,10 +159,16 @@ context "set value as false" do it "determines if choices has blank option or not" do + customer = create(:customer, territory: nil) association = Administrate::Field::BelongsTo.with_options( include_blank: false, ) - field = association.new(:customers, [], :edit) + field = association.new( + :territory, + [], + :edit, + resource: customer, + ) candidates = field.associated_resource_options expect(field.include_blank_option). to eq(false) @@ -67,35 +178,55 @@ end describe "primary_key option" do + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + + Foo = Class.new + FooDashboard = Class.new + uuid = SecureRandom.uuid + allow(Foo).to receive(:all).and_return([Foo]) + allow(Foo).to receive(:uuid).and_return(uuid) + allow(Foo).to receive(:id).and_return(1) + allow_any_instance_of(FooDashboard).to( + receive(:display_resource).and_return(uuid), + ) + end + + after do + remove_constants :Foo, :FooDashboard + end + it "determines what primary key is used on the relationship for the form" do - begin - Foo = Class.new - FooDashboard = Class.new - uuid = SecureRandom.uuid - allow(Foo).to receive(:all).and_return([Foo]) - allow(Foo).to receive(:uuid).and_return(uuid) - allow(Foo).to receive(:id).and_return(1) - allow_any_instance_of(FooDashboard).to( - receive(:display_resource).and_return(uuid) + association = + Administrate::Field::BelongsTo.with_options( + primary_key: "uuid", class_name: "Foo", ) + field = association.new(:customers, [], :show) + field.associated_resource_options - association = - Administrate::Field::BelongsTo.with_options( - primary_key: "uuid", class_name: "Foo" - ) - field = association.new(:customers, [], :show) - field.associated_resource_options - - expect(Foo).to have_received(:all) - expect(Foo).to have_received(:uuid) - expect(Foo).not_to have_received(:id) - ensure - remove_constants :Foo, :FooDashboard - end + expect(Foo).to have_received(:all) + expect(Foo).to have_received(:uuid) + expect(Foo).not_to have_received(:id) + end + + it "triggers a deprecation warning" do + association = + Administrate::Field::BelongsTo.with_options( + primary_key: "uuid", + ) + field = association.new(:foo, double(uuid: nil), :baz) + field.selected_option + + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:primary_key is deprecated/) end end describe "foreign_key option" do + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + end + it "determines what foreign key is used on the relationship for the form" do association = Administrate::Field::BelongsTo.with_options( foreign_key: "foo_uuid", class_name: "Foo", @@ -104,15 +235,29 @@ permitted_attribute = field.permitted_attribute expect(permitted_attribute).to eq("foo_uuid") end + + it "triggers a deprecation warning" do + association = Administrate::Field::BelongsTo.with_options( + foreign_key: "foo_uuid", class_name: "Foo", + ) + field = association.new(:customers, [], :show) + + field.permitted_attribute + + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:foreign_key is deprecated/) + end end - describe "#resources" do + describe "#associated_resource_options" do context "with `order` option" do it "returns the resources in correct order" do - FactoryBot.create_list(:customer, 5) + order = create(:order) + create_list(:customer, 5) options = { order: "name" } association = Administrate::Field::BelongsTo.with_options(options) - field = association.new(:customers, [], :view) + + field = association.new(:customer, [], :show, resource: order) correct_order = Customer.order("name").pluck(:id) @@ -120,14 +265,16 @@ expect(resources).to eq correct_order end - it "rejects order passed in `scope`" do - FactoryBot.create_list(:customer, 3) + it "ignores the order passed in `scope`" do + order = create(:order) + create_list(:customer, 3) options = { order: "name", scope: -> { Customer.order(name: :desc) }, } association = Administrate::Field::BelongsTo.with_options(options) - field = association.new(:customers, [], :view) + + field = association.new(:customer, [], :show, resource: order) correct_order = Customer.order("name").pluck(:id) @@ -138,11 +285,15 @@ context "with `scope` option" do it "returns the resources within the passed scope" do - 1.upto(3) { |i| FactoryBot.create :customer, name: "customer-#{i}" } + # Building instead of creating, to avoid a dependent customer being + # created, leading to random failures + order = build(:order) + + 1.upto(3) { |i| create :customer, name: "customer-#{i}" } scope = -> { Customer.order(name: :desc).limit(2) } association = Administrate::Field::BelongsTo.with_options(scope: scope) - field = association.new(:customers, [], :view) + field = association.new(:customer, [], :show, resource: order) resources = field.associated_resource_options.compact.to_h.keys expect(resources).to eq ["customer-3", "customer-2"] diff --git a/spec/lib/fields/boolean_spec.rb b/spec/lib/fields/boolean_spec.rb index 05d1d704ea..ae6d52d78c 100644 --- a/spec/lib/fields/boolean_spec.rb +++ b/spec/lib/fields/boolean_spec.rb @@ -1,4 +1,4 @@ -require "spec_helper" +require "rails_helper" require "administrate/field/boolean" require "support/field_matchers" @@ -17,7 +17,13 @@ end end - it { should_permit_param(:foo, for_attribute: :foo) } + it do + should_permit_param( + :foo, + on_model: Customer, + for_attribute: :foo, + ) + end describe "#to_s" do it "prints true or false" do diff --git a/spec/lib/fields/deferred_spec.rb b/spec/lib/fields/deferred_spec.rb index 2dad847afb..45ff380602 100644 --- a/spec/lib/fields/deferred_spec.rb +++ b/spec/lib/fields/deferred_spec.rb @@ -1,27 +1,67 @@ +require "rails_helper" require "administrate/field/deferred" require "administrate/field/belongs_to" +require "administrate/field/has_many" require "administrate/field/string" describe Administrate::Field::Deferred do describe "#permitted_attribute" do context "when given a `foreign_key` option" do + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + end + it "returns the value given" do - deferred = Administrate::Field::Deferred. - new(Administrate::Field::BelongsTo.with_options(foreign_key: :bar)) - expect(deferred.permitted_attribute(:foo)).to eq(:bar) + deferred = Administrate::Field::Deferred.new( + Administrate::Field::BelongsTo, + foreign_key: :bar, + ) + expect(deferred.permitted_attribute(:foo, resource_class: LineItem)). + to eq(:bar) + end + + it "triggers a deprecation warning" do + deferred = Administrate::Field::Deferred.new( + Administrate::Field::BelongsTo, + foreign_key: :bar, + ) + deferred.permitted_attribute(:foo, resource_class: LineItem) + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:foreign_key is deprecated/) end end context "when not given a `foreign_key` option" do it "delegates to the backing class" do - deferred = Administrate::Field::Deferred. - new(Administrate::Field::String) + deferred = Administrate::Field::Deferred.new( + Administrate::Field::String, + ) allow(Administrate::Field::String).to receive(:permitted_attribute) - deferred.permitted_attribute(:foo) + deferred.permitted_attribute(:foo, resource_class: LineItem) + + expect(Administrate::Field::String).to( + have_received(:permitted_attribute). + with(:foo, resource_class: LineItem), + ) + end + end + + context "when given a `class_name` option" do + it "passes it on to deferred classes" do + field = double + allow(field).to receive(:permitted_attribute) + deferred = Administrate::Field::Deferred.new( + field, + class_name: "Foo::Bar", + ) + + deferred.permitted_attribute(:bars) - expect(Administrate::Field::String). - to have_received(:permitted_attribute).with(:foo, {}) + expect(field).to( + have_received(:permitted_attribute). + with(:bars, class_name: "Foo::Bar"), + ) end end end @@ -29,10 +69,14 @@ describe "#searchable?" do context "when given a `searchable` option" do it "returns the value given" do - searchable_deferred = Administrate::Field::Deferred. - new(double(searchable?: false), searchable: true) - unsearchable_deferred = Administrate::Field::Deferred. - new(double(searchable?: true), searchable: false) + searchable_deferred = Administrate::Field::Deferred.new( + double(searchable?: false), + searchable: true, + ) + unsearchable_deferred = Administrate::Field::Deferred.new( + double(searchable?: true), + searchable: false, + ) expect(searchable_deferred.searchable?).to eq(true) expect(unsearchable_deferred.searchable?).to eq(false) @@ -41,10 +85,12 @@ context "when not given a `searchable` option" do it "falls back to the default of the deferred class" do - searchable_deferred = Administrate::Field::Deferred. - new(double(searchable?: true)) - unsearchable_deferred = Administrate::Field::Deferred. - new(double(searchable?: false)) + searchable_deferred = Administrate::Field::Deferred.new( + double(searchable?: true), + ) + unsearchable_deferred = Administrate::Field::Deferred.new( + double(searchable?: false), + ) expect(searchable_deferred.searchable?).to eq(true) expect(unsearchable_deferred.searchable?).to eq(false) diff --git a/spec/lib/fields/has_many_spec.rb b/spec/lib/fields/has_many_spec.rb index 15b0838a6e..7fcdd056c4 100644 --- a/spec/lib/fields/has_many_spec.rb +++ b/spec/lib/fields/has_many_spec.rb @@ -18,97 +18,133 @@ describe "#associated_collection" do it "returns an index page for the dashboard of the associated attribute" do - begin - WidgetDashboard = Class.new - widgets = [] - field = Administrate::Field::HasMany.new(:widgets, widgets, :show) + customer = build(:customer) + field = Administrate::Field::HasMany.new( + :orders, + Order.all, + :show, + resource: customer, + ) - page = field.associated_collection + page = field.associated_collection - expect(page).to be_instance_of(Administrate::Page::Collection) - ensure - remove_constants :WidgetDashboard - end + expect(page).to be_instance_of(Administrate::Page::Collection) end end describe "class_name option" do + let(:dashboard_double) { double(collection_attributes: []) } + + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + + FooDashboard = Class.new + allow(FooDashboard).to receive(:new).and_return(dashboard_double) + end + + after do + remove_constants :FooDashboard + end + it "determines what dashboard is used to present the association" do - begin - FooDashboard = Class.new - dashboard_double = double(collection_attributes: []) - allow(FooDashboard).to receive(:new).and_return(dashboard_double) - - association = Administrate::Field::HasMany. - with_options(class_name: "Foo") - field = association.new(:customers, [], :show) - collection = field.associated_collection - attributes = collection.attribute_names - - expect(dashboard_double).to have_received(:collection_attributes) - expect(attributes).to eq([]) - ensure - remove_constants :FooDashboard - end + association = Administrate::Field::HasMany. + with_options(class_name: "Foo") + field = association.new(:customers, [], :show) + collection = field.associated_collection + attributes = collection.attribute_names + + expect(dashboard_double).to have_received(:collection_attributes) + expect(attributes).to eq([]) + end + + it "triggers a deprecation warning" do + association = Administrate::Field::HasMany. + with_options(class_name: "Foo") + field = association.new(:customers, [], :show) + field.associated_collection + + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:class_name is deprecated/) end end describe "primary_key option" do + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + + Foo = Class.new + FooDashboard = Class.new + uuid = SecureRandom.uuid + allow(Foo).to receive(:all).and_return([Foo]) + allow(Foo).to receive(:uuid).and_return(uuid) + allow(Foo).to receive(:id).and_return(1) + allow_any_instance_of(FooDashboard).to( + receive(:display_resource).and_return(uuid), + ) + end + + after do + remove_constants :Foo, :FooDashboard + end + it "determines what primary key is used on the relationship for the form" do - begin - Foo = Class.new - FooDashboard = Class.new - uuid = SecureRandom.uuid - allow(Foo).to receive(:all).and_return([Foo]) - allow(Foo).to receive(:uuid).and_return(uuid) - allow(Foo).to receive(:id).and_return(1) - allow_any_instance_of(FooDashboard).to( - receive(:display_resource).and_return(uuid) + association = + Administrate::Field::HasMany.with_options( + primary_key: "uuid", class_name: "Foo", ) + field = association.new(:customers, [], :show) + field.associated_resource_options - association = - Administrate::Field::HasMany.with_options( - primary_key: "uuid", class_name: "Foo" - ) - field = association.new(:customers, [], :show) - field.associated_resource_options - - expect(Foo).to have_received(:all) - expect(Foo).to have_received(:uuid) - expect(Foo).not_to have_received(:id) - ensure - remove_constants :Foo, :FooDashboard - end + expect(Foo).to have_received(:all) + expect(Foo).to have_received(:uuid) + expect(Foo).not_to have_received(:id) + end + + it "triggers a deprecation warning" do + association = + Administrate::Field::HasMany.with_options( + primary_key: "uuid", class_name: "Foo", + ) + field = association.new(:customers, [], :show) + field.associated_resource_options + + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:primary_key is deprecated/) end end describe "#more_than_limit?" do it "returns true if record count > limit" do limit = Administrate::Field::HasMany::DEFAULT_LIMIT - resources = MockRelation.new([:a] * (limit + 1)) + value = MockRelation.new([:a] * (limit + 1)) association = Administrate::Field::HasMany - field = association.new(:customers, resources, :show) + field = association.new(:customers, value, :show) expect(field.more_than_limit?).to eq(true) end it "returns false if record count <= limit" do limit = Administrate::Field::HasMany::DEFAULT_LIMIT - resources = MockRelation.new([:a] * limit) + value = MockRelation.new([:a] * limit) association = Administrate::Field::HasMany - field = association.new(:customers, resources, :show) + field = association.new(:customers, value, :show) expect(field.more_than_limit?).to eq(false) end context "when there are no records" do it "returns false" do - resources = nil - - association = Administrate::Field::HasMany - field = association.new(:customers, resources, :show) + customer = build(:customer) + value = nil + + field = Administrate::Field::HasMany.new( + :orders, + value, + :show, + resource: customer, + ) expect(field.more_than_limit?).to eq(false) end @@ -118,21 +154,30 @@ describe "#resources" do it "limits the number of records shown" do limit = Administrate::Field::HasMany::DEFAULT_LIMIT - customer = FactoryBot.create(:customer, :with_orders, order_count: 10) - resources = customer.orders + customer = create(:customer, :with_orders, order_count: 10) + value = customer.orders - association = Administrate::Field::HasMany - field = association.new(:orders, resources, :show) + field = Administrate::Field::HasMany.new( + :orders, + value, + :show, + resource: customer, + ) expect(field.resources.size).to eq(limit) end context "when there are no records" do it "returns an empty collection" do - resources = nil - - association = Administrate::Field::HasMany - field = association.new(:customers, resources, :show) + customer = build(:customer) + value = nil + + field = Administrate::Field::HasMany.new( + :orders, + value, + :show, + resource: customer, + ) expect(field.resources).to eq([]) end @@ -140,11 +185,11 @@ context "with `limit` option" do it "limits the number of items returned" do - customer = FactoryBot.create(:customer, :with_orders) - resources = customer.orders + customer = create(:customer, :with_orders) + value = customer.orders association = Administrate::Field::HasMany.with_options(limit: 1) - field = association.new(:orders, resources, :show) + field = association.new(:orders, value, :show, resource: customer) expect(field.resources).to be_one end @@ -152,10 +197,15 @@ context "with `sort_by` option" do it "returns the resources in correct order" do - customer = FactoryBot.create(:customer, :with_orders) + customer = create(:customer, :with_orders) options = { sort_by: :address_line_two } association = Administrate::Field::HasMany.with_options(options) - field = association.new(:orders, customer.orders, :show) + field = association.new( + :orders, + customer.orders, + :show, + resource: customer, + ) correct_order = customer.orders.sort_by(&:address_line_two).map(&:id) reversed_order = customer.orders.sort do |a, b| @@ -169,10 +219,15 @@ context "with `direction` option" do it "returns the resources in correct order" do - customer = FactoryBot.create(:customer, :with_orders) + customer = create(:customer, :with_orders) options = { sort_by: :address_line_two, direction: :desc } association = Administrate::Field::HasMany.with_options(options) - field = association.new(:orders, customer.orders, :show) + field = association.new( + :orders, + customer.orders, + :show, + resource: customer, + ) reversed_order = customer.orders.sort_by(&:address_line_two).map(&:id) correct_order = customer.orders.sort do |a, b| @@ -188,20 +243,25 @@ describe "#selected_options" do it "returns a collection of primary keys" do model = double("model", id: 123) - resources = MockRelation.new([model]) + value = MockRelation.new([model]) association = Administrate::Field::HasMany - field = association.new(:customers, resources, :show) + field = association.new(:customers, value, :show) expect(field.selected_options).to eq([123]) end context "when there are no records" do it "returns an empty collection" do - resources = nil - - association = Administrate::Field::HasMany - field = association.new(:customers, resources, :show) + customer = build(:customer) + value = nil + + field = Administrate::Field::HasMany.new( + :orders, + value, + :show, + resource: customer, + ) expect(field.selected_options).to be_nil end diff --git a/spec/lib/fields/has_one_spec.rb b/spec/lib/fields/has_one_spec.rb index fb6fda08c9..dbc2c53786 100644 --- a/spec/lib/fields/has_one_spec.rb +++ b/spec/lib/fields/has_one_spec.rb @@ -8,12 +8,15 @@ describe Administrate::Field::HasOne do describe "#nested_form" do it "returns a form" do - product_meta_tag = double + resource = build(:product) + value = double field = Administrate::Field::HasOne.new( :product_meta_tag, - product_meta_tag, + value, :show, + resource: resource, ) + form = field.nested_form expect(form).to be_present @@ -22,11 +25,13 @@ describe "#nested_show" do it "returns a Show" do + product = create(:product) product_meta_tag = double field = Administrate::Field::HasOne.new( :product_meta_tag, product_meta_tag, :show, + resource: product, ) show = field.nested_show @@ -37,27 +42,53 @@ describe ".permitted_attribute" do context "with custom class_name" do + before do + allow(ActiveSupport::Deprecation).to receive(:warn) + end + it "returns attributes from correct dashboard" do - field = Administrate::Field::Deferred.new(Administrate::Field::HasOne. - with_options(class_name: :product_meta_tag)) + field = Administrate::Field::Deferred.new( + Administrate::Field::HasOne, + class_name: :product_meta_tag, + ) - field_name = "seo_meta_tag" - attributes = field.permitted_attribute(field_name) + field_name = "product_meta_tag" + attributes = field.permitted_attribute( + field_name, + resource_class: Product, + ) expect(attributes[:"#{field_name}_attributes"]). to eq(%i(meta_title meta_description id)) end + + it "triggers a deprecation warning" do + field = Administrate::Field::Deferred.new( + Administrate::Field::HasOne, + class_name: :product_meta_tag, + ) + field_name = "product_meta_tag" + field.permitted_attribute( + field_name, + resource_class: Product, + ) + expect(ActiveSupport::Deprecation).to have_received(:warn). + with(/:class_name is deprecated/) + end end end describe "#to_partial_path" do it "returns a partial based on the page being rendered" do + resource = double page = :show - product_meta_tag = double + value = double field = Administrate::Field::HasOne.new( :product_meta_tag, - product_meta_tag, - :show, + value, + page, + resource: resource, ) + path = field.to_partial_path expect(path).to eq("/fields/has_one/#{page}") diff --git a/spec/lib/fields/number_spec.rb b/spec/lib/fields/number_spec.rb index 469dd3c232..a74305b5f1 100644 --- a/spec/lib/fields/number_spec.rb +++ b/spec/lib/fields/number_spec.rb @@ -1,3 +1,4 @@ +require "rails_helper" require "administrate/field/number" require "support/field_matchers" @@ -16,7 +17,13 @@ end end - it { should_permit_param(:foo, for_attribute: :foo) } + it do + should_permit_param( + :foo, + on_model: Customer, + for_attribute: :foo, + ) + end describe "#to_s" do it "defaults to displaying no decimal points" do diff --git a/spec/lib/fields/password_spec.rb b/spec/lib/fields/password_spec.rb index 36ec3da12c..908a92ed0d 100644 --- a/spec/lib/fields/password_spec.rb +++ b/spec/lib/fields/password_spec.rb @@ -1,3 +1,4 @@ +require "rails_helper" require "administrate/field/password" require "support/field_matchers" @@ -15,7 +16,13 @@ end end - it { should_permit_param(:foo, for_attribute: :foo) } + it do + should_permit_param( + :foo, + on_model: Customer, + for_attribute: :foo, + ) + end describe "#truncate" do it "renders an empty string for nil" do diff --git a/spec/lib/fields/polymorphic_spec.rb b/spec/lib/fields/polymorphic_spec.rb index a1b5a280e8..102bc9e86f 100644 --- a/spec/lib/fields/polymorphic_spec.rb +++ b/spec/lib/fields/polymorphic_spec.rb @@ -1,3 +1,4 @@ +require "rails_helper" require "administrate/field/belongs_to" require "administrate/field/polymorphic" require "support/constant_helpers" @@ -17,7 +18,13 @@ end end - it { should_permit_param({ foo: %i{type value} }, for_attribute: :foo) } + it do + should_permit_param( + { foo: %i{type value} }, + on_model: Customer, + for_attribute: :foo, + ) + end describe "#display_associated_resource" do it "displays through the dashboard based on the polymorphic class name" do diff --git a/spec/lib/fields/string_spec.rb b/spec/lib/fields/string_spec.rb index 02518b12e6..2cddd43da9 100644 --- a/spec/lib/fields/string_spec.rb +++ b/spec/lib/fields/string_spec.rb @@ -1,3 +1,4 @@ +require "rails_helper" require "administrate/field/string" require "support/field_matchers" @@ -15,7 +16,13 @@ end end - it { should_permit_param(:foo, for_attribute: :foo) } + it do + should_permit_param( + :foo, + on_model: Customer, + for_attribute: :foo, + ) + end describe "#truncate" do it "renders an empty string for nil" do diff --git a/spec/support/field_matchers.rb b/spec/support/field_matchers.rb index e19fe3b897..d66d404f41 100644 --- a/spec/support/field_matchers.rb +++ b/spec/support/field_matchers.rb @@ -1,6 +1,9 @@ module FieldMatchers - def should_permit_param(expected_param, for_attribute:) - permitted_param = described_class.permitted_attribute(for_attribute) + def should_permit_param(expected_param, for_attribute:, on_model:) + permitted_param = described_class.permitted_attribute( + for_attribute, + resource_class: on_model, + ) expect(permitted_param).to eq(expected_param) end end