From 96c0057b03354ead9e171961f645ffa6e23a069f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Fri, 10 Feb 2023 11:59:12 +0100 Subject: [PATCH 1/8] Use Customers as argument for CustomersWithBalance Change argument for CustomersWithBalance from enterprise_id to Customers collection. We have the need to calculate balance for customers in general, not just for customers in a given enterprise. --- app/controllers/admin/customers_controller.rb | 2 +- app/queries/customers_with_balance.rb | 22 +++++++++---------- .../admin/customers_controller_spec.rb | 2 +- spec/queries/customers_with_balance_spec.rb | 19 +++++++++++++++- 4 files changed, 30 insertions(+), 15 deletions(-) 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/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/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/queries/customers_with_balance_spec.rb b/spec/queries/customers_with_balance_spec.rb index 479be6b410..b243962c85 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) + id_b = described_class.new(Customer.where(id: customers)).query + .map{ |c| [c.id, c.balance_value] } + expect(id_b).to eq([[customers.first.id, 0], [customers.second.id, 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') From 22b1dd3232aec6ea1faeeffd5aa8293967697696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Fri, 10 Feb 2023 12:21:18 +0100 Subject: [PATCH 2/8] Add support for extra_fields query parameter Api now supports optional fields. These are included with the extra_fields query param. Syntax: extra_fields[type]=field1,field2 --- app/controllers/api/v1/base_controller.rb | 4 +- app/controllers/concerns/extra_fields.rb | 32 ++++++++++ config/locales/en.yml | 5 ++ .../controllers/concerns/extra_fields_spec.rb | 60 +++++++++++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 app/controllers/concerns/extra_fields.rb create mode 100644 spec/controllers/concerns/extra_fields_spec.rb 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/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/config/locales/en.yml b/config/locales/en.yml index 019042608e..0b0b7f4c6e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1599,6 +1599,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/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 From 3989843a21297454418c0f23e4712ed29ac18eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Fri, 10 Feb 2023 12:01:56 +0100 Subject: [PATCH 3/8] Add support for extra fields in JsonApiSchemas As we are now supporting optional extra fields, We need to be able to construct schemas where extra fields are optionally included. --- app/json_schemas/json_api_schema.rb | 59 ++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/app/json_schemas/json_api_schema.rb b/app/json_schemas/json_api_schema.rb index 069de6633d..1554b21702 100644 --- a/app/json_schemas/json_api_schema.rb +++ b/app/json_schemas/json_api_schema.rb @@ -76,8 +76,10 @@ class JsonApiSchema private - def data_properties(require_all: false) - required = require_all ? all_attributes : required_attributes + 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) { id: { type: :string, example: "1" }, @@ -99,6 +101,59 @@ class JsonApiSchema } end + # Example + # extra_fields: :my_method + # => extra_fields_result = my_method + # => attributes = attributes.merge(extra_fields_result) + # + # extra_fields: {name: :my_method, required: true, opts: {method_opt: true}} + # => extra_fields_result = my_method({method_opt: true}) + # => attributes = attributes.merge(extra_fields_result) + # => required += extra_fields_result.keys + # + # extra_fields: [:my_method, :another_method] + # => extra_fields_result = my_method.merge(another_method) + # => attributes = attribtues.merge(extra_fields_result) + # + # To test use eg:: + # => MySchema.collection(..., extra_fields: ...) + # .dig(:properties, :data, :items, :properties, :attributes) + def get_extra_fields(extra_fields) + case extra_fields + when Symbol + public_send(extra_fields) + when Hash + extra_fields[:opts] && + public_send(extra_fields[:name], extra_fields[:opts]) || public_send(extra_fields[:name]) + 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 + + 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) if is_singular?(name) RelationshipSchema.schema(name) From e95d08cae8d3c3b1934319a27e7a5b0de5c47612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Mon, 13 Feb 2023 11:55:18 +0100 Subject: [PATCH 4/8] Add balance to api v1 customers endpoint - customers#show: Add balance (data_type: double) to customer attributes. - customers#index: Add balance only if specified in extra_fields query parameter: extra_fields[customer]=balance --- .../api/v1/customers_controller.rb | 17 ++- app/json_schemas/customer_schema.rb | 5 + app/serializers/api/v1/customer_serializer.rb | 4 + spec/requests/api/v1/customers_spec.rb | 35 ++++- spec/swagger_helper.rb | 3 +- swagger/v1/swagger.yaml | 124 +++++++++++++++++- 6 files changed, 184 insertions(+), 4 deletions(-) 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/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/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/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..0f4cec8aff 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -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 @@ -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: From f15a4cc9434419255e8dd55785438b17b65138a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Fri, 10 Feb 2023 12:33:08 +0100 Subject: [PATCH 5/8] Add autogenerated whitespaces to swagger.yml These whitespaces were generated by bundle exec rails rswag. Might be related to a new version of rswag or developers' trim trailing whitespace settings. --- swagger/v1/swagger.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 0f4cec8aff..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 @@ -375,7 +375,7 @@ paths: billing_address: type: object nullable: true - example: + example: shipping_address: type: object nullable: true @@ -598,7 +598,7 @@ paths: billing_address: type: object nullable: true - example: + example: shipping_address: type: object nullable: true From af23375b04fe672b93b9cfe3c6794a4653086d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Mon, 13 Feb 2023 11:49:30 +0100 Subject: [PATCH 6/8] Separate Structure from JsonApiSchema Class was getting too long --- app/json_schemas/json_api_schema.rb | 75 ++--------------- app/json_schemas/json_api_schema/structure.rb | 83 +++++++++++++++++++ 2 files changed, 90 insertions(+), 68 deletions(-) create mode 100644 app/json_schemas/json_api_schema/structure.rb diff --git a/app/json_schemas/json_api_schema.rb b/app/json_schemas/json_api_schema.rb index 1554b21702..82c7b26492 100644 --- a/app/json_schemas/json_api_schema.rb +++ b/app/json_schemas/json_api_schema.rb @@ -19,59 +19,11 @@ 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 @@ -81,24 +33,11 @@ class JsonApiSchema attributes = get_attributes(extra_fields_result) required = get_required(require_all, extra_fields, extra_fields_result) - { - 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 - } - } + Structure.data_properties(object_name, attributes, required, relationship_properties) + end + + def relationship_properties + relationships.to_h { |name| [name, relationship_schema(name)] } end # Example 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 From 3d06b758922d6f48ffdf7aee90605f194e533f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Wed, 15 Feb 2023 10:42:28 +0100 Subject: [PATCH 7/8] Simplify customers_with_balance spec Improve variable names and use multiple expectations --- spec/queries/customers_with_balance_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/queries/customers_with_balance_spec.rb b/spec/queries/customers_with_balance_spec.rb index b243962c85..9bf075367a 100644 --- a/spec/queries/customers_with_balance_spec.rb +++ b/spec/queries/customers_with_balance_spec.rb @@ -22,9 +22,9 @@ describe CustomersWithBalance do context 'with customers collection' do it 'returns balance' do customers = create_pair(:customer) - id_b = described_class.new(Customer.where(id: customers)).query - .map{ |c| [c.id, c.balance_value] } - expect(id_b).to eq([[customers.first.id, 0], [customers.second.id, 0]]) + 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 From f645f8fc7993d188c85081243b2499665ac823ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20Norl=C3=A9n?= Date: Wed, 15 Feb 2023 10:51:11 +0100 Subject: [PATCH 8/8] Improve schema method get_extra_fields Use double-splat for passing options and improve examples --- app/json_schemas/json_api_schema.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/app/json_schemas/json_api_schema.rb b/app/json_schemas/json_api_schema.rb index 82c7b26492..53202656d0 100644 --- a/app/json_schemas/json_api_schema.rb +++ b/app/json_schemas/json_api_schema.rb @@ -41,29 +41,28 @@ class JsonApiSchema end # Example - # extra_fields: :my_method - # => extra_fields_result = my_method + # MySchema.schema(extra_fields: :my_method) + # => extra_fields_result = MySchema.my_method # => attributes = attributes.merge(extra_fields_result) # - # extra_fields: {name: :my_method, required: true, opts: {method_opt: true}} - # => extra_fields_result = my_method({method_opt: true}) + # 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 # - # extra_fields: [:my_method, :another_method] - # => extra_fields_result = my_method.merge(another_method) + # 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.collection(..., extra_fields: ...) - # .dig(:properties, :data, :items, :properties, :attributes) + # 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 - extra_fields[:opts] && - public_send(extra_fields[:name], extra_fields[:opts]) || public_send(extra_fields[:name]) + public_send(extra_fields[:name], **extra_fields[:opts].to_h) when Array obj = {}