chore(KNO-11486): exclude metadata when refetching feed after new message received#853
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f71b7c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@cursor review |
|
@cursor review |
|
|
||
| // Always include the default params, if they have been set | ||
| const queryParams: FetchFeedOptionsForRequest = { | ||
| ...this.defaultOptions, |
There was a problem hiding this comment.
Is it okay we are not using mergedOptions here? i.e. we don't include options.
There was a problem hiding this comment.
Good catch. This was working because the destructure of ...mergeDateRangeParams(options) accomplishes the same thing. But I think it’s better if we use mergedOptions everywhere, so I made that change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## mattmik-kno-11394-support-compact-mode-for-feeds #853 +/- ##
====================================================================================
+ Coverage 68.45% 68.58% +0.12%
====================================================================================
Files 193 193
Lines 8062 8088 +26
Branches 1065 1076 +11
====================================================================================
+ Hits 5519 5547 +28
+ Misses 2518 2516 -2
Partials 25 25
|
| "inserted_at.gte"?: string; | ||
| "inserted_at.lte"?: string; | ||
| "inserted_at.gt"?: string; | ||
| "inserted_at.lt"?: string; |
There was a problem hiding this comment.
Did we mean to add these?
There was a problem hiding this comment.
Yes, it’s a mistake that they weren’t already in the FetchFeedOptionsForRequest type.
FetchFeedOptionsForRequest is meant to represent the params included in the API request. Notice that the mergeDateRangeParams function transforms the inserted_at_date_range option on FeedClientOptions into the appropriate date params, e.g. inserted_at.gte, inserted_at.lte, etc.
javascript/packages/client/src/clients/feed/utils.ts
Lines 25 to 50 in 9313398

Description
This PR updates the client SDK so that:
excludeoption, used to exclude specified fields from the list feed items response payload"new-message"is received, the request we make to the list feed items endpoint automatically excludes themetafield from its response.metacontains the badge counts, which are already present on the socket event payload, so querying formetais redundant.Todos
Checklist
Loom demo
https://www.loom.com/share/56c2bb427c31485ea8b837c3c416be04