Merge lp:~elopio/u1-test-utils/payclient_data_objects into lp:u1-test-utils

[Leo Arias]

Updated the payclient helpers to use the new data object factory method of CreditCardRequest. Also removed the duplicated bits of data objects.

[Leo Arias (elopio)]

<pindonga> elopio, on the second mp: l. 8: this should be the result of landing the previous branch, not your own :)
<pindonga> elopio, l. 84: why redefine __init__ ? why not instead call super() and just add the extra pieces you need?
<pindonga> and I think that's sit
<elopio> pindonga: l. 8, yes, I left it in case you wanted to run the tests. I have it as work in progress to make sure I'll change it before landing.
<elopio> l. 84, I changed the country_code to a lazy property. So the super __init__ will fail if it tries to set it. I don't know a better solution. ideas?
<pindonga> elopio, you could just call super.__init__ passing country_code=self._get_country_code(country)
<pindonga> no need to have the lazy property at all
<elopio> pindonga: that makes more sense, right. I'll change it.

[Leo Arias (elopio)]

Reported bug #1165210 to follow up on the magic mock thing.

[Leo Arias (elopio)]

This requires https://code.launchpad.net/~elopio/payclient/data_objects/+merge/157434 to land first.

[Matias Bordese (matiasb)]

+1

review: Approve

=== modified file 'requirements.txt'
--- requirements.txt	2013-03-26 23:14:30 +0000
+++ requirements.txt	2013-04-05 21:17:20 +0000
@@ -5,6 +5,6 @@
 pyflakes
 Twisted
 bzr+ssh://bazaar.launchpad.net/~bloodearnest/localmail/trunk
-bzr+http://bazaar.launchpad.net/~ubuntuone-hackers/payclient/trunk@3
+bzr+http://bazaar.launchpad.net/~ubuntuone-hackers/payclient/trunk@4
 bzr+http://bazaar.launchpad.net/~canonical-isd-qa/selenium-simple-test/trunk@367
 bzr+http://bazaar.launchpad.net/~canonical-isd-hackers/canonical-identity-provider/ssoclient@3

=== modified file 'u1testutils/pay/client.py'
--- u1testutils/pay/client.py	2013-03-27 20:54:15 +0000
+++ u1testutils/pay/client.py	2013-04-05 21:17:20 +0000
@@ -59,22 +59,6 @@
             country is the name of the country in english.
 
         """
-        billing_address_dict = {
-            'street': billing_address.street_line1,
-            'state': billing_address.state,
-            'country': billing_address.get_country_code(),
-            'postal': billing_address.postal_code
-        }
-        kwargs = dict(
-            shop_id=shop_id,
-            open_id=user.openid,
-            card_type=credit_card.card_type,
-            card_holder=credit_card.name_on_card,
-            card_number=credit_card.card_number,
-            card_ccv=credit_card.ccv_number,
-            card_expiration_month=credit_card.expiration_month,
-            card_expiration_year=credit_card.expiration_year,
-            billing_address=billing_address_dict
-        )
-        credit_card_request = payclient.CreditCardRequest(**kwargs)
+        credit_card_request = payclient.CreditCardRequest.from_data_objects(
+            shop_id, user.openid, credit_card, billing_address)
         return self.client.new_credit_card(data=credit_card_request)

=== modified file 'u1testutils/pay/data.py'
--- u1testutils/pay/data.py	2013-03-27 18:36:28 +0000
+++ u1testutils/pay/data.py	2013-04-05 21:17:20 +0000
@@ -14,20 +14,11 @@
 
 import uuid
 
-
-class CreditCard(object):
-
-    def __init__(self, card_type, name_on_card, card_number, ccv_number,
-                 expiration_month, expiration_year):
-        self.card_type = card_type
-        self.name_on_card = name_on_card
-        self.card_number = card_number
-        self.ccv_number = ccv_number
-        self.expiration_month = expiration_month
-        self.expiration_year = expiration_year
-
-    def __repr__(self):
-        return '%s(%r)' % (self.__class__, self.__dict__)
+import payclient.data
+
+
+class CreditCard(payclient.data.CreditCard):
+    """Extends the payclient CreditCard to add factory methods for testing."""
 
     @classmethod
     def make_test_visa_card(cls, unique_id=None):
@@ -43,19 +34,24 @@
                    expiration_month, expiration_year)
 
 
-class Address(object):
-
-    def __init__(self, street_line1=None, street_line2=None, city=None,
-                 state=None, postal_code=None, country=None):
+class Address(payclient.data.Address):
+    """Extends the payclient Address to add factory methods for testing.
+
+    It also receives a country name in the constructor, instead of a country
+    code, receives the street_line2 and city, and concatenates street_line1
+    and street_line2 in the street attribute that the parent class receives.
+
+    """
+
+    def __init__(self, street_line1, street_line2, city, state, postal_code,
+                 country):
+        self.country = country
         self.street_line1 = street_line1
         self.street_line2 = street_line2
         self.city = city
-        self.state = state
-        self.postal_code = postal_code
-        self.country = country
-
-    def __repr__(self):
-        return '%s(%r)' % (self.__class__, self.__dict__)
+        street = '{0}\n{1}'.format(street_line1, street_line2)
+        super(Address, self).__init__(
+            street, state, postal_code, self._get_country_code())
 
     @classmethod
     def make_unique(cls, unique_id=None):
@@ -70,7 +66,7 @@
         return cls(
             street_line1, street_line2, city, state, postal_code, country)
 
-    def get_country_code(self):
+    def _get_country_code(self):
         # TODO add more?
         country_data = {'United States': 'US'}
         return country_data.get(self.country)

=== modified file 'u1testutils/pay/selftests/unit/test_pay_api_client.py'
--- u1testutils/pay/selftests/unit/test_pay_api_client.py	2013-03-27 20:54:15 +0000
+++ u1testutils/pay/selftests/unit/test_pay_api_client.py	2013-04-05 21:17:20 +0000
@@ -73,21 +73,7 @@
                 as mock_api_call:
             api_client.add_new_credit_card(
                 shop_id, self.user, self.credit_card, self.billing_address)
-        expected_params = {
-            'shop_id': shop_id,
-            'open_id': self.user.openid,
-            'card_type': self.credit_card.card_type,
-            'card_holder': self.credit_card.name_on_card,
-            'card_number': self.credit_card.card_number,
-            'card_ccv': self.credit_card.ccv_number,
-            'card_expiration_month': self.credit_card.expiration_month,
-            'card_expiration_year': self.credit_card.expiration_year,
-            'billing_address': {
-                'street': self.billing_address.street_line1,
-                'state': self.billing_address.state,
-                'postal': self.billing_address.postal_code,
-                'country': 'US',
-            }
-        }
         mock_api_call.assert_called_with(
-            data=payclient.CreditCardRequest(**expected_params))
+            data=payclient.CreditCardRequest.from_data_objects(
+                shop_id, self.user.openid, self.credit_card,
+                self.billing_address))