Skip to content

[Mailer] [Infobip] Add trackClicks, trackOpens and trackingUrl as supp… #57424

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

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

ndousson
Copy link
Contributor

@ndousson ndousson commented Jun 17, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
License MIT
Doc PR symfony/symfony-docs#xxxxxxxx # TODO

📍 Context

This PR add three headers to configure Infobip email tracking with API V3.

➕ New feature

New payload attribute was added allowing end users to configure tracking.

Attribute Type Description
X-Infobip-TrackingUrl string The URL on your callback server on which the open and click notifications will be sent.
X-Infobip-TrackClicks boolean Enable or disable track click feature.
X-Infobip-TrackOpens boolean Enable or disable open click feature.

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@ndousson ndousson closed this Jun 17, 2024
@ndousson ndousson reopened this Jun 18, 2024
@ndousson ndousson marked this pull request as ready for review June 18, 2024 07:59
@carsonbot carsonbot added this to the 7.1 milestone Jun 18, 2024
@carsonbot carsonbot changed the title [Mailer][Infobip] Add trackClicks, trackOpens and trackingUrl as supp… [Mailer] [Infobip] Add trackClicks, trackOpens and trackingUrl as supp… Jun 18, 2024
@xabbuh xabbuh modified the milestones: 7.1, 7.2 Jun 18, 2024
@xabbuh
Copy link
Member

xabbuh commented Jun 18, 2024

As this is a new feature the PR must target the 7.2 branch.

@ndousson ndousson force-pushed the feat_mailer_infobip_tracking_headers branch from bf5e3ef to dc60605 Compare June 18, 2024 08:08
@ndousson ndousson changed the base branch from 7.1 to 7.2 June 18, 2024 08:09
@ndousson ndousson requested review from chalasr and dunglas as code owners June 18, 2024 08:09
@ndousson
Copy link
Contributor Author

ndousson commented Jun 18, 2024

I updated the target branch and the MR description 👌🏻
I'll have to rebase as I created my branch from a 7.1

@ndousson ndousson force-pushed the feat_mailer_infobip_tracking_headers branch from dc60605 to c01c981 Compare June 18, 2024 08:15
@ndousson
Copy link
Contributor Author

ndousson commented Jun 18, 2024

I rebased my branch on 7.2 but I don't really understand why I see commit from 7.1 🤔

Do you have an idea ?

Executed commands (from the doc):

git checkout 7.2
git fetch upstream
git merge upstream/7.2
git checkout feat_mailer_infobip_tracking_headers
git rebase 7.2

@fabpot
Copy link
Member

fabpot commented Jun 18, 2024

This should be enough:

git checkout feat_mailer_infobip_tracking_headers
git fetch upstream
git rebase upstream/7.2

@ndousson
Copy link
Contributor Author

This should be enough:

git checkout feat_mailer_infobip_tracking_headers
git fetch upstream
git rebase upstream/7.2

I tested but it doesn't fix the issue.. I'll keep searching

@ndousson ndousson force-pushed the feat_mailer_infobip_tracking_headers branch from c01c981 to add289e Compare June 18, 2024 15:20
@ndousson
Copy link
Contributor Author

git rebase -i upstream/7.2 with a manual removal of unwanted commits fixed the problem 🎉

I think it was due to the fact I first created my branch from 7.1, it just doesn't explain why the non interactive rebase didn't worked 🤷🏼

An other question, could it be possible to add this behavior in the next 7.1.x minor version instead of the 7.2 ?

@fabpot
Copy link
Member

fabpot commented Jun 18, 2024

An other question, could it be possible to add this behavior in the next 7.1.x minor version instead of the 7.2 ?

No, all new features must be done the next version. Maintained versions only receive bug fixes.

@fabpot
Copy link
Member

fabpot commented Jun 18, 2024

Thank you @ndousson.

@fabpot fabpot merged commit 41c6833 into symfony:7.2 Jun 18, 2024
8 of 10 checks passed
fabpot added a commit that referenced this pull request Jun 22, 2024
…(ndousson)

This PR was merged into the 7.2 branch.

Discussion
----------

[Mailer] [Infobip] Document the usage of custom headers

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

See #57424

Commits
-------

fc4403d [Mailer][Infobip] Document the usage of custom headers
symfony-splitter pushed a commit to symfony/infobip-mailer that referenced this pull request Jun 22, 2024
…(ndousson)

This PR was merged into the 7.2 branch.

Discussion
----------

[Mailer] [Infobip] Document the usage of custom headers

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

See symfony/symfony#57424

Commits
-------

fc4403df78 [Mailer][Infobip] Document the usage of custom headers
@fabpot fabpot mentioned this pull request Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants