Merge pull request #8967 from mkllnk/api-customers-final

Serve customers data via first API v1 endpoint in JSON:API format behind feature toggle
This commit is contained in:
Filipe
2022-04-07 21:03:06 +01:00
committed by GitHub
12 changed files with 453 additions and 26 deletions

View File

@@ -79,6 +79,11 @@ Lint/RaiseException:
Lint/StructNewOverride:
Enabled: true
Naming/VariableNumber:
AllowedIdentifiers:
- street_address_1
- street_address_2
Bundler/DuplicatedGem:
Enabled: false

View File

@@ -7,6 +7,7 @@ module Api
include RequestTimeouts
include Pagy::Backend
include JsonApiPagination
include RaisingParameters
check_authorization
@@ -18,6 +19,8 @@ module Api
rescue_from CanCan::AccessDenied, with: :unauthorized
rescue_from ActiveRecord::RecordNotFound, with: :not_found
rescue_from Pagy::VariableError, with: :invalid_pagination
rescue_from ActionController::ParameterMissing, with: :missing_parameter
rescue_from ActionController::UnpermittedParameters, with: :unpermitted_parameters
private
@@ -60,6 +63,20 @@ module Api
json: json_api_error(exception.message)
end
def missing_parameter(error)
message = I18n.t(:missing_parameter, param: error.param, scope: :api)
render status: :unprocessable_entity,
json: json_api_error(message)
end
def unpermitted_parameters(error)
message = I18n.t(:unpermitted_parameters, params: error.params.join(", "), scope: :api)
render status: :unprocessable_entity,
json: json_api_error(message)
end
def invalid_resource!(resource = nil)
render status: :unprocessable_entity,
json: json_api_invalid(

View File

@@ -17,7 +17,7 @@ module Api
end
def show
render json: Api::V1::CustomerSerializer.new(@customer)
render json: Api::V1::CustomerSerializer.new(@customer, include_options)
end
def create
@@ -58,7 +58,7 @@ module Api
end
def search_customers
customers = visible_customers
customers = visible_customers.includes(:bill_address, :ship_address)
customers = customers.where(enterprise_id: params[:enterprise_id]) if params[:enterprise_id]
customers.ransack(params[:q]).result
end
@@ -70,12 +70,67 @@ module Api
end
def customer_params
params.require(:customer).permit(:email, :enterprise_id)
attributes = params.require(:customer).permit(
:email, :enterprise_id,
:code, :first_name, :last_name,
:billing_address, shipping_address: [
:phone, :latitude, :longitude,
:first_name, :last_name,
:street_address_1, :street_address_2,
:postal_code, :locality, :region, :country,
]
).to_h
attributes.merge!(tag_list: params[:tags]) if params.key?(:tags)
transform_address!(attributes, :billing_address, :bill_address)
transform_address!(attributes, :shipping_address, :ship_address)
attributes
end
def transform_address!(attributes, from, to)
return unless attributes.key?(from)
address = attributes.delete(from)
if address.nil?
attributes[to] = nil
return
end
address.transform_keys! do |key|
{
phone: :phone, latitude: :latitude, longitude: :longitude,
first_name: :firstname, last_name: :lastname,
street_address_1: :address1, street_address_2: :address2,
postal_code: :zipcode,
locality: :city,
region: :state_name,
country: :country,
}.with_indifferent_access[key]
end
if address[:state_name].present?
address[:state] = Spree::State.find_by(name: address[:state_name])
end
if address[:country].present?
address[:country] = Spree::Country.find_by(name: address[:country])
end
attributes["#{to}_attributes"] = address
end
def editable_enterprises
OpenFoodNetwork::Permissions.new(current_api_user).editable_enterprises.select(:id)
end
def include_options
fields = [params.fetch(:include, [])].flatten
{ include: fields.map(&:to_s) }
end
end
end
end

View File

@@ -0,0 +1,23 @@
# frozen_string_literal: true
# The API uses strict parameter checking.
#
# We want to raise errors when unused or unpermitted parameters are given
# to the API. You then know straight away when a parameter isn't used.
module RaisingParameters
extend ActiveSupport::Concern
# ActionController manages this config on a per-class basis. The subclass
# enables us to raise errors only here and not in the rest of the app.
class Parameters < ActionController::Parameters
def self.action_on_unpermitted_parameters
:raise
end
end
# We override the params method so that we always use the strict parameters.
# We could rename this method if we need access to the orginal as well.
def params
Parameters.new(super.to_unsafe_hash)
end
end

View File

@@ -12,7 +12,37 @@ class CustomerSchema < JsonApiSchema
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" }
email: { type: :string, example: "alice@example.com" },
allow_charges: { type: :boolean, example: false },
tags: { type: :array, 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: nil,
},
shipping_address: {
type: :object, nullable: true,
example: address_example,
},
}
end
def self.address_example
{
phone: "0404 333 222 111",
latitude: -37.817375100000,
longitude: 144.964803195704,
first_name: "Alice",
last_name: "Springs",
street_address_1: "1 Flinders Street",
street_address_2: "",
postal_code: "1234",
locality: "Melbourne",
region: "Victoria",
country: "Australia",
}
end
@@ -20,6 +50,14 @@ class CustomerSchema < JsonApiSchema
[:enterprise_id, :email]
end
def self.writable_attributes
attributes.except(
:id,
:allow_charges,
:terms_and_conditions_accepted_at,
)
end
def self.relationships
[:enterprise]
end

View File

@@ -0,0 +1,18 @@
# frozen_string_literal: true
module Api
module V1
class AddressSerializer < BaseSerializer
attributes :phone, :latitude, :longitude
attribute :first_name, &:firstname
attribute :last_name, &:lastname
attribute :street_address_1, &:address1
attribute :street_address_2, &:address2
attribute :postal_code, &:zipcode
attribute :locality, &:city
attribute :region, &:state_name
attribute :country, ->(object, _) { object.country.name }
end
end
end

View File

@@ -3,13 +3,28 @@
module Api
module V1
class CustomerSerializer < BaseSerializer
attributes :id, :enterprise_id, :first_name, :last_name, :code, :email
attributes :id, :enterprise_id, :first_name, :last_name, :code, :email,
:allow_charges, :terms_and_conditions_accepted_at
attribute :tags, &:tag_list
attribute :billing_address do |object|
address(object.billing_address)
end
attribute :shipping_address do |object|
address(object.shipping_address)
end
belongs_to :enterprise, links: {
related: ->(object) {
url_helpers.api_v1_enterprise_url(id: object.enterprise_id)
}
}
def self.address(record)
AddressSerializer.new(record).serializable_hash.dig(:data, :attributes)
end
end
end
end

View File

@@ -1414,6 +1414,8 @@ en:
unknown_error: "Something went wrong. Our team has been notified."
invalid_api_key: "Invalid API key (%{key}) specified."
unauthorized: "You are not authorized to perform that action."
unpermitted_parameters: "Parameters not allowed in this request: %{params}"
missing_parameter: "A required parameter is missing or empty: %{param}"
invalid_resource: "Invalid resource. Please fix errors and try again."
resource_not_found: "The resource you were looking for could not be found."
enterprise_logo:

View File

@@ -87,13 +87,11 @@ Openfoodnetwork::Application.routes.draw do
constraints: lambda { |_| Flipper.enabled?(:api_reports) }
end
unless Rails.env.production?
namespace :v1 do
resources :customers
namespace :v1, constraints: ->(request) { Flipper.enabled?(:api_v1, request.env["warden"].user) } do
resources :customers
resources :enterprises do
resources :customers, only: :index
end
resources :enterprises do
resources :customers, only: :index
end
end

View File

@@ -0,0 +1,40 @@
# frozen_string_literal: true
require 'spec_helper'
describe RaisingParameters do
describe "Parameters" do
let(:params) do
RaisingParameters::Parameters.new(
controller: "example",
action: "update",
data: {
id: "unique",
admin: true,
}
)
end
it "raises an error when a parameter is not permitted" do
expect {
params.require(:data).permit(:id)
}.to raise_error(
ActionController::UnpermittedParameters
)
end
it "raises no error when all parameters are permitted" do
expect {
params.require(:data).permit(:id, :admin)
}.to_not raise_error
end
it "doesn't change standard parameter objects" do
original_params = ActionController::Parameters.new(one: 1, two: 2)
expect {
original_params.permit(:one)
}.to_not raise_error
end
end
end

View File

@@ -3,13 +3,24 @@
require "swagger_helper"
describe "Customers", type: :request do
let!(:enterprise1) { create(:enterprise) }
let!(:enterprise1) { create(:enterprise, name: "The Farm") }
let!(:enterprise2) { create(:enterprise) }
let!(:customer1) { create(:customer, enterprise: enterprise1) }
let!(:customer1) {
create(
:customer,
enterprise: enterprise1,
terms_and_conditions_accepted_at: Time.zone.parse("2000-01-01"),
tag_list: ["long-term"],
ship_address: create(:address),
)
}
let!(:customer2) { create(:customer, enterprise: enterprise1) }
let!(:customer3) { create(:customer, enterprise: enterprise2) }
before { login_as enterprise1.owner }
before do
Flipper.enable(:api_v1)
login_as enterprise1.owner
end
path "/api/v1/customers" do
get "List customers" do
@@ -101,11 +112,11 @@ describe "Customers", type: :request do
parameter name: :customer, in: :body, schema: {
type: :object,
properties: CustomerSchema.attributes.except(:id),
properties: CustomerSchema.writable_attributes,
required: CustomerSchema.required_attributes
}
response "201", "Customer created" do
response "201", "Minimal customer created" do
param(:customer) do
{
email: "test@example.com",
@@ -114,14 +125,60 @@ describe "Customers", type: :request do
end
schema "$ref": "#/components/schemas/resources/customer"
run_test!
run_test! do
expect(json_response[:data][:attributes]).to include(
allow_charges: false,
terms_and_conditions_accepted_at: nil,
)
end
end
response "201", "Example customer created" do
param(:customer) do
CustomerSchema.writable_attributes.transform_values do |attribute|
attribute[:example]
end.merge(
enterprise_id: enterprise1.id,
)
end
schema "$ref": "#/components/schemas/resources/customer"
run_test! do
expect(json_response[:data][:attributes]).to include(
first_name: "Alice",
last_name: "Springs",
code: "BUYER1",
email: "alice@example.com",
enterprise_id: enterprise1.id,
tags: ["staff", "discount"],
shipping_address: CustomerSchema.address_example
)
end
end
response "422", "Unpermitted parameter" do
param(:customer) do
{
email: "test@example.com",
enterprise_id: enterprise1.id.to_s,
allow_charges: true,
}
end
schema "$ref": "#/components/schemas/error_response"
run_test! do
expect(json_error_detail).to eq "Parameters not allowed in this request: allow_charges"
end
end
response "422", "Unprocessable entity" do
param(:customer) { {} }
schema "$ref": "#/components/schemas/error_response"
run_test!
run_test! do
expect(json_error_detail).to eq "A required parameter is missing or empty: customer"
expect(json_response[:meta]).to eq nil
end
end
end
end
@@ -136,7 +193,14 @@ describe "Customers", type: :request do
param(:id) { customer1.id }
schema "$ref": "#/components/schemas/resources/customer"
run_test!
run_test! do
date_time_string =
json_response[:data][:attributes][:terms_and_conditions_accepted_at]
expect(date_time_string).to match /^2000-01-01T00:00:00.000[Z+-].*$/
expect(DateTime.parse(date_time_string)).to eq(
customer1.terms_and_conditions_accepted_at
)
end
end
response "404", "Not found" do
@@ -162,6 +226,13 @@ describe "Customers", type: :request do
end
describe "related records" do
it "contains exactly the defined relationships" do
get "/api/v1/customers/#{customer1.id}"
relationships = json_response[:data][:relationships].keys
expect(relationships).to match_array CustomerSchema.relationships.map(&:to_s)
end
it "serializes the enterprise relationship" do
expected_enterprise_data = {
"data" => {
@@ -177,6 +248,28 @@ describe "Customers", type: :request do
expect(json_response[:data][:relationships][:enterprise]).to eq(expected_enterprise_data)
end
end
describe "including related records" do
it "doesn't include other records by default" do
get "/api/v1/customers/#{customer1.id}"
expect(json_response[:included]).to eq nil
end
it "includes enterprise data on request" do
get "/api/v1/customers/#{customer1.id}?include=enterprise"
expect(json_response[:included].size).to eq 1
expect(json_response[:included].first).to include(
id: enterprise1.id.to_s,
type: "enterprise",
attributes: {
id: enterprise1.id,
name: "The Farm",
}
)
end
end
end
put "Update customer" do
@@ -187,7 +280,7 @@ describe "Customers", type: :request do
parameter name: :customer, in: :body, schema: {
type: :object,
properties: CustomerSchema.attributes,
properties: CustomerSchema.writable_attributes,
required: CustomerSchema.required_attributes
}
@@ -195,14 +288,18 @@ describe "Customers", type: :request do
param(:id) { customer1.id }
param(:customer) do
{
id: customer1.id.to_s,
email: "test@example.com",
enterprise_id: enterprise1.id.to_s
}
end
schema "$ref": "#/components/schemas/resources/customer"
run_test!
run_test! do
# Tags should not be overridden when the param is missing:
expect(json_response[:data][:attributes]).to include(
tags: ["long-term"],
)
end
end
response "422", "Unprocessable entity" do

View File

@@ -60,6 +60,38 @@ components:
email:
type: string
example: alice@example.com
allow_charges:
type: boolean
example: false
tags:
type: array
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: Victoria
country: Australia
required:
- id
- enterprise_id
@@ -67,6 +99,11 @@ components:
- last_name
- code
- email
- allow_charges
- tags
- terms_and_conditions_accepted_at
- billing_address
- shipping_address
relationships:
type: object
properties:
@@ -130,6 +167,38 @@ components:
email:
type: string
example: alice@example.com
allow_charges:
type: boolean
example: false
tags:
type: array
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: Victoria
country: Australia
required:
- id
- enterprise_id
@@ -137,6 +206,11 @@ components:
- last_name
- code
- email
- allow_charges
- tags
- terms_and_conditions_accepted_at
- billing_address
- shipping_address
relationships:
type: object
properties:
@@ -236,7 +310,7 @@ paths:
parameters: []
responses:
'201':
description: Customer created
description: Example customer created
content:
application/json:
schema:
@@ -271,6 +345,30 @@ paths:
email:
type: string
example: alice@example.com
tags:
type: array
example:
- staff
- discount
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: Victoria
country: Australia
required:
- enterprise_id
- email
@@ -333,9 +431,6 @@ paths:
schema:
type: object
properties:
id:
type: integer
example: 1
enterprise_id:
type: integer
example: 2
@@ -354,6 +449,30 @@ paths:
email:
type: string
example: alice@example.com
tags:
type: array
example:
- staff
- discount
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: Victoria
country: Australia
required:
- enterprise_id
- email