-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes from 52 commits
f7f4f41
9e70d94
44eaebb
b33d9ca
a88e9b2
7279a3c
cf595a7
1f2245e
ae5dc63
01c92a1
17fa4cb
18ad0ce
eb0f265
b0c130d
93761c9
e7ae593
40525d3
35f0181
22062d0
8561880
606f745
bb9a314
52d7451
e8464b5
98df40d
52b6424
6ea38f9
1997da8
178c115
e0c0715
f426663
138d3e9
462d041
4c52a04
d12a665
bc607da
a891c26
af6d0be
f3c2671
e9ec373
75ee4ea
5e90eb3
a1c93bb
64ee648
88640c4
691358e
493fc40
3fa2b98
e20e9a8
63e9970
62eaea0
e8f2fae
891c75f
eb1894c
ea571c5
a330196
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,4 +39,4 @@ def encoded(part) | |
end | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
require "json_api_client/version" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dont we require this patch anymore? |
||
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 | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our patch needed updating to use the |
||
false | ||
else | ||
self.errors.clear if self.errors | ||
|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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] | ||
|
@@ -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. | ||
|
@@ -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? | ||
|
@@ -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")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,4 +10,4 @@ | |
config.logger = nil | ||
config.order_test_mode = false | ||
config.logger = ActiveSupport::Logger.new(STDOUT) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required as we've removed the
_prefix_path
workaround to help with nested / multitenanted line items endpoint (here) which causes the upgradedjson_api_client
gem version to error.We always have the
container_id
present so this saves us having to pass thepath: { cart_id: ... }
attribute everywhere.The alternative is we make this a requirement and go through the F/E code adding
path: { cart_id: ... }
everywhere like we've done with nested coupons (and I'm not sure this would work if we're loading line items from a cart, updating them, then re-saving)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another fix would have been to update the platform to always include this path parameter in API responses. Not sure this is a good idea as we'd be adding it specifically for
json_api_client
but it isn't necessarily required for any other json API libraries.