laravel-shopify icon indicating copy to clipboard operation
laravel-shopify copied to clipboard

Conditional scripttag creation - Verify support for app blocks

Open brucelee90 opened this issue 3 years ago • 19 comments

I'm not sure if this is considered an issue or not.

I just submitted my app and shopify told me to update my app to theme app extensions to ensure compatibility with Online Store 2.0 themes.

In order to update my app to theme app extensions I will have to verify if a store uses app blocks. If it uses app blocks, then the script tag should not be posted on install.

i.e. it is currently not possible with this package to check if a theme supports app blocks, because if a theme supports app blocks, no scripttags should be created.

I was wondering if there was a way to check if a theme has app blocks available and then implement the logic to post a script tag accordingly?

Did anybody just recently submit an app or has any experience with this?

edit: corrected misleading title and question

brucelee90 avatar Sep 18 '21 01:09 brucelee90

You may get more info there. https://shopify.dev/apps/online-store/verify-support

apurbajnu avatar Sep 22 '21 04:09 apurbajnu

Thank you for your comment! I already read this article too. And I understand that my question is very misleading.

actually the question is, if I can conditionally create a scripttag and how I should do that.

e.g.: case 1: store does not support app blocks- create scripttags

case 2: store supports app block functionality - do not create scripttags and use theme app extension.

I don’t really know where and when scripttags are created by the package and how I could stop them from being created a if theme supports app blocks.

That seems critical right now because that is the reason they won’t accept my app because of this.

Any help or experience will be much appreciated!

thank you

brucelee90 avatar Sep 23 '21 23:09 brucelee90

So I just found the answer. I had to override the Osiset\ShopifyApp\Traits\Authcontroller trait.

function "call_user_func($this->dispatchScriptsAction, $shopId, false);" in line 39 triggers the scripttag install. So now I will have to wrap this into a condition.

brucelee90 avatar Sep 24 '21 21:09 brucelee90

Hi @brucelee90 , can you guide on how to update the app to use theme app extension using this package, as my app also got rejected due to this requirement by shopify. and i dont find much help on how to update to theme app extension using osiset package, previously i just use script tags and the app never got rejected, so any help would be great

yasir-naseer avatar Nov 06 '21 15:11 yasir-naseer

Hi @yasir-naseer,

so what I did was I overwrote the AuthenticateShop.php in my project.

You have to look for the function call_user_func($this->dispatchScriptsAction, $shopId, false);. This function is responsible for dispatching the scripttags.

You have to wrap that function inside of an condition. in this condition you will have to check if app blocks are available. Shopify also has a documentation on how to check if app blocks are available:

https://shopify.dev/apps/online-store/verify-support

brucelee90 avatar Nov 07 '21 18:11 brucelee90

@brucelee90 - are you able to submit a PR for this and I'll take a look?

If it is a minimum requirement for Shopify (which is looks like it is) would be good to get this into the package.

cc: @osiset

Kyon147 avatar Nov 13 '21 10:11 Kyon147

@Kyon147 - yeah sure but it might take a minute for me to figure how that is done 😄

brucelee90 avatar Nov 13 '21 17:11 brucelee90

Hey @Kyon147, I might need your help on this problem. I thought I had an answer to this, but I ran into some issues again.

I believe the problem is this:

Shopify does not want us to use script tags if a theme is installed that supports app-blocks. So we have need to figure out if a theme uses app blocks or not right before the Scripttag jobs are dispatched.

The Scripttag jobs are being dispatched in the file AuthenticateShop.php. So in there I have to query the store's themes.

Now the problem is, that I don't have the store's themes at that point. So my request results in an error, because there are no themes yet.

I don't know the architecture of this package well enough to find any other solutions. Do you have any ideas?

Thank you!

brucelee90 avatar Nov 25 '21 23:11 brucelee90

Hey @brucelee90

Thanks for giving it a go - I'll have a look over Shopify's requirements around script tags and the theme blocks and see where we could place in the check.

If we need the api token before checking for script tags, we might need to defer the script tag installation later down the line after we have the token from the store.

I'll see what the approach could be and talk with @osiset on a path to move forward if this is a blocker for getting apps approved by shopify.

Kyon147 avatar Nov 26 '21 08:11 Kyon147

@Kyon147 kindly include documentation to support adding app block and theme app extension using package in next versions please

yasir-naseer avatar Nov 28 '21 13:11 yasir-naseer

I have tried to solve the issue for my project. It is working for me. Hopefully, It will help you too. What I have done here, let me describe. If you have any suggestions, let me know. I will try to refactor the code. Go to the env file and add this line depending on whatever template you need.

SHOPIFY_APP_BLOCK_TEMPLATES = "product, collection, index"

Go to config/shopify-app.php and add this line.

` /* |--------------------------------------------------------------------------

App Block Templates
This option is for defining appblocktemplates (Ex: 'product, collection, index')
*/

'app_block_templates' => explode(',',env('SHOPIFY_APP_BLOCK_TEMPLATES', '')),

` Go to vendor/osiset/laravel-shopify/src/ShopifyApp/Contracts/ApiHelper.php and add these lines.

public function getThemes(array $params = []): ResponseAccess; public function scriptTagShouldBeEnabled(array $app_block_templates = [], array $params = []):bool;

Go to vendor/osiset/laravel-shopify/src/ShopifyApp/Services/ApiHelper.php and add these lines

`

public function getThemes(array $params = []): ResponseAccess
{
    // Setup the params
    $reqParams = array_merge(
        [
            'limit' => 250,
            'fields' => 'id,role',
        ],
        $params
    );

    // Fire the request
    $response = $this->doRequest(
        ApiMethod::GET(),
        '/admin/themes.json',
        $reqParams
    );

    return $response['body']['themes'];
}

/**
 * {@inheritdoc}
 */
public function scriptTagShouldBeEnabled(array $app_block_templates = [], array $params = []):bool
{
    if (count($app_block_templates) > 0) {
        $themes = $this->getThemes();
        $published_theme = null;
        $templateJSONFiles = [];
        $sectionsWithAppBlock = [];
        $main = false;
        $templateMainSections = [];
        if (count($themes) !== 0) {
            foreach ($themes as $theme) {
                if ($theme['role'] === 'main') {
                    $published_theme = $theme['id'];
                }
            }
        }

        if (!is_null($published_theme)) {
            // Setup the params
            $reqParams = array_merge(
                [
                    'fields' => 'key',
                ],
                $params
            );

            // Fire the request
            $response = $this->doRequest(
                ApiMethod::GET(),
                "/admin/themes/{$published_theme}/assets.json",
                $reqParams
            );

            $assets = $response['body']['assets'];

            if (count($assets) > 0) {
                foreach ($assets as $asset) {
                    foreach ($app_block_templates as $template) {
                        if ($asset['key'] === "templates/{$template}.json") {
                            $templateJSONFiles[] = $asset['key'];
                        }
                    }
                }
                // Log::info(print_r($templateJSONFiles, 1));
                if (count($templateJSONFiles) == count($app_block_templates)) {
                    foreach ($templateJSONFiles as $file) {
                        $acceptsAppBlock = false;
                        $reqParams = array_merge(
                            [
                                'fields' => 'value',
                            ],
                            ['asset[key]' => $file]
                        );

                        // Fire the request
                        $response = $this->doRequest(
                            ApiMethod::GET(),
                            "/admin/themes/{$published_theme}/assets.json",
                            $reqParams
                        );

                        $asset = $response['body']['asset'];

                        $json = json_decode($asset['value'], true);
                        $query = 'main-';
                        // Log::info(print_r($json, 1));

                        if (array_key_exists('sections', (array)$json) && count($json['sections']) > 0) {
                            foreach ($json['sections'] as $key => $value) {
                                if ($key === 'main' || substr($value['type'], 0, strlen($query)) === $query) {
                                    $main = $value;
                                    break;
                                }
                            }
                        }

                        if ($main) {
                            $mainType = $main['type'];
                            if (count($assets) > 0) {
                                foreach ($assets as $asset) {
                                    if ($asset['key'] === "sections/{$mainType}.liquid") {
                                        $templateMainSections[] = $asset['key'];
                                    }
                                }
                            }
                        }
                    }

                    if (count($templateMainSections) > 0) {
                        $templateMainSections = array_unique($templateMainSections);
                        foreach ($templateMainSections as $templateSection) {
                            $acceptsAppBlock = false;
                            $reqParams = array_merge(
                                [
                                    'fields' => 'value',
                                ],
                                ['asset[key]' => $templateSection]
                            );

                            // Fire the request
                            $response = $this->doRequest(
                                ApiMethod::GET(),
                                "/admin/themes/{$published_theme}/assets.json",
                                $reqParams
                            );
                            $asset = $response['body']['asset'];

                            $match = preg_match('/\{\%\s+schema\s+\%\}([\s\S]*?)\{\%\s+endschema\s+\%\}/m', $asset['value'], $matches);

                            // Log::info(print_r($matches,1));
                            $schema = json_decode($matches[1], true);
                            // Log::info(print_r($schema,1));

                            if ($schema && array_key_exists('blocks', $schema)) {
                                foreach ($schema['blocks'] as $block) {
                                    if (array_key_exists('type', (array)$block) && $block['type'] === '@app') {
                                        $acceptsAppBlock = true;
                                    }
                                }
                                //   $acceptsAppBlock = .some((b => b.type === '@app'));
                            }
                            $acceptsAppBlock ? array_push($sectionsWithAppBlock, $templateSection) : null ;
                        }
                    }
                }

                if (count($sectionsWithAppBlock)>0  && count($sectionsWithAppBlock) === count($templateJSONFiles)) {
                    return false;
                }

                // Log::info(print_r($sectionsWithAppBlock, 1));
            }
        }
    }
    return true;
}`

Go To vendor/osiset/laravel-shopify/src/ShopifyApp/Actions/DispatchScripts.php and add these lines

$apiHelper = $shop->apiHelper(); $scriptTagShouldBeEnabled = $apiHelper->scriptTagShouldBeEnabled(Util::getShopifyConfig('app_block_templates')); if($scriptTagShouldBeEnabled === false){ return false; }

I have tried to paste the code with php code indentation but not working with markdown.

apurbajnu avatar Dec 11 '21 13:12 apurbajnu

@apurbajnu That's a great solution! With a few tweaks it seems to work:

In my case the single space between

SHOPIFY_APP_BLOCK_TEMPLATES = "product, collection, index"

Caused a failure, so we should make sure to trim the strings, or the explode function should include a space, or we could trim the string within the foreach loop later.

Also what I don't quite understand yet is this line:

if (count($sectionsWithAppBlock)>0  && count($sectionsWithAppBlock) === count($templateJSONFiles)) {
                    return false;
}

Why do we have to ask if

count($sectionsWithAppBlock) === count($templateJSONFiles) ?

Because from my understanding it should be enough if we ask if there are $sectionsWithAppBlock)>0. If yes then return false.

In shopify's documentation you can see that they actually have three cases:

if (templateJSONFiles.length === sectionsWithAppBlock.length) {
  console.log('All desired templates have main sections that support app blocks!');
} else if (sectionsWithAppBlock.length) {
  console.log('Only some of the desired templates support app blocks.');
} else {
  console.log("None of the desired templates support app blocks");
}

I think it's enough to return false even if only some desired templates support app blocks.

Anyway thank you for that great work! I will implement it in my app!

brucelee90 avatar Feb 27 '22 02:02 brucelee90

Thanks, @brucelee90, for this discussion. I have tried my best to find a solution. You are right. We can make this solution better with some tweaks. if (count($sectionsWithAppBlock)>0 && count($sectionsWithAppBlock) === count($templateJSONFiles)) { return false; }

Explanation I have seen the code from Shopify's Documentation. But they haven't taken any action there. We need to take action. The main thing is, Script tag won't be enqueued if your theme support app block for the template is necessary for your app. But if it supports only a few, then a Script tag is essential. In that case, we need to ensure all template is supported. Otherwise, we attached the script tag. If there is no template name in the env file, then the count will be 0, which can be equal to the app block supported template number. In that case, we ensure the number of templates is greater than zero, and all template is supported for app block. Only then script tag should not be enqueued.

I think this discussion is being little complicated. Please let me know if anything is not clear to you. Sorry. I am not so good at English.

If you think something needs to correct, let me know. It will help me to solve issues near future.

apurbajnu avatar Feb 27 '22 20:02 apurbajnu

@apurbajnu I have seen the shopify example as well and I understood it in a different way. I thought that Scripttag shouldn't be installed even if there are only a few appBlocks 😄

However if it is the case that script tags should only be enqueued if all template is supported for app blocks then I understand what this line

if (count($sectionsWithAppBlock)>0 && count($sectionsWithAppBlock) === count($templateJSONFiles)) { return false; }

is for!

I have tried it with dawn theme and I actually expected it to be completely supported for app blocks, but it turns it's only partly supported.

What I don't understand is the following: on dawn theme I can load a theme app extension on to the product detail page. but if I install the app in then scripttags would be loaded (because dawn theme is only partly supported). Now I would have the problem that I have theme app extension installed AND script tags on the page.

The more I think about it, the more confused I get

brucelee90 avatar Mar 01 '22 10:03 brucelee90

@brucelee90 You have to make JavaScript loaded once App block or Script tag. If app block is supported, that means they can do it easily. Otherwise, they have to do it manually by the following instructions you give. Rest of the job will be done by Script tag.

apurbajnu avatar Mar 02 '22 14:03 apurbajnu

@apurbajnu, thank you very much! So I think I understood it now. There are three cases and in 2 of them Script Tags have to be loaded:

  1. app blocks are supported completely -> No Script tag is loading
  2. app blocks are supported partly -> Script tag has to be loaded but instructions have to be given where to install theme app extensions
  3. app blocks are not supported -> Script tag has to be loaded

brucelee90 avatar Mar 03 '22 05:03 brucelee90

ПР https://github.com/osiset/laravel-shopify/pull/1142

ghost avatar May 23 '22 09:05 ghost

@Kyon147 @brucelee90 I think we can easily create app extension without modifying this package. I already did it while using this package. You can check my comment here: Check this: https://github.com/osiset/laravel-shopify/issues/1167#issuecomment-1202443351

I hope this will help :)

tanseercena avatar Aug 02 '22 12:08 tanseercena

Hi @tanseercena

You are right, I've also recently created an app extension using the shopify CLI. For the embed, it would not require an update to the package.

As the App Embed block, is compatible with vintage and theme 2.0 it would replace most needs for using the script tags from what I can tell.

Kyon147 avatar Aug 02 '22 14:08 Kyon147

Theme extension support has been added in 17.2.0 and the Shopify CLI can be used to scaffold it into the package quite easily.

Kyon147 avatar Sep 11 '22 07:09 Kyon147