Fix entity updates

Co-authored-by: paw <[email protected]>

close: #8223

src/mail-app/mail/model/ConversationListModel.ts

@@ -1,7 +1,7 @@
 import { LoadedMail, MailSetListModel } from "./MailSetListModel"
 import { ListLoadingState, ListState } from "../../../common/gui/base/List"
-import { Mail, MailFolder, MailSetEntry, MailSetEntryTypeRef, MailTypeRef } from "../../../common/api/entities/tutanota/TypeRefs"
-import { EntityUpdateData } from "../../../common/api/common/utils/EntityUpdateUtils"
+import { Mail, MailFolder, MailFolderTypeRef, MailSetEntry, MailSetEntryTypeRef, MailTypeRef } from "../../../common/api/entities/tutanota/TypeRefs"
+import { EntityUpdateData, isUpdateForTypeRef } from "../../../common/api/common/utils/EntityUpdateUtils"
 import { ListFilter, ListModel } from "../../../common/misc/ListModel"
 import Stream from "mithril/stream"
 import { ConversationPrefProvider } from "../view/ConversationViewModel"
@@ -21,7 +21,7 @@ import {
 import { assertNotNull, compare, promiseFilter } from "@tutao/tutanota-utils"
 import { ListFetchResult } from "../../../common/gui/base/ListUtils"
 import { isOfflineError } from "../../../common/api/common/utils/ErrorUtils"
-import { MailSetKind } from "../../../common/api/common/TutanotaConstants"
+import { MailSetKind, OperationType } from "../../../common/api/common/TutanotaConstants"
 
 export interface LoadedConversation {
 	conversationId: Id
@@ -53,14 +53,7 @@ export class ConversationListModel implements MailSetListModel {
 				return this.loadMails(lastFetchedId, count)
 			},
 
-			sortCompare: (item1, item2) => {
-				// Mail set entry ID has the timestamp and mail element ID
-				const item1Id = this.effectiveIdOfConversation(item1)
-				const item2Id = this.effectiveIdOfConversation(item2)
-
-				// Sort in reverse order to ensure newer mails are first
-				return compare(customIdToUint8array(item2Id), customIdToUint8array(item1Id))
-			},
+			sortCompare: (item1, item2) => this.reverseSortConversation(item1, item2),
 
 			getItemId: (item) => this.effectiveIdOfConversation(item),
 
@@ -99,8 +92,104 @@ export class ConversationListModel implements MailSetListModel {
 	}
 
 	async handleEntityUpdate(update: EntityUpdateData) {
-		// FIXME
-		return Promise.resolve(undefined)
+		if (isUpdateForTypeRef(MailFolderTypeRef, update)) {
+			// If a label is modified, we want to update all mails that reference it, which requires linearly iterating
+			// through all mails. There are more efficient ways we could do this, such as by keeping track of each label
+			// we've retrieved from the database and just update that, but we want to avoid adding more maps that we
+			// have to maintain.
+			if (update.operation === OperationType.UPDATE) {
+				const mailSetId: IdTuple = [update.instanceListId, update.instanceId]
+				for (const loadedMail of this.mailMap.values()) {
+					const hasMailSet = loadedMail.latestMail.labels.some((label) => isSameId(mailSetId, label._id))
+					if (!hasMailSet) {
+						continue
+					}
+					// MailModel's entity event listener should have been fired first
+					const labels = this.mailModel.getLabelsForMail(loadedMail.latestMail.mail)
+					const newMailEntry = {
+						...loadedMail,
+						labels,
+					}
+					this._updateSingleMail(newMailEntry)
+				}
+			}
+		} else if (isUpdateForTypeRef(MailSetEntryTypeRef, update) && isSameId(this.mailSet.entries, update.instanceListId)) {
+			// Adding/removing to this list (MailSetEntry doesn't have any fields to update, so we don't need to handle this)
+			if (update.operation === OperationType.DELETE) {
+				// TODO: what happens if we delete a mail set entry where an existing email in that conversation also exists in the folder?
+				const { mailId } = deconstructMailSetEntryId(update.instanceId)
+				const conversation = this.mailMap.get(mailId)
+				if (conversation) {
+					this.mailMap.delete(mailId)
+					this.conversationMap.delete(conversation.conversationId)
+				}
+				await this.listModel.deleteLoadedItem(update.instanceId)
+			} else if (update.operation === OperationType.CREATE) {
+				const loadedMail = await this.loadSingleMail([update.instanceListId, update.instanceId])
+				const addedMail = loadedMail.addedItems[0]
+				if (addedMail != null) {
+					await this.listModel.waitLoad(async () => {
+						if (this.listModel.canInsertItem(addedMail)) {
+							this.listModel.insertLoadedItem(addedMail)
+						}
+						for (const oldEntry of loadedMail.deletedItems) {
+							const id = this.effectiveIdOfConversation(oldEntry)
+							const wasSelected = this.listModel.isItemSelected(id)
+							const inMultiselect = this.isInMultiselect()
+							const selection = this.listModel.getSelectedAsArray()
+							this.mailMap.delete(getElementId(oldEntry.latestMail.mail))
+							await this.listModel.deleteLoadedItem(id)
+
+							// ensure the selection does not change
+							if (wasSelected) {
+								if (inMultiselect) {
+									for (const s of selection) {
+										if (this.effectiveIdOfConversation(s) !== this.effectiveIdOfConversation(oldEntry)) {
+											this.listModel.onSingleInclusiveSelection(s)
+										}
+									}
+									this.listModel.onSingleInclusiveSelection(addedMail)
+								} else {
+									this.listModel.onSingleSelection(addedMail)
+								}
+							}
+						}
+					})
+				}
+			}
+		} else if (isUpdateForTypeRef(MailTypeRef, update)) {
+			// We only need to handle updates for Mail.
+			// Mail deletion will also be handled in MailSetEntry delete/create.
+			const mailItem = this.mailMap.get(update.instanceId)
+			if (mailItem != null && update.operation === OperationType.UPDATE) {
+				const newMailData = await this.entityClient.load(MailTypeRef, [update.instanceListId, update.instanceId])
+				const labels = this.mailModel.getLabelsForMail(newMailData) // in case labels were added/removed
+				const loadedMail = {
+					...mailItem.latestMail,
+					labels,
+					mail: newMailData,
+				}
+				this._updateSingleMail({
+					...mailItem,
+					latestMail: loadedMail,
+				})
+			}
+		}
+	}
+
+	// @VisibleForTesting
+	_updateSingleMail(loadedConversation: LoadedConversation) {
+		this.updateMailMap(loadedConversation)
+		this.listModel.updateLoadedItem(loadedConversation)
+	}
+
+	private async loadSingleMail(id: IdTuple): Promise<{
+		addedItems: LoadedConversation[]
+		deletedItems: LoadedConversation[]
+	}> {
+		const mailSetEntry = await this.entityClient.load(MailSetEntryTypeRef, id)
+		const loadedMails = await this.resolveMailSetEntries([mailSetEntry], this.defaultMailProvider)
+		return this.filterAndUpdateMails(loadedMails)
 	}
 
 	isEmptyAndDone(): boolean {
@@ -259,7 +348,8 @@ export class ConversationListModel implements MailSetListModel {
 		}
 
 		return {
-			items: this.updateMailMap(items),
+			// there should be no deleted items since we're loading older mails
+			items: this.filterAndUpdateMails(items)?.addedItems,
 			complete,
 		}
 	}
@@ -281,25 +371,39 @@ export class ConversationListModel implements MailSetListModel {
 		})
 	}
 
-	private updateMailMap(mails: LoadedMail[]): LoadedConversation[] {
-		const conversations: LoadedConversation[] = []
+	private filterAndUpdateMails(mails: LoadedMail[]): {
+		addedItems: LoadedConversation[]
+		deletedItems: LoadedConversation[]
+	} {
+		const addedItems: LoadedConversation[] = []
+		const deletedItems: LoadedConversation[] = []
 
 		for (const mail of mails) {
 			const conversation = {
 				conversationId: listIdPart(mail.mail.conversationEntry),
 				latestMail: mail,
 			}
 
-			if (this.conversationMap.has(conversation.conversationId)) {
+			const existingConversation = this.conversationMap.get(conversation.conversationId)
+			if (existingConversation != null && this.reverseSortConversation(conversation, existingConversation) >= 0) {
 				continue
 			}
 
-			this.mailMap.set(getElementId(conversation.latestMail.mail), conversation)
-			this.conversationMap.set(conversation.conversationId, conversation)
-			conversations.push(conversation)
+			if (existingConversation != null) {
+				this.mailMap.delete(getElementId(existingConversation.latestMail.mail))
+				deletedItems.push(existingConversation)
+			}
+
+			this.updateMailMap(conversation)
+			addedItems.push(conversation)
 		}
 
-		return conversations
+		return { addedItems, deletedItems }
+	}
+
+	private updateMailMap(conversation: LoadedConversation) {
+		this.mailMap.set(getElementId(conversation.latestMail.mail), conversation)
+		this.conversationMap.set(conversation.conversationId, conversation)
 	}
 
 	private async resolveMailSetEntries(
@@ -309,6 +413,15 @@ export class ConversationListModel implements MailSetListModel {
 		return resolveMailSetEntries(mailSetEntries, mailProvider, this.mailModel)
 	}
 
+	private reverseSortConversation(item1: LoadedConversation, item2: LoadedConversation): number {
+		// Mail set entry ID has the timestamp and mail element ID
+		const item1Id = this.effectiveIdOfConversation(item1)
+		const item2Id = this.effectiveIdOfConversation(item2)
+
+		// Sort in reverse order to ensure newer mails are first
+		return compare(customIdToUint8array(item2Id), customIdToUint8array(item1Id))
+	}
+
 	/**
 	 * Load mails from the cache rather than remotely
 	 */

test/tests/mail/model/MailListModelTest.ts

@@ -1,5 +1,5 @@
 import o from "../../../../packages/otest/dist/otest"
-import { LoadedMail, MailListModel } from "../../../../src/mail-app/mail/model/MailListModel"
+import { MailListModel } from "../../../../src/mail-app/mail/model/MailListModel"
 import {
 	createMailFolder,
 	createMailSetEntry,
@@ -37,6 +37,7 @@ import { MailboxDetail } from "../../../../src/common/mailFunctionality/MailboxM
 import { GroupInfoTypeRef, GroupTypeRef } from "../../../../src/common/api/entities/sys/TypeRefs"
 import { ConnectionError } from "../../../../src/common/api/common/error/RestError"
 import { clamp, pad } from "@tutao/tutanota-utils"
+import { LoadedMail } from "../../../../src/mail-app/mail/model/MailSetListModel"
 
 o.spec("MailListModelTest", () => {
 	let model: MailListModel