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

Update client library to support entitlements #595

Merged
merged 2 commits into from
Oct 26, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions recurly/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,42 @@ def create_usage(self, sub_add_on, usage):

Subscription._classes_for_nodename['subscription'] = Subscription

class CustomerPermission(Resource):
"""CustomerPermission"""

attributes = (
'id',
'code',
'name',
'description',
)

class Entitlements(Resource):
"""Entitlments available on an account"""

member_path = 'entitlements/%s'
collection_path = 'entitlements'

nodename = 'entitlement'

attributes = (
'created_at',
'updated_at'
)
_classes_for_nodename = { 'customer_permission': CustomerPermission }

# This is a 'bit' of a workaround in that we have an array of Entitlements, which include
# a nested array of 'granted_by', which is just single elements in which we need the href only.
# The loop in resource.py#value_for_element assumes that the array will be the child of the main class,
# in this case Entitlement, which it is not. So override it to get the array of hrefs
@classmethod
def value_for_element(cls, elem):
excludes = ['granted_by']
if elem is None or elem.tag not in excludes or elem.attrib.get('type') != 'array':
return super(Entitlements, cls).value_for_element(elem)

return [code_elem.attrib['href'] for code_elem in elem]

class TransactionBillingInfo(recurly.Resource):
node_name = 'billing_info'
attributes = (
Expand Down
32 changes: 32 additions & 0 deletions tests/fixtures/entitlements/account-created.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
POST https://api.recurly.com/v2/accounts HTTP/1.1
X-Api-Version: {api-version}
Accept: application/xml
Authorization: Basic YXBpa2V5Og==
User-Agent: {user-agent}
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="UTF-8"?>
<account>
<account_code>entitlementmock</account_code>
</account>

HTTP/1.1 201 Created
Content-Type: application/xml; charset=utf-8
Location: https://api.recurly.com/v2/accounts/entitlementmock

<?xml version="1.0" encoding="UTF-8"?>
<account href="https://api.recurly.com/v2/accounts/entitlementmock">
<adjustments href="https://api.recurly.com/v2/accounts/entitlementmock/adjustments"/>
<billing_info href="https://api.recurly.com/v2/accounts/entitlementmock/billing_info"/>
<invoices href="https://api.recurly.com/v2/accounts/entitlementmock/invoices"/>
<subscriptions href="https://api.recurly.com/v2/accounts/entitlementmock/subscriptions"/>
<transactions href="https://api.recurly.com/v2/accounts/entitlementmock/transactions"/>
<entitlements href="https://api.recurly.com/v2/accounts/entitlementmock/entitlements"/>
<account_code>entitlementmock</account_code>
<username nil="nil"></username>
<email nil="nil"></email>
<first_name nil="nil"></first_name>
<last_name nil="nil"></last_name>
<company_name nil="nil"></company_name>
<accept_language nil="nil"></accept_language>
</account>
28 changes: 28 additions & 0 deletions tests/fixtures/entitlements/list.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
GET https://api.recurly.com/v2/accounts/entitlementmock/entitlements HTTP/1.1
X-Api-Version: {api-version}
Accept: application/xml
Authorization: Basic YXBpa2V5Og==
User-Agent: {user-agent}


HTTP/1.1 200 OK
X-Records: 1
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="UTF-8"?>
<entitlements type="array">
<entitlement>
<customer_permission>
<id>rlvkez1y6fip</id>
<code>VIP-Meet-And-Greet</code>
<name>Meet the team!</name>
<description>All VIP members can meet the team.</description>
</customer_permission>
<granted_by type="array">
<subscription href="https://api.recurly.com/v2/subscriptions/rhind9aehvrt"/>
<external_subscription href="https://api.recurly.com/v2/external_subscriptions/rlhjggnogtc5"/>
</granted_by>
<created_at type="datetime">2022-09-20T15:50:56Z</created_at>
<updated_at type="datetime">2022-09-20T15:50:56Z</updated_at>
</entitlement>
</entitlements>
19 changes: 19 additions & 0 deletions tests/test_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,25 @@ def test_dunning_campaign(self):
update = campaign.bulk_update('testmock', ['pc-967-343'])
self.assertIsNone(update)

def test_account_entitlements(self):
account = Account(account_code='entitlement%s' % self.test_id)
with self.mock_request('entitlements/account-created.xml'):
account.save()

with self.mock_request('entitlements/list.xml'):
entitlement = account.entitlements()[0]
customer_permission = entitlement.customer_permission
self.assertEqual(customer_permission.id, 'rlvkez1y6fip')
self.assertEqual(customer_permission.code, 'VIP-Meet-And-Greet')
self.assertEqual(customer_permission.name, 'Meet the team!')
self.assertEqual(customer_permission.description, 'All VIP members can meet the team.')
self.assertEqual(entitlement.created_at.strftime('%x'), '09/20/22')
self.assertEqual(entitlement.updated_at.strftime('%x'), '09/20/22')
self.assertEqual(entitlement.granted_by, [
'https://api.recurly.com/v2/subscriptions/rhind9aehvrt',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the client lib perspective, do we really want to respond with the resource link? I mean, I know that the entitlements API endpoint already responds with the granted_by node with one array that contains the resource links, but there it makes sense because, in my opinion, it's better to keep the API response clean and it is easy to navigate and check out the resource(by just click into the resource link, for example). But for the client lib, we should expect that someone is embedding calls from this lib into their code. For example:

try {
  $entitlements = Recurly_EntitlementList::get('<ACCOUNT_CODE>');
  foreach ($entitlements as $entitlement) {
    print "Entitlement: {$entitlement->granted_by}\n";
  }
} catch (Recurly_NotFoundError $e) {
  print "Invalid account code: $e";
}

So what the consumer of this lib is supposed to do with the subscription/external subscription resource link? Isn't better to respond with the full subscription or external subscription object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I somewhat disagree with the client lib doing more than the API response. Why would they be any different? If we wanted the user to have more detail we would include it in the presenter.

When the API discussion occurred the idea was that if the merchant wanted more details, they would make the request themselves. I think that the logic process is "First and foremost does this user have this entitlement?" and that typically the answer is yes or no and that's the end.

If the merchant thinks the user shouldn't have it, then they can go digging on which granted_by is giving them the details.

I see this as a similar way that we show plan_codes in the coupon request. We simply return the plan_code and no other details about the plan.

Copy link

@lsfernandes92 lsfernandes92 Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would they be any different?

I don't know really and I am just questioning hehehehe. Why does the merchant prefer to use the client lib instead of just requesting the entitlements API?

What's the value of having the resource link in the client lib response? What really matters here is the subscriptions IDs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value is that they don't have to write their own API wrapper. In theory what we publish is 'official' and won't break, unlike a piece of code written by a dev who leaves a year later and no one understands it.

I think the point is that we as developers don't actually know what the client wants or what the value is, we rely on Chris and Dom to figure that out for us.

'https://api.recurly.com/v2/external_subscriptions/rlhjggnogtc5'
])

def test_invoice_templates(self):
with self.mock_request('invoice_templates/list.xml'):
template = InvoiceTemplate.all()[0]
Expand Down