Migrate improvements for CI from Laravel-Shopify by gnikyt · Pull Request #103 · gnikyt/Basic-Shopify-API · GitHub
Skip to content

Migrate improvements for CI from Laravel-Shopify#103

Merged
gnikyt merged 5 commits into
masterfrom
feature/ci-improvements
Jul 2, 2021
Merged

Migrate improvements for CI from Laravel-Shopify#103
gnikyt merged 5 commits into
masterfrom
feature/ci-improvements

Conversation

@gnikyt

@gnikyt gnikyt commented Jun 30, 2021

Copy link
Copy Markdown
Owner

@lucasmichot Migrating over some good changes you did for Laravel Shopify.

@gnikyt gnikyt self-assigned this Jun 30, 2021
@gnikyt

gnikyt commented Jun 30, 2021

Copy link
Copy Markdown
Owner Author

@lucasmichot

Copy link
Copy Markdown
Contributor

Nice move @osiset - you might also need to run composer normalize to update the composer.json file

@gnikyt

gnikyt commented Jun 30, 2021

Copy link
Copy Markdown
Owner Author

Yes just noticed it failed, easy fix, thanks for the work!

@gnikyt gnikyt marked this pull request as ready for review July 2, 2021 13:59
@gnikyt gnikyt merged commit 64188ab into master Jul 2, 2021
@squatto

squatto commented Jul 2, 2021

Copy link
Copy Markdown

Marking this as final completely broke my entire app 🔥 I pinned it to 10.0.1 so that I could keep running for now.

I extended it so that I could add a few of my own utility methods that I use as entry points to the response object. Nothing I can't fix...I just thought I'd give you a chuckle over "I see no need for anyone to extend this" 😂


The sky is falling, the sky is falling. haha

image

@gnikyt

gnikyt commented Jul 5, 2021

Copy link
Copy Markdown
Owner Author

@squatto Interesting... I checked with a couple dozen people and no one was extending it, figured it would be safe since it was just for accessing. In this case, I am not sure I can solve this easily without making an interface... are you extending the constructor? I could add it there.

@squatto

squatto commented Jul 6, 2021

Copy link
Copy Markdown

@osiset It's no problem at all! You don't need to change anything. I will change it on my side to be a property of my class instead of extending it. I'm mapping the Shopify API responses into models/DTOs, and extending ResponseAccess was really just a simple convenience and isn't necessary. I really only used ->offsetExists() and ->offsetGet() in my extended class, both of which are public methods and can be accessed from ResponseAccess when set to a class property.

This is all I'm currently doing:

<?php

namespace App\Services\Shopify;

use App\Exceptions\ShopifyApiException;
use App\Models\Client;
use App\Models\User;
use App\Services\Shopify\Models\ShopifyModel;
use Illuminate\Support\Collection;
use Illuminate\Support\Str;
use Osiset\BasicShopifyAPI\ResponseAccess;

class ShopifyApiResponse extends ResponseAccess
{
    protected array $resourceClassMap = [
        'customer'          => \App\Services\Shopify\Models\Customer::class,
        'fulfillment'       => \App\Services\Shopify\Models\Fulfillment::class,
        'image'             => \App\Services\Shopify\Models\Image::class,
        'metafield'         => \App\Services\Shopify\Models\Metafield::class,
        'order'             => \App\Services\Shopify\Models\Order::class,
        'draft_order'       => \App\Services\Shopify\Models\DraftOrder::class,
        'product'           => \App\Services\Shopify\Models\Product::class,
        'custom_collection' => \App\Services\Shopify\Models\CustomCollection::class,
        'collect'           => \App\Services\Shopify\Models\Collect::class,
        'inventory_level'   => \App\Services\Shopify\Models\InventoryLevel::class,
        'inventory_item'    => \App\Services\Shopify\Models\InventoryItem::class,
        'shop'              => \App\Services\Shopify\Models\Shop::class,
        'transaction'       => \App\Services\Shopify\Models\Transaction::class,
        'variant'           => \App\Services\Shopify\Models\Variant::class,
        'webhook'           => \App\Services\Shopify\Models\Webhook::class,
    ];

    /**
     * Get all possible keys for a resource (singular, plural)
     *
     * Shopify uses a singular or plural key based on the resource:
     * - Resource "/orders/123456" = key "order" (singular)
     * - Resource "/orders/123456/fulfillments" = key "fulfillments" (plural)
     *
     * @param string $resource
     *
     * @return array
     */
    protected function getPossibleResourceKeys(string $resource): array
    {
        $resource = ShopifyApiService::fixResource($resource);

        $base = pathinfo($resource, PATHINFO_FILENAME);

        if (is_numeric($base)) {
            // e.g. /admin/products/123456.json
            return static::getPossibleResourceKeys(dirname($resource));
        }

        return array_values(array_unique([
            $base,
            Str::singular($base),
            Str::plural($base),
        ]));
    }

    /**
     * Return the object(s) from the response that pertains to the resource's calculated key.
     *
     * The response object(s) are mapped to resource-specific models (e.g. \App\Services\Shopify\Models\Product)
     *
     * The response is either a single model instance, a Collection of model instances,
     * or an integer (for count requests)
     *
     * Shopify returns the resource's data in a top-level key that is named after the resource.
     * For example, the "orders" resource returns...
     *   Single order: {"order": {...}}
     *   Multiple orders: {"orders": [{...}, {...}, ...]}
     * If none of the known keys are found, an exception is thrown
     *
     * @param string $resource
     * @param Client|User $owner
     *
     * @return Collection|ShopifyModel|ShopifyModel[]|int|null
     * @throws ShopifyApiException
     */
    public function forResource(string $resource, $owner)
    {
        foreach ($this->getPossibleResourceKeys($resource) as $key) {
            if ($this->offsetExists($key)) {
                $value = $this->offsetGet($key);

                return is_numeric($value)
                    ? $value
                    : $this->toModel($key, new self($value->toArray()), $owner);
            }
        }

        throw new ShopifyApiException("Unable to identify the correct response key for resource '$resource'", $owner);
    }

    /**
     * Map the response to a Shopify model (if single result)
     * or a Collection of Shopify models (if multiple results)
     *
     * @param string $resource
     * @param ShopifyApiResponse $response
     * @param Client|User $owner
     *
     * @return Collection|ShopifyModel[]|ShopifyModel
     * @throws ShopifyApiException
     */
    protected function toModel(string $resource, self $response, $owner)
    {
        $resource = Str::singular($resource);

        if (! isset($this->resourceClassMap[$resource])) {
            throw new ShopifyApiException("Model class is not known for resource '$resource'", $owner);
        }

        $class = $this->resourceClassMap[$resource];

        return call_user_func([$class, 'createFromResponse'], $response, $owner);
    }
}

@gnikyt

gnikyt commented Jul 6, 2021

Copy link
Copy Markdown
Owner Author

In this case, I'll move the final to the constructor, this will solve issues just the same. The constructor doesn't need to be extended.

@gnikyt

gnikyt commented Jul 6, 2021

Copy link
Copy Markdown
Owner Author

I also have a similar API to model on some internal projects, I made a factory service to essentially do what you have above to create new responseaccess return. All good :)

@squatto

squatto commented Jul 6, 2021

Copy link
Copy Markdown

@gnikyt gnikyt deleted the feature/ci-improvements branch July 8, 2021 19:43
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