From 8cadc40975bffe62b25c3202758978e75e8b559e Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Thu, 29 Jul 2021 09:29:07 -0500 Subject: [PATCH] Replace MessageModel#isUnread with isMessageUnread utility --- ts/messageModifiers/ReadSyncs.ts | 3 ++- ts/models/messages.ts | 9 +++------ ts/state/ducks/conversations.ts | 5 +++-- ts/test-both/util/isMessageUnread_test.ts | 21 +++++++++++++++++++++ ts/util/isMessageUnread.ts | 8 ++++++++ ts/views/conversation_view.ts | 15 ++++++++------- 6 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 ts/test-both/util/isMessageUnread_test.ts create mode 100644 ts/util/isMessageUnread.ts diff --git a/ts/messageModifiers/ReadSyncs.ts b/ts/messageModifiers/ReadSyncs.ts index 847d15441..af8ba3606 100644 --- a/ts/messageModifiers/ReadSyncs.ts +++ b/ts/messageModifiers/ReadSyncs.ts @@ -7,6 +7,7 @@ import { Collection, Model } from 'backbone'; import { MessageModel } from '../models/messages'; import { isIncoming } from '../state/selectors/message'; +import { isMessageUnread } from '../util/isMessageUnread'; type ReadSyncAttributesType = { senderId: string; @@ -109,7 +110,7 @@ export class ReadSyncs extends Collection { // If message is unread, we mark it read. Otherwise, we update the expiration // timer to the time specified by the read sync if it's earlier than // the previous read time. - if (message.isUnread()) { + if (isMessageUnread(message.attributes)) { // TODO DESKTOP-1509: use MessageUpdater.markRead once this is TS message.markRead(readAt, { skipSave: true }); diff --git a/ts/models/messages.ts b/ts/models/messages.ts index dd1c0c148..64a4f26b3 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -56,6 +56,7 @@ import { import { migrateLegacySendAttributes } from '../messages/migrateLegacySendAttributes'; import { getOwn } from '../util/getOwn'; import { markRead } from '../services/MessageUpdater'; +import { isMessageUnread } from '../util/isMessageUnread'; import { isDirectConversation, isGroupV1, @@ -745,10 +746,6 @@ export class MessageModel extends window.Backbone.Model { } } - isUnread(): boolean { - return !!this.get('unread'); - } - merge(model: MessageModel): void { const attributes = model.attributes || model; this.set(attributes); @@ -847,7 +844,7 @@ export class MessageModel extends window.Backbone.Model { return; } - if (this.get('unread')) { + if (isMessageUnread(this.attributes)) { this.set(markRead(this.attributes)); } @@ -3175,7 +3172,7 @@ export class MessageModel extends window.Backbone.Model { const isFirstRun = false; await this.modifyTargetMessage(conversation, isFirstRun); - if (this.get('unread')) { + if (isMessageUnread(this.attributes)) { await conversation.notify(this); } diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 16f780d85..661f8d0b0 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -47,6 +47,7 @@ import { getGroupSizeRecommendedLimit, getGroupSizeHardLimit, } from '../../groups/limits'; +import { isMessageUnread } from '../../util/isMessageUnread'; import { toggleSelectedContactForGroupAddition } from '../../groups/toggleSelectedContactForGroupAddition'; import { GroupNameCollisionsWithIdsByTitle } from '../../util/groupMemberNameCollisions'; import { ContactSpoofingType } from '../../util/contactSpoofing'; @@ -2241,7 +2242,7 @@ export function reducer( const oldestId = newMessageIds.find(messageId => { const message = lookup[messageId]; - return Boolean(message.unread); + return message && isMessageUnread(message); }); if (oldestId) { @@ -2257,7 +2258,7 @@ export function reducer( const newUnread: number = newMessageIds.reduce((sum, messageId) => { const message = lookup[messageId]; - return sum + (message && message.unread ? 1 : 0); + return sum + (message && isMessageUnread(message) ? 1 : 0); }, 0); totalUnread = (totalUnread || 0) + newUnread; } diff --git a/ts/test-both/util/isMessageUnread_test.ts b/ts/test-both/util/isMessageUnread_test.ts new file mode 100644 index 000000000..e5da7a195 --- /dev/null +++ b/ts/test-both/util/isMessageUnread_test.ts @@ -0,0 +1,21 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; + +import { isMessageUnread } from '../../util/isMessageUnread'; + +describe('isMessageUnread', () => { + it("returns false if the message's `unread` field is undefined", () => { + assert.isFalse(isMessageUnread({})); + assert.isFalse(isMessageUnread({ unread: undefined })); + }); + + it('returns false if the message is read', () => { + assert.isFalse(isMessageUnread({ unread: false })); + }); + + it('returns true if the message is unread', () => { + assert.isTrue(isMessageUnread({ unread: true })); + }); +}); diff --git a/ts/util/isMessageUnread.ts b/ts/util/isMessageUnread.ts new file mode 100644 index 000000000..3c58a9b2c --- /dev/null +++ b/ts/util/isMessageUnread.ts @@ -0,0 +1,8 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { MessageAttributesType } from '../model-types.d'; + +export const isMessageUnread = ( + message: Readonly> +): boolean => Boolean(message.unread); diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts index 08406a8b7..af9481f08 100644 --- a/ts/views/conversation_view.ts +++ b/ts/views/conversation_view.ts @@ -40,6 +40,7 @@ import { isOutgoing, isTapToView, } from '../state/selectors/message'; +import { isMessageUnread } from '../util/isMessageUnread'; import { getMessagesByConversation } from '../state/selectors/conversations'; import { ConversationDetailsMembershipList } from '../components/conversation/conversation-details/ConversationDetailsMembershipList'; import { showSafetyNumberChangeDialog } from '../shims/showSafetyNumberChangeDialog'; @@ -1313,17 +1314,17 @@ Whisper.ConversationView = Whisper.View.extend({ const newestInMemoryMessage = await getMessageById(newestMessageId, { Message: Whisper.Message, }); - if (!newestInMemoryMessage) { + if (newestInMemoryMessage) { + // If newest in-memory message is unread, scrolling down would mean going to + // the very bottom, not the oldest unread. + if (isMessageUnread(newestInMemoryMessage.attributes)) { + scrollToLatestUnread = false; + } + } else { window.log.warn( `loadNewestMessages: did not find message ${newestMessageId}` ); } - - // If newest in-memory message is unread, scrolling down would mean going to - // the very bottom, not the oldest unread. - if (newestInMemoryMessage && newestInMemoryMessage.isUnread()) { - scrollToLatestUnread = false; - } } const metrics = await getMessageMetricsForConversation(conversationId);