Added webhook trigger for secret reminder #3199
Conversation
WalkthroughThe pull request consolidates and streamlines the handling of projects, secret events, and webhook payloads across multiple services. In the secret approval request service, the project details are now obtained together with the bot key, eliminating an extra database call. The queue service and secret queue now import and use a new webhook payload type ( Possibly related PRs
Suggested reviewers
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
backend/src/queue/queue-service.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. backend/src/ee/services/secret-approval-request/secret-approval-request-service.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. backend/src/services/secret/secret-queue.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 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: 1
🧹 Nitpick comments (6)
docs/documentation/platform/webhooks.mdx (1)
39-53: Good addition of documentation for the new webhook eventThe documentation for the "secrets.reminder-expired" webhook event payload is clear and well-structured. This will help users understand what data to expect when a secret reminder expires.
One minor observation: there's a slight inconsistency in the event naming convention between "secret.modified" (singular) and "secrets.reminder-expired" (plural). Consider standardizing the naming pattern for consistency.
backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts (1)
506-506: Performance optimization: Retrieving project details with bot keyThis change eliminates an additional database query by retrieving the
projectinformation together with thebotKeyandshouldUseSecretV2Bridgein a single call. The project details are later used in email substitutions (line ~878), making this a well-executed optimization.Previously, a separate call to
projectDAL.findById(projectId)would have been needed to get this information.backend/src/queue/queue-service.ts (1)
24-24: Improved type safety with centralized webhook payload typeReplacing the inline payload type definition with the imported
TWebhookPayloadstype is a good refactoring that improves maintainability and consistency. This ensures that all parts of the codebase are using the same payload structure for webhook events.The change is part of a broader refactoring to support multiple webhook event types, including the new secret reminder expiration event.
Also applies to: 111-111
backend/src/services/webhook/webhook-types.ts (1)
40-49: Consider clarifying the “type” property naming to avoid confusion.
Having both the top-leveltype: WebhookEvents.SecretModifiedand an internaltype?: string | null;in the payload can introduce ambiguity. Renaming one of these properties can improve clarity.Also applies to: 51-63
backend/src/services/secret/secret-queue.ts (1)
627-634: Webhook payload for “SecretModified” is concise and clear.
The queued object includes environment, projectId, and secretPath. Double-check if additional information (e.g., projectName) is needed. If it is, consider adding it here to save a database lookup downstream.backend/src/services/webhook/webhook-fns.ts (1)
61-100: Refine the switch statement for maintainability.
There’s acase WebhookType.GENERAL:immediately followed bydefault:which can be combined. Consider removing the redundant case to simplify.Apply this diff to remove the useless case:
- case WebhookType.GENERAL: default: return { event: event.type, ... };🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts(1 hunks)backend/src/queue/queue-service.ts(2 hunks)backend/src/services/secret/secret-queue.ts(5 hunks)backend/src/services/webhook/webhook-fns.ts(7 hunks)backend/src/services/webhook/webhook-service.ts(2 hunks)backend/src/services/webhook/webhook-types.ts(1 hunks)docs/documentation/platform/webhooks.mdx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
backend/src/services/webhook/webhook-fns.ts
[error] 88-88: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run integration test
- GitHub Check: Check TS and Lint
🔇 Additional comments (7)
backend/src/services/webhook/webhook-types.ts (1)
33-66: Structuring event types and payloads looks good!
These new definitions cleanly separate “modified” and “reminder expired” event payloads and enhance maintainability by using a union type for strict type checks.backend/src/services/secret/secret-queue.ts (3)
64-64: No issues with the new import.
ImportingWebhookEventshere is consistent with the rest of the changes.
1066-1108: Verify required fields in the “SecretReminderExpired” payload.
EnsuresecretNameis always present and not null at runtime before queuing, as notifications or webhook handling may fail otherwise.
1516-1527: Check thatjob.datamatches theTWebhookPayloadsshape.
Currently,event: job.datais passed directly. Ifjob.datamight contain additional fields or differ in shape, consider a type guard or validation step to avoid runtime errors.backend/src/services/webhook/webhook-fns.ts (3)
57-155: Overall logic for generating webhook payloads is well-structured.
Switching based onevent.typeand further customizing Slack vs. general payloads ensures clarity.🧰 Tools
🪛 Biome (1.9.4)
[error] 88-88: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 141-141: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
187-192: Fallback to database for project name is a good safeguard.
This ensures webhooks are populated with a valid project name without failing if not present in the payload.
194-201: Preserve event type consistency when reassigning tohook.type.
Overriding thetypefield in the payload withhook.typeis acceptable, but verify thathook.typeis a valid subset ofWebhookEventsto avoid unexpected merges.
Description 📣
PR to trigger webhook event for secret reminder expiry
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippetsSummary by CodeRabbit
Refactor
New Features
Documentation