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

Don't activate charges via API #281

Merged
merged 3 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
nodejs 13.7.0
postgres 10.4
ruby 2.6.5
yarn 1.22.10
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Change Log
All notable changes to this project will be documented in this file.

## Unreleased
### Changed
- Address the [Shopify API deprecation of charge activation](https://shopify.dev/changelog/auto-activation-of-charges-and-subscriptions)

## 0.18.3 - 2021-07-23
### Changed
- Address the [Shopify API deprecation of page-based pagination](https://shopify.dev/changelog/page-based-pagination-replaced-by-cursor-based-pagination-across-multiple-rest-endpoints) in favour of cursor-based in the `Synchronises` concern.
Expand Down
7 changes: 6 additions & 1 deletion app/controllers/disco_app/charges_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@ def create
end
end

# Attempt to activate a charge after a user has accepted or declined it.
# Attempt to activate a charge (locally) after a user has accepted or declined it.
# Redirect to the main application's root URL immediately afterwards - if the
# charge wasn't accepted, the flow will start again.
#
# Previously, the activation of a charge also required updating Shopify via the
# API, but that requirement has been removed.
#
# See https://shopify.dev/changelog/auto-activation-of-charges-and-subscriptions
def activate
# First attempt to find a matching charge.
if (charge = @subscription.charges.find_by(id: params[:id], shopify_id: params[:charge_id])).nil?
Expand Down
20 changes: 9 additions & 11 deletions app/services/disco_app/charges_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,13 @@ def self.create(shop, subscription)
charge
end

# Attempt to activate the given Shopify charge for the given Shop using the
# Shopify API. Returns true on successful activation, false otherwise.
# Synchronises the status of a given charge from the Shopify API and returns
# true if it's active (and false otherwise).
#
# Previously, the activation of a charge also required updating Shopify via the
# API, but that requirement has been removed.
#
# See https://shopify.dev/changelog/auto-activation-of-charges-and-subscriptions
def self.activate(shop, charge)
# Start by fetching the Shopify charge to check that it was accepted.
shopify_charge = shop.with_api_context do
Expand All @@ -43,20 +48,13 @@ def self.activate(shop, charge)
# Update the status of the local charge based on the Shopify charge.
charge.send("#{shopify_charge.status}!") if charge.respond_to? "#{shopify_charge.status}!"

# If the charge wasn't accepted, fail and return.
return false unless charge.accepted?

# If the charge was indeed accepted, activate it via Shopify.
charge.shop.with_api_context do
shopify_charge.activate
end
# If the charge isn't active, fail and return.
return false unless charge.active?

# If the charge was recurring, make sure that all other local recurring
# charges are marked inactive.
cancel_recurring_charges(shop, charge) if charge.recurring?

charge.active!

true
rescue StandardError
false
Expand Down
3 changes: 1 addition & 2 deletions test/controllers/disco_app/charges_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ def teardown
end

test 'user trying to activate accepted charge succeeds and is redirected to the root page' do
stub_api_request(:get, "#{@shop.admin_url}/recurring_application_charges/654381179.json", 'widget_store/charges/get_accepted_recurring_application_charge')
stub_api_request(:post, "#{@shop.admin_url}/recurring_application_charges/654381179/activate.json", 'widget_store/charges/activate_recurring_application_charge')
stub_api_request(:get, "#{@shop.admin_url}/recurring_application_charges/654381179.json", 'widget_store/charges/get_active_recurring_application_charge')

@current_subscription.active_charge.destroy
get :activate, params: { subscription_id: @current_subscription, id: @new_charge.id, charge_id: @new_charge.shopify_id }
Expand Down
6 changes: 4 additions & 2 deletions test/dummy/package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{
"dependencies": {
"@rails/webpacker": "^4.0.7"
"@rails/webpacker": "5.4.3",
"webpack": "^4.46.0",
"webpack-cli": "^3.3.12"
},
"devDependencies": {
"webpack-dev-server": "^3.9.0"
"webpack-dev-server": "^3"
}
}
3,944 changes: 1,989 additions & 1,955 deletions test/dummy/yarn.lock

Large diffs are not rendered by default.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"name": "Lifetime",
"api_client_id": 755357713,
"price": "49.00",
"status": "accepted",
"status": "active",
"return_url": "https://test.example.com/subscriptions/304261385/charges/629961174/activate",
"created_at": "2015-09-02T14:52:56-04:00",
"updated_at": "2015-09-02T14:52:56-04:00",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
"name": "Basic",
"api_client_id": 755357713,
"price": "9.99",
"status": "accepted",
"status": "active",
"return_url": "https://test.example.com/subscriptions/304261385/charges/629961174/activate",
"billing_on": null,
"created_at": "2015-09-02T14:50:32-04:00",
"updated_at": "2015-09-02T14:50:32-04:00",
"test": true,
"activated_on": null,
"activated_on": "2015-09-02T14:50:32-04:00",
"trial_ends_on": null,
"cancelled_on": null,
"trial_days": 14,
Expand Down
9 changes: 3 additions & 6 deletions test/services/disco_app/charges_service_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ def teardown
end

test 'activating an accepted recurring charge is successful and cancels any existing recurring charges' do
stub_api_request(:get, "#{@shop.admin_url}/recurring_application_charges/654381179.json", 'widget_store/charges/get_accepted_recurring_application_charge')
stub_api_request(:post, "#{@shop.admin_url}/recurring_application_charges/654381179/activate.json", 'widget_store/charges/activate_recurring_application_charge')
stub_api_request(:get, "#{@shop.admin_url}/recurring_application_charges/654381179.json", 'widget_store/charges/get_active_recurring_application_charge')

old_charge = @subscription.active_charge
assert old_charge.active?
Expand All @@ -71,8 +70,7 @@ def teardown
end

test 'activating an accepted recurring charge cancels other recurring charges' do
stub_api_request(:get, "#{@shop.admin_url}/recurring_application_charges/654381179.json", 'widget_store/charges/get_accepted_recurring_application_charge')
stub_api_request(:post, "#{@shop.admin_url}/recurring_application_charges/654381179/activate.json", 'widget_store/charges/activate_recurring_application_charge')
stub_api_request(:get, "#{@shop.admin_url}/recurring_application_charges/654381179.json", 'widget_store/charges/get_active_recurring_application_charge')

assert DiscoApp::ChargesService.activate(@shop, @new_charge)
assert @new_charge.active?
Expand Down Expand Up @@ -101,8 +99,7 @@ def teardown
end

test 'activating an accepted one-time charge is successful' do
stub_api_request(:get, "#{@dev_shop.admin_url}/application_charges/1012637323.json", 'widget_store/charges/get_accepted_application_charge')
stub_api_request(:post, "#{@dev_shop.admin_url}/application_charges/1012637323/activate.json", 'widget_store/charges/activate_application_charge')
stub_api_request(:get, "#{@dev_shop.admin_url}/application_charges/1012637323.json", 'widget_store/charges/get_active_application_charge')

assert DiscoApp::ChargesService.activate(@dev_shop, @dev_new_charge)
assert @dev_new_charge.active?
Expand Down