refactor(smtppostmaster): use centralized env system for SMTP config #617
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add SmtpOptions interface and defaults to @pgpmjs/types - Add SMTP_* env var parsing to @pgpmjs/env getEnvVars() - Refactor smtppostmaster to use getEnvOptions() instead of direct process.env - Add smtpOverrides parameter to send() for programmatic configuration - Add resetTransport() helper for test isolation - Update tests to use smtpOverrides instead of env manipulation - Update README with new programmatic override examples Addresses PR #577 feedback to move process.env access into the centralized environment system.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR builds on top of PR #577 (feat/cloud-fn-test) and addresses the feedback to move
process.envaccess insmtppostmasterinto the centralized environment system (@pgpmjs/env/@pgpmjs/types).Changes:
SmtpOptionsinterface and defaults to@pgpmjs/typesgetEnvVars()in@pgpmjs/envsmtppostmasterto usegetEnvOptions()instead of directprocess.envaccesssmtpOverridesparameter tosend()for programmatic configurationresetTransport()helper for test isolationUpdates since last revision
pgpm/env/__tests__/__snapshots__/merge.test.ts.snapto include the newsmtpdefaultsReview & Testing Checklist for Human
pgpm/types/src/pgpm.tsincludes all necessary SMTP fields (host, port, secure, user, pass, from, replyTo, TLS options, pool settings, etc.)getTransport()function inpackages/smtppostmaster/src/index.tscreates a new transport when overrides are provided. Verify this doesn't cause issues with connection pooling or resource leaks.send()function now accepts an optional second parameter. Verify existing callers infunctions/send-email-linkandfunctions/simple-emailwork correctly.getEnvOptions()Recommended test plan:
pnpm --filter @constructive-io/smtppostmaster testto verify unit tests passsend()directlyNotes
This PR includes all changes from PR #577 plus the env refactor. The original PR changes (jobs consolidation, E2E tests, SMTP switch for cloud functions) should be reviewed as part of PR #577.
Link to Devin run: https://app.devin.ai/sessions/74d34686e5f2432ba8efd913b271b98c
Requested by: Dan Lynch (@pyramation)