Adds preliminary support for Klarna #1059

edward · 2014-03-04T16:17:17Z

@bslobodin @louiskearns Here’s my first draft for Klarna support.

Plan:

*Cart item data that Klarna needs: 'type', 'reference', 'quantity', 'unit_price', 'tax_rate'

edward · 2014-03-04T16:27:05Z

Test failed due to a missing SSH key in Circle CI. Related issue logged here: https://github.com/Shopify/ops/issues/569

bslobodin · 2014-03-04T17:29:48Z

AM is actually using Travis (https://travis-ci.org/Shopify/active_merchant). Yes, someone added a Circle project as well, but it was never fully set up AFAICT.

edward · 2014-03-04T18:52:13Z

@bslobodin Ah, yeah, I see that now.

I’m going to have the generated tests be skipped for now.

odorcicd · 2014-03-19T15:17:29Z

lib/active_merchant/billing/integrations/klarna.rb

+      module Klarna
+
+        mattr_accessor :service_url
+        self.service_url = 'https://hpp-staging-eu.herokuapp.com/api/v1/checkout'


Add support for both staging and live? This looks like just a test url

edward · 2014-03-24T14:49:02Z

@bslobodin @odorcicd Could you please take a look at what’s been done so far?

edward · 2014-03-27T19:08:39Z

lib/active_merchant/billing/integrations/klarna/notification.rb

+          end
+
+          # Acknowledge the transaction to Klarna. This method has to be called after a new
+          # apc arrives. Klarna will verify that all the information we received are correct and will return a


@odorcicd @bslobodin What’s an apc in this context?

(This is part of the generated stuff that AM made for me.)

I have no idea what it actually stands for, but it's a callback we get from the gateway with the status of the payment and acknowledge needs to check if it can be trusted before further processing takes place, i.e. order update.

edward · 2014-04-16T21:38:04Z

@bslobodin @odorcicd I think this guy is pretty much done. Can you please take a look?

bslobodin · 2014-04-17T13:00:08Z

lib/active_merchant/billing/integrations/klarna.rb

+             merchant_checkout_uri
+             merchant_base_uri
+             merchant_confirmation_uri).each do |required_field|
+            raise ArgumentError, "Missing required field #{required_field}" unless fields.has_key?(required_field)


Perhaps unless fields[required_field].present??

I feel like has_key? is more readable here and tells a better story – I care about whether a key is set at all.

Conversely, fields[required_field].present? is more about whether the value of that hash key is nil or not, which isn’t exactly the same thing.

To me required suggests that it should be raising when values are nil or empty as well.

edward · 2014-04-28T17:53:07Z

@bslobodin @odorcicd

I think we’re good here, but I just wanted to ping you before I squash, bump the version number, and release a new version of ActiveMerchant.

The remaining unresolved issues are:

guessing the locale from the billing address country may cause an exception, but only under truly exceptional circumstances – Klarna shouldn’t even appear as an option if the buyer hasn’t specified one of the countries that the locale-guessing method is aware of
Klarna has confirmed to be throwing away information other than the shipping address email and shipping address postal code – beyond pushing Klarna not to do this, there’s not much we can do on the AM side to correct this UX oddity

Anything else that I’ve missed before I squash and bump?

odorcicd · 2014-04-28T17:57:08Z

lib/active_merchant/billing/integrations/klarna/helper.rb

+
+          private
+
+          def street_address


odorcicd · 2014-04-28T18:05:56Z

@edward still find it weird that Klarna would blow up if there's someone making a purchase from outside one of those countries. This means any merchant using Klarna can't have any international customers?

bslobodin · 2014-04-28T18:07:07Z

lib/active_merchant/billing/integrations/helper.rb

+          self.currency    = options[:currency]
+          self.credential2 = options[:credential2]
+          self.credential3 = options[:credential3]
+          self.credential4 = options[:credential4]


These used to be aligned?

edward · 2014-04-28T18:43:33Z

@odorcicd the Klarna gateway page will currently blow up if they are somehow sent a customer with a billing address country that is not in their supported list. Here’s a picture of the error:

odorcicd · 2014-04-28T18:49:15Z

@edward do they also blowup if the billing information put in is a US address, for example?

bslobodin · 2014-04-28T18:49:28Z

test/unit/integrations/helpers/klarna_helper_test.rb

+
+  def test_basic_helper_fields
+    assert_field 'purchase_currency', @options[:currency]
+


Empty line?

bslobodin · 2014-04-28T18:52:34Z

test/unit/integrations/klarna_module_test.rb

+      'merchant_confirmation_uri' => 'http://some-webstore.se?URI=confirmation'
+    }
+
+    cart_items =[{:type => 'physical',


Missing space after =.

edward · 2014-04-29T15:16:25Z

test/unit/integrations/helpers/klarna_helper_test.rb

+    assert_field 'cart_item-0_name', item[:name].to_s
+    assert_field 'cart_item-0_quantity', item[:quantity].to_s
+    assert_field 'cart_item-0_unit_price', item[:unit_price].to_s
+    assert_field 'cart_item-0_tax_rate', '1000'


Something feels wrong about this calculated amount. Is the formula correct?

Hm, yea, the rate should be %ge? If you set the below, the rate should be 10% or 0.1, depending on how it needs to be expressed to Klarna.

:unit_price => Money.new(9.00), :tax_amount => Money.new(0.90)

The rate as expressed to Klarna should be a percentage multiplied by 100
and converted to an integer.

Am I missing something?

—
Edward https://www.google.ca/search?q=edward+ocampo-gooding at Shopify

On Tue, Apr 29, 2014 at 11:21 AM, Boris Slobodin
notifications@github.comwrote:

In test/unit/integrations/helpers/klarna_helper_test.rb:

def test_merchant_digest

@Helper = valid_helper

assert_field 'merchant_digest', "/fUAJJiWEluzrwL1AX8pptGFYP3T1gWIv+tafLnARzY="

end

def test_line_item

item = example_line_item

@helper.line_item(item)

assert_field 'cart_item-0_type', item[:type].to_s

assert_field 'cart_item-0_reference', item[:reference].to_s

assert_field 'cart_item-0_name', item[:name].to_s

assert_field 'cart_item-0_quantity', item[:quantity].to_s

assert_field 'cart_item-0_unit_price', item[:unit_price].to_s

assert_field 'cart_item-0_tax_rate', '1000'

Hm, yea, the rate should be %ge? If you set the below, the rate should be
10% or 0.1, depending on how it needs to be expressed to Klarna.

:unit_price => Money.new(9.00),
:tax_amount => Money.new(0.90)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1059/files#r12098609
.

edward · 2014-04-29T15:27:10Z

Squash-merged into master

edward · 2014-06-16T13:35:49Z

@ntalbott Ok, deleted

patrickdet · 2014-10-01T15:45:24Z

Whatever happened to this Klarna patch? It's nowhere to be found but in this pull-request. Did you decide to drop support for Klarna?
/cc @edward

edward · 2014-10-01T18:52:45Z

@patrickdet “It’s complicated”

Shopify and Klarna are still talking about how we’re going to integrate. I forget what happened to the PR.

beranPro · 2016-07-14T07:55:14Z

anything new on this PR ?
Are there any plans to integrate Klarna ?

odorcicd reviewed Mar 19, 2014
View reviewed changes

edward reviewed Mar 27, 2014
View reviewed changes

bslobodin reviewed Apr 17, 2014
View reviewed changes

odorcicd reviewed Apr 28, 2014
View reviewed changes

bslobodin reviewed Apr 28, 2014
View reviewed changes

Inline street_address

116cf51

bslobodin reviewed Apr 28, 2014
View reviewed changes

Correct indentation

5560cf1

bslobodin reviewed Apr 28, 2014
View reviewed changes

Edward Ocampo-Gooding added 8 commits April 28, 2014 19:00

Fix formatting

71cd3ce

Split test out

9a3ce57

Add signature test

d521276

Add calculated tax rates

09de314

Actually implement tax rates properly

9d99fc1

Default to using sv-se as fallback locale

95d5880

Remove minitest/autorun

cae9e8b

whitespace

2c57418

edward reviewed Apr 29, 2014
View reviewed changes

Do tax calculation math correctly

b53abb4

edward closed this Apr 29, 2014

edward deleted the klarna branch June 16, 2014 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds preliminary support for Klarna #1059

Adds preliminary support for Klarna #1059

edward commented Mar 4, 2014

edward commented Mar 4, 2014

bslobodin commented Mar 4, 2014

edward commented Mar 4, 2014

odorcicd Mar 19, 2014

edward commented Mar 24, 2014

edward Mar 27, 2014

edward Mar 27, 2014

bslobodin Mar 27, 2014

edward commented Apr 16, 2014

bslobodin Apr 17, 2014

edward Apr 17, 2014

bslobodin Apr 17, 2014

edward commented Apr 28, 2014

odorcicd Apr 28, 2014

edward Apr 28, 2014

odorcicd commented Apr 28, 2014

bslobodin Apr 28, 2014

edward commented Apr 28, 2014

odorcicd commented Apr 28, 2014

bslobodin Apr 28, 2014

bslobodin Apr 28, 2014

edward Apr 29, 2014

bslobodin Apr 29, 2014

edward Apr 29, 2014

edward commented Apr 29, 2014

edward commented Jun 16, 2014

patrickdet commented Oct 1, 2014

edward commented Oct 1, 2014

beranPro commented Jul 14, 2016


		def test_basic_helper_fields
		assert_field 'purchase_currency', @options[:currency]

Adds preliminary support for Klarna #1059

Adds preliminary support for Klarna #1059

Conversation

edward commented Mar 4, 2014

edward commented Mar 4, 2014

bslobodin commented Mar 4, 2014

edward commented Mar 4, 2014

Choose a reason for hiding this comment

edward commented Mar 24, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edward commented Apr 16, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edward commented Apr 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odorcicd commented Apr 28, 2014

Choose a reason for hiding this comment

edward commented Apr 28, 2014

odorcicd commented Apr 28, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edward commented Apr 29, 2014

edward commented Jun 16, 2014

patrickdet commented Oct 1, 2014

edward commented Oct 1, 2014

beranPro commented Jul 14, 2016