-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: send test message #44
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,9 +5,12 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
FindDigestByTypeArgs, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
UpsertDigest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Digest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
DigestWithRelations, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from "./digest.types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { assign, isObject } from "radash"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { DigestType } from "@prisma/client"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { sendTeamMetricsDigest } from "./digest-team-metrics.service"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { sendTeamWipDigest } from "./digest-team-wip.service"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const findDigestByType = async <T extends DigestType>({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
workspaceId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -100,3 +103,13 @@ export const upsertDigest = async ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
return updatedDigest as unknown as Digest; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const sendDigest = async (digest: DigestWithRelations) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (digest.type === DigestType.TEAM_METRICS) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
await sendTeamMetricsDigest(digest); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (digest.type === DigestType.TEAM_WIP) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
await sendTeamWipDigest(digest); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+107
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling and type safety. The current implementation has several areas for improvement:
export const sendDigest = async (digest: DigestWithRelations) => {
+ try {
- if (digest.type === DigestType.TEAM_METRICS) {
- await sendTeamMetricsDigest(digest);
- }
-
- if (digest.type === DigestType.TEAM_WIP) {
- await sendTeamWipDigest(digest);
- }
+ const digestHandlers = {
+ [DigestType.TEAM_METRICS]: sendTeamMetricsDigest,
+ [DigestType.TEAM_WIP]: sendTeamWipDigest,
+ } as const;
+
+ const handler = digestHandlers[digest.type];
+ if (!handler) {
+ throw new Error(`Unsupported digest type: ${digest.type}`);
+ }
+
+ await handler(digest);
+ } catch (error) {
+ throw new Error(`Failed to send digest: ${error.message}`);
+ }
}; 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default /* GraphQL */ ` | ||
directive @rateLimit( | ||
max: Int | ||
window: String | ||
message: String | ||
identityArgs: [String] | ||
arrayLengthField: String | ||
) on FIELD_DEFINITION | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { | ||
BaseException, | ||
BaseExceptionOptions, | ||
ErrorCode, | ||
} from "./base.exception"; | ||
|
||
export class RateLimitException extends BaseException { | ||
name = "RateLimitException"; | ||
|
||
constructor( | ||
message = "Rate limit exceeded", | ||
options?: Partial<BaseExceptionOptions> | ||
) { | ||
super(message, { | ||
code: ErrorCode.RATE_LIMIT_EXCEEDED, | ||
userFacingMessage: "You have exceeded the rate limit.", | ||
severity: "log", | ||
...options, | ||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { z } from "zod"; | ||
import { createMutationResolver } from "../../../../lib/graphql"; | ||
import { logger } from "../../../../lib/logger"; | ||
import { validateInputOrThrow } from "../../../../lib/validate-input"; | ||
import { protectWithPaywall } from "../../../billing/services/billing.service"; | ||
import { authorizeWorkspaceOrThrow } from "../../../workspace-authorization.service"; | ||
import { sendTestMessage } from "../../slack/services/slack-integration.service"; | ||
import { IntegrationApp } from "@sweetr/graphql-types/api"; | ||
|
||
export const sendTestMessageMutation = createMutationResolver({ | ||
sendTestMessage: async (_, { input }, context) => { | ||
logger.info("mutation.sendTestMessage", { input }); | ||
|
||
validateInputOrThrow( | ||
z.object({ | ||
workspaceId: z.number(), | ||
channel: z.string().max(80), | ||
app: z.nativeEnum(IntegrationApp), | ||
}), | ||
Comment on lines
+16
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Remove unused The
🔗 Analysis chainVerify the usage of the The
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if sendTestMessage is implemented for other apps
rg -A 5 "sendTestMessage.*\(" apps/api/src/app/integrations/
Length of output: 2642 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just future-proofing for when we support MS Teams. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @waltergalvao Ah, that makes perfect sense! Thanks for the clarification. (`∀´)D ✏️ Learnings added
🧠 Learnings used
|
||
input | ||
); | ||
|
||
await authorizeWorkspaceOrThrow({ | ||
workspaceId: input.workspaceId, | ||
gitProfileId: context.currentToken.gitProfileId, | ||
}); | ||
|
||
await protectWithPaywall(input.workspaceId); | ||
|
||
await sendTestMessage(input.workspaceId, input.channel); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider error handling for sendTestMessage. The service call should be wrapped in a try-catch block to handle potential integration errors gracefully. - await sendTestMessage(input.workspaceId, input.channel);
+ try {
+ await sendTestMessage(input.workspaceId, input.channel);
+ } catch (error) {
+ logger.error("Failed to send test message", { error, input });
+ throw error;
+ }
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GraphQL already handles this. We show a specific user facing error message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @waltergalvao Understood, thanks for the clarification. The GraphQL layer's error handling is sufficient here. ✏️ Learnings added
|
||
|
||
return null; | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { useRateLimiter } from "@envelop/rate-limiter"; | ||
import { GraphQLContext } from "@sweetr/graphql-types/shared"; | ||
import { RateLimitException } from "../app/errors/exceptions/rate-limit.exception"; | ||
|
||
export const rateLimiterPlugin = useRateLimiter({ | ||
identifyFn: (context: GraphQLContext) => | ||
context.workspaceId?.toString() ?? context.currentToken?.userId.toString(), | ||
transformError: (message: string) => | ||
new RateLimitException("Rate limit exceeded", { | ||
userFacingMessage: message, | ||
}), | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,39 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { IntegrationApp } from "@sweetr/graphql-types/api"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { showSuccessNotification } from "../../../providers/notification.provider"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { getErrorMessage } from "../../../providers/error-message.provider"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useSendTestMessageMutation } from "../../../api/integrations.api"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useWorkspace } from "../../../providers/workspace.provider"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { showErrorNotification } from "../../../providers/notification.provider"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const useSendTestMessage = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { workspace } = useWorkspace(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { mutate, isPending: isSendingTestMessage } = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useSendTestMessageMutation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const sendTestMessage = (channel: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mutate( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
input: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
workspaceId: workspace.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app: IntegrationApp.SLACK, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
channel: channel, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onError: (error) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
showErrorNotification({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
message: getErrorMessage(error), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onSuccess: () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
showSuccessNotification({ message: "Test message sent" }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+13
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation for the channel parameter. The const sendTestMessage = (channel: string) => {
+ if (!channel?.trim()) {
+ showErrorNotification({
+ message: "Please enter a valid channel name",
+ });
+ return;
+ }
+
mutate(
{
input: {
workspaceId: workspace.id,
app: IntegrationApp.SLACK,
- channel: channel,
+ channel: channel.trim(),
},
}, 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sendTestMessage, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isSendingTestMessage, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Missing required peer dependency for rate limiter
The @envelop/rate-limiter package requires @envelop/core ^5.0.2 as a peer dependency, but it's not found in package.json. Please add it to ensure proper functionality.
🔗 Analysis chain
Verify rate limiter package compatibility
The @envelop/rate-limiter package is appropriately added. Let's verify the version compatibility and check for any known issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 799
Script:
Length of output: 411
Script:
Length of output: 162