diff --git a/app/controllers/administrate/application_controller.rb b/app/controllers/administrate/application_controller.rb index 3c13d9c281..905385fad6 100644 --- a/app/controllers/administrate/application_controller.rb +++ b/app/controllers/administrate/application_controller.rb @@ -11,6 +11,7 @@ def index resources = paginate_resources(resources) @resources = resources page = Administrate::Page::Collection.new(dashboard, order: order) + page.context = self filters = Administrate::Search.new(scoped_resource, dashboard, search_term).valid_filters render locals: { @@ -24,29 +25,38 @@ def index def show @resource = resource = requested_resource + page = Administrate::Page::Show.new(dashboard, resource) + page.context = self render locals: { - page: Administrate::Page::Show.new(dashboard, resource) + page: page } end def new - @resource = resource = new_resource - authorize_resource(resource) + @resource = resource = new_resource.tap do |resource| + authorize_resource(resource) + end + + page = Administrate::Page::Form.new(dashboard, resource) + page.context = self render locals: { - page: Administrate::Page::Form.new(dashboard, resource) + page: page } end def edit @resource = resource = requested_resource + page = Administrate::Page::Form.new(dashboard, resource) + page.context = self render locals: { - page: Administrate::Page::Form.new(dashboard, resource) + page: page } end def create - @resource = resource = new_resource(resource_params) - authorize_resource(resource) + @resource = resource = new_resource(resource_params).tap do |resource| + authorize_resource(resource) + end if resource.save yield(resource) if block_given? @@ -55,8 +65,10 @@ def create notice: translate_with_resource("create.success") ) else + page = Administrate::Page::Form.new(dashboard, resource) + page.context = self render :new, locals: { - page: Administrate::Page::Form.new(dashboard, resource) + page: page }, status: :unprocessable_entity end end @@ -70,8 +82,10 @@ def update status: :see_other ) else + page = Administrate::Page::Form.new(dashboard, resource) + page.context = self render :edit, locals: { - page: Administrate::Page::Form.new(dashboard, resource) + page: page }, status: :unprocessable_entity end end @@ -178,7 +192,9 @@ def sorting_params end def dashboard - @dashboard ||= dashboard_class.new + @dashboard ||= dashboard_class.new.tap do |d| + d.context = self + end end def requested_resource diff --git a/docs/customizing_controller_actions.md b/docs/customizing_controller_actions.md index 73d98ca1e6..5ef960ad45 100644 --- a/docs/customizing_controller_actions.md +++ b/docs/customizing_controller_actions.md @@ -31,8 +31,8 @@ class Admin::FoosController < Admin::ApplicationController # Foo.find_by!(slug: param) # end - # Override this if you have certain roles that require a subset - # this will be used to set the records shown on the `index` action. + # Override this if you have certain roles that require a subset. + # This will be used in all actions except for the `new` and `create` actions # # def scoped_resource # if current_user.super_admin? diff --git a/lib/administrate/base_dashboard.rb b/lib/administrate/base_dashboard.rb index 1df429b8e9..82b89cc6f3 100644 --- a/lib/administrate/base_dashboard.rb +++ b/lib/administrate/base_dashboard.rb @@ -70,7 +70,7 @@ def specific_form_attributes_for(action) end def permitted_attributes(action = nil) - attributes = form_attributes action + attributes = form_attributes(action) if attributes.is_a? Hash attributes = attributes.values.flatten @@ -120,6 +120,8 @@ def item_associations attribute_associated attributes end + attr_accessor :context + private def attribute_not_found_message(attr) diff --git a/lib/administrate/field/associative.rb b/lib/administrate/field/associative.rb index 5e87c54fab..b2c55573c8 100644 --- a/lib/administrate/field/associative.rb +++ b/lib/administrate/field/associative.rb @@ -53,7 +53,9 @@ def html_controller private def associated_dashboard - "#{associated_class_name}Dashboard".constantize.new + "#{associated_class_name}Dashboard".constantize.new.tap do |d| + d.context = context + end end def association_primary_key diff --git a/lib/administrate/field/base.rb b/lib/administrate/field/base.rb index 7b7f0375d6..4468a01e01 100644 --- a/lib/administrate/field/base.rb +++ b/lib/administrate/field/base.rb @@ -65,18 +65,22 @@ def self.local_partial_prefixes(look: :default) end end - def initialize(attribute, data, page, options = {}) + def initialize(attribute, raw_data, page, options = {}) @attribute = attribute @page = page @resource = options.delete(:resource) @options = options - @data = read_value(data) + @raw_data = raw_data end def html_class self.class.html_class end + def data + read_value(@raw_data) + end + def html_controller nil end @@ -133,7 +137,8 @@ def required? end end - attr_reader :attribute, :data, :options, :page, :resource + attr_reader :attribute, :options, :page, :resource + attr_accessor :context end end end diff --git a/lib/administrate/field/has_many.rb b/lib/administrate/field/has_many.rb index 6678692ef9..739e3d52fc 100644 --- a/lib/administrate/field/has_many.rb +++ b/lib/administrate/field/has_many.rb @@ -26,7 +26,9 @@ def associated_collection(order = self.order) associated_dashboard, order: order, collection_attributes: options[:collection_attributes] - ) + ).tap do |page| + page.context = context + end end def attribute_key @@ -76,7 +78,7 @@ def more_than_limit? end def data - @data ||= associated_class.none + super || associated_class.none end def order_from_params(params) @@ -106,12 +108,11 @@ def includes end def candidate_resources - if options.key?(:includes) - includes = options.fetch(:includes) - associated_class.includes(*includes).all - else - associated_class.all - end + scope = options[:scope] ? options[:scope].call(self) : associated_class.all + scope = scope.includes(options.fetch(:includes, [])) + + order = options[:order] + order ? scope.reorder(order) : scope end def display_candidate_resource(resource) diff --git a/lib/administrate/field/has_one.rb b/lib/administrate/field/has_one.rb index 1e715abc01..491bc03f4c 100644 --- a/lib/administrate/field/has_one.rb +++ b/lib/administrate/field/has_one.rb @@ -26,14 +26,18 @@ def nested_form @nested_form ||= Administrate::Page::Form.new( resolver.dashboard_class.new, data || resolver.resource_class.new - ) + ).tap do |page| + page.context = context + end end def nested_show @nested_show ||= Administrate::Page::Show.new( resolver.dashboard_class.new, data || resolver.resource_class.new - ) + ) do |page| + page.context = context + end end def linkable? diff --git a/lib/administrate/field/polymorphic.rb b/lib/administrate/field/polymorphic.rb index d0e9dbb13e..d17c70c4be 100644 --- a/lib/administrate/field/polymorphic.rb +++ b/lib/administrate/field/polymorphic.rb @@ -26,12 +26,14 @@ def selected_global_id private def associated_dashboard(klass = data.class) - "#{klass.name}Dashboard".constantize.new + "#{klass.name}Dashboard".constantize.new.tap do |d| + d.context = context + end end def classes klasses = options.fetch(:classes, []) - klasses.respond_to?(:call) ? klasses.call : klasses + klasses.respond_to?(:call) ? klasses.call(self) : klasses end private diff --git a/lib/administrate/page/base.rb b/lib/administrate/page/base.rb index 56c61892d3..82576b05ad 100644 --- a/lib/administrate/page/base.rb +++ b/lib/administrate/page/base.rb @@ -27,11 +27,15 @@ def item_associations dashboard.try(:item_associations) || [] end + attr_accessor :context + private def attribute_field(dashboard, resource, attribute_name, page) field = dashboard.attribute_type_for(attribute_name) - field.new(attribute_name, nil, page, resource: resource) + field.new(attribute_name, nil, page, resource: resource).tap do |f| + f.context = context + end end attr_reader :dashboard, :options diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 84e88d44c6..a679199c4a 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -3,10 +3,10 @@ describe Admin::CustomersController, type: :controller do describe "GET index" do it "passes all customers to the view" do - customer = create(:customer) + customers = create_list(:customer, 5) locals = capture_view_locals { get :index } - expect(locals[:resources]).to eq([customer]) + expect(locals[:resources]).to eq(customers) end it "applies any scope overrides" do @@ -47,6 +47,22 @@ expect(locals[:resources].map(&:id)).to eq customers.map(&:id).sort end + context "when the user is an admin" do + controller(Admin::CustomersController) do + def authenticate_admin + @current_user = Customer.last + end + end + + it "passes one customers to the view" do + _other_customers = create_list(:customer, 5) + customer = create(:customer) + + locals = capture_view_locals { get :index } + expect(locals[:resources]).to eq([customer]) + end + end + context "with alternate sorting attributes" do controller(Admin::CustomersController) do def default_sorting_attribute diff --git a/spec/controllers/admin/orders_controller_spec.rb b/spec/controllers/admin/orders_controller_spec.rb index da11846659..d9e07ebdb0 100644 --- a/spec/controllers/admin/orders_controller_spec.rb +++ b/spec/controllers/admin/orders_controller_spec.rb @@ -34,8 +34,8 @@ def pundit_user end describe "GET new" do - it "raises a Pundit error" do - expect { get :new }.to raise_error(Pundit::NotAuthorizedError) + it "allows access to /new" do + expect { get :new }.not_to raise_error end end @@ -106,5 +106,34 @@ def send_request(order:) end end end + + context "when the user is not an admin" do + controller(Admin::OrdersController) do + def pundit_user + Customer.find_by(name: "Current User") + end + end + + let!(:user) { create(:customer, name: "Current User") } + + describe "GET new" do + it "allows access to /new" do + expect { get :new }.not_to raise_error + end + end + + describe "POST create" do + it "allows creating records with the current customer" do + post( + :create, + params: { + order: attributes_for(:order) + } + ) + expect(response).to redirect_to([:admin, (order = Order.last)]) + expect(order.customer).to eq(user) + end + end + end end # standard:enable Lint/ConstantDefinitionInBlock diff --git a/spec/dashboards/order_dashboard_spec.rb b/spec/dashboards/order_dashboard_spec.rb index 95872566c8..9d6cb2a881 100644 --- a/spec/dashboards/order_dashboard_spec.rb +++ b/spec/dashboards/order_dashboard_spec.rb @@ -8,10 +8,74 @@ expect(dashboard.permitted_attributes).to include(:address_line_one) end - it "returns the attribute_id name for belongs_to relationships" do - dashboard = OrderDashboard.new + context "when the page context is 'new' or 'create'" do + let(:ctx_with_non_admin_user) do + Struct.new(:pundit_user).new(Struct.new(:admin?).new(false)) + end + let(:ctx_with_admin_user) do + Struct.new(:pundit_user).new(Struct.new(:admin?).new(true)) + end + + context "when the user is not an admin" do + it "not returns attributes with customer_id" do + dashboard = OrderDashboard.new + dashboard.context = ctx_with_non_admin_user + expect( + dashboard.permitted_attributes("new") + ).not_to include("customer_id") + expect( + dashboard.permitted_attributes("create") + ).not_to include("customer_id") + end + end + + context "when the user is an admin" do + it "returns attributes with customer_id" do + dashboard = OrderDashboard.new + dashboard.context = ctx_with_admin_user + expect( + dashboard.permitted_attributes("new") + ).to include("customer_id") + expect( + dashboard.permitted_attributes("create") + ).to include("customer_id") + end + end + end + + context "when the page context is 'edit' or 'update'" do + let(:ctx_with_non_admin_user) do + Struct.new(:pundit_user).new(Struct.new(:admin?).new(false)) + end + let(:ctx_with_admin_user) do + Struct.new(:pundit_user).new(Struct.new(:admin?).new(true)) + end + + context "when the user is not an admin" do + it "not returns attributes with customer_id" do + dashboard = OrderDashboard.new + dashboard.context = ctx_with_non_admin_user + expect( + dashboard.permitted_attributes("edit") + ).not_to include("customer_id") + expect( + dashboard.permitted_attributes("update") + ).not_to include("customer_id") + end + end - expect(dashboard.permitted_attributes).to include("customer_id") + context "when the user is an admin" do + it "also no returns attributes with customer_id" do + dashboard = OrderDashboard.new + dashboard.context = ctx_with_admin_user + expect( + dashboard.permitted_attributes("edit") + ).not_to include("customer_id") + expect( + dashboard.permitted_attributes("update") + ).not_to include("customer_id") + end + end end end end diff --git a/spec/example_app/app/controllers/admin/customers_controller.rb b/spec/example_app/app/controllers/admin/customers_controller.rb index 163d4358bd..fcd0386bf7 100644 --- a/spec/example_app/app/controllers/admin/customers_controller.rb +++ b/spec/example_app/app/controllers/admin/customers_controller.rb @@ -15,7 +15,7 @@ def become private def scoped_resource - Customer.where(hidden: false) + super.where(hidden: false) end def with_variant diff --git a/spec/example_app/app/controllers/admin/orders_controller.rb b/spec/example_app/app/controllers/admin/orders_controller.rb index d43cc85c2b..dbdd46973e 100644 --- a/spec/example_app/app/controllers/admin/orders_controller.rb +++ b/spec/example_app/app/controllers/admin/orders_controller.rb @@ -1,4 +1,12 @@ module Admin class OrdersController < Admin::ApplicationController + private + + def authorize_resource(resource) + super + if %w[new create].include?(action_name) && !pundit_user.admin? + resource.customer = pundit_user + end + end end end diff --git a/spec/example_app/app/dashboards/order_dashboard.rb b/spec/example_app/app/dashboards/order_dashboard.rb index bc028096f9..6f71f34bd1 100644 --- a/spec/example_app/app/dashboards/order_dashboard.rb +++ b/spec/example_app/app/dashboards/order_dashboard.rb @@ -70,4 +70,14 @@ class OrderDashboard < Administrate::BaseDashboard "details" => %i[line_items total_price shipped_at payments] ) .freeze + + def form_attributes(action = nil) + if %w[new create].include?(action.to_s) && context.try(:pundit_user).try(:admin?) + super + else + super.dup + .transform_values { |v| v.without(:customer) } + .delete_if { |_k, v| v.blank? } + end + end end diff --git a/spec/example_app/app/policies/customer_policy.rb b/spec/example_app/app/policies/customer_policy.rb index 784d10d58b..6faf8d3c79 100644 --- a/spec/example_app/app/policies/customer_policy.rb +++ b/spec/example_app/app/policies/customer_policy.rb @@ -1,2 +1,11 @@ class CustomerPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user.admin? + scope + else + scope.where(id: user.id) + end + end + end end diff --git a/spec/example_app/app/policies/order_policy.rb b/spec/example_app/app/policies/order_policy.rb index 2d0dbeb2cf..17f97810f2 100644 --- a/spec/example_app/app/policies/order_policy.rb +++ b/spec/example_app/app/policies/order_policy.rb @@ -1,12 +1,16 @@ class OrderPolicy < ApplicationPolicy class Scope < Scope def resolve - scope.all + if user.admin? + scope.all + else + scope.where(customer: user) + end end end def create? - user.admin? + true end def update? diff --git a/spec/features/orders_form_spec.rb b/spec/features/orders_form_spec.rb index bcb58df30d..2f60156008 100644 --- a/spec/features/orders_form_spec.rb +++ b/spec/features/orders_form_spec.rb @@ -21,22 +21,23 @@ describe "belongs_to relationships" do it "has stored value selected" do - create(:customer) - order = create(:order) + customer = create(:customer) - visit edit_admin_order_path(order) - expected = order.customer.id.to_s - expect(find_field("Customer").value).to eq expected + visit new_admin_order_path + select(customer.name, from: "Customer") + click_on "Create Order" + + expect(find_field("Customer").value).to eq customer.id.to_s end it "displays translated label when translation for the attribute is available" do order = create(:order) - custom_attribute_name = "Client" + custom_attribute_name = "Lines" translations = { activerecord: { attributes: { order: { - customer: custom_attribute_name + line_items: custom_attribute_name } } } diff --git a/spec/lib/fields/base_spec.rb b/spec/lib/fields/base_spec.rb index ee30f78caa..06de81ff0a 100644 --- a/spec/lib/fields/base_spec.rb +++ b/spec/lib/fields/base_spec.rb @@ -300,5 +300,15 @@ expect(field.data).to eq(nil) end end + + context "when context is set and a getter block is provided" do + it "lazily reads the value from the context" do + resource = double("Model") + field = field_class.new(:attribute, :date, :page, resource: resource, getter: ->(f) { f.context.custom_value + " from block" }) + field.context = double("Context", custom_value: "custom value") + + expect(field.data).to eq("custom value from block") + end + end end end diff --git a/spec/lib/fields/belongs_to_spec.rb b/spec/lib/fields/belongs_to_spec.rb index e70e087c3c..74161530a1 100644 --- a/spec/lib/fields/belongs_to_spec.rb +++ b/spec/lib/fields/belongs_to_spec.rb @@ -260,7 +260,7 @@ create_list(:customer, 3) options = { order: "name", - scope: -> { Customer.order(name: :desc) } + scope: ->(_field) { Customer.order(name: :desc) } } association = Administrate::Field::BelongsTo.with_options(options) @@ -280,7 +280,7 @@ order = build(:order) 1.upto(3) { |i| create :customer, name: "customer-#{i}" } - scope = -> { Customer.order(name: :desc).limit(2) } + scope = ->(_field) { Customer.order(name: :desc).limit(2) } association = Administrate::Field::BelongsTo.with_options(scope: scope) field = association.new(:customer, [], :show, resource: order) diff --git a/spec/lib/fields/has_many_spec.rb b/spec/lib/fields/has_many_spec.rb index ede93d044e..92e14b58f6 100644 --- a/spec/lib/fields/has_many_spec.rb +++ b/spec/lib/fields/has_many_spec.rb @@ -54,6 +54,7 @@ before do stub_const("FooDashboard", Class.new) allow(FooDashboard).to receive(:new).and_return(dashboard_double) + allow(dashboard_double).to receive(:context=).with(nil).and_return(nil) end it "determines what dashboard is used to present the association" do @@ -250,4 +251,56 @@ end end end + + describe "#associated_resource_options" do + context "with `order` option" do + it "returns the resources in correct order" do + order = create(:order) + create_list(:customer, 5) + options = {order: "name"} + association = Administrate::Field::HasMany.with_options(options) + + field = association.new(:customer, [], :show, resource: order) + + correct_order = Customer.order("name").pluck(:id) + + resources = field.associated_resource_options.compact.to_h.values + expect(resources).to eq correct_order + end + + it "ignores the order passed in `scope`" do + order = create(:order) + create_list(:customer, 3) + options = { + order: "name", + scope: ->(_field) { Customer.order(name: :desc) } + } + association = Administrate::Field::HasMany.with_options(options) + + field = association.new(:customer, [], :show, resource: order) + + correct_order = Customer.order("name").pluck(:id) + + resources = field.associated_resource_options.compact.to_h.values + expect(resources).to eq correct_order + end + end + + context "with `scope` option" do + it "returns the resources within the passed scope" do + # 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 = ->(_field) { Customer.order(name: :desc).limit(2) } + + association = Administrate::Field::HasMany.with_options(scope: scope) + field = association.new(:customer, [], :show, resource: order) + resources = field.associated_resource_options.compact.to_h.keys + + expect(resources).to eq ["customer-3", "customer-2"] + end + end + end end diff --git a/spec/lib/fields/polymorphic_spec.rb b/spec/lib/fields/polymorphic_spec.rb index 461a772fb3..9b02ca76a3 100644 --- a/spec/lib/fields/polymorphic_spec.rb +++ b/spec/lib/fields/polymorphic_spec.rb @@ -50,6 +50,7 @@ def display_resource(*) :success end + attr_accessor :context end field = Administrate::Field::Polymorphic.new(:foo, Thing.new, :show) @@ -96,10 +97,10 @@ def display_resource(*) context "present in options as a call-able object" do it "returns the called value" do - classes = -> { ["one", "two", "three"] } + classes = ->(field) { ["one", "two", "three"] } allow(field).to receive(:options).and_return(classes: classes) - expect(field.send(:classes)).to eq(classes.call) + expect(field.send(:classes)).to eq(classes.call(field)) end end end