Skip to content

Allow custom sender#10945

Merged
ItzNotABug merged 6 commits into1.8.xfrom
allow-custom-sender
Dec 12, 2025
Merged

Allow custom sender#10945
ItzNotABug merged 6 commits into1.8.xfrom
allow-custom-sender

Conversation

@ItzNotABug
Copy link
Copy Markdown
Contributor

@ItzNotABug ItzNotABug commented Dec 12, 2025

What does this PR do?

Allow changing sender name, email and reply-to name and email.

Test Plan

N/A.

Related PRs and Issues

N/A.

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@ItzNotABug ItzNotABug self-assigned this Dec 12, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 12, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

Adds a new protected array property customMailOptions to the Mail event class with public setters/getters for senderEmail, senderName, replyToEmail, and replyToName. reset() now clears customMailOptions. preparePayload() includes customMailOptions in the returned payload. The Mails worker logic is updated to prefer values from customMailOptions when resolving From and Reply-To, falling back to existing SMTP/default resolution when those custom values are absent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect Mail.php to confirm all getters/setters consistently read/write customMailOptions and types/return chaining are correct.
  • Verify reset() clears customMailOptions and no residual state remains between events.
  • Review preparePayload() to ensure payload shape is stable and backward compatible.
  • Review src/Appwrite/Platform/Workers/Mails.php to confirm precedence: use customMailOptions for From and Reply-To, and that original SMTP/default fallback logic remains intact.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Allow custom sender' is concise and directly relates to the main change of enabling custom sender configuration, though it doesn't mention reply-to functionality also added.
Description check ✅ Passed The description clearly explains that the PR allows changing sender name, email, and reply-to name and email, which aligns with the actual changes made to support custom mail options.
Docstring Coverage ✅ Passed Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch allow-custom-sender
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2052496 and 37b43b0.

📒 Files selected for processing (1)
  • src/Appwrite/Platform/Workers/Mails.php (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (Migrations)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (Tokens)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Projects)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E General Test
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: Unit Test
  • GitHub Check: scan
🔇 Additional comments (3)
src/Appwrite/Platform/Workers/Mails.php (3)

155-156: LGTM!

Clean extraction of custom mail options with a safe default. The empty array fallback ensures subsequent checks won't fail on undefined keys.

164-170: LGTM! Well-structured fallback hierarchy.

The reply-to logic correctly implements the documented fallback hierarchy. The initialized defaults on lines 152-153 ensure partial overrides (providing only email or only name) are safe, and the conditional structure cleanly separates custom options from SMTP config.

158-162: The OR condition and fallback logic are safe; no changes needed.

Both code paths pre-populate From and FromName before reaching this logic: the registry setup (app/init/registers.php line 21) calls setFrom($email, $from), and the custom SMTP path (getMailer at line 213) does the same. The subsequent clearAddresses()/clearRecipients() calls only clear recipient data, not sender info. Therefore, partial overrides (e.g., only senderName without senderEmail) safely fall back to pre-configured values and will never pass an empty email address to setFrom().

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 12, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

ChiragAgg5k
ChiragAgg5k reviewed Dec 12, 2025
View reviewed changes
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 12, 2025

✨ Benchmark results

  • Requests per second: 1,177
  • Requests with 200 status code: 211,972
  • P99 latency: 0.163645094

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,177 1,256
200 211,972 226,181
P99 0.163645094 0.161462366

@ItzNotABug ItzNotABug merged commit 8e434b7 into 1.8.x Dec 12, 2025
73 of 74 checks passed
@ItzNotABug ItzNotABug deleted the allow-custom-sender branch December 12, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChiragAgg5k ChiragAgg5k ChiragAgg5k approved these changes

Assignees

@ItzNotABug ItzNotABug

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ItzNotABug @ChiragAgg5k