-
Notifications
You must be signed in to change notification settings - Fork 14
feat(KNO-11394): initialize feed clients in compact mode by default #851
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
base: main
Are you sure you want to change the base?
Changes from all commits
0947e69
dc3c347
14a98d9
1d15435
dbaf175
a16267a
f7c5d46
b1e6054
7dff37e
4980591
e6014f4
db04746
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 |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| --- | ||
| "@knocklabs/client": minor | ||
| "@knocklabs/react-core": minor | ||
| "@knocklabs/expo": minor | ||
| "@knocklabs/react": minor | ||
| "@knocklabs/react-native": minor | ||
| --- | ||
|
|
||
| Initialize feeds in `"compact"` mode by default | ||
|
|
||
| The feed client can now be initialized with a `mode` option, set to either `"compact"` or `"rich"`. When `mode` is `"compact"`, the `activities` and `total_activities` fields will _not_ be present on feed items, and the `data` field will not include nested arrays and objects. | ||
|
|
||
| **By default, feeds are initialized in `"compact"` mode. If you need to access `activities`, `total_activities`, or the complete `data`, you must initialize your feed in `"rich"` mode.** | ||
|
|
||
| If you are using the feed client via `@knocklabs/client` directly: | ||
|
|
||
| ```js | ||
| const knockFeed = knockClient.feeds.initialize( | ||
| process.env.KNOCK_FEED_CHANNEL_ID, | ||
| { mode: "full" }, | ||
| ); | ||
| ``` | ||
|
|
||
| If you are using `<KnockFeedProvider>` via `@knocklabs/react`, `@knocklabs/react-native`, or `@knocklabs/expo`: | ||
|
|
||
| ```tsx | ||
| <KnockFeedProvider | ||
| feedId={process.env.KNOCK_FEED_CHANNEL_ID} | ||
| defaultFeedOptions={{ mode: "full" }} | ||
| /> | ||
| ``` | ||
|
|
||
| If you are using the `useNotifications` hook via `@knocklabs/react-core`: | ||
|
|
||
| ```js | ||
| const feedClient = useNotifications( | ||
| knockClient, | ||
| process.env.KNOCK_FEED_CHANNEL_ID, | ||
| { mode: "full" }, | ||
| ); | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { describe, expect, test, vi } from "vitest"; | ||
|
|
||
| import type { FetchFeedOptions } from "../../../src"; | ||
| import ApiClient from "../../../src/api"; | ||
| import Feed from "../../../src/clients/feed/feed"; | ||
| import { FeedSocketManager } from "../../../src/clients/feed/socket-manager"; | ||
|
|
@@ -83,6 +84,7 @@ describe("Feed", () => { | |
| ); | ||
|
|
||
| expect(feed.defaultOptions.archived).toBe("exclude"); | ||
| expect(feed.defaultOptions.mode).toBe("compact"); | ||
| } finally { | ||
| cleanup(); | ||
| } | ||
|
|
@@ -546,7 +548,7 @@ describe("Feed", () => { | |
| expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ | ||
| method: "GET", | ||
| url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", | ||
| params: { archived: "exclude" }, | ||
| params: { archived: "exclude", mode: "compact" }, | ||
| }); | ||
| expect(result).toBeDefined(); | ||
| if (result && "entries" in result) { | ||
|
|
@@ -579,10 +581,11 @@ describe("Feed", () => { | |
| undefined, | ||
| ); | ||
|
|
||
| const options = { | ||
| const options: FetchFeedOptions = { | ||
| page_size: 25, | ||
| source: "workflow_123", | ||
| tenant: "tenant_456", | ||
| mode: "rich", | ||
| }; | ||
|
|
||
| await feed.fetch(options); | ||
|
|
@@ -595,6 +598,7 @@ describe("Feed", () => { | |
| page_size: 25, | ||
| source: "workflow_123", | ||
| tenant: "tenant_456", | ||
| mode: "rich", | ||
| }, | ||
| }); | ||
| } finally { | ||
|
|
@@ -650,6 +654,7 @@ describe("Feed", () => { | |
| url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", | ||
| params: { | ||
| archived: "exclude", | ||
| mode: "compact", | ||
| after: "cursor_123", | ||
| }, | ||
| }); | ||
|
|
@@ -1582,4 +1587,131 @@ describe("Feed", () => { | |
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe("Feed Mode", () => { | ||
| test("sets mode query param to compact by default", 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(); | ||
|
|
||
| expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ | ||
| method: "GET", | ||
| url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", | ||
| params: { | ||
| archived: "exclude", | ||
| mode: "compact", | ||
| }, | ||
| }); | ||
| } finally { | ||
| cleanup(); | ||
| } | ||
| }); | ||
|
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. Duplicate test for default compact mode behaviorLow Severity The test "sets mode query param to compact by default" is nearly identical to "fetches feed data successfully" (lines 516-560). Both create a feed with empty options and verify that |
||
|
|
||
| test("sets mode query param to rich when initialized in rich mode", 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", | ||
| { mode: "rich" }, | ||
| undefined, | ||
| ); | ||
|
|
||
| await feed.fetch(); | ||
|
|
||
| expect(mockApiClient.makeRequest).toHaveBeenCalledWith({ | ||
| method: "GET", | ||
| url: "/v1/users/user_123/feeds/01234567-89ab-cdef-0123-456789abcdef", | ||
| params: { | ||
| archived: "exclude", | ||
| mode: "rich", | ||
| }, | ||
| }); | ||
| } finally { | ||
| cleanup(); | ||
| } | ||
| }); | ||
|
|
||
| test("handles lack of activities and total_activities in compact mode", async () => { | ||
| const { knock, mockApiClient, cleanup } = getTestSetup(); | ||
|
|
||
| try { | ||
| // Create a compact mode feed item (no activities or total_activities) | ||
| const compactFeedItem = { | ||
| __cursor: "cursor_123", | ||
| id: "msg_123", | ||
| actors: [], | ||
| blocks: [], | ||
| archived_at: null, | ||
| inserted_at: new Date().toISOString(), | ||
| read_at: null, | ||
| seen_at: null, | ||
| clicked_at: null, | ||
| interacted_at: null, | ||
| link_clicked_at: null, | ||
| source: { key: "workflow", version_id: "v1", categories: [] }, | ||
| tenant: null, | ||
| total_actors: 1, | ||
| updated_at: new Date().toISOString(), | ||
| data: { message: "Hello" }, | ||
| }; | ||
|
|
||
| const mockFeedResponse = { | ||
| entries: [compactFeedItem], | ||
| meta: { total_count: 1, unread_count: 1, unseen_count: 1 }, | ||
| 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", | ||
| { mode: "compact" }, | ||
| undefined, | ||
| ); | ||
|
|
||
| const result = await feed.fetch(); | ||
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result!.data).toEqual(mockFeedResponse); | ||
| } finally { | ||
| cleanup(); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||


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.
Sounds like there may be a potential breakage if you upgrade without any code change?
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.
Correct. As per Chris’s comment here: