From fc9f1955301fcf6013e3a605a80fab81742eba40 Mon Sep 17 00:00:00 2001 From: Matthew Mikolay Date: Thu, 12 Feb 2026 16:18:24 -0500 Subject: [PATCH] Revert "chore(KNO-11486): exclude metadata when refetching feed after new message received (#853)" This reverts commit 1e75952ebe4757930ba72a996f331e6c662c556e. --- .changeset/chatty-cars-care.md | 16 -- packages/client/src/clients/feed/feed.ts | 40 ++--- .../client/src/clients/feed/interfaces.ts | 20 +-- packages/client/src/clients/feed/store.ts | 4 +- packages/client/src/clients/feed/types.ts | 2 +- packages/client/src/clients/feed/utils.ts | 24 --- .../client/test/clients/feed/feed.test.ts | 143 +---------------- .../client/test/clients/feed/store.test.ts | 26 ---- .../client/test/clients/feed/utils.test.ts | 145 ------------------ 9 files changed, 18 insertions(+), 402 deletions(-) delete mode 100644 .changeset/chatty-cars-care.md diff --git a/.changeset/chatty-cars-care.md b/.changeset/chatty-cars-care.md deleted file mode 100644 index d78249f6c..000000000 --- a/.changeset/chatty-cars-care.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -"@knocklabs/client": minor ---- - -Exclude metadata when refetching feed after new message received - -Starting with this release, if you configure a feed client to listen for events via [`Feed.on()`](https://docs.knock.app/in-app-ui/javascript/sdk/feed-client#on), the payload for feed events of type `"items.received.realtime"` will always have `metadata` set to `undefined`. - -```js -const knockFeed = knock.feeds.initialize(process.env.KNOCK_FEED_CHANNEL_ID); - -knockFeed.on("items.received.realtime", (eventPayload) => { - // eventPayload.metadata will always be undefined here - const { items, metadata } = eventPayload; -}); -``` diff --git a/packages/client/src/clients/feed/feed.ts b/packages/client/src/clients/feed/feed.ts index b4cd71055..01518180d 100644 --- a/packages/client/src/clients/feed/feed.ts +++ b/packages/client/src/clients/feed/feed.ts @@ -33,12 +33,7 @@ import { FeedMessagesReceivedPayload, FeedRealTimeCallback, } from "./types"; -import { - getFormattedExclude, - getFormattedTriggerData, - mergeAndDedupeArrays, - mergeDateRangeParams, -} from "./utils"; +import { getFormattedTriggerData, mergeDateRangeParams } from "./utils"; // Default options to apply const feedClientDefaults: Pick = { @@ -541,27 +536,19 @@ class Feed { // Set the loading type based on the request type it is state.setNetworkStatus(options.__loadingType ?? NetworkStatus.loading); - const mergedOptions = { - ...this.defaultOptions, - ...mergeDateRangeParams(options), - exclude: mergeAndDedupeArrays( - this.defaultOptions.exclude, - options.exclude, - ), - }; - // trigger_data should be a JSON string for the API // this function will format the trigger data if it's an object - // https://docs.knock.app/api-reference/users/feeds/list_items - const formattedTriggerData = getFormattedTriggerData(mergedOptions); - - const formattedExclude = getFormattedExclude(mergedOptions); + // https://docs.knock.app/reference#get-feed + const formattedTriggerData = getFormattedTriggerData({ + ...this.defaultOptions, + ...options, + }); // Always include the default params, if they have been set const queryParams: FetchFeedOptionsForRequest = { - ...mergedOptions, + ...this.defaultOptions, + ...mergeDateRangeParams(options), trigger_data: formattedTriggerData, - exclude: formattedExclude, // Unset options that should not be sent to the API __loadingType: undefined, __fetchSource: undefined, @@ -612,8 +599,7 @@ class Feed { const eventPayload = { items: response.entries as FeedItem[], - // meta will not be present on the response when __fetchSource is "socket" - metadata: response.meta as FeedMetadata | undefined, + metadata: response.meta as FeedMetadata, event: feedEventType, }; @@ -664,13 +650,7 @@ class Feed { } // Fetch the items before the current head (if it exists) - this.fetch({ - before: currentHead?.__cursor, - __fetchSource: "socket", - // The socket event payload should *always* include metadata, - // but to be safe, only exclude meta from the fetch when it's present - exclude: metadata ? ["meta"] : [], - }); + this.fetch({ before: currentHead?.__cursor, __fetchSource: "socket" }); } private buildUserFeedId() { diff --git a/packages/client/src/clients/feed/interfaces.ts b/packages/client/src/clients/feed/interfaces.ts index f90b2c75e..eeb9a3f41 100644 --- a/packages/client/src/clients/feed/interfaces.ts +++ b/packages/client/src/clients/feed/interfaces.ts @@ -58,11 +58,6 @@ export interface FeedClientOptions { * @default "compact" */ mode?: "rich" | "compact"; - /** - * Field paths to exclude from the response. Use dot notation for nested fields - * (e.g., "entries.archived_at"). Limited to 3 levels deep. - */ - exclude?: string[]; } export type FetchFeedOptions = { @@ -70,23 +65,14 @@ export type FetchFeedOptions = { __fetchSource?: "socket" | "http"; } & Omit; -/** - * The final data shape that is sent to the the list feed items endpoint of the Knock API. - * - * @see https://docs.knock.app/api-reference/users/feeds/list_items - */ +// The final data shape that is sent to the API +// Should match types here: https://docs.knock.app/reference#get-feed export type FetchFeedOptionsForRequest = Omit< FeedClientOptions, - "trigger_data" | "exclude" + "trigger_data" > & { // Formatted trigger data into a string trigger_data?: string; - /** Fields to exclude from the response, joined by commas. */ - exclude?: string; - "inserted_at.gte"?: string; - "inserted_at.lte"?: string; - "inserted_at.gt"?: string; - "inserted_at.lt"?: string; // Unset options that should not be sent to the API __loadingType: undefined; __fetchSource: undefined; diff --git a/packages/client/src/clients/feed/store.ts b/packages/client/src/clients/feed/store.ts index d8c33cb87..6e48fedc9 100644 --- a/packages/client/src/clients/feed/store.ts +++ b/packages/client/src/clients/feed/store.ts @@ -73,9 +73,7 @@ const initalizeStore = () => { return { ...state, items, - // Preserve existing metadata if meta is not provided - // (e.g., when excluded via `exclude` param) - metadata: meta ?? state.metadata, + metadata: meta, pageInfo: options.shouldSetPage ? page_info : state.pageInfo, loading: false, networkStatus: NetworkStatus.ready, diff --git a/packages/client/src/clients/feed/types.ts b/packages/client/src/clients/feed/types.ts index e0c7f66d1..b83df1d62 100644 --- a/packages/client/src/clients/feed/types.ts +++ b/packages/client/src/clients/feed/types.ts @@ -56,7 +56,7 @@ export type BindableFeedEvent = FeedEvent | "items.received.*" | "items.*"; export interface FeedEventPayload { event: Omit; items: FeedItem[]; - metadata?: FeedMetadata; + metadata: FeedMetadata; } export type FeedRealTimeCallback = (resp: FeedResponse) => void; diff --git a/packages/client/src/clients/feed/utils.ts b/packages/client/src/clients/feed/utils.ts index e4cbb5bd4..9d764c513 100644 --- a/packages/client/src/clients/feed/utils.ts +++ b/packages/client/src/clients/feed/utils.ts @@ -66,27 +66,3 @@ export function getFormattedTriggerData(options: FeedClientOptions) { return undefined; } - -export function getFormattedExclude(options: FeedClientOptions) { - if (!options?.exclude?.length) { - return undefined; - } - - const fields = options.exclude - .map((field) => field.trim()) - .filter((field) => !!field); - - return fields.length ? fields.join(",") : undefined; -} - -/** - * Merges two arrays, deduplicating values. - * Returns undefined if the merged result is empty. - */ -export function mergeAndDedupeArrays( - array1: T[] | undefined, - array2: T[] | undefined, -): T[] | undefined { - const merged = [...(array1 ?? []), ...(array2 ?? [])]; - return merged.length ? [...new Set(merged)] : undefined; -} diff --git a/packages/client/test/clients/feed/feed.test.ts b/packages/client/test/clients/feed/feed.test.ts index d2b52a732..0cdf9a3cf 100644 --- a/packages/client/test/clients/feed/feed.test.ts +++ b/packages/client/test/clients/feed/feed.test.ts @@ -1253,7 +1253,7 @@ describe("Feed", () => { event: "new-message" as const, metadata: { total_count: 2, unread_count: 2, unseen_count: 2 }, data: { - [feed.referenceId]: { + client_ref_id: { metadata: { total_count: 2, unread_count: 2, unseen_count: 2 }, }, }, @@ -1261,66 +1261,8 @@ describe("Feed", () => { await feed.handleSocketEvent(newMessagePayload); - // Should trigger a fetch to get the latest data with exclude: "meta" - expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ - method: "GET", - url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", - params: { - archived: "exclude", - mode: "compact", - exclude: "meta", - }, - }); - } finally { - cleanup(); - } - }); - - test("handles new message socket events without metadata in payload", async () => { - const { knock, mockApiClient, cleanup } = getTestSetup(); - - try { - const mockSocketManager = { - join: vi.fn().mockReturnValue(vi.fn()), - leave: vi.fn(), - }; - - // Mock the store response for the feed fetch - mockApiClient.makeRequest.mockResolvedValue({ - statusCode: "ok", - body: { - entries: [], - page_info: { before: null, after: null }, - meta: { total_count: 1, unread_count: 1, unseen_count: 1 }, - }, - }); - - const feed = new Feed( - knock, - "01234567-89ab-cdef-0123-456789abcdef", - {}, - mockSocketManager as unknown as FeedSocketManager, - ); - - // Payload lacking metadata for this client's referenceId. - // This should never happen in practice, but we should handle it gracefully. - const newMessagePayload = { - event: "new-message" as const, - metadata: { total_count: 2, unread_count: 2, unseen_count: 2 }, - data: {}, - }; - - await feed.handleSocketEvent(newMessagePayload); - - // Should trigger a fetch WITHOUT exclude: "meta" to get badge counts from API - expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ - method: "GET", - url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", - params: { - archived: "exclude", - mode: "compact", - }, - }); + // Should trigger a fetch to get the latest data + expect(mockApiClient.makeRequest).toHaveBeenCalled(); } finally { cleanup(); } @@ -1735,83 +1677,4 @@ describe("Feed", () => { } }); }); - - describe("Exclude Option", () => { - test("converts exclude array to comma-separated string in query params", async () => { - const { knock, mockApiClient, cleanup } = getTestSetup(); - - try { - const mockFeedResponse = { - entries: [], - meta: { total_count: 0, unread_count: 0, unseen_count: 0 }, - page_info: { before: null, after: null, page_size: 50 }, - }; - - mockApiClient.makeRequest.mockResolvedValue({ - statusCode: "ok", - body: mockFeedResponse, - }); - - const feed = new Feed( - knock, - "01234567-89ab-cdef-0123-456789abcdef", - {}, - undefined, - ); - - await feed.fetch({ - exclude: ["entries.archived_at", "meta.total_count"], - }); - - expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ - method: "GET", - url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", - params: { - archived: "exclude", - mode: "compact", - exclude: "entries.archived_at,meta.total_count", - }, - }); - } finally { - cleanup(); - } - }); - - test("ignores empty exclude array", async () => { - const { knock, mockApiClient, cleanup } = getTestSetup(); - - try { - const mockFeedResponse = { - entries: [], - meta: { total_count: 0, unread_count: 0, unseen_count: 0 }, - page_info: { before: null, after: null, page_size: 50 }, - }; - - mockApiClient.makeRequest.mockResolvedValue({ - statusCode: "ok", - body: mockFeedResponse, - }); - - const feed = new Feed( - knock, - "01234567-89ab-cdef-0123-456789abcdef", - {}, - undefined, - ); - - await feed.fetch({ exclude: [] }); - - expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ - method: "GET", - url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", - params: { - archived: "exclude", - mode: "compact", - }, - }); - } finally { - cleanup(); - } - }); - }); }); diff --git a/packages/client/test/clients/feed/store.test.ts b/packages/client/test/clients/feed/store.test.ts index c3465fc31..4e03f8905 100644 --- a/packages/client/test/clients/feed/store.test.ts +++ b/packages/client/test/clients/feed/store.test.ts @@ -232,32 +232,6 @@ describe("feed store", () => { const state = store.getState(); expect(state.items).toHaveLength(2); // Should deduplicate }); - - test("preserves existing metadata when meta is undefined", () => { - const store = createStore(); - - // Set initial result with metadata - store.getState().setResult({ - entries: mockItems, - meta: mockMetadata, - page_info: mockPageInfo, - }); - - // Verify initial metadata is set - expect(store.getState().metadata).toEqual(mockMetadata); - - // Set new result without meta (simulating exclude param) - store.getState().setResult({ - entries: [mockItems[0]!], - meta: undefined, - page_info: mockPageInfo, - }); - - // Metadata should be preserved - const state = store.getState(); - expect(state.metadata).toEqual(mockMetadata); - expect(state.items).toHaveLength(1); - }); }); describe("setMetadata", () => { diff --git a/packages/client/test/clients/feed/utils.test.ts b/packages/client/test/clients/feed/utils.test.ts index 9bcb581fe..d8395df27 100644 --- a/packages/client/test/clients/feed/utils.test.ts +++ b/packages/client/test/clients/feed/utils.test.ts @@ -7,9 +7,7 @@ import type { } from "../../../src/clients/feed/interfaces"; import { deduplicateItems, - getFormattedExclude, getFormattedTriggerData, - mergeAndDedupeArrays, mergeDateRangeParams, sortItems, } from "../../../src/clients/feed/utils"; @@ -419,147 +417,4 @@ describe("feed utils", () => { }); }); }); - - describe("getFormattedExclude", () => { - test("returns undefined when no exclude option", () => { - const options: FeedClientOptions = { - archived: "exclude", - }; - - const result = getFormattedExclude(options); - - expect(result).toBeUndefined(); - }); - - test("returns undefined when exclude is undefined", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: undefined, - }; - - const result = getFormattedExclude(options); - - expect(result).toBeUndefined(); - }); - - test("returns undefined when exclude is empty array", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: [], - }; - - const result = getFormattedExclude(options); - - expect(result).toBeUndefined(); - }); - - test("returns single field as-is", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: ["entries.archived_at"], - }; - - const result = getFormattedExclude(options); - - expect(result).toBe("entries.archived_at"); - }); - - test("joins multiple fields with commas", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: ["entries.archived_at", "meta.total_count", "entries.data"], - }; - - const result = getFormattedExclude(options); - - expect(result).toBe("entries.archived_at,meta.total_count,entries.data"); - }); - - test("trims whitespace before joining fields", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: [ - " entries.archived_at ", - "meta.total_count\n", - "\tentries.data ", - ], - }; - - const result = getFormattedExclude(options); - - expect(result).toBe("entries.archived_at,meta.total_count,entries.data"); - }); - - test("filters out empty fields", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: ["entries.archived_at", " ", "entries.data"], - }; - - const result = getFormattedExclude(options); - - expect(result).toBe("entries.archived_at,entries.data"); - }); - - test("returns undefined for empty options object", () => { - const options: FeedClientOptions = {}; - - const result = getFormattedExclude(options); - - expect(result).toBeUndefined(); - }); - - test("returns undefined when all fields are whitespace-only", () => { - const options: FeedClientOptions = { - archived: "exclude", - exclude: [" ", " ", "\t", "\n"], - }; - - const result = getFormattedExclude(options); - - expect(result).toBeUndefined(); - }); - }); - - describe("mergeAndDedupeArrays", () => { - test("returns undefined when both arrays are undefined", () => { - const result = mergeAndDedupeArrays(undefined, undefined); - expect(result).toBeUndefined(); - }); - - test("returns undefined when both arrays are empty", () => { - const result = mergeAndDedupeArrays([], []); - expect(result).toBeUndefined(); - }); - - test("returns first array when second is undefined", () => { - const result = mergeAndDedupeArrays(["a", "b"], undefined); - expect(result).toEqual(["a", "b"]); - }); - - test("returns first array when second is empty", () => { - const result = mergeAndDedupeArrays(["a", "b"], []); - expect(result).toEqual(["a", "b"]); - }); - - test("returns second array when first is undefined", () => { - const result = mergeAndDedupeArrays(undefined, ["c", "d"]); - expect(result).toEqual(["c", "d"]); - }); - - test("returns second array when first is empty", () => { - const result = mergeAndDedupeArrays([], ["c", "d"]); - expect(result).toEqual(["c", "d"]); - }); - - test("merges both arrays", () => { - const result = mergeAndDedupeArrays(["a", "b"], ["c", "d"]); - expect(result).toEqual(["a", "b", "c", "d"]); - }); - - test("deduplicates merged arrays", () => { - const result = mergeAndDedupeArrays(["a", "b"], ["b", "c"]); - expect(result).toEqual(["a", "b", "c"]); - }); - }); });