Improve chat message create performance (#5981)

Signed-off-by: Kristina Fefelova <kristin.fefelova@gmail.com>

packages/query/src/index.ts

@@ -960,6 +960,10 @@ export class LiveQuery implements WithTx, Client {
     result: LookupData<T>
   ): Promise<void> {
     for (const key in lookup._id) {
+      if ((doc as any)[key] === undefined || (doc as any)[key] === 0) {
+        continue
+      }
+
       const value = lookup._id[key]
 
       let _class: Ref<Class<Doc>>

plugins/attachment-resources/src/components/AttachmentRefInput.svelte

@@ -50,6 +50,7 @@
   export let placeholder: IntlString | undefined = undefined
   export let extraActions: RefAction[] = []
   export let boundary: HTMLElement | undefined = undefined
+  export let skipAttachmentsPreload = false
 
   let refInput: ReferenceInput
 
@@ -72,9 +73,30 @@
 
   let refContainer: HTMLElement
 
+  const existingAttachmentsQuery = createQuery()
+  let existingAttachments: Ref<Attachment>[] = []
+
+  $: if (Array.from(attachments.keys()).length > 0) {
+    existingAttachmentsQuery.query(
+      attachment.class.Attachment,
+      {
+        space,
+        attachedTo: objectId,
+        attachedToClass: _class,
+        _id: { $in: Array.from(attachments.keys()) }
+      },
+      (res) => {
+        existingAttachments = res.map((p) => p._id)
+      }
+    )
+  } else {
+    existingAttachments = []
+    existingAttachmentsQuery.unsubscribe()
+  }
+
   $: objectId && updateAttachments(objectId)
 
-  async function updateAttachments (objectId: Ref<Doc>) {
+  async function updateAttachments (objectId: Ref<Doc>): Promise<void> {
     draftAttachments = $draftsStore[draftKey]
     if (draftAttachments && shouldSaveDraft) {
       attachments.clear()
@@ -87,7 +109,8 @@
       })
       originalAttachments.clear()
       removedAttachments.clear()
-    } else {
+      query.unsubscribe()
+    } else if (!skipAttachmentsPreload) {
       query.query(
         attachment.class.Attachment,
         {
@@ -103,17 +126,23 @@
           }
         }
       )
+    } else {
+      attachments.clear()
+      newAttachments.clear()
+      originalAttachments.clear()
+      removedAttachments.clear()
+      query.unsubscribe()
     }
   }
 
-  async function saveDraft () {
+  function saveDraft (): void {
     if (shouldSaveDraft) {
       draftAttachments = Object.fromEntries(attachments)
       draftController.save(draftAttachments)
     }
   }
 
-  async function createAttachment (file: File) {
+  async function createAttachment (file: File): Promise<void> {
     try {
       const uuid = await uploadFile(file)
       const metadata = await getFileMetadata(file, uuid)
@@ -137,27 +166,13 @@
       })
       newAttachments.add(_id)
       attachments = attachments
+      dispatch('update', { message: content, attachments: attachments.size })
       saveDraft()
     } catch (err: any) {
-      setPlatformStatus(unknownError(err))
+      void setPlatformStatus(unknownError(err))
     }
   }
 
-  const existingAttachmentsQuery = createQuery()
-  let existingAttachments: Ref<Attachment>[] = []
-  $: existingAttachmentsQuery.query(
-    attachment.class.Attachment,
-    {
-      space,
-      attachedTo: objectId,
-      attachedToClass: _class,
-      _id: { $in: Array.from(attachments.keys()) }
-    },
-    (res) => {
-      existingAttachments = res.map((p) => p._id)
-    }
-  )
-
   async function saveAttachment (doc: Attachment): Promise<void> {
     if (!existingAttachments.includes(doc._id)) {
       await client.addCollection(attachment.class.Attachment, space, objectId, _class, 'attachments', doc, doc._id)
@@ -199,6 +214,7 @@
       await createAttachments()
     }
     attachments = attachments
+    dispatch('update', { message: content, attachments: attachments.size })
     saveDraft()
   }
 
@@ -231,7 +247,7 @@
     }
   })
 
-  export function removeDraft (removeFiles: boolean) {
+  export function removeDraft (removeFiles: boolean): void {
     draftController.remove()
     if (removeFiles) {
       newAttachments.forEach((p) => {
@@ -258,14 +274,14 @@
     return Promise.all(promises).then()
   }
 
-  async function onMessage (event: CustomEvent) {
+  async function onMessage (event: CustomEvent): Promise<void> {
     loading = true
     await createAttachments()
     loading = false
     dispatch('message', { message: event.detail, attachments: attachments.size })
   }
 
-  async function onUpdate (event: CustomEvent) {
+  function onUpdate (event: CustomEvent): void {
     dispatch('update', { message: event.detail, attachments: attachments.size })
   }
 
@@ -326,7 +342,7 @@
     <ReferenceInput
       {focusIndex}
       bind:this={refInput}
-      {content}
+      bind:content
       {iconSend}
       {labelSend}
       {showSend}
@@ -358,7 +374,7 @@
       {placeholder}
     >
       <div slot="header">
-        {#if attachments.size || progress}
+        {#if attachments.size > 0 || progress}
           <div class="flex-row-center list scroll-divider-color">
             {#if progress}
               <div class="flex p-3">
@@ -371,7 +387,7 @@
                   value={attachment}
                   removable
                   on:remove={(result) => {
-                    if (result !== undefined) removeAttachment(attachment)
+                    if (result !== undefined) void removeAttachment(attachment)
                   }}
                 />
               </div>

plugins/chunter-resources/src/channelDataProvider.ts

@@ -131,8 +131,10 @@ export class ChannelDataProvider implements IChannelDataProvider {
     const metadata = get(this.metadataStore)
 
     if (mode === 'forward') {
+      const isTailLoading = get(this.isTailLoading)
+      const tail = get(this.tailStore)
       const last = metadata[metadata.length - 1]?.createdOn ?? 0
-      return last > timestamp
+      return last > timestamp && !isTailLoading && tail.length === 0
     } else {
       const first = metadata[0]?.createdOn ?? 0
       return first < timestamp

plugins/chunter-resources/src/components/ChannelScrollView.svelte

@@ -29,11 +29,17 @@
   } from '@hcengineering/activity-resources'
   import { InboxNotificationsClientImpl } from '@hcengineering/notification-resources'
   import { get } from 'svelte/store'
-  import { tick, beforeUpdate, afterUpdate } from 'svelte'
+  import { tick, beforeUpdate, afterUpdate, onMount, onDestroy } from 'svelte'
   import { getResource } from '@hcengineering/platform'
 
   import ActivityMessagesSeparator from './ChannelMessagesSeparator.svelte'
-  import { filterChatMessages, getClosestDate, readChannelMessages } from '../utils'
+  import {
+    filterChatMessages,
+    getClosestDate,
+    readChannelMessages,
+    chatReadMessagesStore,
+    recheckNotifications
+  } from '../utils'
   import HistoryLoading from './LoadingHistory.svelte'
   import { ChannelDataProvider, MessageMetadata } from '../channelDataProvider'
   import JumpToDateSelector from './JumpToDateSelector.svelte'
@@ -113,8 +119,11 @@
 
   $: displayMessages = filterChatMessages(messages, filters, filterResources, objectClass, selectedFilters)
 
-  inboxClient.inboxNotificationsByContext.subscribe(() => {
-    readViewportMessages()
+  const unsubscribe = inboxClient.inboxNotificationsByContext.subscribe(() => {
+    if (notifyContext !== undefined) {
+      recheckNotifications(notifyContext)
+      readViewportMessages()
+    }
   })
 
   function scrollToBottom (afterScrollFn?: () => void): void {
@@ -589,6 +598,14 @@
 
     return canGroupMessages(message, prevMessage ?? prevMetadata)
   }
+
+  onMount(() => {
+    chatReadMessagesStore.update(() => new Set())
+  })
+
+  onDestroy(() => {
+    unsubscribe()
+  })
 </script>
 
 {#if isLoading}
@@ -635,22 +652,20 @@
           <JumpToDateSelector selectedDate={message.createdOn} on:jumpToDate={jumpToDate} />
         {/if}
 
-        <div class="msg">
-          <ActivityMessagePresenter
-            doc={object}
-            value={message}
-            skipLabel={skipLabels}
-            {showEmbedded}
-            hoverStyles="filledHover"
-            isHighlighted={isSelected}
-            shouldScroll={isSelected}
-            withShowMore={false}
-            attachmentImageSize="x-large"
-            showLinksPreview={false}
-            type={canGroup ? 'short' : 'default'}
-            hideLink
-          />
-        </div>
+        <ActivityMessagePresenter
+          doc={object}
+          value={message}
+          skipLabel={skipLabels}
+          {showEmbedded}
+          hoverStyles="filledHover"
+          isHighlighted={isSelected}
+          shouldScroll={isSelected}
+          withShowMore={false}
+          attachmentImageSize="x-large"
+          showLinksPreview={false}
+          type={canGroup ? 'short' : 'default'}
+          hideLink
+        />
       {/each}
 
       {#if loadMoreAllowed && provider.canLoadMore('forward', messages[messages.length - 1]?.createdOn)}
@@ -670,14 +685,6 @@
 {/if}
 
 <style lang="scss">
-  .msg {
-    margin: 0;
-    height: auto;
-    display: flex;
-    flex-direction: column;
-    flex-shrink: 0;
-  }
-
   .grower {
     flex-grow: 10;
     flex-shrink: 5;

plugins/chunter-resources/src/components/chat-message/ChatMessageInput.svelte

@@ -75,7 +75,7 @@
     inputRef.removeDraft(false)
   }
 
-  function objectChange (draft: MessageDraft, empty: Partial<MessageDraft>) {
+  function objectChange (draft: MessageDraft, empty: Partial<MessageDraft>): void {
     if (shouldSaveDraft) {
       draftController.save(draft, empty)
     }
@@ -90,7 +90,7 @@
     }
   }
 
-  async function onUpdate (event: CustomEvent) {
+  function onUpdate (event: CustomEvent): void {
     if (!shouldSaveDraft) {
       return
     }
@@ -99,25 +99,11 @@
     currentMessage.attachments = attachments
   }
 
-  async function onMessage (event: CustomEvent) {
-    if (chatMessage) {
-      loading = true
-    } // for new messages we use instant txes
-
+  async function handleCreate (event: CustomEvent, _id: Ref<ChatMessage>): Promise<void> {
     const doneOp = getClient().measure(`chunter.create.${_class} ${object._class}`)
     try {
-      draftController.remove()
-      inputRef.removeDraft(false)
+      await createMessage(event, _id)
 
-      if (chatMessage) {
-        await editMessage(event)
-      } else {
-        await createMessage(event)
-      }
-
-      // Remove draft from Local Storage
-      currentMessage = getDefault()
-      _id = currentMessage._id
       const d1 = Date.now()
       void (await doneOp)().then((res) => {
         console.log(`create.${_class} measure`, res, Date.now() - d1)
@@ -127,11 +113,44 @@
       Analytics.handleError(err)
       console.error(err)
     }
-    dispatch('submit', false)
-    loading = false
   }
 
-  async function createMessage (event: CustomEvent) {
+  async function handleEdit (event: CustomEvent): Promise<void> {
+    const doneOp = getClient().measure(`chunter.edit.${_class} ${object._class}`)
+    try {
+      await editMessage(event)
+
+      const d1 = Date.now()
+      void (await doneOp)().then((res) => {
+        console.log(`edit.${_class} measure`, res, Date.now() - d1)
+      })
+    } catch (err: any) {
+      void (await doneOp)()
+      Analytics.handleError(err)
+      console.error(err)
+    }
+  }
+
+  async function onMessage (event: CustomEvent): Promise<void> {
+    draftController.remove()
+    inputRef.removeDraft(false)
+
+    if (chatMessage !== undefined) {
+      loading = true
+      await handleEdit(event)
+    } else {
+      void handleCreate(event, _id)
+    }
+
+    // Remove draft from Local Storage
+    clear()
+    currentMessage = getDefault()
+    _id = currentMessage._id
+    loading = false
+    dispatch('submit', false)
+  }
+
+  async function createMessage (event: CustomEvent, _id: Ref<ChatMessage>): Promise<void> {
     const { message, attachments } = event.detail
     const operations = client.apply(_id)
 
@@ -153,8 +172,6 @@
         _id as Ref<ThreadMessage>
       )
 
-      clear()
-
       await operations.update(parentMessage, { lastReply: Date.now() })
 
       const hasPerson = !!parentMessage.repliedPersons?.includes(account.person)
@@ -172,12 +189,11 @@
         { message, attachments },
         _id
       )
-      clear()
     }
     await operations.commit()
   }
 
-  async function editMessage (event: CustomEvent) {
+  async function editMessage (event: CustomEvent): Promise<void> {
     if (chatMessage === undefined) {
       return
     }
@@ -194,7 +210,8 @@
   bind:this={inputRef}
   bind:content={inputContent}
   {_class}
-  space={object.space}
+  space={isSpace(object) ? object._id : object.space}
+  skipAttachmentsPreload={(currentMessage.attachments ?? 0) === 0}
   bind:objectId={_id}
   {shouldSaveDraft}
   {boundary}

plugins/chunter-resources/src/utils.ts

@@ -20,7 +20,6 @@ import {
   type Class,
   type Client,
   type Doc,
-  generateId,
   getCurrentAccount,
   type IdMap,
   type Ref,
@@ -41,10 +40,11 @@ import activity, {
 import {
   archiveContextNotifications,
   InboxNotificationsClientImpl,
+  isActivityNotification,
   isMentionNotification
 } from '@hcengineering/notification-resources'
 import notification, { type DocNotifyContext } from '@hcengineering/notification'
-import { get, type Unsubscriber } from 'svelte/store'
+import { get, type Unsubscriber, writable } from 'svelte/store'
 
 import chunter from './plugin'
 import DirectIcon from './components/DirectIcon.svelte'
@@ -348,6 +348,57 @@ export async function leaveChannel (channel: Channel, value: Ref<Account> | Arra
   }
 }
 
+// NOTE: Store timestamp updates to avoid unnecessary updates when if the server takes a long time to respond
+const contextsTimestampStore = writable<Map<Ref<DocNotifyContext>, number>>(new Map())
+// NOTE: Sometimes user can read message before notification is created and we should mark it as viewed when notification is received
+export const chatReadMessagesStore = writable<Set<Ref<ActivityMessage>>>(new Set())
+
+function getAllIds (messages: DisplayActivityMessage[]): Array<Ref<ActivityMessage>> {
+  return messages
+    .map((message) => {
+      const combined =
+        message._class === activity.class.DocUpdateMessage
+          ? (message as DisplayDocUpdateMessage)?.combinedMessagesIds
+          : undefined
+
+      return [message._id, ...(combined ?? [])]
+    })
+    .flat()
+}
+
+export function recheckNotifications (context: DocNotifyContext): void {
+  const client = getClient()
+  const inboxClient = InboxNotificationsClientImpl.getClient()
+
+  const messages = get(chatReadMessagesStore)
+
+  if (messages.size === 0) {
+    return
+  }
+
+  const notifications = get(inboxClient.inboxNotificationsByContext).get(context._id) ?? []
+
+  const toRead = notifications
+    .filter((it) => {
+      if (it.isViewed) {
+        return false
+      }
+
+      if (isMentionNotification(it)) {
+        return messages.has(it.mentionedIn as Ref<ActivityMessage>)
+      }
+
+      if (isActivityNotification(it)) {
+        return messages.has(it.attachedTo)
+      }
+
+      return false
+    })
+    .map((n) => n._id)
+
+  void inboxClient.readNotifications(client, toRead)
+}
+
 export async function readChannelMessages (
   messages: DisplayActivityMessage[],
   context: DocNotifyContext | undefined
@@ -359,40 +410,36 @@ export async function readChannelMessages (
   const inboxClient = InboxNotificationsClientImpl.getClient()
   const client = getClient()
 
-  const allIds = messages
-    .map((message) => {
-      const combined =
-        message._class === activity.class.DocUpdateMessage
-          ? (message as DisplayDocUpdateMessage)?.combinedMessagesIds
-          : undefined
+  const readMessages = get(chatReadMessagesStore)
+  const allIds = getAllIds(messages).filter((id) => !readMessages.has(id))
 
-      return [message._id, ...(combined ?? [])]
-    })
-    .flat()
-  const relatedMentions = get(inboxClient.otherInboxNotifications).filter(
-    (n) => !n.isViewed && isMentionNotification(n) && allIds.includes(n.mentionedIn as Ref<ActivityMessage>)
-  )
+  const notifications = get(inboxClient.activityInboxNotifications)
+    .filter(({ _id, attachedTo }) => allIds.includes(attachedTo))
+    .map((n) => n._id)
 
-  const ops = getClient().apply(generateId())
+  const relatedMentions = get(inboxClient.otherInboxNotifications)
+    .filter((n) => !n.isViewed && isMentionNotification(n) && allIds.includes(n.mentionedIn as Ref<ActivityMessage>))
+    .map((n) => n._id)
 
-  void inboxClient.readMessages(ops, allIds).then(() => {
-    void ops.commit()
-  })
+  chatReadMessagesStore.update((store) => new Set([...store, ...allIds]))
 
-  void inboxClient.readNotifications(
-    client,
-    relatedMentions.map((n) => n._id)
-  )
+  void inboxClient.readNotifications(client, [...notifications, ...relatedMentions])
 
   if (context === undefined) {
     return
   }
 
-  const lastTimestamp = messages[messages.length - 1].createdOn ?? 0
+  const storedTimestampUpdates = get(contextsTimestampStore).get(context._id)
+  const newTimestamp = messages[messages.length - 1].createdOn ?? 0
+  const prevTimestamp = Math.max(storedTimestampUpdates ?? 0, context.lastViewedTimestamp ?? 0)
 
-  if ((context.lastViewedTimestamp ?? 0) < lastTimestamp) {
-    context.lastViewedTimestamp = lastTimestamp
-    void client.update(context, { lastViewedTimestamp: lastTimestamp })
+  if (prevTimestamp < newTimestamp) {
+    context.lastViewedTimestamp = newTimestamp
+    contextsTimestampStore.update((store) => {
+      store.set(context._id, newTimestamp)
+      return store
+    })
+    void client.update(context, { lastViewedTimestamp: newTimestamp })
   }
 }
 

plugins/notification-resources/src/inboxNotificationsClient.ts

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //
-import activity, { type ActivityMessage } from '@hcengineering/activity'
+import activity from '@hcengineering/activity'
 import {
   SortingOrder,
   generateId,
@@ -216,29 +216,10 @@ export class InboxNotificationsClientImpl implements InboxNotificationsClient {
     })
   }
 
-  async readMessages (client: TxOperations, ids: Array<Ref<ActivityMessage>>): Promise<void> {
-    const alreadyReadIds = get(this.activityInboxNotifications)
-      .filter(({ attachedTo, isViewed }) => ids.includes(attachedTo) && isViewed)
-      .map(({ attachedTo }) => attachedTo)
-
-    const toReadIds = ids.filter((id) => !alreadyReadIds.includes(id))
-
-    if (toReadIds.length === 0) {
-      return
-    }
-
-    const notificationsToRead = get(this.activityInboxNotifications).filter(({ attachedTo }) =>
-      toReadIds.includes(attachedTo)
-    )
-
-    for (const notification of notificationsToRead) {
-      notification.isViewed = true
-      await client.update(notification, { isViewed: true })
-    }
-  }
-
   async readNotifications (client: TxOperations, ids: Array<Ref<InboxNotification>>): Promise<void> {
-    const notificationsToRead = (get(this.inboxNotifications) ?? []).filter(({ _id }) => ids.includes(_id))
+    const notificationsToRead = (get(this.inboxNotifications) ?? []).filter(
+      ({ _id, isViewed }) => ids.includes(_id) && !isViewed
+    )
 
     for (const notification of notificationsToRead) {
       await client.update(notification, { isViewed: true })

plugins/notification/src/index.ts

@@ -286,7 +286,6 @@ export interface InboxNotificationsClient {
 
   readDoc: (client: TxOperations, _id: Ref<Doc>) => Promise<void>
   forceReadDoc: (client: TxOperations, _id: Ref<Doc>, _class: Ref<Class<Doc>>) => Promise<void>
-  readMessages: (client: TxOperations, ids: Ref<ActivityMessage>[]) => Promise<void>
   readNotifications: (client: TxOperations, ids: Array<Ref<InboxNotification>>) => Promise<void>
   unreadNotifications: (client: TxOperations, ids: Array<Ref<InboxNotification>>) => Promise<void>
   archiveNotifications: (client: TxOperations, ids: Array<Ref<InboxNotification>>) => Promise<void>