-
Notifications
You must be signed in to change notification settings - Fork 0
[APP-7066] - consider local send status messages as current users' #32
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ import format from 'date-fns/format'; | |
|
|
||
| import { ReplyType } from '../types'; | ||
| import type { CoreMessageType } from '.'; | ||
| import { isReadMessage } from '.'; | ||
| import { isReadMessage, isSendableMessage } from '.'; | ||
|
|
||
| /** | ||
| * exported, should be backward compatible | ||
|
|
@@ -16,6 +16,7 @@ export const compareMessagesForGrouping = ( | |
| nextMessage: CoreMessageType, | ||
| currentChannel?: GroupChannel, | ||
| replyType?: ReplyType, | ||
| currentUserId?: string, | ||
| ) => { | ||
| if (!currentChannel || (currentChannel as GroupChannel).channelType !== 'group') { | ||
| return [ | ||
|
|
@@ -30,18 +31,43 @@ export const compareMessagesForGrouping = ( | |
| const sendingStatus = (currMessage as UserMessage)?.sendingStatus || ''; | ||
| const isAcceptable = sendingStatus !== 'failed'; | ||
| return [ | ||
| isSameGroup(prevMessage, currMessage, currentChannel) && isAcceptable, | ||
| isSameGroup(currMessage, nextMessage, currentChannel) && isAcceptable, | ||
| isSameGroup(prevMessage, currMessage, currentUserId) && isAcceptable, | ||
| isSameGroup(currMessage, nextMessage, currentUserId) && isAcceptable, | ||
| ]; | ||
| }; | ||
|
|
||
| export const getMessageCreatedAt = (message: BaseMessage) => format(message.createdAt, 'p'); | ||
|
|
||
| // Group current user's messages together. The current user's messages | ||
| // may not have their userId on it, and if not, we assume that messages w/ a | ||
| // local send status is the current user's as well. | ||
| // Given the above is true, group by timestamp | ||
| export const areBothFromMyUserAndInSameGroup = ( | ||
| message: CoreMessageType, | ||
| comparingMessage: CoreMessageType, | ||
| currentUserId?: string | ||
| ) => { | ||
| if (!currentUserId || !message.createdAt || !comparingMessage.createdAt) return false; | ||
|
|
||
| const isFirstMessageByMe = getIsByMe(currentUserId, message); | ||
| const isSecondMessageByMe = getIsByMe(currentUserId, comparingMessage); | ||
|
|
||
| if (isFirstMessageByMe && isSecondMessageByMe) { | ||
| return getMessageCreatedAt(message) === getMessageCreatedAt(comparingMessage); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| export const isSameGroup = ( | ||
| message: CoreMessageType, | ||
| comparingMessage: CoreMessageType, | ||
| currentChannel?: GroupChannel, | ||
| currentUserId?: string | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What was the group being used for before?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was used previously to determine grouping by The usage was removed but the parameter got left in |
||
| ) => { | ||
| if (areBothFromMyUserAndInSameGroup(message, comparingMessage, currentUserId)) { | ||
| return true; | ||
| } | ||
|
|
||
| if ( | ||
| !( | ||
| message | ||
|
|
@@ -61,23 +87,20 @@ export const isSameGroup = ( | |
| return false; | ||
| } | ||
|
|
||
| // group pending messages with any message type other than failed | ||
| // Given the above is true, group by timestamp and sender id | ||
| if ([message.sendingStatus, comparingMessage.sendingStatus].includes(SendingStatus.PENDING) && | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this fixed a different bug, but will be obsolete/unnecessary with the new fix. |
||
| ![message.sendingStatus, comparingMessage.sendingStatus].includes(SendingStatus.FAILED)) { | ||
| return ( | ||
| message?.sender?.userId === comparingMessage?.sender?.userId | ||
| && getMessageCreatedAt(message) === getMessageCreatedAt(comparingMessage) | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| message?.sendingStatus === comparingMessage?.sendingStatus | ||
| && message?.sender?.userId === comparingMessage?.sender?.userId | ||
| && getMessageCreatedAt(message) === getMessageCreatedAt(comparingMessage) | ||
| ); | ||
| }; | ||
|
|
||
| export const getIsByMe = (userId: string, message: BaseMessage) => { | ||
| if (!isSendableMessage(message) || !userId) return false; | ||
| const messageIsLocalType = [SendingStatus.FAILED, SendingStatus.PENDING].includes(message.sendingStatus); | ||
|
|
||
| return userId === message.sender.userId || messageIsLocalType; | ||
| } | ||
|
|
||
| export default { | ||
| compareMessagesForGrouping, | ||
| getMessageCreatedAt, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentUserId will be undefined by default anyway right? Any reason to explicitly given it a default of undefined here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah no ur right, I had previously had it as
nulland refactored it to undefined