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

[ACCOUNT-2797] refactor: remove guzzle dependency #468

Open
wants to merge 98 commits into
base: main
Choose a base branch
from

Conversation

hschoenenberger
Copy link
Contributor

@hschoenenberger hschoenenberger commented Jan 14, 2025

To go a step further reducing friction with the PrestaShop module's ecosystem, reducing module weight.

As a module we just need to do a few things, not to embed huge libraries so let's get straight to the point and isolate a much a possible from other modules to avoid collisions, also stay lighter.

  • Copy and adapt tests from oauth2-prestashop
  • HttpClient tests (and create an interface)
  • Default scopes config & redirect_uri mandatory field

Still a POC, but fully functional.

  • remove guzzle
  • remove theleagueoauth via prestashop-oauth2

Benefits:

  • reduce the zip size
  • eventually avoid future deprecation issues with Guzzle
  • note: module cannibalism was already fixed with php-scoping, however this simplifies the whole module packaging.

$oauth2Client = $this->getOAuth2Client();

$logoutUrl = $oauth2Client->getLogoutUri(
$oauth2Client->getPostLogoutRedirectUri(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decide if we also allow setting alternative postLoginUri with login flow

/**
* @var string
*/
protected $userAgent = 'ps_accounts/' . \Ps_accounts::VERSION;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Header string to be decided with the team


return $this->getSafeResponse($ch);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Put ?

} else {
$decodedBody = json_decode($body, true);

return is_array($decodedBody) ? $decodedBody : [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We expect only json object / list

*
* @throws OAuth2Exception
*/
public function getAuthorizationUri(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allow to specify another redirect uri here ?

@@ -0,0 +1,33 @@
<?php

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO

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

Successfully merging this pull request may close these issues.

3 participants