Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not check version if not provided #624

Merged
merged 1 commit into from Sep 18, 2023
Merged

Conversation

matks
Copy link
Contributor

@matks matks commented Sep 8, 2023

Questions Answers
Description? See below
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#33886
Sponsor company
How to test? See ticket

Description

When the Configuration page of the module is opened, the checklist is being verified. Function UpgradeSelfCheck::checkKeyGeneration() is called but at this moment $this->upgrader->version_num is NULL so the use of version_compare triggers a warning.

At the beginning I was worried about $this->upgrader->version_num being NULL. But then I explored the code and actually I found that $this->upgrader->version_num is set in UpgradeContainer::getUpgrader() and it depends on the channel chosen. The version number variable will be set from different locations if the target version is available on the API, in a file or in a directory. So it makes sense that, when the page is opened, it is not yet set and then it is NULL.

So my solution to avoid the warning is simply not to perform the comparison if $this->upgrader->version_num is NULL as this is a valid usecase.

@djoelleuch djoelleuch self-assigned this Sep 18, 2023
Copy link

@djoelleuch djoelleuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @matks ,
I have checked your PR the issue is well fixed ,

Untitled_.Sep.18.2023.10_12.AM.webm

It's QA approved ✔️
Thank you for your PR 🚀

@matks matks added this to the 4.16.4 milestone Sep 18, 2023
@matks matks merged commit f17f38b into PrestaShop:dev Sep 18, 2023
26 checks passed
@matks matks deleted the fix-php81-warning branch September 18, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
5 participants