Closed tm1000 closed 3 years ago
@Skullbock š
Hi @tm1000 i've not been able to test this yet. I have a project that uses this, and i'll be updating in this week so i'll probably have some feedback
Hey @tm1000 i confirm that this works as expected. I think we're ready for a release of a v4. If you have any addition / breaking change, now's the time!
@Skullbock I think its fine. Im not using it quite yet (Im still on v3 for my unreleased work) the reasoning is only because I'd have to fork Crm and Books to be able to accept our changes to client
@tm1000 you can fetch my branch for zoho crm using the dev-
syntax for composer.
But anyway, i think i'll release this as v4 next week and work on stabilizing the crm library with a v5.
Don't forget about the books library too!
@tm1000 i release v4 of zoho client, and tagged a v4.5.0 of zoho books that allows for v3 and v4 of the client This way, along with the crm branch, you should be able to use v4 on the 2 projects
cheers
@Skullbock I think you broke your stable releases by doing that. I'm getting reports from my engineers that books broke between books v4 and the new crm.
I'm going to investigate in a few minutes to see what the issue is
Really strange, since the v4 public api is the same.
Maybe it's the usage of Region
?
If so, i'll revert with a 4.5.1
and publish a 5.0.0
with just the new dependency
It might be. I haven't had a chance to check yet. This staff member works in Columbia so he's three hours ahead of me. I was going to check the issue and give you some prs to fix it but wanted to let you know if you were still around
I'll be around for another hour or so. Let me know, i'll revert / fix if need be in this hour
Actually I believe The reasoning is because we don't have a dependency on the client. So if crm or books has a dependency of 4 or 5 composer will choose 5 and the client constructor is different between old client and new client.
You can fix that by forcing the require. Anyway, i'll avoid that problem for you and revert with a 4.5.1 and push a 5.0.0 so you can choose
Pushed 4.5.1
and 5.0.0
@Skullbock these changes made it into the v4 branch but you merged the develop branch into master not v4. So the published 4.0.0 is actually missing all of the work from this PR and is essentially broken for what I need.
I don't know what the fuck happened. I think my local git client got fucked up or something.
Anyway I'll merge this tomorrow morning. šš»
@Skullbock don't worry. I was just as confused š
@tm1000 should've release 4.1.0 correctly! let me know if anything still is amiss
@Skullbock you didnt publish to packagist https://packagist.org/packages/weble/zohoclient
Additionally crm and books still link against -dev. I've figured out a way to work around it for now for testing (my CRM tests all passed yay!)
Hey @tm1000 this is on pourpose, we were testing it It seems to work, so we'll probably release later today
Asad\OAuth2\Client\AccessToken\ZohoAccessToken
in place ofLeague\OAuth2\Client\Token\AccessToken
since we are usingAsad
anywaysAdditionally
Thoughts
getAuthenticatedRequest
against the provider for other libraries. However there might be a benefit in supplying a different client between oauth and crm/books for things like retries.setAccessToken
? Seems counter-intuitive. Additionally testing this is kind of difficult as we'd need a way to abstractaccessToken
, the call togetAccessToken()
returns a string (maybe it should return theaccessToken
itself). So at the very least you'd need asetAccessTokenObjectThing()
to be able to testsetAccessToken