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

Add a method to return an ID from an href. #95

Closed
wants to merge 1 commit into from

Conversation

deviantintegral
Copy link

This PR adds a method to return an ID of an object without having to get() it from the Recurly API - particularly useful for stubs. For example, given an invoice UUID, to find the ID of an account you have to get() on the account stub, which can add quite a bit of lag to any requests. When all you want is the account code, it's a bit of overkill.

Now, this method works for me, but will certainly need some improvement. I went with the most non-invasive change, just to unblock my own work, but I have ideas for other, better approaches.

  1. Push the method down to Recurly_Stub, since if you've loaded an object you can directly access the id and you shouldn't need this method.
  2. Make the method abstract (since we don't have an interface) and require classes to implement it.
  3. Change every Recurly object to store the unique ID as a property, and assign it when constructing or getting.
  4. Decide that this isn't the library's problem at all, and make href the canonical identifier. However... I noticed that sometimes _href doesn't have my account's subdomain and is just api.recurly.com, so I don't know if this would cause problems.

Anyways, I figured I'd throw this out there before I did much more.

reden-tm pushed a commit to Theatermania-Inc/recurly-client-php that referenced this pull request Mar 31, 2015
@bhelx bhelx self-assigned this Oct 22, 2015
@bhelx
Copy link
Contributor

bhelx commented Oct 22, 2015

This is actually pretty useful, sorry we are only now looking at it. I'll review it as soon as I get the chance.

@aaron-junot
Copy link

I agree that this is pretty useful. Thank you for calling out the assumption that the last portion of the URL is an id. A few things this would not work for:
<redemptions href="https://your-subdomain.recurly.com/v2/invoices/1007/redemptions"/>
<subscriptions href="https://your-subdomain.recurly.com/v2/invoices/1007/subscriptions"/>
<credit_invoices href="https://your-subdomain.recurly.com/v2/invoices/1325/credit_invoices"/>
<billing_info href="https://your-subdomain.recurly.com/v2/accounts/1/billing_info"/>
<transactions href="https://your-subdomain.recurly.com/v2/accounts/1/transactions"/>
<notes href="https://your-subdomain.recurly.com/v2/accounts/1/notes"/>

This was just a quick glance through the API docs, I think there are probably more. I'm going to continue to think of a way around this, but since most of our ids are arbitrary strings (rather than, say, integers or a specific formatted string), it's going to be hard to figure out what the relevant ID in a generic way. And it would be unwise to hard-code in a solution to these edge cases because what if the API changes and the ID gets shifted around?

If anyone sees a solution that I've missed, please comment. This type of feature would be really nice if we can get it working for every href

@bhelx
Copy link
Contributor

bhelx commented Oct 15, 2018

I agree with @aaron-suarez here. Although this could be helpful, I don't want to officially support it if it doesn't handle every case and I can't see a way forward to make it work for every case. I'm going to close it since it's been 4 years. Feel free to continue discussion or re-open.

@bhelx bhelx closed this Oct 15, 2018
@bhelx bhelx added the V2 V2 Client label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants