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.

Sign up for GitHub

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

fix: Update Conversation List when new conversation is streamed #1333

Merged
merged 1 commit into from
Dec 10, 2024
Merged

fix: Update Conversation List when new conversation is streamed #1333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions features/conversation-requests-list/useV3RequestItems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logger from "@/utils/logger";
import { getMessageContentType } from "@/utils/xmtpRN/contentTypes";
import { getV3SpamScore } from "@data/helpers/conversations/spamScore";
import { useCurrentAccount } from "@data/store/accountsStore";
Expand Down Expand Up @@ -44,17 +45,21 @@ export const useV3RequestItems = () => {
} catch {
content = lastMessage?.fallback ?? "";
}

const contentType = getMessageContentType(
lastMessage?.contentTypeId!
);

const spamScore = contentType
? await getV3SpamScore({
let spamScore = 0;
try {
if (contentType) {
spamScore = await getV3SpamScore({
message: content,
contentType,
})
: 0; // TODO: Handle this, maybe not 0 if we can't find content type?
});
}
} catch (error) {
logger.error("Couldn't get spam score", error);
}

if (spamScore > 1) {
likelySpam.push(conversation);
} else {
Expand Down
1 change: 1 addition & 0 deletions queries/useV3ConversationListQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export const updateConversationDataToConversationListQuery = (
conversation: Partial<ConversationWithCodecsType>
) => {
const previousConversationsData = getConversationListQueryData(account);

if (!previousConversationsData) return;
Comment on lines +170 to 171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider initializing empty conversation list instead of early return

The early return when no previous conversations exist could lead to missed updates. Consider initializing with an empty array instead.

- if (!previousConversationsData) return;
+ if (!previousConversationsData) {
+   setConversationListQueryData(account, [conversation]);
+   return;
+ }

This ensures that updates are not lost when the conversation list is initially empty.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!previousConversationsData) return;
if (!previousConversationsData) {
setConversationListQueryData(account, [conversation]);
return;
}

const newConversations: V3ConversationListType =
previousConversationsData.map((c) => {
Expand Down
5 changes: 4 additions & 1 deletion utils/xmtpRN/contentTypes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ export function isContentType(args: {
return contentType.startsWith(prefix);
}

export function getMessageContentType(contentType: string) {
export function getMessageContentType(contentType: string | undefined) {
if (!contentType) {
return undefined;
}
Comment on lines +30 to +32
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should put | undefined here 🤔

And instead handle the undefined before calling the function.

Curious on your thoughts. It's just that I feel it's counterintuitive to call this function with a contentType that is undefined haha.

Love the | undefined instead of ?: string tho! At least it shows it wants an argument

return ObjectTyped.keys(contentTypesPrefixes).find((key) =>
contentType.startsWith(contentTypesPrefixes[key])
);
Expand Down
4 changes: 3 additions & 1 deletion utils/xmtpRN/conversations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import {
DmWithCodecsType,
} from "./client";
import { getXmtpClient } from "./sync";
import { addConversationToConversationListQuery } from "@/queries/useV3ConversationListQuery";

export const streamConversations = async (account: string) => {
await stopStreamingConversations(account);
const client = (await getXmtpClient(account)) as ConverseXmtpClientType;
await client.conversations.stream(async (conversation) => {
logger.info("[XMTPRN Conversations] GOT A NEW CONVO");
// handleNewConversation(client, conversation);

addConversationToConversationListQuery(account, conversation);
Comment on lines +24 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for the query update

The conversation list update should be wrapped in a try-catch block to handle potential failures gracefully. Also, consider removing the empty line for better readability.

  logger.info("[XMTPRN Conversations] GOT A NEW CONVO");
-
-    addConversationToConversationListQuery(account, conversation);
+  try {
+    await addConversationToConversationListQuery(account, conversation);
+  } catch (error) {
+    logger.error("[XMTPRN Conversations] Failed to update conversation list", error);
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
addConversationToConversationListQuery(account, conversation);
logger.info("[XMTPRN Conversations] GOT A NEW CONVO");
try {
await addConversationToConversationListQuery(account, conversation);
} catch (error) {
logger.error("[XMTPRN Conversations] Failed to update conversation list", error);
}

});
logger.info("STREAMING CONVOS");
};
Expand Down
Loading