Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade json_api_client gem #158

Merged
merged 56 commits into from
Oct 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
f7f4f41
Update client gem
JoshuaTatterton Aug 16, 2018
9e70d94
Update patches code
JoshuaTatterton Aug 16, 2018
44eaebb
Update jsonapi page size param
JoshuaTatterton Aug 16, 2018
b33d9ca
Same as before but it works
JoshuaTatterton Aug 16, 2018
a88e9b2
Update page number param
JoshuaTatterton Aug 16, 2018
7279a3c
Fix pagination params
JoshuaTatterton Aug 16, 2018
cf595a7
Update patch
JoshuaTatterton Aug 21, 2018
1f2245e
Narrow down version
Sep 3, 2018
ae5dc63
Puts gem version
Sep 3, 2018
01c92a1
Fix version of json_api_client
Sep 4, 2018
17fa4cb
Try v1.6.0 of json_api_client
Sep 4, 2018
18ad0ce
Fix 'unknown keyword: data'
Sep 4, 2018
eb0f265
Fix 'unknown keyword: data' in 'update'
Sep 4, 2018
b0c130d
Fix 'unknown keyword: filter' and maybe others
Sep 4, 2018
93761c9
Potentially fix 'unknown keywords: ...'
Sep 4, 2018
e7ae593
Fix more
Sep 4, 2018
40525d3
Fix syntax issue
Sep 4, 2018
35f0181
Fix 'wrong number of arguments'
Sep 4, 2018
22062d0
Update stubs
Sep 4, 2018
8561880
Temporarily revert 1.6.0 to 1.5.3
Sep 5, 2018
606f745
Revert to 1.6.0
Sep 5, 2018
bb9a314
Try just setting Accept-Encoding header
Sep 5, 2018
52d7451
Revert to Faraday Gzip middleware
Sep 6, 2018
e8464b5
Remove unneeded middleware
Sep 10, 2018
98df40d
Remove stub
Sep 10, 2018
52b6424
CustomerAccountSpec extra request stub
Sep 10, 2018
6ea38f9
Test without webhook stub
Sep 10, 2018
1997da8
Revert
Sep 10, 2018
178c115
Revert spec changes completely
Sep 10, 2018
e0c0715
Remove unneeded patch
Sep 10, 2018
f426663
Revert to 1.5.3 and remove workarounds for 1.6.0
Sep 10, 2018
138d3e9
Add patch back in
Sep 10, 2018
462d041
Remove patch
Sep 10, 2018
4c52a04
Merge branch 'master' into update_json_api_client_gem
Sep 19, 2018
d12a665
Only use gzip compression for known compatible adapters
Sep 19, 2018
bc607da
Add big obvious puts
Sep 19, 2018
a891c26
Remove big obvious puts
Sep 19, 2018
af6d0be
Add logging
Sep 20, 2018
f3c2671
Move logging
Sep 20, 2018
e9ec373
Add config option for disabling gzip
Sep 20, 2018
75ee4ea
Disable gzip in paypal specs
Sep 20, 2018
5e90eb3
Add patch back in
Sep 21, 2018
a1c93bb
Remove GZIP
Sep 24, 2018
64ee648
More removing of GZIP
Sep 24, 2018
88640c4
Fix tax code error spec failure
Sep 24, 2018
691358e
Remove unused transaltion, move error message into lib
Sep 24, 2018
493fc40
Add failing spec
Sep 26, 2018
3fa2b98
Add fix (hopefully?)
Sep 27, 2018
e20e9a8
Remove fix
Sep 27, 2018
63e9970
Override path method to automatically set path param
Sep 27, 2018
62eaea0
Update fix
Sep 27, 2018
e8f2fae
Update again
Sep 27, 2018
891c75f
Hack
Sep 28, 2018
eb1894c
Add fix for 'last' link
Oct 1, 2018
ea571c5
Remove hardcoded timeout
Oct 1, 2018
a330196
Tweak fix and update comment
Oct 1, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion app/models/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,16 @@ class LineItem < FlexCommerceApi::ApiBase
has_one :item
belongs_to :cart, class_name: "::FlexCommerce::Cart"


# note: only embedded in order responses, not carts
has_many :line_item_discounts, class_name: "::FlexCommerce::LineItemDiscount"

# This allows us to set the `path` parameter automatically from the provided
# cart_id or container_id
def self.path(params = nil, *args)
cart_id = params["cart_id"] || params["container_id"]
params["path"] = { "cart_id" => cart_id } if cart_id

super
end
end
end
3 changes: 0 additions & 3 deletions config/locales/payments.en.yml

This file was deleted.

2 changes: 1 addition & 1 deletion flex-commerce-api.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency "json_matchers", ["~> 0.5", ">= 0.5.1"]

spec.add_dependency "oj", "~> 2.12"
spec.add_runtime_dependency "json_api_client", "1.1.1"
spec.add_runtime_dependency "json_api_client", "1.5.3"
spec.add_runtime_dependency "activesupport", ">= 4.0"
spec.add_runtime_dependency "rack", ">= 1.6"
spec.add_runtime_dependency "faraday-http-cache", "1.3.0"
Expand Down
6 changes: 6 additions & 0 deletions lib/flex_commerce_api/base_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
require "flex_commerce_api/json_api_client_extension/remote_builder"

module FlexCommerceApi
# Set the pagination param
JsonApiClientExtension::Paginator.page_param = "number"
JsonApiClientExtension::Paginator.per_page_param = "size"

#
# Base class for all flex commerce models
#
Expand Down Expand Up @@ -106,12 +110,14 @@ def reconfigure_api_base options = {}

def reconfigure options = {}
self.site = append_version(FlexCommerceApi.config.api_base_url)

base_options = {
adapter: FlexCommerceApi.config.adapter || :net_http,
http_cache: FlexCommerceApi.config.http_cache,
timeout: FlexCommerceApi.config.timeout,
open_timeout: FlexCommerceApi.config.open_timeout
}

self.connection_options.delete(:include_previewed)
self.connection_options = connection_options.merge(base_options).merge(options)
reload_connection_if_required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def initialize(options = {})
add_json_api_extension = options.fetch(:add_json_api_extension, true)
authenticate = options.fetch(:authenticate, true)
include_previewed = options.fetch :include_previewed, false

@faraday = Faraday.new(site) do |builder|
builder.request :json
builder.use JsonApiClientExtension::SaveRequestBodyMiddleware
Expand Down Expand Up @@ -42,6 +43,7 @@ def run(*args)
end

private

def cache_options(options)
{
# treats the cache like a client, not a proxy
Expand All @@ -54,6 +56,7 @@ def cache_options(options)
logger: FlexCommerceApi.logger
}.merge(options.fetch(:http_cache, {}))
end

end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ def encoded(part)
end
end
end
end
end
17 changes: 17 additions & 0 deletions lib/patches/json_api_client/builder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module JsonApiClient
module Query
class Builder
# This is a workaround because in the json_api_client gem they now parse
# the "last" link instead of the original behaviour:
# https://github.com/JsonApiClient/json_api_client/blob/v1.5.3/lib/json_api_client/query/builder.rb#L69
# This causes an error when Faraday parses our params due to the filter
# param appearing to be malformed.
#
# TODO: Look at fixing our links so that we can remove this workaround
# (platform ticket #7293)
def last
self.to_a.last
end
end
end
end
15 changes: 4 additions & 11 deletions lib/patches/json_api_client/resource.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require "json_api_client/version"
if ["1.1.1"].include?(JsonApiClient::VERSION)

if ["1.1.1", "1.5.3"].include?(JsonApiClient::VERSION)
require "json_api_client/resource"
module JsonApiClient
class Resource
Expand All @@ -11,15 +12,8 @@ def save
else
self.class.requestor.create(self)
end

if last_result_set.has_errors?
last_result_set.errors.each do |error|
if error.source_parameter
errors.add(error.source_parameter, error.title || error.detail)
else
errors.add(:base, error.title || error.detail)
end
end
fill_errors
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our patch needed updating to use the json_api_client gem's latest method of dealing with errors

false
else
self.errors.clear if self.errors
Expand All @@ -43,8 +37,7 @@ def save
Please check these two PRs:
* https://github.com/chingor13/json_api_client/pull/238 (This was released in version 1.5.0)
* https://github.com/JsonApiClient/json_api_client/pull/285 (This hasn't yet been released at the time of writing this)

If both have been merged into the gem version you are using, remove this file (#{__FILE__}).
If both have been merged into the gem version you are using, remove this file (#{__FILE__}).
If not, add the current version to the allowed array at the top of this file.
)
end
22 changes: 14 additions & 8 deletions lib/paypal_express/setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
module FlexCommerce
module PaypalExpress
# @class Setup
#
#
# This is the main class, which talks to ActiveMerchant gem to initiate a transaction using Paypal
class Setup
include ::FlexCommerce::PaypalExpress::Api


# @initialize
#
#
# @param {FlexCommerce::PaymentProviderSetup} payment_provider_setup
# @param {FlexCommerce::Cart} cart
# @param {Paypal Gateway} [gateway_class = ::ActiveMerchant::Billing::PaypalExpressGateway]
Expand All @@ -23,7 +23,7 @@ class Setup
# @param {FlexCommerce::ShippingMethod} shipping_method_model = FlexCommerce::ShippingMethod
# @param {boolean} [use_mobile_payments = false]
# @param {String} [description = nil]
#
#
# @note:
# For `::ActiveMerchant::Billing::PaypalExpressGateway` to work
# rails-site should include active merchant gem. Ideally this gem should be included in the gemspec.
Expand All @@ -43,7 +43,7 @@ def initialize(cart:, gateway_class: ::ActiveMerchant::Billing::PaypalExpressGat

def call
validate_shipping_method

response = gateway.setup_order(convert_amount(cart.total), paypal_params)
# If paypal setup went fine, redirect to the paypal page
if response.success?
Expand Down Expand Up @@ -76,15 +76,21 @@ def paypal_params
end

# @method shipping_methods
#
#
# @returns shipping methods with promotions applied
def shipping_methods
@shipping_methods ||= ShippingMethodsForCart.new(cart: cart, shipping_methods: shipping_method_model.all).call.sort_by(&:total)
end

def validate_shipping_method
unless cart.shipping_method_id.nil? || shipping_methods.any? {|sm| sm.id == cart.shipping_method_id} then
raise ::FlexCommerce::PaypalExpress::Exception::AccessDenied.new(I18n.t("payment_setup.shipping_method_not_available"))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This translation wasn't loading and we were getting a translation missing error in the test runs. It was the only one in the locale file anyway, so I've moved it into the library. We could add another error class here as well (open to naming ideas)

unless cart.shipping_method_id.nil? || shipping_methods.any? { |sm| sm.id == cart.shipping_method_id }
if cart.shipping_method_id.nil?
error_message = "Shipping method not specified on cart."
else
error_message = "Shipping method not available: #{cart.shipping_method_id}"
error_message += " (in #{shipping_methods.to_a.collect(&:id).join(", ")})"
end
raise ::FlexCommerce::PaypalExpress::Exception::AccessDenied.new(error_message)
end
end
end
Expand Down
58 changes: 58 additions & 0 deletions spec/e2e/line_items_api_e2e_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
require "e2e_spec_helper"
RSpec.describe "Line items API end to end spec", vcr: true do
# As the "before context" blocks cannot access let vars, the "context store" simply defines a method called "context_store"
# that stores whatever you want - it is an "OpenStruct" so you can just write anything to it and read it back at any time
# This is cleared at the start of the context, but the idea is so that you can share stuff between examples.
# This obviously means that the examples are tied together - in terms of the read, update and delete methods all rely
# on having created an object in the first place.
# This also means that this test suite must be run in the order defined, not random.
include_context "context store"

# We define the model in advance, mainly allowing the code in the examples to be fairly generic and can be copied / pasted
# into other tests without changing the model all over the place.
let(:model) { FlexCommerce::LineItem }

before(:context) do
context_store.uuid = uuid = SecureRandom.uuid
context_store.foreign_resources = OpenStruct.new
context_store.foreign_resources.product = FlexCommerce::Product.create! title: "Title for product 1 for variant #{uuid}",
reference: "reference for product 1 for variant #{uuid}",
content_type: "markdown"
context_store.foreign_resources.variant = FlexCommerce::Variant.create title: "Title for Test Variant #{uuid}",
description: "Description for Test Variant #{uuid}",
reference: "reference_for_test_variant_#{uuid}",
price: 5.50,
price_includes_taxes: false,
sku: "sku_for_test_variant_#{uuid}",
product_id: context_store.foreign_resources.product.id,
stock_level: 1
context_store.created_cart = FlexCommerce::Cart.create!
end

context "#create" do
it "should persist when valid attributes are used" do
expect {
FlexCommerce::LineItem.create! \
item_id: context_store.foreign_resources.variant.id,
item_type: "Variant",
unit_quantity: 1,
container_id: context_store.created_cart.id
}.not_to raise_error
end
end

context "#update" do
it "should persist when valid attributes are used" do
FlexCommerce::LineItem.create! \
item_id: context_store.foreign_resources.variant.id,
item_type: "Variant",
unit_quantity: 1,
container_id: context_store.created_cart.id
created_cart = FlexCommerce::Cart.find(context_store.created_cart.id) # Reload it
line_item = created_cart.line_items.first
line_item.update(unit_quantity: 5)
line_item.reload
expect(line_item.unit_quantity).to eq(5)
end
end
end
2 changes: 1 addition & 1 deletion spec/support/global_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
config.logger = nil
config.order_test_mode = false
config.logger = ActiveSupport::Logger.new(STDOUT)
end
end