Skip to content

Conversation

@Kaikina
Copy link

@Kaikina Kaikina commented May 22, 2025

Questions Answers
Description? It makes sense to add the instance of the MailAlert into the hook params as we probably want to use its properties inside our hook code. For instance, it would be great to have a practical access to id_product_attribute.
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? NA
How to test? Register this hook, access the passed array and test that mailAlert key is set and is an instance of the MailAlert object that is executing the hook.

@ps-jarvis
Copy link

Hello @Kaikina!

This is your first pull request on ps_emailalerts repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@github-project-automation github-project-automation bot moved this to Ready for review in PR Dashboard May 22, 2025
boherm
boherm previously approved these changes May 22, 2025
Copy link
Member

@boherm boherm left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I agree with you, that could be nice to have :)

@Kaikina
Copy link
Author

Kaikina commented May 22, 2025

I updated to "id_product_attribute". I didn't see it was a static function (I don't like static functions). So no way of passing "$this". But actually, I think the only missing information while using the hook was the id_product_attribute, so I added it.

@Kaikina
Copy link
Author

Kaikina commented Jul 28, 2025

May we merge this branch ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

5 participants