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

Redirect on Error #7

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

Michael-Stokoe
Copy link

A project I'm working on requires that the user be redirected back to my application if the user hits cancel in Xero's OAuth.

Luckily this is handled nicely by a request parameter.

See "Redirect on Error", here.

I've added the parameter to the Xero class and defaulted it to false.

Usage is pretty simple:

    $api = new XeroApi();
    $api->setRedirectOnError(true);

Now if the Xero part of OAuth fails for any reason (User hits cancel, can't remember password etc.), Xero will redirect us back to the application's callback url.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.5%) to 97.521% when pulling 61fd29c on consilience:master into dfa6b5b on Invoiced:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.5%) to 97.521% when pulling 61fd29c on consilience:master into dfa6b5b on Invoiced:master.

@coveralls
Copy link

coveralls commented May 8, 2019

Coverage Status

Coverage decreased (-0.8%) to 99.18% when pulling 78d7ebd on consilience:master into ea1af9f on Invoiced:master.

@Michael-Stokoe
Copy link
Author

Travis error was a composer error, unrelated to this PR, I believe.

@jaredtking
Copy link
Contributor

Thank you for the contribution! Can you add a test case for when the redirectOnError parameter is used?

@Michael-Stokoe
Copy link
Author

Michael-Stokoe commented May 8, 2019

No problem.

I realised after coveralls reported -2.5% C.C. that I hadn't included any tests with it. Been a pretty busy day but I'll get it sorted. Cheers.

@Michael-Stokoe
Copy link
Author

Sorry it's taken me so long to get around to it, I'm sure you know how it is 😅

@Michael-Stokoe
Copy link
Author

Sorry about that. I had to remove a typehint to get the PHP5.6 tests to pass.

@jaredtking
Copy link
Contributor

Thank you for the PR! Can you add a test case for urlAuthorization() when the redirectOnError parameter is enabled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants