Skip to content

Commit

Permalink
Initially disable Segment app webhooks (#1678)
Browse files Browse the repository at this point in the history
  • Loading branch information
krzysztofzuraw authored Jan 9, 2025
1 parent 1f524da commit b4196e7
Show file tree
Hide file tree
Showing 19 changed files with 404 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/sharp-mirrors-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"segment": patch
---

After this change webhooks in Segment app will be by default disabled. After configuration is successfully set app will enable webhooks.
44 changes: 44 additions & 0 deletions apps/segment/generated/graphql.ts

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions apps/segment/graphql/mutations/enable-webhook.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
mutation EnableWebhook($id: ID!) {
webhookUpdate(id: $id, input: { isActive: true }) {
errors {
message
code
}
}
}
8 changes: 8 additions & 0 deletions apps/segment/graphql/queries/fetch-app-webhooks.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
query FetchAppWebhooks($id: ID!) {
app(id: $id) {
webhooks {
id
isActive
}
}
}
3 changes: 1 addition & 2 deletions apps/segment/scripts/run-webhooks-migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ const runMigrations = async () => {

const baseUrl = new URL(targetUrl).origin;

// All webhooks in this application are turned on or off. If any of them is enabled, we enable all of them.
return appWebhooks.map((w) => ({ ...w.getWebhookManifest(baseUrl), enabled }));
return appWebhooks.map((w) => w.getWebhookManifest(baseUrl));
},
});

Expand Down
34 changes: 34 additions & 0 deletions apps/segment/src/modules/configuration/configuration.router.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,34 @@
import { TRPCError } from "@trpc/server";
import { z } from "zod";

import { createLogger } from "@/logger";

import { protectedClientProcedure } from "../trpc/protected-client-procedure";
import { router } from "../trpc/trpc-server";
import { WebhooksActivityClient } from "../webhooks/webhook-activity/webhook-activity-client";
import { WebhookActivityService } from "../webhooks/webhook-activity/webhook-activity-service";
import { AppConfigMetadataManager } from "./app-config-metadata-manager";

const logger = createLogger("configurationRouter");

export const configurationRouter = router({
getWebhookConfig: protectedClientProcedure.query(async ({ ctx }) => {
const webhookActivityClient = new WebhooksActivityClient(ctx.apiClient);
const webhookActivityService = new WebhookActivityService(ctx.appId, webhookActivityClient);

const isActiveResult = await webhookActivityService.getWebhooksIsActive();

if (isActiveResult.isErr()) {
logger.error("Error during fetching webhooks isActive", { error: isActiveResult.error });

throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "There was an error with fetching webhooks information. Contact Saleor support.",
});
}

return { areWebhooksActive: isActiveResult.value.some(Boolean) };
}),
getConfig: protectedClientProcedure.query(async ({ ctx }) => {
const manager = AppConfigMetadataManager.createFromAuthData({
appId: ctx.appId,
Expand Down Expand Up @@ -36,5 +56,19 @@ export const configurationRouter = router({
await manager.set(config);

logger.debug("Successfully set config");

const webhookActivityClient = new WebhooksActivityClient(ctx.apiClient);
const webhookActivityService = new WebhookActivityService(ctx.appId, webhookActivityClient);

const enableAppWebhooksResult = await webhookActivityService.enableAppWebhooks();

if (enableAppWebhooksResult.isErr()) {
logger.error("Error during enabling app webhooks", { error: enableAppWebhooksResult.error });

throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "There with enabling app webhooks. Contact Saleor support.",
});
}
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ export const SegmentConfigForm = () => {
const { notifySuccess, notifyError } = useDashboardNotification();

const { data: config, isLoading, refetch } = trpcClient.configuration.getConfig.useQuery();
const utils = trpcClient.useUtils();

const { mutate } = trpcClient.configuration.setConfig.useMutation({
onSuccess() {
notifySuccess("Configuration saved");
refetch();
utils.configuration.getWebhookConfig.refetch();
},
onError() {
notifyError("Error saving configuration");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { Layout, SemanticChip, SkeletonLayout } from "@saleor/apps-ui";
import { Box, Text } from "@saleor/macaw-ui";

import { trpcClient } from "@/modules/trpc/trpc-client";

export const WebhookStatus = () => {
const { data: config, isLoading } = trpcClient.configuration.getWebhookConfig.useQuery();

if (isLoading || !config) {
return <SkeletonLayout.Section />;
}

return (
<Layout.AppSectionCard>
<Box display={"flex"} gap={4} alignItems={"center"} justifyContent={"space-between"}>
{config.areWebhooksActive ? (
<>
<Text size={2} color="default2">
Webhooks are active. For more information about webhooks check Manage app button in
header above.
</Text>
<SemanticChip marginLeft={"auto"} size="small" variant={"success"}>
ACTIVE
</SemanticChip>
</>
) : (
<>
<Text size={2} color="default2">
Your webhooks were disabled. Most likely, your configuration is invalid. Check your
credentials.
</Text>
<SemanticChip marginLeft={"auto"} size="small" variant={"error"}>
DISABLED
</SemanticChip>
</>
)}
</Box>
</Layout.AppSectionCard>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ export const orderCancelledAsyncWebhook =
event: "ORDER_CANCELLED",
apl: saleorApp.apl,
query: OrderCancelledDocument,
/**
* Webhook is disabled by default. Will be enabled by the app when configuration succeeds
*/
isActive: false,
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ export const orderCreatedAsyncWebhook =
event: "ORDER_CREATED",
apl: saleorApp.apl,
query: OrderCreatedDocument,
/**
* Webhook is disabled by default. Will be enabled by the app when configuration succeeds
*/
isActive: false,
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ export const orderFullyPaidAsyncWebhook =
event: "ORDER_FULLY_PAID",
apl: saleorApp.apl,
query: OrderFullyPaidDocument,
/**
* Webhook is disabled by default. Will be enabled by the app when configuration succeeds
*/
isActive: false,
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ export const orderRefundedAsyncWebhook =
event: "ORDER_REFUNDED",
apl: saleorApp.apl,
query: OrderRefundedDocument,
/**
* Webhook is disabled by default. Will be enabled by the app when configuration succeeds
*/
isActive: false,
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ export const orderUpdatedAsyncWebhook =
event: "ORDER_UPDATED",
apl: saleorApp.apl,
query: OrderUpdatedDocument,
/**
* Webhook is disabled by default. Will be enabled by the app when configuration succeeds
*/
isActive: false,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { err, ok, Result } from "neverthrow";
import { Client } from "urql";

import { BaseError } from "@/errors";
import { EnableWebhookDocument, FetchAppWebhooksDocument } from "@/generated/graphql";

export interface IWebhooksActivityClient {
fetchAppWebhooksInformation(
id: string,
): Promise<Result<{ id: string; isActive: boolean }[], unknown>>;
enableSingleWebhook(id: string): Promise<Result<undefined, unknown>>;
}

export class WebhooksActivityClient implements IWebhooksActivityClient {
private static FetchAppWebhooksError = BaseError.subclass("FetchAppWebhooksError");

constructor(private client: Pick<Client, "query" | "mutation">) {}

async fetchAppWebhooksInformation(id: string) {
const result = await this.client.query(FetchAppWebhooksDocument, { id }).toPromise();

if (result.error || !result.data) {
return err(
new WebhooksActivityClient.FetchAppWebhooksError("Error fetching app webhooks", {
cause: result.error,
}),
);
}

if (!result.data?.app?.webhooks) {
return err(
new WebhooksActivityClient.FetchAppWebhooksError(
"Couldnt find webhooks registered for app. Check if app is properly registered",
),
);
}
return ok(result.data.app.webhooks.map((w) => w));
}

async enableSingleWebhook(id: string) {
const result = await this.client
.mutation(EnableWebhookDocument, {
id,
})
.toPromise();

if (result.error || !result.data) {
return err(
new WebhooksActivityClient.FetchAppWebhooksError("Error fetching app webhooks", {
cause: result.error,
}),
);
}

const possibleErrors = result.data.webhookUpdate?.errors ?? [];

if (possibleErrors.length > 0) {
return err(
new WebhooksActivityClient.FetchAppWebhooksError("Error enabling webhook", {
errors: possibleErrors.map((error) =>
WebhooksActivityClient.FetchAppWebhooksError.normalize(error),
),
}),
);
}

return ok(undefined);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { err, ok } from "neverthrow";
import { describe, expect, it, vi } from "vitest";

import { IWebhooksActivityClient } from "./webhook-activity-client";
import { WebhookActivityService } from "./webhook-activity-service";

describe("WebhookActivityService", () => {
it("should enable app webhooks", async () => {
const mockedClient: IWebhooksActivityClient = {
enableSingleWebhook: vi.fn(() => Promise.resolve(ok(undefined))),
fetchAppWebhooksInformation: vi.fn(() =>
Promise.resolve(
ok([
{ id: "webhook-id-1", isActive: true },
{ id: "webhook-id-2", isActive: true },
]),
),
),
};

const service = new WebhookActivityService("app-id", mockedClient);

await service.enableAppWebhooks();

expect(mockedClient.fetchAppWebhooksInformation).toHaveBeenCalled();
expect(mockedClient.enableSingleWebhook).toHaveBeenCalledTimes(2);
expect(mockedClient.enableSingleWebhook).toHaveBeenNthCalledWith(1, "webhook-id-1");
expect(mockedClient.enableSingleWebhook).toHaveBeenNthCalledWith(2, "webhook-id-2");
});

it("should get information if webhooks are active", async () => {
const mockedClient: IWebhooksActivityClient = {
enableSingleWebhook: vi.fn(() => Promise.resolve(ok(undefined))),
fetchAppWebhooksInformation: vi.fn(() =>
Promise.resolve(
ok([
{ id: "webhook-id-1", isActive: true },
{ id: "webhook-id-2", isActive: false },
]),
),
),
};

const service = new WebhookActivityService("app-id", mockedClient);

const response = await service.getWebhooksIsActive();

expect(response._unsafeUnwrap()).toStrictEqual([true, false]);
});

it("should throw error when fetching webhooks information fails", async () => {
const mockedClient: IWebhooksActivityClient = {
enableSingleWebhook: vi.fn(),
fetchAppWebhooksInformation: vi.fn(() =>
Promise.resolve(err("Error during fetching webhooks information")),
),
};

const service = new WebhookActivityService("app-id", mockedClient);

const result = await service.enableAppWebhooks();

expect(result._unsafeUnwrapErr()).toBeInstanceOf(
WebhookActivityService.WebhookActivityServiceWebhooksError,
);

expect(mockedClient.fetchAppWebhooksInformation).toHaveBeenCalled();
expect(mockedClient.enableSingleWebhook).not.toHaveBeenCalled();
});

it("should throw error when enabling app webhooks fails", async () => {
const mockedClient: IWebhooksActivityClient = {
enableSingleWebhook: vi.fn(() => Promise.resolve(err("Error during enabling webhooks"))),
fetchAppWebhooksInformation: vi.fn(() =>
Promise.resolve(ok([{ id: "webhook-id", isActive: false }])),
),
};

const service = new WebhookActivityService("app-id", mockedClient);

const result = await service.enableAppWebhooks();

expect(result._unsafeUnwrapErr()).toBeInstanceOf(
WebhookActivityService.WebhookActivityServiceWebhooksError,
);

expect(mockedClient.fetchAppWebhooksInformation).toHaveBeenCalled();
expect(mockedClient.enableSingleWebhook).toHaveBeenCalled();
});
});
Loading

0 comments on commit b4196e7

Please sign in to comment.