From 44db76531e59fd715b6bcc5ecf9b10f59659a306 Mon Sep 17 00:00:00 2001 From: Alvaro <110414366+alvaro-signal@users.noreply.github.com> Date: Mon, 3 Oct 2022 17:10:20 -0600 Subject: [PATCH] markStoryRead: More logging in unusual cases --- ts/components/StoryListItem.stories.tsx | 1 + ts/components/StoryViewer.tsx | 15 +++++++++++---- ts/messages/helpers.ts | 8 +++++--- ts/state/ducks/stories.ts | 12 ++++++++++++ ts/state/selectors/stories.ts | 7 +++++++ ts/test-both/helpers/getFakeStory.tsx | 5 ++++- ts/types/Stories.ts | 1 + ts/util/idForLogging.ts | 7 ++++++- 8 files changed, 47 insertions(+), 9 deletions(-) diff --git a/ts/components/StoryListItem.stories.tsx b/ts/components/StoryListItem.stories.tsx index 1e4df0fef..cd28f663a 100644 --- a/ts/components/StoryListItem.stories.tsx +++ b/ts/components/StoryListItem.stories.tsx @@ -53,6 +53,7 @@ SomeonesStory.args = { hasReplies: true, isUnread: true, messageId: '123', + messageIdForLogging: 'for logging 123', sender: getDefaultConversation(), timestamp: Date.now(), expirationTimestamp: undefined, diff --git a/ts/components/StoryViewer.tsx b/ts/components/StoryViewer.tsx index d6f38659f..4d066a1d2 100644 --- a/ts/components/StoryViewer.tsx +++ b/ts/components/StoryViewer.tsx @@ -145,8 +145,15 @@ export const StoryViewer = ({ StoryViewType | undefined >(); - const { attachment, canReply, isHidden, messageId, sendState, timestamp } = - story; + const { + attachment, + canReply, + isHidden, + messageId, + messageIdForLogging, + sendState, + timestamp, + } = story; const { acceptedMessageRequest, avatarPath, @@ -323,8 +330,8 @@ export const StoryViewer = ({ useEffect(() => { markStoryRead(messageId); - log.info('stories.markStoryRead', { messageId }); - }, [markStoryRead, messageId]); + log.info('stories.markStoryRead', { message: messageIdForLogging }); + }, [markStoryRead, messageId, messageIdForLogging]); const canFreelyNavigateStories = storyViewMode === StoryViewModeType.All || diff --git a/ts/messages/helpers.ts b/ts/messages/helpers.ts index 46998e20d..b76aee555 100644 --- a/ts/messages/helpers.ts +++ b/ts/messages/helpers.ts @@ -72,7 +72,9 @@ export function getContact( return window.ConversationController.get(id); } -export function getSource(message: MessageAttributesType): string | undefined { +export function getSource( + message: Pick +): string | undefined { if (isIncoming(message) || isStory(message)) { return message.source; } @@ -84,7 +86,7 @@ export function getSource(message: MessageAttributesType): string | undefined { } export function getSourceDevice( - message: MessageAttributesType + message: Pick ): string | number | undefined { const { sourceDevice } = message; @@ -101,7 +103,7 @@ export function getSourceDevice( } export function getSourceUuid( - message: MessageAttributesType + message: Pick ): UUIDStringType | undefined { if (isIncoming(message) || isStory(message)) { return message.sourceUuid; diff --git a/ts/state/ducks/stories.ts b/ts/state/ducks/stories.ts index a4988a5e4..b3a46c7c5 100644 --- a/ts/state/ducks/stories.ts +++ b/ts/state/ducks/stories.ts @@ -72,6 +72,7 @@ export type StoryDataType = { | 'sendStateByConversationId' | 'source' | 'sourceUuid' + | 'sourceDevice' | 'storyDistributionListId' | 'timestamp' | 'type' @@ -426,16 +427,27 @@ function markStoryRead( !isDownloaded(matchingStory.attachment) && !hasFailed(matchingStory.attachment) ) { + log.warn( + `markStoryRead: not downloaded: ${messageId} ${ + matchingStory.attachment?.error + ? `error: ${matchingStory.attachment?.error}` + : '' + }` + ); return; } if (matchingStory.readStatus !== ReadStatus.Unread) { + log.warn( + `markStoryRead: not unread, ${messageId} read status: ${matchingStory.readStatus}` + ); return; } const message = await getMessageById(messageId); if (!message) { + log.warn(`markStoryRead: no message found ${messageId}`); return; } diff --git a/ts/state/selectors/stories.ts b/ts/state/selectors/stories.ts index bad039ae3..728b22705 100644 --- a/ts/state/selectors/stories.ts +++ b/ts/state/selectors/stories.ts @@ -35,6 +35,7 @@ import { getUserConversationId } from './user'; import { getDistributionListSelector } from './storyDistributionLists'; import { getStoriesEnabled } from './items'; import { calculateExpirationTimestamp } from '../../util/expirationTimer'; +import { getMessageIdForLogging } from '../../util/idForLogging'; export const getStoriesState = (state: StateType): StoriesStateType => state.stories; @@ -191,12 +192,18 @@ export function getStoryView( views = innerViews; } + const messageIdForLogging = getMessageIdForLogging({ + ...pick(story, 'type', 'sourceUuid', 'sourceDevice'), + sent_at: story.timestamp, + }); + return { attachment, canReply: canReply(story, ourConversationId, conversationSelector), isHidden: Boolean(sender.hideStory), isUnread: story.readStatus === ReadStatus.Unread, messageId: story.messageId, + messageIdForLogging, readAt, sender, sendState, diff --git a/ts/test-both/helpers/getFakeStory.tsx b/ts/test-both/helpers/getFakeStory.tsx index aa16f6bba..8e2411a7d 100644 --- a/ts/test-both/helpers/getFakeStory.tsx +++ b/ts/test-both/helpers/getFakeStory.tsx @@ -43,13 +43,16 @@ export function getFakeStoryView( ): StoryViewType { const sender = getDefaultConversation(); + const messageId = UUID.generate().toString(); + return { attachment: getAttachmentWithThumbnail( attachmentUrl || '/fixtures/tina-rolf-269345-unsplash.jpg' ), hasReplies: Boolean(casual.coin_flip), isUnread: Boolean(casual.coin_flip), - messageId: UUID.generate().toString(), + messageId, + messageIdForLogging: `${messageId} (for logging)`, sender, timestamp: timestamp || Date.now() - 2 * durations.MINUTE, expirationTimestamp: undefined, diff --git a/ts/types/Stories.ts b/ts/types/Stories.ts index a4bf3e2d3..42be8112f 100644 --- a/ts/types/Stories.ts +++ b/ts/types/Stories.ts @@ -73,6 +73,7 @@ export type StoryViewType = { isHidden?: boolean; isUnread?: boolean; messageId: string; + messageIdForLogging: string; readAt?: number; sender: Pick< ConversationType, diff --git a/ts/util/idForLogging.ts b/ts/util/idForLogging.ts index d03d7fbe2..8d0096f98 100644 --- a/ts/util/idForLogging.ts +++ b/ts/util/idForLogging.ts @@ -8,7 +8,12 @@ import type { import { getSource, getSourceDevice, getSourceUuid } from '../messages/helpers'; import { isDirectConversation, isGroupV2 } from './whatTypeOfConversation'; -export function getMessageIdForLogging(message: MessageAttributesType): string { +export function getMessageIdForLogging( + message: Pick< + MessageAttributesType, + 'type' | 'sourceUuid' | 'sourceDevice' | 'sent_at' + > +): string { const account = getSourceUuid(message) || getSource(message); const device = getSourceDevice(message); const timestamp = message.sent_at;