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

Log Mail subject message correctly in case of alteration #17439

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

davideapvd
Copy link
Contributor

@davideapvd davideapvd commented Jan 30, 2020

Questions Answers
Branch? develop
Description? In Mail.php, in case we alter the message subject with actionMailAlterMessageBeforeSend, the message is logged wrongly, this fixes the log
Type? bug fix
Category? CO
BC breaks? probably if relying on backend logs wrong
Deprecations? no
Fixed ticket?
How to test? Implement actionMailAlterMessageBeforeSend and alter the subject like in the following code, check in backend email logs to see the subject is logged correctly

public function hookActionMailAlterMessageBeforeSend($params)
    {
        /**
         * @var Swift_Message $message
         */
        $message = &$params['message'];
        $message->setSubject('altered subject test');
    }


This change is Reviewable

Progi1984
Progi1984 previously approved these changes Jan 31, 2020
PierreRambaud
PierreRambaud previously approved these changes Jan 31, 2020
@PierreRambaud PierreRambaud added the Waiting for QA Status: action required, waiting for test feedback label Jan 31, 2020
@SD1982 SD1982 self-assigned this Mar 26, 2020
@SD1982 SD1982 added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 26, 2020
@SD1982
Copy link
Contributor

SD1982 commented Mar 26, 2020

Hi @davideapvd Could you provide us more info on "how to test" as a merchant or user would do without having access to the code? And could you also fix the PrettyCi error please?

@PierreRambaud
Copy link
Contributor

Hi @davideapvd Could you provide us more info on "how to test" as a merchant or user would do without having access to the code? And could you also fix the PrettyCi error please?

You can't test without using a module

@davideapvd
Copy link
Contributor Author

Hi @davideapvd Could you provide us more info on "how to test" as a merchant or user would do without having access to the code? And could you also fix the PrettyCi error please?

hello @SD1982 I think prettyci is reporting something I didn't modify, am I correct?

@SD1982 SD1982 removed their assignment Mar 27, 2020
@PierreRambaud
Copy link
Contributor

Hi @davideapvd Could you provide us more info on "how to test" as a merchant or user would do without having access to the code? And could you also fix the PrettyCi error please?

hello @SD1982 I think prettyci is reporting something I didn't modify, am I correct?

When this happens, you need to rebase your branch based on latest change on the develop branch.

@Progi1984 Progi1984 added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for author Status: action required, waiting for author feedback labels Mar 30, 2020
@Robin-Fischer-PS
Copy link
Contributor

Hi @davideapvd @PierreRambaud ! If a module is required to test the PR, could you provide us one, with a scenario ? Thanks ;)

@Robin-Fischer-PS Robin-Fischer-PS added Waiting for author Status: action required, waiting for author feedback Waiting for dev Status: action required, waiting for tech feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Jun 4, 2020
@davideapvd
Copy link
Contributor Author

@Robin-Fischer-PS sure, here it is the simplest one https://github.com/davideapvd/pr17439 , the method to test the pr is as described in the original message, let me know if that is enough.

@Robin-Fischer-PS
Copy link
Contributor

Thanks a lot @davideapvd ! It's OK for QA :)

@Robin-Fischer-PS Robin-Fischer-PS added QA ✔️ Status: check done, code approved and removed Waiting for author Status: action required, waiting for author feedback Waiting for dev Status: action required, waiting for tech feedback labels Jun 4, 2020
@Progi1984 Progi1984 merged commit 7641f78 into PrestaShop:develop Jun 4, 2020
@Progi1984
Copy link
Member

Thanks @davideapvd

@Progi1984 Progi1984 added this to the 1.7.8.0 milestone Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Type: Bug develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants