Skip to content

Commit

Permalink
Merge pull request #281 from discolabs/feature/dont-activate-charges
Browse files Browse the repository at this point in the history
Don't activate charges via API
  • Loading branch information
gavinballard committed Oct 11, 2021
2 parents d720793 + 68cea47 commit f4c2b77
Show file tree
Hide file tree
Showing 14 changed files with 2,020 additions and 2,018 deletions.
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

0 comments on commit f4c2b77

Please sign in to comment.