Alert emails now contain the Org name #1555
Conversation
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/webapp/app/v3/services/alerts/deliverAlert.server.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces a new property, Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
internal-packages/emails/emails/deployment-success.tsx (1)
Line range hint
42-44: Add organization name to the email body for consistencyWhile the organization is shown in the preview, it should also be displayed in the email body for better context, similar to how it's shown in other alert emails.
<Text style={h1} ->{`Version ${version} successfully deployed ${taskCount} tasks in ${environment}`}</Text> +>{`[${organization}] Version ${version} successfully deployed ${taskCount} tasks in ${environment}`}</Text>internal-packages/emails/emails/deployment-failure.tsx (1)
Line range hint
57-58: Add organization name to the email body for consistencyThe organization name should be displayed in the email body to maintain consistency with other alert emails.
-<Text style={h1}>{`An error occurred deploying ${version} in ${environment}`}</Text> +<Text style={h1}>{`[${organization}] An error occurred deploying ${version} in ${environment}`}</Text>internal-packages/emails/emails/alert-attempt-failure.tsx (3)
65-65: Standardize preview text format across all alert emailsThe preview text format differs from other alert emails. Consider standardizing the format:
-<Preview>{`${organization}: [${version}.${environment} ${taskIdentifier}] ${error.message}`}</Preview> +<Preview>{`[${organization}] ${taskIdentifier} [${environment}] ${error.message}`}</Preview>
48-57: Simplify props destructuring for better readabilityThe current multi-line props destructuring can be simplified.
- const { - taskIdentifier, - fileName, - exportName, - version, - environment, - error, - attemptLink, - organization, - } = { + const { taskIdentifier, fileName, exportName, version, environment, error, attemptLink, organization } = {
Organization implementation is inconsistent across email templates
•
deployment-success.tsxanddeployment-failure.tsxuse organization in preview text as[${organization}]
•invite.tsxuses organization name in preview asYou've been invited to ${orgName}
•alert-attempt-failure.tsxandalert-run-failure.tsxhave the organization field in schema but preview text implementation is not visible in the first 50 lines
• Some templates use "organization" while others use "orgName" as the property name🔗 Analysis chain
Line range hint
1-1: Verify organization name implementation across all email templatesLet's ensure the organization name has been consistently implemented across all email templates.
The previous results confirmed that all email schemas include the organization field, but we need to verify the actual usage of the organization name in the email templates for consistency. Let's check how the organization name is displayed in the preview text and email body across all templates.
Let's verify the consistency of how organization is used in email subjects and preview texts. The previous results showed schema definitions and mock data, but we need to check the actual implementation in the email templates.
Let's try a broader search to see the email template implementations, as the previous search patterns might have missed the actual usage patterns. This will help us verify how the organization name is being used in the email content.
Let's examine the actual content of these email template files to verify the organization implementation, as our previous pattern matching attempts didn't yield the expected results. This will show us the beginning of each file where the email components are typically defined.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if organization field is added to all email schemas and previews echo "Checking email templates for organization implementation..." rg -l "AlertAttemptEmailSchema|AlertRunEmailSchema|AlertDeploymentFailureEmailSchema|AlertDeploymentSuccessEmailSchema" internal-packages/emails/emails/ rg "organization: z.string" internal-packages/emails/emails/ rg "\[${organization}\]|\${organization}:" internal-packages/emails/emails/Length of output: 1075
Script:
#!/bin/bash # Check for organization name usage in email templates echo "Checking organization name usage in email templates..." rg -A 2 "\$\{organization\}" internal-packages/emails/emails/ rg -A 2 "organization:" internal-packages/emails/emails/Length of output: 1865
Script:
#!/bin/bash # Check email preview text and subject implementations echo "Checking email preview and subject implementations..." rg -B 2 -A 4 "subject:|previewText:" internal-packages/emails/emails/Length of output: 188
Script:
#!/bin/bash # Check email template implementations more broadly echo "Checking email template implementations..." rg -A 10 "export const.*Email = " internal-packages/emails/emails/Length of output: 163
Script:
#!/bin/bash # Check content of email template files echo "Checking content of email template files..." for file in $(find internal-packages/emails/emails/ -name "*.tsx"); do echo "=== $file ===" head -n 50 "$file" echo -e "\n" doneLength of output: 13900
internal-packages/emails/emails/alert-run-failure.tsx (1)
76-80: Consider improving the preview line readabilityWhile the organization name addition is good, the preview line might be getting too dense. Consider breaking it into two parts:
- <Preview>{`${organization}: [${version}.${environment} ${taskIdentifier}] ${error.message}`}</Preview> + <Preview>{`${organization} - ${error.message}\n[${version}.${environment} ${taskIdentifier}]`}</Preview>The organization display in the email body is well-implemented, with good placement and consistent styling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal-packages/emails/emails/alert-attempt-failure.tsx(3 hunks)internal-packages/emails/emails/alert-run-failure.tsx(4 hunks)internal-packages/emails/emails/deployment-failure.tsx(3 hunks)internal-packages/emails/emails/deployment-success.tsx(2 hunks)
🔇 Additional comments (3)
internal-packages/emails/emails/alert-run-failure.tsx (3)
33-33: LGTM: Schema change correctly adds organization field
The addition of the required organization field to the schema is well-implemented and aligns with the PR objectives.
53-53: LGTM: Preview default value is appropriate
The default value "my-organization" is descriptive and follows the existing pattern of preview defaults.
67-67: LGTM: Props destructuring properly includes organization
The organization prop is correctly destructured and follows the existing code style.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (1)
Line range hint
737-737: Consider adding organization name to Slack message contextWhile the Slack messages include the project name, consider adding the organization name to the context line for consistency with email alerts and better clarity in multi-org environments. For example:
-text: `${runId} | ${taskIdentifier} | ${version}.${environment} | ${alert.project.name}`, +text: `${runId} | ${taskIdentifier} | ${version}.${environment} | ${alert.project.organization.title} / ${alert.project.name}`,Also applies to: 800-800, 896-896, 970-970
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/v3/services/alerts/deliverAlert.server.ts(4 hunks)
🔇 Additional comments (2)
apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (2)
222-222: LGTM: Organization name consistently added to all email alerts
The changes successfully add the organization name to all types of alert emails (attempt, run, deployment failure, and deployment success) in a consistent manner.
Also applies to: 248-248, 281-281, 302-302
Line range hint 1051-1074: LGTM: Robust error handling and helper methods
The error handling is comprehensive, covering various Slack API error types with appropriate logging and retry logic. The helper methods for text truncation and formatting are well-implemented with proper length checks and logging.
Added the Org name to the alert emails. This was from a customer with multiple orgs who couldn't tell where the errors were coming from easily.
Org name has been added to:
Summary by CodeRabbit
New Features
organizationproperty to various email schemas, enhancing email content with organizational context.Bug Fixes