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

BrevoPayloadConverter causes type error if tags are not present #54197

Closed
palgalik opened this issue Mar 7, 2024 · 0 comments · Fixed by #54199
Closed

BrevoPayloadConverter causes type error if tags are not present #54197

palgalik opened this issue Mar 7, 2024 · 0 comments · Fixed by #54199

Comments

@palgalik
Copy link
Contributor

palgalik commented Mar 7, 2024

Symfony version(s) affected

6.4.5

Description

BrevoPayloadConverter handles tags parameter, that it's always present, but it's not since 6.4.5.

Since tags was removed from mandatory parameters in #54088 it should have been defaulted to empty array in the payload converter.

TypeError: Symfony\\Component\\RemoteEvent\\Event\\Mailer\\AbstractMailerEvent::setTags(): Argument #1 ($tags) must be of type array, null given, called in /app/vendor/symfony/brevo-mailer/RemoteEvent/BrevoPayloadConverter.php

How to reproduce

Call webhook controller without any tags present in the request.

Possible Solution

Add default empty array as fallback value
or
Check tags is present, before calling setTags

Additional Context

No response

@palgalik palgalik added the Bug label Mar 7, 2024
@palgalik palgalik changed the title BrevoPayloadConverter BrevoPayloadConverter causes type error if tags are not present Mar 7, 2024
@fabpot fabpot closed this as completed Mar 15, 2024
fabpot added a commit that referenced this issue Mar 15, 2024
…ore calling setTags (palgalik)

This PR was merged into the 6.4 branch.

Discussion
----------

[Mailer] [Brevo] Check that tags is present in payload before calling setTags

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix [#54197](#54197)
| License       | MIT

`tags` was removed as a mandatory parameter from Brevo remote event requests in this [PR](https://github.com/symfony/symfony/pull/54089/files) by me. Sadly, I wasn't wary enough, and I didn't added payload check before calling setTags in the PayloadConverter, and it causes TypeErrors.

- With this fix, `setTags` won't be called anymore if tags are not present in the request
- `AbstractMailerEvent` defaults tags as empty array, so there is no need to call the setter with empty array parameter

Commits
-------

868939c [Mailer] [Brevo] Check that tags is present in payload before calling setTags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants