ampersand-magento2-upgrade-patch-helper

ampersand-magento2-upgrade-patch-helper copied to clipboard

For a while now, https://hyva.io/ is used more and more, and we use it in some projects as well. There it would be awesome, if some parts could be improved:

• Ignore overrides in hyva modules as these are maintained by hyva itself anyway and we don't want to adapt them in an update
• Check also custom fallback themes: In hyva it is possible to define a fallback theme for specific URLs. For that, there is a config setting, where you provide a full path of a fallback theme. This theme is currently not taken into account as far as I see, as it is not defined as a "normal" theme and so ignored. Would be awesome, if this could be checked as well.

Do you see any way of approaching this?

All of these kinds of things are possible @norgeindian

We have a flag for --vendor-namespaces so something like --exclude-vendor-namespaces could perhaps take hyva ?

For the theme fallback currently we hook into the magento minification and simple resolvers

https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/blob/c60d335518e38e38254d246c185275cb85f864cc/src/Ampersand/PatchHelper/Helper/PatchOverrideValidator.php#L446-L463

I do not have any Hyva stores, thus the effort to develop these changes are less of a personal priority me. If you know how the file resolution works for hyva, pull requests are very welcome. I can help out in terms of testing / pointers / debugging / etc once something is "off the ground"

@convenient , thanks for your fast reply. I checked that now a bit more detailed in our current updates and fear, that just excluding is not enough. So the structure of hyva is the following, assuming that you have your own theme on top of it:

• your theme
• Hyva/default
• Hyva/reset

So any changes in luma or blank do not affect the custom theme at all, as it is only dependent on the hyva themes, which are on their side not dependent on any core theme. So what we would need is a setting to define a substitution for the core themes, so that our custom theme is checked against this one. This would not only apply for hyva, but in general for all shops, which have an independent theme included and a custom theme built on top of this. Do you see any easy way of having such a functionality with the given structure?

The fallback theme is then another thing, actually hyva specific. @Vinai, do you maybe have an idea, how we could include this setting into this helper here? This would make hyva updates a lot easier.

@norgeindian So for hyva it would be like

1. we need to pretend that the magento themes don't exist at all (except for adminhtml)
2. the hyva theme would not be identified as the custom theme, but would itself be the "core" theme the system is built upon?

Currently the way we grab custom themes is here

https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/blob/e52a434ca5be461ae8ebfac6d28a9e5fd7d9a6fb/src/Ampersand/PatchHelper/Helper/Magento2Instance.php#L65-L80

Perhaps we can do something sneaky like so

• detect if hyva is in that list of custom themes
  • then alter the behaviour so that hyva is not detected as a custom theme.
  • we can iterate through all magento core themes, grab the paths, and exclude any files in those themes from analysis to remove them from the report.

This would not only apply for hyva, but in general for all shops, which have an independent theme included and a custom theme built on top of this.

Do you know off the top of your head where this is defined in the configuration ? We do boot the magento instance, so we could possibly grab and work this information out

@convenient , exactly what you wrote. We pretend, core themes do not exist and hyva should be the "new" core theme.

Do you know off the top of your head where this is defined in the configuration ? We do boot the magento instance, so we could possibly grab and work this information out

You mean, the definition of the fallback theme? This is defined in hyva_theme_fallback/general/theme_full_path and the value is something like frontend/vendor/fallbackTheme

Right interesting. Currently the tool works without connecting to the DB which I find useful for having it in CI so I'll have a think upon this.

Is this theme ALWAYS guaranteed to exist on hyva stores? Can I string match against this code? Hyva/reset

Happy to provide any info. Not 100% sure what is required after reading this thread.

Regarding the hyva theme detection, what we do to determine if a theme is a Hyvä theme or not is to follow the parent themes, and if that ends at Hyva/reset it's a Hyvä theme, otherwise it's not.

    public function isHyva(): bool
    {
        $theme = $this->viewDesign->getDesignTheme();
        while ($theme) {
            if (strpos($theme->getCode(), 'Hyva/') === 0) {
                return true;
            }
            $theme = $theme->getParentTheme();
        }
        return false;
    }

Regarding the theme fallback module, this is based on system configuration. I think this should be if one in regards to the upgrade helper, since any page that it refers to will then simply use a Luma theme - no special treatment necessary.

Thanks for the isHyva @Vinai :) exactly what I was wondering how to implement

Hey @norgeindian I've take an hour or so shotgun programming at this.

https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/pull/80

There's still a massive TODO and question mark about how the theme fallbacking configuration you mentioned actually works. That would likely need implemented.

This is likely as far as I can push this code change myself, we don't have any Hyva sites, and setting one up for a test instance is possible but I honestly do not know when I would get the time to do that.

Anyone who wants to use that PR as the basis or inspiration for the implementation of this, that's great.

@convenient , awesome, really cool. I will try to test a Hyva update as soon as possible and let you know how it goes.

Is the fallback support supposed to work yet in the PR?

We have jajuma/prg and the compat module hyva-themes/magento2-jajuma-prg installed.

I belive on the left side, the hyva-themes/magento2-jajuma-prg/src/view/frontend/templates/product/list/toolbar/sorter.phtml should be checked for diffs?

Instead, I get this in the Elgentos UI:

| WARN  | Override (phtml/js/html) | vendor/jajuma/prg/view/frontend/templates/product/list/toolbar/sorter.phtml                                                 | app/design/frontend/OurCustomer/Hyvaaaaa/Jajuma_PRG/templates/product/list/toolbar/sorter.phtml                           |

Also there is this:

| WARN | Override (phtml/js/html) | vendor/magento/module-catalog/view/base/templates/product/price/tier_prices.phtml | app/design/frontend/OurCustomer/Hyvaaaaa/Magento_Catalog/templates/product/price/tier_prices.phtml |

but the template is copied from

vendor/hyva-themes/magento2-default-theme/Magento_Catalog/templates/product/price/tier_prices.phtml