Allow multiple reactions to stories

This commit is contained in:
Josh Perez 2022-04-28 18:06:28 -04:00 committed by GitHub
parent 42554ebaf0
commit 6d576ed901
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 211 additions and 119 deletions

View File

@ -40,7 +40,6 @@ export type StoryViewType = {
isHidden?: boolean; isHidden?: boolean;
isUnread?: boolean; isUnread?: boolean;
messageId: string; messageId: string;
selectedReaction?: string;
sender: Pick< sender: Pick<
ConversationType, ConversationType,
| 'acceptedMessageRequest' | 'acceptedMessageRequest'

View File

@ -313,7 +313,8 @@ export async function sendReaction(
const newReactions = reactionUtil.markOutgoingReactionSent( const newReactions = reactionUtil.markOutgoingReactionSent(
getReactions(message), getReactions(message),
pendingReaction, pendingReaction,
successfulConversationIds successfulConversationIds,
message.attributes
); );
setReactions(message, newReactions); setReactions(message, newReactions);

View File

@ -152,6 +152,7 @@ import { shouldDownloadStory } from '../util/shouldDownloadStory';
import { shouldShowStoriesView } from '../state/selectors/stories'; import { shouldShowStoriesView } from '../state/selectors/stories';
import type { ContactWithHydratedAvatar } from '../textsecure/SendMessage'; import type { ContactWithHydratedAvatar } from '../textsecure/SendMessage';
import { SeenStatus } from '../MessageSeenStatus'; import { SeenStatus } from '../MessageSeenStatus';
import { isNewReactionReplacingPrevious } from '../reactions/util';
/* eslint-disable camelcase */ /* eslint-disable camelcase */
/* eslint-disable more/no-then */ /* eslint-disable more/no-then */
@ -233,12 +234,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const { storyChanged } = window.reduxActions.stories; const { storyChanged } = window.reduxActions.stories;
if (isStory(this.attributes)) { if (isStory(this.attributes)) {
const ourConversationId = const storyData = getStoryDataFromMessageAttributes(this.attributes);
window.ConversationController.getOurConversationIdOrThrow();
const storyData = getStoryDataFromMessageAttributes(
this.attributes,
ourConversationId
);
if (!storyData) { if (!storyData) {
return; return;
@ -2892,14 +2888,15 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
const reactions = reactionUtil.addOutgoingReaction( const reactions = reactionUtil.addOutgoingReaction(
this.get('reactions') || [], this.get('reactions') || [],
newReaction newReaction,
isStory(this.attributes)
); );
this.set({ reactions }); this.set({ reactions });
} else { } else {
const oldReactions = this.get('reactions') || []; const oldReactions = this.get('reactions') || [];
let reactions: Array<MessageReactionType>; let reactions: Array<MessageReactionType>;
const oldReaction = oldReactions.find( const oldReaction = oldReactions.find(re =>
re => re.fromId === reaction.get('fromId') isNewReactionReplacingPrevious(re, reaction.attributes, this.attributes)
); );
if (oldReaction) { if (oldReaction) {
this.clearNotifications(oldReaction); this.clearNotifications(oldReaction);
@ -2914,12 +2911,20 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
if (reaction.get('source') === ReactionSource.FromSync) { if (reaction.get('source') === ReactionSource.FromSync) {
reactions = oldReactions.filter( reactions = oldReactions.filter(
re => re =>
re.fromId !== reaction.get('fromId') || !isNewReactionReplacingPrevious(
re.timestamp > reaction.get('timestamp') re,
reaction.attributes,
this.attributes
) || re.timestamp > reaction.get('timestamp')
); );
} else { } else {
reactions = oldReactions.filter( reactions = oldReactions.filter(
re => re.fromId !== reaction.get('fromId') re =>
!isNewReactionReplacingPrevious(
re,
reaction.attributes,
this.attributes
)
); );
} }
this.set({ reactions }); this.set({ reactions });
@ -2948,7 +2953,12 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
} }
reactions = oldReactions.filter( reactions = oldReactions.filter(
re => re.fromId !== reaction.get('fromId') re =>
!isNewReactionReplacingPrevious(
re,
reaction.attributes,
this.attributes
)
); );
reactions.push(reactionToAdd); reactions.push(reactionToAdd);
this.set({ reactions }); this.set({ reactions });

View File

@ -2,8 +2,12 @@
// SPDX-License-Identifier: AGPL-3.0-only // SPDX-License-Identifier: AGPL-3.0-only
import { findLastIndex, has, identity, omit, negate } from 'lodash'; import { findLastIndex, has, identity, omit, negate } from 'lodash';
import type { MessageReactionType } from '../model-types.d'; import type {
MessageAttributesType,
MessageReactionType,
} from '../model-types.d';
import { areObjectEntriesEqual } from '../util/areObjectEntriesEqual'; import { areObjectEntriesEqual } from '../util/areObjectEntriesEqual';
import { isStory } from '../state/selectors/message';
const isReactionEqual = ( const isReactionEqual = (
a: undefined | Readonly<MessageReactionType>, a: undefined | Readonly<MessageReactionType>,
@ -31,8 +35,13 @@ const isOutgoingReactionCompletelyUnsent = ({
export function addOutgoingReaction( export function addOutgoingReaction(
oldReactions: ReadonlyArray<MessageReactionType>, oldReactions: ReadonlyArray<MessageReactionType>,
newReaction: Readonly<MessageReactionType> newReaction: Readonly<MessageReactionType>,
isStoryMessage = false
): Array<MessageReactionType> { ): Array<MessageReactionType> {
if (isStoryMessage) {
return [...oldReactions, newReaction];
}
const pendingOutgoingReactions = new Set( const pendingOutgoingReactions = new Set(
oldReactions.filter(isOutgoingReactionPending) oldReactions.filter(isOutgoingReactionPending)
); );
@ -101,6 +110,17 @@ export function* getUnsentConversationIds({
} }
} }
// This function is used when filtering reactions so that we can limit normal
// messages to a single reactions but allow multiple reactions from the same
// sender for stories.
export function isNewReactionReplacingPrevious(
reaction: MessageReactionType,
newReaction: MessageReactionType,
messageAttributes: MessageAttributesType
): boolean {
return !isStory(messageAttributes) && reaction.fromId === newReaction.fromId;
}
export const markOutgoingReactionFailed = ( export const markOutgoingReactionFailed = (
reactions: Array<MessageReactionType>, reactions: Array<MessageReactionType>,
reaction: Readonly<MessageReactionType> reaction: Readonly<MessageReactionType>
@ -116,7 +136,8 @@ export const markOutgoingReactionFailed = (
export const markOutgoingReactionSent = ( export const markOutgoingReactionSent = (
reactions: ReadonlyArray<MessageReactionType>, reactions: ReadonlyArray<MessageReactionType>,
reaction: Readonly<MessageReactionType>, reaction: Readonly<MessageReactionType>,
conversationIdsSentTo: Iterable<string> conversationIdsSentTo: Iterable<string>,
messageAttributes: MessageAttributesType
): Array<MessageReactionType> => { ): Array<MessageReactionType> => {
const result: Array<MessageReactionType> = []; const result: Array<MessageReactionType> = [];
@ -135,7 +156,8 @@ export const markOutgoingReactionSent = (
if (!isReactionEqual(re, reaction)) { if (!isReactionEqual(re, reaction)) {
const shouldKeep = !isFullySent const shouldKeep = !isFullySent
? true ? true
: re.fromId !== reaction.fromId || re.timestamp > reaction.timestamp; : !isNewReactionReplacingPrevious(re, reaction, messageAttributes) ||
re.timestamp > reaction.timestamp;
if (shouldKeep) { if (shouldKeep) {
result.push(re); result.push(re);
} }

View File

@ -17,8 +17,7 @@ export async function loadStories(): Promise<void> {
} }
export function getStoryDataFromMessageAttributes( export function getStoryDataFromMessageAttributes(
message: MessageAttributesType, message: MessageAttributesType
ourConversationId?: string
): StoryDataType | undefined { ): StoryDataType | undefined {
const { attachments } = message; const { attachments } = message;
const unresolvedAttachment = attachments ? attachments[0] : undefined; const unresolvedAttachment = attachments ? attachments[0] : undefined;
@ -33,17 +32,13 @@ export function getStoryDataFromMessageAttributes(
? getAttachmentsForMessage(message) ? getAttachmentsForMessage(message)
: [unresolvedAttachment]; : [unresolvedAttachment];
const selectedReaction = (
(message.reactions || []).find(re => re.fromId === ourConversationId) || {}
).emoji;
return { return {
attachment, attachment,
messageId: message.id, messageId: message.id,
selectedReaction,
...pick(message, [ ...pick(message, [
'conversationId', 'conversationId',
'deletedForEveryone', 'deletedForEveryone',
'reactions',
'readStatus', 'readStatus',
'sendStateByConversationId', 'sendStateByConversationId',
'source', 'source',
@ -57,11 +52,8 @@ export function getStoryDataFromMessageAttributes(
export function getStoriesForRedux(): Array<StoryDataType> { export function getStoriesForRedux(): Array<StoryDataType> {
strictAssert(storyData, 'storyData has not been loaded'); strictAssert(storyData, 'storyData has not been loaded');
const ourConversationId =
window.ConversationController.getOurConversationId();
const stories = storyData const stories = storyData
.map(story => getStoryDataFromMessageAttributes(story, ourConversationId)) .map(getStoryDataFromMessageAttributes)
.filter(isNotNil); .filter(isNotNil);
storyData = undefined; storyData = undefined;

View File

@ -37,11 +37,11 @@ import { viewedReceiptsJobQueue } from '../../jobs/viewedReceiptsJobQueue';
export type StoryDataType = { export type StoryDataType = {
attachment?: AttachmentType; attachment?: AttachmentType;
messageId: string; messageId: string;
selectedReaction?: string;
} & Pick< } & Pick<
MessageAttributesType, MessageAttributesType,
| 'conversationId' | 'conversationId'
| 'deletedForEveryone' | 'deletedForEveryone'
| 'reactions'
| 'readStatus' | 'readStatus'
| 'sendStateByConversationId' | 'sendStateByConversationId'
| 'source' | 'source'
@ -65,7 +65,6 @@ export type StoriesStateType = {
const LOAD_STORY_REPLIES = 'stories/LOAD_STORY_REPLIES'; const LOAD_STORY_REPLIES = 'stories/LOAD_STORY_REPLIES';
const MARK_STORY_READ = 'stories/MARK_STORY_READ'; const MARK_STORY_READ = 'stories/MARK_STORY_READ';
const REACT_TO_STORY = 'stories/REACT_TO_STORY';
const REPLY_TO_STORY = 'stories/REPLY_TO_STORY'; const REPLY_TO_STORY = 'stories/REPLY_TO_STORY';
export const RESOLVE_ATTACHMENT_URL = 'stories/RESOLVE_ATTACHMENT_URL'; export const RESOLVE_ATTACHMENT_URL = 'stories/RESOLVE_ATTACHMENT_URL';
const STORY_CHANGED = 'stories/STORY_CHANGED'; const STORY_CHANGED = 'stories/STORY_CHANGED';
@ -84,14 +83,6 @@ type MarkStoryReadActionType = {
payload: string; payload: string;
}; };
type ReactToStoryActionType = {
type: typeof REACT_TO_STORY;
payload: {
messageId: string;
selectedReaction: string;
};
};
type ReplyToStoryActionType = { type ReplyToStoryActionType = {
type: typeof REPLY_TO_STORY; type: typeof REPLY_TO_STORY;
payload: MessageAttributesType; payload: MessageAttributesType;
@ -119,7 +110,6 @@ export type StoriesActionType =
| MarkStoryReadActionType | MarkStoryReadActionType
| MessageChangedActionType | MessageChangedActionType
| MessageDeletedActionType | MessageDeletedActionType
| ReactToStoryActionType
| ReplyToStoryActionType | ReplyToStoryActionType
| ResolveAttachmentUrlActionType | ResolveAttachmentUrlActionType
| StoryChangedActionType | StoryChangedActionType
@ -286,27 +276,24 @@ function queueStoryDownload(
function reactToStory( function reactToStory(
nextReaction: string, nextReaction: string,
messageId: string, messageId: string
previousReaction?: string ): ThunkAction<void, RootStateType, unknown, NoopActionType> {
): ThunkAction<void, RootStateType, unknown, ReactToStoryActionType> {
return async dispatch => { return async dispatch => {
try { try {
await enqueueReactionForSend({ await enqueueReactionForSend({
messageId, messageId,
emoji: nextReaction, emoji: nextReaction,
remove: nextReaction === previousReaction, remove: false,
});
dispatch({
type: REACT_TO_STORY,
payload: {
messageId,
selectedReaction: nextReaction,
},
}); });
} catch (error) { } catch (error) {
log.error('Error enqueuing reaction', error, messageId, nextReaction); log.error('Error enqueuing reaction', error, messageId, nextReaction);
showToast(ToastReactionFailed); showToast(ToastReactionFailed);
} }
dispatch({
type: 'NOOP',
payload: null,
});
}; };
} }
@ -403,8 +390,8 @@ export function reducer(
'conversationId', 'conversationId',
'deletedForEveryone', 'deletedForEveryone',
'messageId', 'messageId',
'reactions',
'readStatus', 'readStatus',
'selectedReaction',
'sendStateByConversationId', 'sendStateByConversationId',
'source', 'source',
'sourceUuid', 'sourceUuid',
@ -424,9 +411,14 @@ export function reducer(
!isDownloaded(prevStory.attachment) && !isDownloaded(prevStory.attachment) &&
isDownloaded(newStory.attachment); isDownloaded(newStory.attachment);
const readStatusChanged = prevStory.readStatus !== newStory.readStatus; const readStatusChanged = prevStory.readStatus !== newStory.readStatus;
const reactionsChanged =
prevStory.reactions?.length !== newStory.reactions?.length;
const shouldReplace = const shouldReplace =
isDownloadingAttachment || hasAttachmentDownloaded || readStatusChanged; isDownloadingAttachment ||
hasAttachmentDownloaded ||
readStatusChanged ||
reactionsChanged;
if (!shouldReplace) { if (!shouldReplace) {
return state; return state;
} }
@ -448,22 +440,6 @@ export function reducer(
}; };
} }
if (action.type === REACT_TO_STORY) {
return {
...state,
stories: state.stories.map(story => {
if (story.messageId === action.payload.messageId) {
return {
...story,
selectedReaction: action.payload.selectedReaction,
};
}
return story;
}),
};
}
if (action.type === MARK_STORY_READ) { if (action.type === MARK_STORY_READ) {
return { return {
...state, ...state,

View File

@ -5,10 +5,12 @@ import { createSelector } from 'reselect';
import { pick } from 'lodash'; import { pick } from 'lodash';
import type { GetConversationByIdType } from './conversations'; import type { GetConversationByIdType } from './conversations';
import type { ConversationType } from '../ducks/conversations';
import type { import type {
ConversationStoryType, ConversationStoryType,
StoryViewType, StoryViewType,
} from '../../components/StoryListItem'; } from '../../components/StoryListItem';
import type { MessageReactionType } from '../../model-types.d';
import type { ReplyStateType } from '../../types/Stories'; import type { ReplyStateType } from '../../types/Stories';
import type { StateType } from '../reducer'; import type { StateType } from '../reducer';
import type { StoryDataType, StoriesStateType } from '../ducks/stories'; import type { StoryDataType, StoriesStateType } from '../ducks/stories';
@ -55,6 +57,35 @@ function sortByRecencyAndUnread(
return storyA.timestamp > storyB.timestamp ? -1 : 1; return storyA.timestamp > storyB.timestamp ? -1 : 1;
} }
function getReactionUniqueId(reaction: MessageReactionType): string {
return `${reaction.fromId}:${reaction.targetAuthorUuid}:${reaction.timestamp}`;
}
function getAvatarData(
conversation: ConversationType
): Pick<
ConversationType,
| 'acceptedMessageRequest'
| 'avatarPath'
| 'color'
| 'isMe'
| 'name'
| 'profileName'
| 'sharedGroupNames'
| 'title'
> {
return pick(conversation, [
'acceptedMessageRequest',
'avatarPath',
'color',
'isMe',
'name',
'profileName',
'sharedGroupNames',
'title',
]);
}
function getConversationStory( function getConversationStory(
conversationSelector: GetConversationByIdType, conversationSelector: GetConversationByIdType,
story: StoryDataType, story: StoryDataType,
@ -92,7 +123,6 @@ function getConversationStory(
canReply: canReply(story, ourConversationId, conversationSelector), canReply: canReply(story, ourConversationId, conversationSelector),
isUnread: story.readStatus === ReadStatus.Unread, isUnread: story.readStatus === ReadStatus.Unread,
messageId: story.messageId, messageId: story.messageId,
selectedReaction: story.selectedReaction,
sender, sender,
timestamp, timestamp,
}; };
@ -153,38 +183,56 @@ export const getStoryReplies = createSelector(
conversationSelector, conversationSelector,
contactNameColorSelector, contactNameColorSelector,
me, me,
{ replyState }: Readonly<StoriesStateType> { stories, replyState }: Readonly<StoriesStateType>
): ReplyStateType | undefined => { ): ReplyStateType | undefined => {
if (!replyState) { if (!replyState) {
return; return;
} }
const foundStory = stories.find(
story => story.messageId === replyState.messageId
);
const reactions = foundStory
? (foundStory.reactions || []).map(reaction => {
const conversation = conversationSelector(reaction.fromId);
return {
...getAvatarData(conversation),
contactNameColor: contactNameColorSelector(
foundStory.conversationId,
conversation.id
),
id: getReactionUniqueId(reaction),
reactionEmoji: reaction.emoji,
timestamp: reaction.timestamp,
};
})
: [];
const replies = replyState.replies.map(reply => {
const conversation =
reply.type === 'outgoing'
? me
: conversationSelector(reply.sourceUuid || reply.source);
return {
...getAvatarData(conversation),
...pick(reply, ['body', 'deletedForEveryone', 'id', 'timestamp']),
contactNameColor: contactNameColorSelector(
reply.conversationId,
conversation.id
),
};
});
const combined = [...replies, ...reactions].sort((a, b) =>
a.timestamp > b.timestamp ? 1 : -1
);
return { return {
messageId: replyState.messageId, messageId: replyState.messageId,
replies: replyState.replies.map(reply => { replies: combined,
const conversation =
reply.type === 'outgoing'
? me
: conversationSelector(reply.sourceUuid || reply.source);
return {
...pick(conversation, [
'acceptedMessageRequest',
'avatarPath',
'color',
'isMe',
'name',
'profileName',
'sharedGroupNames',
'title',
]),
...pick(reply, ['body', 'deletedForEveryone', 'id', 'timestamp']),
contactNameColor: contactNameColorSelector(
reply.conversationId,
conversation.id
),
};
}),
}; };
} }
); );

View File

@ -69,8 +69,8 @@ export function SmartStoryViewer({
onNextUserStories={onNextUserStories} onNextUserStories={onNextUserStories}
onPrevUserStories={onPrevUserStories} onPrevUserStories={onPrevUserStories}
onReactToStory={async (emoji, story) => { onReactToStory={async (emoji, story) => {
const { messageId, selectedReaction: previousReaction } = story; const { messageId } = story;
storiesActions.reactToStory(emoji, messageId, previousReaction); storiesActions.reactToStory(emoji, messageId);
}} }}
onReplyToStory={(message, mentions, timestamp, story) => { onReplyToStory={(message, mentions, timestamp, story) => {
storiesActions.replyToStory( storiesActions.replyToStory(

View File

@ -4,7 +4,10 @@
import { assert } from 'chai'; import { assert } from 'chai';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
import { omit } from 'lodash'; import { omit } from 'lodash';
import type { MessageReactionType } from '../../model-types.d'; import type {
MessageAttributesType,
MessageReactionType,
} from '../../model-types.d';
import { isEmpty } from '../../util/iterables'; import { isEmpty } from '../../util/iterables';
import { import {
@ -48,6 +51,18 @@ describe('reaction utilities', () => {
const newReactions = addOutgoingReaction(oldReactions, reaction); const newReactions = addOutgoingReaction(oldReactions, reaction);
assert.deepStrictEqual(newReactions, [oldReactions[1], reaction]); assert.deepStrictEqual(newReactions, [oldReactions[1], reaction]);
}); });
it('does not remove any pending reactions if its a story', () => {
const oldReactions = [
{ ...rxn('😭', { isPending: true }), timestamp: 3 },
{ ...rxn('💬'), fromId: uuid() },
{ ...rxn('🥀', { isPending: true }), timestamp: 1 },
{ ...rxn('🌹', { isPending: true }), timestamp: 2 },
];
const reaction = rxn('😀');
const newReactions = addOutgoingReaction(oldReactions, reaction, true);
assert.deepStrictEqual(newReactions, [...oldReactions, reaction]);
});
}); });
describe('getNewestPendingOutgoingReaction', () => { describe('getNewestPendingOutgoingReaction', () => {
@ -199,21 +214,36 @@ describe('reaction utilities', () => {
const reactions = [star, none, { ...rxn('🔕'), timestamp: 1 }]; const reactions = [star, none, { ...rxn('🔕'), timestamp: 1 }];
function getMessage(): MessageAttributesType {
const now = Date.now();
return {
conversationId: uuid(),
id: uuid(),
received_at: now,
sent_at: now,
timestamp: now,
type: 'incoming',
};
}
it("does nothing if the reaction isn't in the list", () => { it("does nothing if the reaction isn't in the list", () => {
const result = markOutgoingReactionSent( const result = markOutgoingReactionSent(
reactions, reactions,
rxn('🥀', { isPending: true }), rxn('🥀', { isPending: true }),
[uuid()] [uuid()],
getMessage()
); );
assert.deepStrictEqual(result, reactions); assert.deepStrictEqual(result, reactions);
}); });
it('updates reactions to be partially sent', () => { it('updates reactions to be partially sent', () => {
[star, none].forEach(reaction => { [star, none].forEach(reaction => {
const result = markOutgoingReactionSent(reactions, reaction, [ const result = markOutgoingReactionSent(
uuid1, reactions,
uuid2, reaction,
]); [uuid1, uuid2],
getMessage()
);
assert.deepStrictEqual( assert.deepStrictEqual(
result.find(re => re.emoji === reaction.emoji) result.find(re => re.emoji === reaction.emoji)
?.isSentByConversationId, ?.isSentByConversationId,
@ -227,11 +257,12 @@ describe('reaction utilities', () => {
}); });
it('removes sent state if a reaction with emoji is fully sent', () => { it('removes sent state if a reaction with emoji is fully sent', () => {
const result = markOutgoingReactionSent(reactions, star, [ const result = markOutgoingReactionSent(
uuid1, reactions,
uuid2, star,
uuid3, [uuid1, uuid2, uuid3],
]); getMessage()
);
const newReaction = result.find(re => re.emoji === '⭐️'); const newReaction = result.find(re => re.emoji === '⭐️');
assert.isDefined(newReaction); assert.isDefined(newReaction);
@ -239,11 +270,12 @@ describe('reaction utilities', () => {
}); });
it('removes a fully-sent reaction removal', () => { it('removes a fully-sent reaction removal', () => {
const result = markOutgoingReactionSent(reactions, none, [ const result = markOutgoingReactionSent(
uuid1, reactions,
uuid2, none,
uuid3, [uuid1, uuid2, uuid3],
]); getMessage()
);
assert( assert(
result.every(({ emoji }) => typeof emoji === 'string'), result.every(({ emoji }) => typeof emoji === 'string'),
@ -252,13 +284,25 @@ describe('reaction utilities', () => {
}); });
it('removes older reactions of mine', () => { it('removes older reactions of mine', () => {
const result = markOutgoingReactionSent(reactions, star, [ const result = markOutgoingReactionSent(
uuid1, reactions,
uuid2, star,
uuid3, [uuid1, uuid2, uuid3],
]); getMessage()
);
assert.isUndefined(result.find(re => re.emoji === '🔕')); assert.isUndefined(result.find(re => re.emoji === '🔕'));
}); });
it('does not remove my older reactions if they are on a story', () => {
const result = markOutgoingReactionSent(
reactions,
star,
[uuid1, uuid2, uuid3],
{ ...getMessage(), type: 'story' }
);
assert.isDefined(result.find(re => re.emoji === '🔕'));
});
}); });
}); });