Hi,

What do you think about the switching to the official PHP SDK https://github.com/VKCOM/vk-php-sdk?

CommentFileSizeAuthor
#11 switch_to_official_sdk-2953973-12.patch21.51 KBugolek
#10 switch_to_official_sdk-2953973-10.patch19.76 KBugolek
#5 switch_to_official_sdk-2953973-5.patch19.51 KBdavps

Comments

lebaster@gmail.com created an issue. See original summary.

gvso’s picture

gvso
Primary language Spanish
commented

One of the reasons the current release is an alpha version is because we want to use the official library instead of the PHP League OAuth2 client library. If you want to work on this, please go ahead!

davps’s picture

davps commented
Status: Active » Needs review

Ported and manually tested.

https://github.com/drupalsocialinitiative/social_auth_vk/pull/3/files

ugolek’s picture

ugolek commented

Hi david.pashaev, I just tested on http://dev-social-auth-vk.pantheonsite.io/, and looks like all working well with the latest Drupal 8 and PHP 7.1.

Hi gvso, I think you could accept the PR from david.pashaev.

Thanks

davps’s picture

davps commented
StatusFileSize
new switch_to_official_sdk-2953973-5.patch19.51 KB

Attaching patch from pull request

Status: Needs review » Needs work

The last submitted patch, 5: switch_to_official_sdk-2953973-5.patch, failed testing. View results

gvso’s picture

gvso
Primary language Spanish
commented

I will look at your patches to the Social API by this Sunday. Thanks for your contributions.

gvso’s picture

gvso
Primary language Spanish
commented

I wonder why the tests failed

gvso’s picture

gvso
Primary language Spanish
commented 
  1. +++ b/src/Controller/VkontakteAuthController.php
@@ -108,85 +106,102 @@ public static function create(ContainerInterface $container) {
+    $plugin = $this->networkManager->createInstance('social_auth_vk');
+
+    return $plugin->getSdk();

    Just return the SDK directly without the temporal variable $plugin 

  2. +++ b/src/Controller/VkontakteAuthController.php
@@ -108,85 +106,102 @@ public static function create(ContainerInterface $container) {
+      drupal_set_message($this->t($exception->getMessage()), 'error');

    Print an "user-friendly" message here and log the error with the logger 

  3. +++ b/src/Controller/VkontakteAuthController.php
@@ -108,85 +106,102 @@ public static function create(ContainerInterface $container) {
+      drupal_set_message($this->t($exception->getMessage()), 'error');

    Same as the previous one. 

  4. +++ b/src/VkontakteAuthManager.php
@@ -2,81 +2,280 @@
 class VkontakteAuthManager extends OAuth2Manager {

    I haven't tested this patch, but I don't like the way this file was implemented. It doesn't follow the same behavior as the other implementers, and that's something we want to avoid as much as possible

ugolek’s picture

ugolek commented
StatusFileSize
new switch_to_official_sdk-2953973-10.patch19.76 KB

Hi @gvso,

I fixed some mentioned issue. But I also check other Social Auth related projects and they not follow the right way. We need to sort out this issue asap.

Thank you

ugolek’s picture

ugolek commented
StatusFileSize
new switch_to_official_sdk-2953973-12.patch21.51 KB

  • lebster committed 419a7e2 on 8.x-2.x 
    Issue #2953973 by lebster, david.pashaev: Switch to official PHP SDK
ugolek’s picture

ugolek commented
Status: Needs work » Fixed
ugolek’s picture

ugolek commented
Status: Fixed » Closed (fixed)