Remove space from email display name#1462
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (15)
📝 WalkthroughWalkthroughThe PR standardizes branding by replacing "Compact Connect" with "CompactConnect" across Node.js email code, tests, package metadata, and docs; upgrades nodemailer/@types; updates IAM SES policy conditions to allow both old and new FromDisplayName values for migration; and bumps many Python dev/runtime dependency pins. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
5c75275 to
50a7049
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/cosmetology-app/stacks/persistent_stack/__init__.py (1)
372-372: Track removal of the legacy display-name permission with a concrete follow-up reference.Line [372] has the right temporary TODO; adding an issue ID/target release in the comment would reduce risk of leaving legacy permission indefinitely.
If useful, I can draft a short follow-up issue template for the cleanup step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cosmetology-app/stacks/persistent_stack/__init__.py` at line 372, Update the TODO comment that reads 'Remove the "Compact Connect" display name once all Lambda code exclusively uses "CompactConnect"' to include a concrete follow-up reference (e.g., an issue/PR ID or target release and owner) so the legacy display-name permission removal is tracked; locate the comment in persistent_stack/__init__.py (the TODO mentioning "Compact Connect"/"CompactConnect") and append the issue number or release milestone and an owner/assignee and expected completion date.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/compact-connect/lambdas/nodejs/package.json`:
- Line 52: Update the dev/prod dependency for the TypeScript definitions to
match nodemailer 8.x by changing the `@types/nodemailer` entry to ^8.0.0 in
package.json; locate the existing "nodemailer": "^8.0.5" and the pinned
"@types/nodemailer": "7.0.9" and update the latter to "^8.0.0", then run your
package manager install and rebuild to ensure types align with the nodemailer
8.x API.
---
Nitpick comments:
In `@backend/cosmetology-app/stacks/persistent_stack/__init__.py`:
- Line 372: Update the TODO comment that reads 'Remove the "Compact Connect"
display name once all Lambda code exclusively uses "CompactConnect"' to include
a concrete follow-up reference (e.g., an issue/PR ID or target release and
owner) so the legacy display-name permission removal is tracked; locate the
comment in persistent_stack/__init__.py (the TODO mentioning "Compact
Connect"/"CompactConnect") and append the issue number or release milestone and
an owner/assignee and expected completion date.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94803ecd-c831-488a-b1f9-c351099251ef
⛔ Files ignored due to path filters (1)
backend/compact-connect/lambdas/nodejs/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
backend/compact-connect/lambdas/nodejs/lib/email/base-email-service.tsbackend/compact-connect/lambdas/nodejs/lib/email/email-notification-service.tsbackend/compact-connect/lambdas/nodejs/package.jsonbackend/compact-connect/lambdas/nodejs/tests/email-notification-service.test.tsbackend/compact-connect/lambdas/nodejs/tests/lib/email/email-notification-service.test.tsbackend/compact-connect/lambdas/nodejs/tests/lib/email/encumbrance-notification-service.test.tsbackend/compact-connect/lambdas/nodejs/tests/lib/email/ingest-event-email-service.test.tsbackend/compact-connect/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.tsbackend/compact-connect/lambdas/python/cognito-backup/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements-dev.inbackend/compact-connect/lambdas/python/common/requirements-dev.txtbackend/compact-connect/lambdas/python/common/requirements.txtbackend/compact-connect/lambdas/python/compact-configuration/requirements-dev.txtbackend/compact-connect/lambdas/python/custom-resources/requirements-dev.txtbackend/compact-connect/lambdas/python/data-events/requirements-dev.txtbackend/compact-connect/lambdas/python/disaster-recovery/requirements-dev.txtbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.inbackend/compact-connect/lambdas/python/provider-data-v1/requirements-dev.txtbackend/compact-connect/lambdas/python/purchases/requirements-dev.inbackend/compact-connect/lambdas/python/purchases/requirements-dev.txtbackend/compact-connect/lambdas/python/search/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/compact-connect/lambdas/python/staff-users/requirements-dev.inbackend/compact-connect/lambdas/python/staff-users/requirements-dev.txtbackend/compact-connect/requirements-dev.inbackend/compact-connect/requirements-dev.txtbackend/compact-connect/requirements.txtbackend/compact-connect/stacks/persistent_stack/__init__.pybackend/cosmetology-app/lambdas/nodejs/lib/email/base-email-service.tsbackend/cosmetology-app/lambdas/nodejs/package.jsonbackend/cosmetology-app/lambdas/nodejs/tests/email-notification-service.test.tsbackend/cosmetology-app/lambdas/nodejs/tests/lib/email/encumbrance-notification-service.test.tsbackend/cosmetology-app/lambdas/nodejs/tests/lib/email/ingest-event-email-service.test.tsbackend/cosmetology-app/lambdas/nodejs/tests/lib/email/investigation-notification-service.test.tsbackend/cosmetology-app/lambdas/python/cognito-backup/requirements-dev.txtbackend/cosmetology-app/lambdas/python/cognito-backup/requirements.txtbackend/cosmetology-app/lambdas/python/common/cc_common/data_model/provider_record_util.pybackend/cosmetology-app/lambdas/python/common/requirements-dev.inbackend/cosmetology-app/lambdas/python/common/requirements-dev.txtbackend/cosmetology-app/lambdas/python/common/requirements.txtbackend/cosmetology-app/lambdas/python/compact-configuration/requirements-dev.txtbackend/cosmetology-app/lambdas/python/custom-resources/requirements-dev.txtbackend/cosmetology-app/lambdas/python/data-events/requirements-dev.txtbackend/cosmetology-app/lambdas/python/disaster-recovery/requirements-dev.txtbackend/cosmetology-app/lambdas/python/provider-data-v1/requirements-dev.inbackend/cosmetology-app/lambdas/python/provider-data-v1/requirements-dev.txtbackend/cosmetology-app/lambdas/python/search/requirements-dev.txtbackend/cosmetology-app/lambdas/python/search/requirements.txtbackend/cosmetology-app/lambdas/python/staff-user-pre-token/requirements-dev.txtbackend/cosmetology-app/lambdas/python/staff-users/requirements-dev.inbackend/cosmetology-app/lambdas/python/staff-users/requirements-dev.txtbackend/cosmetology-app/requirements-dev.inbackend/cosmetology-app/requirements-dev.txtbackend/cosmetology-app/requirements.txtbackend/cosmetology-app/stacks/persistent_stack/__init__.py
This system is not live in prod yet, so we can perform a hard cut-over to the new name
There was a problem hiding this comment.
There are many more references to Compact Connect throughout the code base, most of them are internal so not super important but the couple that I think should be addressed are:
Thank you for integrating with Compact Connect! You have been designated as the IT professional who is able to handle
credentials for secure machine-to-machine authentication between your state and CompactConnect.
And the external facing readmes
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen This is good to merge!
The display name for the email addresses was originally configured with a space between Compact and Connect. This display name is specified by an IAM policy that must be updated in order to specify the corrected 'CompactConnect' display name. To avoid downtime, we need to update the policy to support both during the first deployment, then remove the old policy once the change is verified in production.
This also makes several needed dependency updates in an attempt to cut down on the many Dependabot PRs we currently have in place.
Closes #1428
Summary by CodeRabbit
Chores
Tests