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

Add controller for CRON tasks and minor fixes #175

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Jan 5, 2024

Questions Answers
Description? Added new controller to make module compatible with Prestashop 9. Removes some unused code, adds some missing configuration values. Adds a deprecation notice to current cron task.
Type? refacto
BC breaks? yes
Deprecations? yes
Fixed ticket?
How to test? Test that sitemap generates normally. Test that if you call the old URL, you get a notice. http://localhost/dev/modules/gsitemap/gsitemap-cron.php?token=YOURTOKEN&id_shop=1

@Hlavtox Hlavtox added this to the 4.4.0 milestone Jan 5, 2024
*/
trigger_error('This file should no longer be used and will be removed in 5.0.0 version of this module. Please update your cron tasks to use the new URL available in module settings.', E_USER_NOTICE);
Copy link
Contributor

Choose a reason for hiding this comment

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

No Tools::displayFileAsDeprecated() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into it, but I didn't like that you can't specify the message and it throws the error only in debug mode.

@AureRita AureRita self-assigned this Jan 10, 2024
@AureRita
Copy link

AureRita commented Jan 10, 2024

Hi @Hlavtox

Thank you for your PR, I tested it and it seems to works as you can see :

recording.47.webm

As you can see on the start there is a little message about the version of PHP that is not supported and we have a red notice, except that all seems to works.

Tested on 8.1.x and develop,
FYI I have a green message on update on 8.1.x

red message :
image

Because the PR seems to works as expected, if this error message is not problematic for you, It's QA ✔️

Thank you

@Hlavtox
Copy link
Contributor Author

Hlavtox commented Jan 10, 2024

No that message is something unrelated to the project. ;-)

@Hlavtox Hlavtox merged commit 3d610ce into PrestaShop:dev Jan 10, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants