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

Update new and obsolete hooks for 9.0.0 #957

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

jolelievre
Copy link
Contributor

@jolelievre jolelievre commented Oct 16, 2024

Questions Answers
Description? Update new and obsolete hooks for 9.0.0
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? ~
Sponsor company ~
How to test? ~

This PR was auto-generated thanks to the core Symfony command, which was improved in this PR PrestaShop/PrestaShop#37168

Quetzacoalt91
Quetzacoalt91 previously approved these changes Oct 22, 2024
Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

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

The query is missing ON DUPLICATE KEY UPDATE `title` = VALUES(`title`), `description` = VALUES(`description`);

(This was added by my PR to update automatically-added hooks that would now be without description.)

@jolelievre
Copy link
Contributor Author

@Hlavtox you're right I integrated it in the command later, but I must have updated this PR improperly

@jolelievre
Copy link
Contributor Author

@Hlavtox @Quetzacoalt91 this should be good now

Hlavtox
Hlavtox previously approved these changes Oct 23, 2024
Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

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

Thanks @jolelievre 👍

@Quetzacoalt91
Copy link
Member

Incoming conflict with #981

Copy link

sonarqubecloud bot commented Nov 4, 2024

@jolelievre
Copy link
Contributor Author

Thanks @Quetzacoalt91 it's fixed, it would be great if this one can be validated soon to avoid any other conflicts because it is blocking the core PR that generates this automatically PrestaShop/PrestaShop#37168

Once the new command is merged we'll update the release process so that it is included in the steps to do by the core team, this means people won't need to create update hooks PR because we can semi-automatically handle all of them as long as the hook.xml file has been correctly updated in the core.

One possible improvement of the CLI command would be to update an already existing SQL query for hooks, but this will be done in some other PR if needed.

@AureRita AureRita self-assigned this Nov 4, 2024
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @jolelievre

Thank you for your PR, I tested it and it seems to works.

Tested from :
8.0.4 to 8.2
8.2 to 9.0.0
8.0.4 to 9.0.0

Because the PR seems to works as expected, It's QA ✔️

Thank you

@M0rgan01 M0rgan01 added this to the 7.0.0 milestone Nov 4, 2024
@M0rgan01 M0rgan01 merged commit dd65b98 into PrestaShop:dev Nov 4, 2024
36 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