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

Integration binding, docs #78

Open
fidel-ruiz opened this issue Jun 22, 2018 · 9 comments
Open

Integration binding, docs #78

fidel-ruiz opened this issue Jun 22, 2018 · 9 comments

Comments

@fidel-ruiz
Copy link

I'm trying to follow the steps to integrate the wrapper with my store, according to the readme basically I have to bind my own shop model that implements the interface ShopBaseInfo and the ShopAccessInfo with this one is just passing the access token or is another Eloquent Model implementing that interface.

I couldn't instantiate a Variant as is shown in the Readme

@rickywiens
Copy link
Contributor

If you are copying this part verbatim:

/** @var BoldApps\ShopifyToolkit\Models\Variant $variant */
$variant = $variantService->getById(2641814487051);

You will need to replace that integer with a variant_id from a variant in your Shopify store.

Where specifically are you running into an issue after you have created classes that implement the 3 required interfaces in the README?

@fidel-ruiz
Copy link
Author

fidel-ruiz commented Jun 22, 2018

Let me try to break things here:
this code from register function on my AppServiceProvider

public function register()
    {
        $this->app->bind(\BoldApps\ShopifyToolkit\Contracts\ApiSleeper::class,
        \BoldApps\ShopifyToolkit\Support\ShopifyApiHandler::class);
        
        $this->app->instance(ShopBaseInfo::class, Shop::class );
        $this->app->instance(ShopAccessInfo::class, AccessToken::class);

    }

Then in one of my routes, I'm trying to create an instance of the services

Route::get('/test', function () {
    $variantService = new Variant();
    $variant = $variantService->getById($product_id);
    return 'Hello world';
});

I'm getting this(I should've sent this before sorry for that):
Type error: Too few arguments to function BoldApps\ShopifyToolkit\Services\Base::__construct(), 0 passed in /home/fidel/workspace/poc/php/bold-shopify-integration/routes/api.php on line 23 and exactly 1 expected

This is because the Variant service extends from the Base Service which needs a Client passed to his constructor

    $client = new Client(new Shop(), new AccessToken(), new GuzzleClient(), new ShopifyApiHandler());

I now have this part working by setting this and then using it like:

     $variantService = new Variant($client);

Is this the correct way of using it???
Now I getting a:
Client error: GET https://havana-meal.myshopify.com/admin/variants/1523866566714.json resulted in a 401 Unauthorized response:\n
{"errors":"[API] Invalid API key or access token (unrecognized login or wrong password)"}

Which implies that something must be wrong with my credentials but I'm using the exact same in mine POSTMAN test with the Rest Admin API from Shopify and is properly working.

@theRTC204
Copy link
Contributor

When newing an instance of Client you need to pass in hydrated models that implement ShopBaseInfo and ShopAccessInfo contracts. Client will use those models to grab your auth token & shop domain.

Looking at your code snippet you're just passing in an empty model that you're initializing in the constructor.

@fidel-ruiz
Copy link
Author

fidel-ruiz commented Jun 22, 2018

I didn't post my models code but is something like:

namespace App;

use BoldApps\ShopifyToolkit\Contracts\ShopBaseInfo;
use Illuminate\Database\Eloquent\Model;

class Shop extends Model implements ShopBaseInfo
{
    private $myshopifyDomain = 'havana-meal.myshopify.com';

    public function getMyShopifyDomain()
    {
        return $this->myshopifyDomain;
    }

    public function getDomain()
    {
        // TODO: Implement getDomain() method.
    }

}
namespace App;

use BoldApps\ShopifyToolkit\Contracts\ShopAccessInfo;
use Illuminate\Database\Eloquent\Model;

class AccessToken extends Model implements ShopAccessInfo
{
    public function getToken()
    {
        return 'token****'; 
    }
}

***token = btoa('apikey:password') is this correct?

@rickywiens
Copy link
Contributor

rickywiens commented Jun 25, 2018

The token for the ShopAccessInfo is usually the access_token from a successful oAuth process with a Shop after installation: https://help.shopify.com/api/getting-started/authentication/oauth#step-3-confirm-installation

If you are using a private Shopify application, the getToken function should return the "Shared Secret" from your private apps "Admin API" section.

@fidel-ruiz
Copy link
Author

Well, I've tried with returning the Shared Secret from the getToken function and isn't working, I'll take a deeper look into it later today.
Thanks for your answer

@fidel-ruiz
Copy link
Author

fidel-ruiz commented Jun 26, 2018

Hey, I think that I've found where this could be fixed.
Example:

$headers = ['X-Shopify-Access-Token' => $this->shopAccessInfo->getToken()];

This class handles the entire HTTP integration and the headers for each HTTP verb is set as shown below:

//Current Line:
$headers = ['X-Shopify-Access-Token' => $this->shopAccessInfo->getToken()];
//Changed to this: 
        $headers = ['Authorization' => 'Basic token***'];

And it worked, the solution could be changing the signature of the interface ShopAccessInfo to specify which header to use also like I pointed out above, this is done for each HTTP verb, it could be improved by creating something like a helper headers function that simply returns the headers for authorization, and adds new headers in cases where it's needed.

Anyways thanks again, if it helps I can make a PR for this.

***token = btoa('apikey:password')

@theRTC204
Copy link
Contributor

I think if we were to introduce support for Private apps, I'd prefer to see the authentication method be a little bit more prescribed rather than require the user to implement their own header generation method. Perhaps something in the Client that informs whether to use the Public or Private authentication method.

@fidel-ruiz
Copy link
Author

Totally agree, as a mather of fact that was one of my suggestion when I spoke with Eric and sorry I forgot his name the guy from the pair programming exercise. How much is necessary create a model that implement the interface when this could be handle using an environment configuration for token and header and the client itself could be responsible for implementing that logic

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

No branches or pull requests

3 participants