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

Recurly_CouponRedemption::get() method throws inappropriate Recurly_NotFoundError exception. #81

Closed
trip-somers opened this issue Apr 14, 2014 · 8 comments
Labels
V2 V2 Client

Comments

@trip-somers
Copy link

A Recurly_NotFoundError exception is thrown by Recurly_CouponRedemption::get() when a valid/active account code has no coupons. Given that there is no other way to look for active coupons on an account, it can not be considered bad practice to use the get() method to check for them. As such, the get() method should return FALSE (or null) instead of an exception when the account code is valid/active and only return a Recurly_NotFoundError exception when the account code is invalid.

@drewish
Copy link

drewish commented Apr 14, 2014

Could you provide a little more sample code for what you're trying?

If you know the account you can just do:

$account = Recurly_Account::get('d1c1ded5e7c7c6db1cbfda420b1d5c');
if ($account->redemption) {
  var_dump($account->redemption->get());
}

@trip-somers
Copy link
Author

I do know the account code. The problem is that get() throws an exception if the selected account has no coupon. Here is my code:

    try
    {
        return Recurly_CouponRedemption::get( $account->account_code );
    }
    catch (Recurly_NotFoundError $e)
    {
        return throw new Exception('Account not found.');
    }
    catch (Recurly_Error $e)
    {
        throw new Exception($e->getMessage());
    }

In this case, the account_code was '14'. Initially, there was no coupon on the account, and my code was spitting out 'Account not found.' Once I added a coupon to account, the exception disappeared. My conclusion: it throws an exception when there's no coupon.

I updated my function to look like this:

    try
    {
        return Recurly_CouponRedemption::get( $account->account_code );
    }
    catch (Recurly_NotFoundError $e)
    {
        return null;
    }
    catch (Recurly_Error $e)
    {
        throw new Exception($e->getMessage());
    }

I consider that a work-around because if there ever is a bad account code passed to this function, it will now just skip right over the exception.

@trip-somers
Copy link
Author

Whoops. I just noticed that your comment suggested going through the Recurly_Account object. I will try that, but I still think the way this exception is thrown is an issue.

@drewish
Copy link

drewish commented Apr 14, 2014

I definitely see what you're saying. While I agree that the throwing obnoxious, it is consistent with the rest of the API so I'm reluctant to change it. Give the approach I'd outlined with the fetch of the account a try.

@drewish
Copy link

drewish commented Apr 14, 2014

Side note: we really need to a better job of documenting the use of ->get() to follow relations between records. It opens up a lot of functionality that's not obvious at first.

@trip-somers
Copy link
Author

Yes. Agree about ->get(). I discovered it by accident on Friday.

I disagree about the consistency with the rest of the API, though. The other NotFoundError exceptions are typically thrown when looking for something specific (account, transaction, subscription, something with an ID), not something that may or may not exist (like a redemption that has no ID).

Ultimately, I can't make you change your mind, but at the very least, it would make sense to be able to differentiate between a "bad account code" and a "no redemptions found" response.

@trip-somers
Copy link
Author

Your suggestion is working for me, so I will leave this issue here for you to handle as you see fit.

Thanks for the help and the quick responses.

@drewish
Copy link

drewish commented Apr 14, 2014

Well thank you for the feedback. I'm going to close this but I updated the docs on http://docs.recurly.com/api/coupons/coupon-redemption#lookup-redemption-account to use the snipped I posted here. Hopefully it'll make it easier for the next person.

@drewish drewish closed this as completed Apr 14, 2014
@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
V2 V2 Client
Projects
None yet
Development

No branches or pull requests

3 participants