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 Sugar client token handling #135

Merged
merged 3 commits into from
Jul 16, 2020
Merged

Update Sugar client token handling #135

merged 3 commits into from
Jul 16, 2020

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Jun 8, 2020

auth._bearerToken is undefined as of 3.3.0, so pass the token to Sugar from auth.checkBearerToken().
Check the current token before each Sugar API request.

auth._bearerToken is undefined as of 3.3.0, so pass the token to Sugar from auth.checkBearerToken().
Check the current token before each Sugar API request.
@eatyourgreens eatyourgreens requested a review from a team June 18, 2020 12:46
@wgranger wgranger self-requested a review July 9, 2020 14:37
Copy link
Contributor

@wgranger wgranger left a comment

Choose a reason for hiding this comment

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

There's a breaking change here in the inclusion of the beforeEveryRequest mixin with the JSONAPIClient instantiation. For reference, here is where mixins are merged in the object constructor.

I tested this with npm link on this branch against a local version of PFE. I noticed a crash when visiting the "I Fancy Cats" project on staging. PFE worked well when I took out the mixin (lines 13-19 in sugar.js).

lib/sugar.js Outdated Show resolved Hide resolved
lib/sugar.js Outdated Show resolved Hide resolved
@eatyourgreens
Copy link
Contributor Author

Testing this, I noticed that PFE is still on 3.0.0-rc.0 so is behind the latest client by a couple of minor versions.

Copy link
Contributor

@wgranger wgranger left a comment

Choose a reason for hiding this comment

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

Nice. This looks good to me. I can't think of any reason not to bump PJC on PFE. Perhaps that can happen after this is merged and published.

@eatyourgreens eatyourgreens merged commit f4ecebe into master Jul 16, 2020
@eatyourgreens eatyourgreens deleted the sugar-token branch July 16, 2020 21:19
eatyourgreens added a commit that referenced this pull request May 6, 2021
Missed in #135: Promise.all should take an array as an argument. I've also added better error-logging to debug Sugar client errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants