feat: Update commercial if it is also installed#13
feat: Update commercial if it is also installed#13Michael Telgmann (mitelg) wants to merge 1 commit into
Conversation
Michael Telgmann (mitelg)
requested review from
Jonas Elfering (keulinho),
Marcel Kräml (mkraeml) and
Soner (shyim)
Michael Telgmann (mitelg)
force-pushed
the
fix/commercial-handling
branch
2 times, most recently
from
619a625 to
80e1d38
Compare
|
Yes to do it like that, we need to always release a new commercial version when we release a Shopware version, even when there are no changes to Commercial on a patch release. |
Jonas Elfering (keulinho)
left a comment
Jonas Elfering (keulinho)
left a comment
There was a problem hiding this comment.
Thanks for cleaning up and for putting that into two seperate commits, that made it way easier to review 👍
I only have one small nitpick
|
|
||
| foreach ($this->installedPackages as $package) { | ||
| if (isset($package['type']) && $package['type'] === $type) { | ||
| if (isset($package['type']) && $package['type'] === 'shopware-platform-plugin') { |
There was a problem hiding this comment.
when we are at it we should make this a const ;)
Michael Telgmann (mitelg)
left a comment
Michael Telgmann (mitelg)
left a comment
There was a problem hiding this comment.
TODO:
- build phar from this changes and test if it works as intended
| } | ||
|
|
||
| if (isset($composerJson['require']['shopware/commercial'])) { | ||
| // If commercial is installed, also update it directly as part of the core update to keep them in sync |
There was a problem hiding this comment.
we need to ensure, that the given $version is of format (v)11.22.33.44.
I am not sure, if "versions" like dev-trunk are also possible at this point 🤔
| if (isset($composerJson['require']['shopware/commercial'])) { | ||
| // If commercial is installed, also update it directly as part of the core update to keep them in sync | ||
| // Remove leading "(v)6." from Shopware version to match commercial release versions | ||
| $composerJson['require']['shopware/commercial'] = substr($version, strpos($version, '.') + 1); |
There was a problem hiding this comment.
The entire bootstrapping of extension loading is forgotten at this place.
How does Composer will get the next version? The plugin can be in:
- vendor (required store version)
- custom/static-plugins (someone put it there)
- custom/plugins (downloaded through plugin manager)
To fulfill this, you will need packages.shopware.com. This is right now on 0% of Shops configured who uses the WebInstaller, if they already now our packages, they might not use this tool at all anymore.
There are still no API to gather the packages token, so you will have to fallback to regular API Download which we have in core. But the application might me such in a unstable state, that an bin/console store:download SwagCommercial may also fail. Also that command is not forward-compatible, when you run it on the old version, you get the old version. On the new version, you may already too late and it's broken because "you didnt updated them at same time". So the only real viable solution is to make those SBP API Calls also in the WebInstaller and grab the store token manually from Shopware DB.
When we wanna open this Topic really, I would push further that we authenticate every Shopware instance against packages.shopware.com, and only use that mechanism. I would not port the existing uggly zip downloading to there. The store packages, would solve this problem also for PayPal plugin whatever else requires this in future
There was a problem hiding this comment.
this is the mentioned issue, we should do first shopware/shopware#13631
Can we please do this next time in another PRs. With AI is already hard enough to keep your brain active while reviewing and unnecessary changes makes it unnecessary hard to review |
Michael Telgmann (mitelg)
force-pushed
the
fix/commercial-handling
branch
from
80e1d38 to
3c844e5
Compare
5 participants
Currently commercial has a too broad upper version constraint, which could lead to errors like this: shopware/shopware#12229
Before the version of commercial was in sync with the SW version. meaning SW 6.7.6.0 -> commercial 7.6.0
no other minor version of commercial was allowed to be installed with SW 6.7.6.0.
This did not work good in update cases as you can get in a state where you have installed a non-compatible version of commercial. Therefore the broad upper boundary was introduced.
As preparation to re-introduce the in-sync versions again, the web installer needs a change. The idea is to update Shopware and Commercial simultaneously. updating not only the core packages in the composer.json but also the commercial version.
This only works, if we always tag a commercial release, when a Shopware release tag is created. Marcel Kräml (@mkraeml) please confirm this assumption 🙂