Skip to content
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

Initially disable Segment app webhooks #1678

Merged
merged 5 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading