autoupgrade
autoupgrade copied to clipboard
Fix version compare check to use CoreUpgrader 8 with 8.0 version
This PR fixes CoreUpgrader file
If you want to update to version 8.0, the core upgrader uses the CoreUpgrader17 instead of CoreUpgrader8.
(the selected core upgrader depends on the targeted version and not from the source version)
Fixes https://github.com/PrestaShop/PrestaShop/issues/29248
HOW TO TEST:
1/ In the autoupgrade form, choose upgrade via local archive 2/ In the field for the upgrade version, you should only be able to enter versions with a patch precision, example:
- 1.7.7.8
- 8.0.0
- 1.6.1.24
Examples of wrong versions (you will get an error message):
- 1.7.2
- 8.0
- 8.0.0.0
(Notice how for PS 8 and above, the version is made of 3 numbers, while it's 4 numbers for older versions)
Some weird PHP I tested to find the bug and help you understand this PR.
php > var_dump(version_compare("8.0", "8.0.0", '<'));
bool(true)
php > var_dump(version_compare("8.0.0", "8.0.0", '<'));
bool(false)
php > var_dump(version_compare("8", "8.0.0", '<'));
bool(true)
@FabienPapet I added a commit with some wording modification
About the regex: did you test it in the form ? I tried your PR and it seems like I can put any value I want in the field, I don't think it comes from your regex, more like it's not even taken into account, but maybe I'm missing something ? 🤔
the regex seems good otherwise 👍
@FabienPapet small reminder so the PR can be ready for the auto-upgrade release :)
Should be good now
Hello @khouloudbelguith I updated the pull request, now you will have this message if you don't give a correct version number
![Capture d’écran 2022-09-15 à 11 12 15](https://user-images.githubusercontent.com/1784781/190365044-b44f8a80-7565-4818-8619-f723cb31cff6.png)
thank you @khouloudbelguith
I fixed your first issue :+1:
About the second one:
- I can't reproduce it following your steps, I guess something else went wrong in your case but I'm not sure what, we should definitely investigate
- I don't think it's linked to this PR (this PR only modifies JS, and version check in PHP, nothing else, it would be weird that it breaks the upgrade process... and I didn't reproduce with the PR)
Do you think we can create an issue for your second point, with reproducible steps, then it would be fixed in another PR
Hello @FabienPapet @matthieu-rolland @matks !
As seen with @khouloudbelguith , since the second issue cannot be reproduced by everyone and It seems that it's more subtle than we thought (yeah, 17 steps where not enough).
So we will create an issue for this second issue and investigate on it later. So we only need the first issue to be fixed and we will test it to check the fix, if it's OK, then merge.
Thank you everybody