Skip to content

Fixing the FROM email creds for tokenized email transports #14661

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 5, 2025
Merged

Fixing the FROM email creds for tokenized email transports #14661

merged 3 commits into from
Mar 5, 2025

Conversation

escopecz
Copy link
Member

Q A
Bug fix? (use the a.b branch) ✔️
New feature/enhancement? (use the a.x branch) ❌
Deprecations? ❌
BC breaks? (use the c.x branch) ❌
Automated tests included? In acquia/mc-cs-plugin-sparkpost#4
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes acquia/mc-cs-plugin-sparkpost#4

Description

The existing test in the Sparkpost plugin started to fail due to bug in Mautic. This PR fixes the issue and makes the test pass again. This bug will be present in all tokenized email transports that use API to send emails instead of SMTP.

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Install and configure the Sparkpost plugin. You'll need a Sparkpost account to test with.
  3. Create a contact with an email address
  4. From the contact detail page click on Send Email.
  5. Notice the FROM email address is pre-filled with email address of your user, not the default sending address
  6. Fill in the subject and email body and send the email

Before this change the FROM address and FROM name on the sent email was the default from the global configuration.

With this change the FROM address and name is from your user or whatever email address you define in the FROM address field.

@escopecz escopecz added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging email Anything related to email labels Feb 28, 2025
@escopecz escopecz added the unforking Used for PRs in the Acquia's unforking initiative label Feb 28, 2025
@escopecz escopecz mentioned this pull request Feb 28, 2025
Merged
@escopecz escopecz requested review from fedys and dadarya0 February 28, 2025 14:20
@codecov Codecov
Copy link

codecov bot commented Feb 28, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.67%. Comparing base (376eb99) to head (2dcc76d).
Report is 4 commits behind head on 6.x.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                6.x   #14661   +/-   ##
=========================================
  Coverage     64.67%   64.67%           
  Complexity    34678    34678           
=========================================
  Files          2273     2273           
  Lines        103599   103599           
=========================================
+ Hits          66999    67000    +1     
+ Misses        36600    36599    -1
Files with missing lines Coverage Δ
app/bundles/EmailBundle/Helper/FromEmailHelper.php 96.05% <100.00%> (ø)
app/bundles/EmailBundle/Helper/MailHelper.php 79.01% <100.00%> (ø)

... and 1 file with indirect coverage changes

@escopecz escopecz moved this to 🦸🏻 Needs 2 tests in Open Source Fridays Feb 28, 2025
fedys
fedys approved these changes Mar 3, 2025
View reviewed changes
Copy link
Member

@fedys fedys left a comment

Choose a reason for hiding this comment

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

Thanks @escopecz!

@dadarya0
Copy link
Contributor

dadarya0 commented Mar 4, 2025

@escopecz sparkpost is required for testing of this PR , do we have any any documentation to install sparkpost plugin ?
it is not available in market place list.

@escopecz
Copy link
Member Author

escopecz commented Mar 4, 2025

@dadarya0 It's linked int he PR description. Let me know if you have some trouble installing it.

@dadarya0
Copy link
Contributor

dadarya0 commented Mar 4, 2025

2. Install and configure the Sparkpost plugin.

In PR description link is redirecting to sparkpost repository and did not found more information in readme file.

@escopecz
Copy link
Member Author

escopecz commented Mar 4, 2025

We should fix that!

In the mean time, you can:

  1. cd plugins
  2. git clone git@github.com:acquia/mc-cs-plugin-sparkpost.git SparkpostBundle
  3. cd ..
  4. bin/console m:p:i

@dadarya0
Copy link
Contributor

dadarya0 commented Mar 5, 2025

Tested, working as expected.

without this PR
From address was showing user's email but in mailbox it was default from the global configuration

After applying this PR :-
From address is showing user's email and sending mail from same email which is showing in FROM address field.

image image image

@dadarya0 dadarya0 added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Mar 5, 2025
@escopecz escopecz added this to the 6.0.0-beta milestone Mar 5, 2025
@escopecz escopecz merged commit 2cf6356 into mautic:6.x Mar 5, 2025
17 checks passed
@github-project-automation github-project-automation bot moved this from 🦸🏻 Needs 2 tests to 🥳 Done in Open Source Fridays Mar 5, 2025
@RCheesley RCheesley modified the milestones: 6.0.0-beta, 6.0.0-beta2 Mar 5, 2025
@escopecz escopecz mentioned this pull request Mar 6, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fedys fedys fedys approved these changes

+1 more reviewer

@dadarya0 dadarya0 dadarya0 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@dadarya0 dadarya0

Labels
bug Issues or PR's relating to bugs email Anything related to email ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged unforking Used for PRs in the Acquia's unforking initiative
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
6.0.0-beta2
Development

Successfully merging this pull request may close these issues.
4 participants
@escopecz @dadarya0 @fedys @RCheesley