search

Yoast / api-libs

4 stars 5 forks source link

Refresh tokens is broken #19

Closed Rarst closed 9 years ago

Rarst commented 9 years ago

Yoast_Google_Analytics_Client->refresh_tokens() attempts to get response via $response = $this->refreshToken( $refresh_token );, which is void return method.

I am not confident in what should this be adjusted to?

Likely cause for recurrent HTTP requests, see https://github.com/Yoast/google-analytics-for-wordpress/issues/261

andizer commented 9 years ago

I was diving into this piece and as far as I see, I did a wrong implementation of this. The method refreshToken will call refreshTokenRequest that already sets the accesstoken.

We should actually call method getAccessToken to store this temporary and validate this with isAccessTokenExpired.

jdevalk commented 9 years ago

As @rarst just showed me, this means we're doing a request to Google on every page load in the admin... We should fix this and release it ASAP.

andizer commented 9 years ago

@jdevalk I've fixed it for the GSC-client. I will fix this for the analytics client also. Probably I can take some of this into one file (like a yoast-api-client) for both clients.

GaryJones commented 9 years ago

As @rarst just showed me, this means we're doing a request to Google on every page load in the admin... We should fix this and release it ASAP.

screenshot 2015-07-09 21 10 56

Yup, every. single. page. And I've just had it delay loading a page by 7.9 seconds :-1:

jdevalk commented 9 years ago

We know. But I want to make sure it's 100% tested before we release it. We've done worse than this in a previous update, unfortunately. On Thu 9 Jul 2015 at 22:14 Gary Jones notifications@github.com wrote:

As @rarst https://github.com/rarst just showed me, this means we're doing a request to Google on every page load in the admin... We should fix this and release it ASAP.

[image: screenshot 2015-07-09 21 10 56] https://cloud.githubusercontent.com/assets/88371/8605784/795fd9b6-267f-11e5-83cd-0dab05d3883c.png

Yup, every. single. page.

— Reply to this email directly or view it on GitHub https://github.com/Yoast/api-libs/issues/19#issuecomment-120133378.