diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 4e48ac68ea..3d4f47bd07 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -67,7 +67,7 @@ module Admin def collection if json_request? && params[:enterprise_id].present? - CustomersWithBalance.new(managed_enterprise_id).query. + CustomersWithBalance.new(Customer.of(managed_enterprise_id)).query. includes( :enterprise, { bill_address: [:state, :country] }, diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index e01fe3aab3..cc62b6978f 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -106,7 +106,9 @@ module Api end def json_api_error(message, **options) - { errors: [{ detail: message }] }.merge(options) + error_options = options.delete(:error_options) || {} + + { errors: [{ detail: message }.merge(error_options)] }.merge(options) end def json_api_invalid(message, errors) diff --git a/app/controllers/api/v1/customers_controller.rb b/app/controllers/api/v1/customers_controller.rb index 24bd1db8ee..86bf05fc65 100644 --- a/app/controllers/api/v1/customers_controller.rb +++ b/app/controllers/api/v1/customers_controller.rb @@ -6,11 +6,17 @@ module Api module V1 class CustomersController < Api::V1::BaseController include AddressTransformation + include ExtraFields skip_authorization_check only: :index before_action :authorize_action, only: [:show, :update, :destroy] + # Query parameters + before_action only: [:index] do + @extra_customer_fields = extra_fields :customer, [:balance] + end + def index @pagy, customers = pagy(search_customers, pagy_options) @@ -51,7 +57,11 @@ module Api private def customer - @customer ||= Customer.find(params[:id]) + @customer ||= if action_name == "show" + CustomersWithBalance.new(Customer.where(id: params[:id])).query.first! + else + Customer.find(params[:id]) + end end def authorize_action @@ -61,6 +71,11 @@ module Api def search_customers customers = visible_customers.includes(:bill_address, :ship_address) customers = customers.where(enterprise_id: params[:enterprise_id]) if params[:enterprise_id] + + if @extra_customer_fields.include?(:balance) + customers = CustomersWithBalance.new(customers).query + end + customers.ransack(params[:q]).result.order(:id) end diff --git a/app/controllers/concerns/extra_fields.rb b/app/controllers/concerns/extra_fields.rb new file mode 100644 index 0000000000..058e986a13 --- /dev/null +++ b/app/controllers/concerns/extra_fields.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# To be included in api controllers for handeling query params +module ExtraFields + extend ActiveSupport::Concern + + def invalid_query_param(name, status, msg) + render status: status, json: json_api_error(msg, error_options: + { + title: I18n.t("api.query_param.error.title"), + source: { parameter: name }, + status: status, + code: Rack::Utils::SYMBOL_TO_STATUS_CODE[status] + }) + end + + def extra_fields(type, available_fields) + fields = params.dig(:extra_fields, type)&.split(',')&.compact&.map(&:to_sym) + return [] if fields.blank? + + unknown_fields = fields - available_fields + + if unknown_fields.present? + invalid_query_param( + "extra_fields[#{type}]", :unprocessable_entity, + I18n.t("api.query_param.error.extra_fields", fields: unknown_fields.join(', ')) + ) + end + + fields + end +end diff --git a/app/json_schemas/customer_schema.rb b/app/json_schemas/customer_schema.rb index ecced41793..1156ef8327 100644 --- a/app/json_schemas/customer_schema.rb +++ b/app/json_schemas/customer_schema.rb @@ -61,4 +61,9 @@ class CustomerSchema < JsonApiSchema def self.relationships [:enterprise] end + + # Optional attributes included with eg: CustomerSchema.schema(extra_fields: :balance) + def self.balance + { balance: { type: :number, format: :double } } + end end diff --git a/app/json_schemas/json_api_schema.rb b/app/json_schemas/json_api_schema.rb index 069de6633d..53202656d0 100644 --- a/app/json_schemas/json_api_schema.rb +++ b/app/json_schemas/json_api_schema.rb @@ -19,84 +19,77 @@ class JsonApiSchema end def schema(options = {}) - { - type: :object, - properties: { - data: { - type: :object, - properties: data_properties(**options) - }, - meta: { type: :object }, - links: { type: :object } - }, - required: [:data] - } + Structure.schema(data_properties(**options)) end def collection(options) - { - type: :object, - properties: { - data: { - type: :array, - items: { - type: :object, - properties: data_properties(**options) - } - }, - meta: { - type: :object, - properties: { - pagination: { - type: :object, - properties: { - results: { type: :integer, example: 250 }, - pages: { type: :integer, example: 5 }, - page: { type: :integer, example: 2 }, - per_page: { type: :integer, example: 50 }, - } - } - }, - required: [:pagination] - }, - links: { - type: :object, - properties: { - self: { type: :string }, - first: { type: :string }, - prev: { type: :string, nullable: true }, - next: { type: :string, nullable: true }, - last: { type: :string } - } - } - }, - required: [:data, :meta, :links] - } + Structure.collection(data_properties(**options)) end private - def data_properties(require_all: false) + def data_properties(require_all: false, extra_fields: nil) + extra_fields_result = get_extra_fields(extra_fields) + attributes = get_attributes(extra_fields_result) + required = get_required(require_all, extra_fields, extra_fields_result) + + Structure.data_properties(object_name, attributes, required, relationship_properties) + end + + def relationship_properties + relationships.to_h { |name| [name, relationship_schema(name)] } + end + + # Example + # MySchema.schema(extra_fields: :my_method) + # => extra_fields_result = MySchema.my_method + # => attributes = attributes.merge(extra_fields_result) + # + # MySchema.schema(extra_fields: {name: :my_method, required: true, opts: {method_opt: true}}) + # => extra_fields_result = MySchema.my_method(method_opt: true) + # => attributes = attributes.merge(extra_fields_result) + # => required += extra_fields_result.keys + # + # MySchema.schema(extra_fields: [:my_method, :another_method]) + # => extra_fields_result = MySchema.my_method.merge(another_method) + # => attributes = attribtues.merge(extra_fields_result) + # + # To test use eg: + # MySchema.schema(extra_fields: :my_method) + # .dig(:properties, :data, :properties, :attributes) + def get_extra_fields(extra_fields) + case extra_fields + when Symbol + public_send(extra_fields) + when Hash + public_send(extra_fields[:name], **extra_fields[:opts].to_h) + when Array + obj = {} + + extra_fields.each do |w| + obj.merge!(get_extra_fields(w)) + end + + obj + end + end + + def get_required(require_all, extra_fields, extra_fields_result) required = require_all ? all_attributes : required_attributes - { - id: { type: :string, example: "1" }, - type: { type: :string, example: object_name }, - attributes: { - type: :object, - properties: attributes, - required: required - }, - relationships: { - type: :object, - properties: relationships.to_h do |name| - [ - name, - relationship_schema(name) - ] - end - } - } + if extra_fields.is_a?(Hash) && extra_fields[:required] == true && extra_fields_result.present? + required += extra_fields_result.keys + end + + required + end + + def get_attributes(extra_fields_result) + if [extra_fields_result, attributes].all?{ |obj| obj.respond_to?(:merge) } + attributes.merge(extra_fields_result) + else + attributes + end end def relationship_schema(name) diff --git a/app/json_schemas/json_api_schema/structure.rb b/app/json_schemas/json_api_schema/structure.rb new file mode 100644 index 0000000000..ea72753776 --- /dev/null +++ b/app/json_schemas/json_api_schema/structure.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +# rubocop:disable Metrics/MethodLength + +class JsonApiSchema + module Structure + extend self + + def schema(data_properties) + { + type: :object, + properties: { + data: { + type: :object, + properties: data_properties + }, + meta: { type: :object }, + links: { type: :object } + }, + required: [:data] + } + end + + def collection(data_properties) + { + type: :object, + properties: { + data: { + type: :array, + items: { + type: :object, + properties: data_properties + } + }, + meta: { + type: :object, + properties: { + pagination: { + type: :object, + properties: { + results: { type: :integer, example: 250 }, + pages: { type: :integer, example: 5 }, + page: { type: :integer, example: 2 }, + per_page: { type: :integer, example: 50 }, + } + } + }, + required: [:pagination] + }, + links: { + type: :object, + properties: { + self: { type: :string }, + first: { type: :string }, + prev: { type: :string, nullable: true }, + next: { type: :string, nullable: true }, + last: { type: :string } + } + } + }, + required: [:data, :meta, :links] + } + end + + def data_properties(object_name, attributes, required, relationship_properties) + { + id: { type: :string, example: "1" }, + type: { type: :string, example: object_name }, + attributes: { + type: :object, + properties: attributes, + required: required + }, + relationships: { + type: :object, + properties: relationship_properties + } + } + end + end +end + +# rubocop:enable Metrics/MethodLength diff --git a/app/queries/customers_with_balance.rb b/app/queries/customers_with_balance.rb index 4abd070d34..553ca88cec 100644 --- a/app/queries/customers_with_balance.rb +++ b/app/queries/customers_with_balance.rb @@ -1,28 +1,22 @@ # frozen_string_literal: true -# Fetches the customers of the specified enterprise including the aggregated balance across the -# customer's orders. That is, we get the total balance for each customer with this enterprise. +# Adds an aggregated 'balance_value' to each customer based on their order history +# class CustomersWithBalance - def initialize(enterprise) - @enterprise = enterprise + def initialize(customers) + @customers = customers end def query - Customer.of(enterprise). + @customers. joins(left_join_complete_orders). group("customers.id"). select("customers.*"). - select(outstanding_balance_sum) + select("#{outstanding_balance_sum} AS balance_value") end private - attr_reader :enterprise - - def outstanding_balance_sum - "SUM(#{OutstandingBalance.new.statement}) AS balance_value" - end - # The resulting orders are in states that belong after the checkout. Only these can be considered # for a customer's balance. def left_join_complete_orders @@ -36,4 +30,8 @@ class CustomersWithBalance states = Spree::Order::FINALIZED_STATES.map { |state| Arel::Nodes.build_quoted(state) } Arel::Nodes::In.new(Spree::Order.arel_table[:state], states) end + + def outstanding_balance_sum + "SUM(#{OutstandingBalance.new.statement})::float" + end end diff --git a/app/serializers/api/v1/customer_serializer.rb b/app/serializers/api/v1/customer_serializer.rb index b5e59648b5..d558728d75 100644 --- a/app/serializers/api/v1/customer_serializer.rb +++ b/app/serializers/api/v1/customer_serializer.rb @@ -16,6 +16,10 @@ module Api address(object.shipping_address) end + attribute :balance, if: proc { |record| + record.respond_to?(:balance_value) + }, &:balance_value + belongs_to :enterprise, links: { related: ->(object) { url_helpers.api_v1_enterprise_url(id: object.enterprise_id) diff --git a/config/locales/en.yml b/config/locales/en.yml index 18d8764c05..e2f487ad7e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1624,6 +1624,11 @@ en: destroy_attachment_does_not_exist: "Terms and Conditions file does not exist" orders: failed_to_update: "Failed to update order" + query_param: + error: + title: Invalid query parameter + extra_fields: "Unsupported fields: %{fields}" + # Frontend views # diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 9630772d86..a60402f0bf 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -45,7 +45,7 @@ module Admin it 'calls CustomersWithBalance' do customers_with_balance = instance_double(CustomersWithBalance) allow(CustomersWithBalance) - .to receive(:new).with(enterprise) { customers_with_balance } + .to receive(:new).with(Customer.of(enterprise)) { customers_with_balance } expect(customers_with_balance).to receive(:query) { Customer.none } diff --git a/spec/controllers/concerns/extra_fields_spec.rb b/spec/controllers/concerns/extra_fields_spec.rb new file mode 100644 index 0000000000..463427926b --- /dev/null +++ b/spec/controllers/concerns/extra_fields_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ExtraFields do + let(:dummy_controller) { Api::V1::BaseController.new.extend ExtraFields } + + describe "#invalid_query_param" do + it "renders error" do + allow(dummy_controller).to receive(:render) {} + dummy_controller.invalid_query_param("param", :unprocessable_entity, "error message") + expect(dummy_controller).to have_received(:render).with( + json: + { + errors: + [{ + code: 422, + detail: "error message", + source: { parameter: "param" }, + status: :unprocessable_entity, + title: "Invalid query parameter" + }] + }, + status: :unprocessable_entity + ) + end + end + + describe "#extra_fields" do + context "when fields present and available" do + it "returns extra fields" do + allow(dummy_controller).to receive(:params). + and_return({ extra_fields: { customer: "balance" } }) + expect(dummy_controller.extra_fields(:customer, [:balance])).to eq([:balance]) + end + end + + context "when fields missing" do + it "returns empty arr" do + allow(dummy_controller).to receive(:params).and_return({}) + expect(dummy_controller.extra_fields(:customer, [:balance])).to eq([]) + end + end + + context "when fields not in available fields" do + it "calls invalid_query_param" do + allow(dummy_controller).to receive(:invalid_query_param) {} + allow(dummy_controller).to receive(:params). + and_return({ extra_fields: { customer: "unknown" } }) + dummy_controller.extra_fields(:customer, [:balance]) + + expect(dummy_controller).to have_received(:invalid_query_param).with( + "extra_fields[customer]", + :unprocessable_entity, + "Unsupported fields: unknown" + ) + end + end + end +end diff --git a/spec/queries/customers_with_balance_spec.rb b/spec/queries/customers_with_balance_spec.rb index 479be6b410..9bf075367a 100644 --- a/spec/queries/customers_with_balance_spec.rb +++ b/spec/queries/customers_with_balance_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe CustomersWithBalance do - subject(:customers_with_balance) { described_class.new(customer.enterprise.id) } + subject(:customers_with_balance) { described_class.new(Customer.where(id: customer)) } describe '#query' do let(:customer) { create(:customer) } @@ -18,6 +18,23 @@ describe CustomersWithBalance do customers_with_balance.query end + describe 'arguments' do + context 'with customers collection' do + it 'returns balance' do + customers = create_pair(:customer) + query = described_class.new(Customer.where(id: customers)).query + expect(query.pluck(:id)).to eq([customers.first.id, customers.second.id]) + expect(query.map(&:balance_value)).to eq([0, 0]) + end + end + + context 'with empty customers collection' do + it 'returns empty customers collection' do + expect(described_class.new(Customer.none).query).to eq([]) + end + end + end + context 'when orders are in cart state' do before do create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart') diff --git a/spec/requests/api/v1/customers_spec.rb b/spec/requests/api/v1/customers_spec.rb index 047c5e0438..3a4b7b38bb 100644 --- a/spec/requests/api/v1/customers_spec.rb +++ b/spec/requests/api/v1/customers_spec.rb @@ -26,10 +26,13 @@ describe "Customers", type: :request do get "List customers" do tags "Customers" parameter name: :enterprise_id, in: :query, type: :string + parameter name: "extra_fields[customer]", in: :query, type: :string, example: :balance, + description: "Add extra fields to each customer" produces "application/json" response "200", "Customers list" do param(:enterprise_id) { enterprise1.id } + param("extra_fields[customer]") { :balance } schema "$ref": "#/components/schemas/customers_collection" run_test! @@ -105,6 +108,33 @@ describe "Customers", type: :request do end end + describe "query parameters" do + describe "extra_fields[customer]" do + context "with balance" do + it "adds balance to each customer" do + get "/api/v1/customers", params: { extra_fields: { customer: :balance } } + balances = json_response[:data].map{ |c| c[:attributes][:balance] } + expect(balances.all?{ |b| b.is_a? Numeric }).to eq(true) + end + end + + context "with unknown field" do + it "returns unprocessable entity" do + get "/api/v1/customers", params: { extra_fields: { customer: :unknown } } + expect([response.status, json_error_detail]).to eq [422, "Unsupported fields: unknown"] + end + end + + context "when not recevied" do + it "does not add balances" do + get "/api/v1/customers" + balances = json_response[:data].map{ |c| c[:attributes][:balance] } + expect([response.status, balances.compact]).to eq [200, []] + end + end + end + end + post "Create customer" do tags "Customers" consumes "application/json" @@ -191,7 +221,10 @@ describe "Customers", type: :request do response "200", "Customer" do param(:id) { customer1.id } - schema "$ref": "#/components/schemas/customer" + schema CustomerSchema.schema( + require_all: true, + extra_fields: { name: :balance, required: true } + ) run_test! do date_time_string = diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index ad4cd3c34e..bebca37075 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -27,8 +27,9 @@ RSpec.configure do |config| components: { schemas: { error_response: ErrorsSchema.schema, + # only customer#show is with extra_fields: {name: :balance, required: true} customer: CustomerSchema.schema(require_all: true), - customers_collection: CustomerSchema.collection(require_all: true) + customers_collection: CustomerSchema.collection(require_all: true, extra_fields: :balance) }, securitySchemes: { api_key_header: { diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 2909306e6e..82ae1aab26 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -77,7 +77,7 @@ components: billing_address: type: object nullable: true - example: + example: shipping_address: type: object nullable: true @@ -190,7 +190,7 @@ components: billing_address: type: object nullable: true - example: + example: shipping_address: type: object nullable: true @@ -210,6 +210,9 @@ components: country: code: AU name: Australia + balance: + type: number + format: double required: - id - enterprise_id @@ -307,6 +310,12 @@ paths: in: query schema: type: string + - name: extra_fields[customer] + in: query + example: balance + description: Add extra fields to each customer + schema: + type: string responses: '200': description: Customers list @@ -366,7 +375,7 @@ paths: billing_address: type: object nullable: true - example: + example: shipping_address: type: object nullable: true @@ -406,7 +415,120 @@ paths: content: application/json: schema: - "$ref": "#/components/schemas/customer" + type: object + properties: + data: + type: object + properties: + id: + type: string + example: '1' + type: + type: string + example: customer + attributes: + type: object + properties: + id: + type: integer + example: 1 + enterprise_id: + type: integer + example: 2 + first_name: + type: string + nullable: true + example: Alice + last_name: + type: string + nullable: true + example: Springs + code: + type: string + nullable: true + example: BUYER1 + email: + type: string + example: alice@example.com + allow_charges: + type: boolean + example: false + tags: + type: array + items: + type: string + example: + - staff + - discount + terms_and_conditions_accepted_at: + type: string + format: date-time + nullable: true + example: '2022-03-12T15:55:00.000+11:00' + billing_address: + type: object + nullable: true + example: + shipping_address: + type: object + nullable: true + example: + phone: 0404 333 222 111 + latitude: -37.8173751 + longitude: 144.964803195704 + first_name: Alice + last_name: Springs + street_address_1: 1 Flinders Street + street_address_2: '' + postal_code: '1234' + locality: Melbourne + region: + code: Vic + name: Victoria + country: + code: AU + name: Australia + balance: + type: number + format: double + required: + - id + - enterprise_id + - first_name + - last_name + - code + - email + - allow_charges + - tags + - terms_and_conditions_accepted_at + - billing_address + - shipping_address + - balance + relationships: + type: object + properties: + enterprise: + type: object + properties: + data: + type: object + properties: + id: + type: string + type: + type: string + example: enterprise + links: + type: object + properties: + related: + type: string + meta: + type: object + links: + type: object + required: + - data '404': description: Not found content: @@ -476,7 +598,7 @@ paths: billing_address: type: object nullable: true - example: + example: shipping_address: type: object nullable: true