-
Notifications
You must be signed in to change notification settings - Fork 78
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
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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: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?
There was a problem hiding this comment.
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 theplan_code
and no other details about the plan.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
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.