Use resolve conversations service when executing mail actions

Close #8506

Co-authored-by: paw <paw-hub@users.noreply.github.com>
Co-authored-by: wrd <wrd@tutao.de>

src/common/api/common/TutanotaConstants.ts

@@ -11,6 +11,7 @@ import { ProgrammingError } from "./error/ProgrammingError"
 import { TranslationKey } from "../../misc/LanguageViewModel.js"
 
 export const MAX_NBR_MOVE_DELETE_MAIL_SERVICE = 50
+export const MAX_NBR_OF_CONVERSATIONS = 50
 
 // visible for testing
 export const MAX_BLOB_SIZE_BYTES = 1024 * 1024 * 10

src/common/api/main/OperationProgressTracker.ts

@@ -5,6 +5,17 @@ export type OperationId = number
 
 export type ExposedOperationProgressTracker = Pick<OperationProgressTracker, "onProgress">
 
+/**
+ *  - id for sending updates
+ *  - progress, a stream to observe
+ *  - done, a handle to stop tracking the operation progress
+ */
+export interface OperationHandle {
+	id: OperationId
+	progress: Stream<number>
+	done: () => unknown
+}
+
 /**
  * This is a multiplexer for tracking individual remote async operations.
  * Unlike {@link ProgressTracker} does not accumulate the progress and doesn't compute the percentage from units of work.
@@ -16,12 +27,9 @@ export class OperationProgressTracker {
 	private operationId = 0
 
 	/**
-	 * Prepares a new operation and gives a handle for it which contains:
-	 *   - id for sending updates
-	 *   - progress, a stream to observe
-	 *   - done, a handle to stop tracking the operation progress
+	 * Prepares a new operation and gives a handle.
 	 */
-	startNewOperation(): { id: OperationId; progress: Stream<number>; done: () => unknown } {
+	startNewOperation(): OperationHandle {
 		const id = this.operationId++
 		const progress = stream<number>(0)
 		this.progressPerOp.set(id, progress)

src/common/api/worker/facades/lazy/MailFacade.ts

@@ -9,6 +9,7 @@ import {
 	ManageLabelService,
 	MoveMailService,
 	ReportMailService,
+	ResolveConversationsService,
 	SendDraftService,
 	UnreadMailStateService,
 } from "../../../entities/tutanota/Services.js"
@@ -25,6 +26,7 @@ import {
 	MailMethod,
 	MailReportType,
 	MAX_NBR_MOVE_DELETE_MAIL_SERVICE,
+	MAX_NBR_OF_CONVERSATIONS,
 	OperationType,
 	PhishingMarkerStatus,
 	PublicKeyIdentifierType,
@@ -53,6 +55,7 @@ import {
 	createMoveMailData,
 	createNewDraftAttachment,
 	createReportMailPostData,
+	createResolveConversationsServiceGetIn,
 	createSecureExternalRecipientKeyData,
 	createSendDraftData,
 	createUnreadMailStatePostIn,
@@ -78,13 +81,16 @@ import {
 } from "../../../entities/tutanota/TypeRefs.js"
 import { RecipientsNotFoundError } from "../../../common/error/RecipientsNotFoundError.js"
 import { NotFoundError } from "../../../common/error/RestError.js"
-import type { EntityUpdate, ExternalUserReference, User } from "../../../entities/sys/TypeRefs.js"
 import {
 	BlobReferenceTokenWrapper,
+	createGeneratedIdWrapper,
+	EntityUpdate,
+	ExternalUserReference,
 	ExternalUserReferenceTypeRef,
 	GroupInfoTypeRef,
 	GroupRootTypeRef,
 	GroupTypeRef,
+	User,
 	UserTypeRef,
 } from "../../../entities/sys/TypeRefs.js"
 import {
@@ -102,8 +108,8 @@ import {
 	ofClass,
 	promiseFilter,
 	promiseMap,
-	Versioned,
 	splitInChunks,
+	Versioned,
 } from "@tutao/tutanota-utils"
 import { BlobFacade } from "./BlobFacade.js"
 import { assertWorkerOrNode, isApp, isDesktop } from "../../../common/Env.js"
@@ -1113,6 +1119,22 @@ export class MailFacade {
 			{ concurrency: 5 },
 		)
 	}
+
+	/** Resolve conversation list ids to the IDs of mails in those conversations. */
+	async resolveConversations(conversationListIds: readonly Id[]): Promise<IdTuple[]> {
+		const result = await promiseMap(
+			splitInChunks(MAX_NBR_OF_CONVERSATIONS, conversationListIds),
+			async (conversationListIds) =>
+				this.serviceExecutor.get(
+					ResolveConversationsService,
+					createResolveConversationsServiceGetIn({
+						conversationLists: conversationListIds.map((id) => createGeneratedIdWrapper({ value: id })),
+					}),
+				),
+			{ concurrency: 2 },
+		)
+		return result.flatMap((response) => response.mailIds).map((idTupleWrapper) => [idTupleWrapper.listId, idTupleWrapper.listElementId])
+	}
 }
 
 export function phishingMarkerValue(type: ReportedMailFieldType, value: string): string {

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

@@ -433,18 +433,12 @@ export class MailModel {
 		return !this.logins.isEnabled(FeatureType.DisableMailExport)
 	}
 
-	async markMails(mails: readonly Mail[], unread: boolean): Promise<void> {
-		await this.mailFacade.markMails(
-			mails.map(({ _id }) => _id),
-			unread,
-		)
+	async markMails(mails: readonly IdTuple[], unread: boolean): Promise<void> {
+		await this.mailFacade.markMails(mails, unread)
 	}
 
-	async applyLabels(mails: readonly Mail[], addedLabels: readonly MailFolder[], removedLabels: readonly MailFolder[]): Promise<void> {
-		const groupedByListIds = groupBy(
-			mails.map((m) => m._id),
-			(mailId) => listIdPart(mailId),
-		)
+	async applyLabels(mails: readonly IdTuple[], addedLabels: readonly MailFolder[], removedLabels: readonly MailFolder[]): Promise<void> {
+		const groupedByListIds = groupBy(mails, (mailId) => listIdPart(mailId))
 		for (const [_, groupedMails] of groupedByListIds) {
 			const mailChunks = splitInChunks(MAX_NBR_MOVE_DELETE_MAIL_SERVICE, groupedMails)
 			for (const mailChunk of mailChunks) {
@@ -672,4 +666,9 @@ export class MailModel {
 
 		return allMails
 	}
+
+	/** Resolve conversation list ids to the IDs of mails in those conversations. */
+	async resolveConversationsForMails(mails: readonly Mail[]): Promise<IdTuple[]> {
+		return await this.mailFacade.resolveConversations(mails.map((m) => listIdPart(m.conversationEntry)))
+	}
 }

src/mail-app/mail/view/MailView.ts

@@ -486,7 +486,7 @@ export class MailView extends BaseTopLevelView implements TopLevelView<MailViewA
 			{
 				key: Keys.U,
 				exec: () => {
-					if (this.mailViewModel.listModel) this.toggleUnreadMails(this.mailViewModel.listModel.getSelectedAsArray())
+					if (this.mailViewModel.listModel) this.toggleUnreadMails()
 				},
 				help: "toggleUnread_action",
 			},
@@ -877,12 +877,15 @@ export class MailView extends BaseTopLevelView implements TopLevelView<MailViewA
 		m.route.set("/")
 	}
 
-	private async toggleUnreadMails(mails: Mail[]): Promise<void> {
-		if (mails.length == 0) {
+	private async toggleUnreadMails(): Promise<void> {
+		const selectedMails = this.mailViewModel.listModel?.getSelectedAsArray() ?? []
+		if (isEmpty(selectedMails)) {
 			return
 		}
+		const mailIds = await this.mailViewModel.getActionableMails(selectedMails)
+
 		// set all selected emails to the opposite of the first email's unread state
-		await mailLocator.mailModel.markMails(mails, !mails[0].unread)
+		await mailLocator.mailModel.markMails(mailIds, !selectedMails[0].unread)
 	}
 
 	private deleteMails(mails: Mail[]): Promise<boolean> {

src/mail-app/mail/view/MailViewModel.ts

@@ -329,11 +329,11 @@ export class MailViewModel {
         If ConversationInListView is active, all mails in the conversation are returned (so they can be processed in a group)
         If not, only the primary mail is returned, since that is the one being looked at/interacted with.
      */
-	async getActionableMails(mails: Mail[]): Promise<ReadonlyArray<Mail>> {
+	async getActionableMails(mails: Mail[]): Promise<ReadonlyArray<IdTuple>> {
 		if (this.conversationPrefProvider.getMailListDisplayMode() === MailListDisplayMode.CONVERSATIONS) {
-			return this.mailModel.loadConversationsForAllMails(mails)
+			return this.mailModel.resolveConversationsForMails(mails)
 		} else {
-			return mails
+			return mails.map((m) => m._id)
 		}
 	}
 

src/mail-app/mail/view/MailViewerHeader.ts

@@ -788,7 +788,7 @@ export class MailViewerHeader implements Component<MailViewerHeaderAttrs> {
 									styles.isDesktopLayout() ? 300 : 200,
 									viewModel.mailModel.getLabelsForMails([viewModel.mail]),
 									viewModel.mailModel.getLabelStatesForMails([viewModel.mail]),
-									(addedLabels, removedLabels) => viewModel.mailModel.applyLabels([viewModel.mail], addedLabels, removedLabels),
+									(addedLabels, removedLabels) => viewModel.mailModel.applyLabels([viewModel.mail._id], addedLabels, removedLabels),
 								)
 								// waiting for the dropdown to be closed
 								setTimeout(() => {

src/mail-app/mail/view/MailViewerToolbar.ts

@@ -23,10 +23,10 @@ export interface MailViewerToolbarAttrs {
 	mailboxModel: MailboxModel
 	mailModel: MailModel
 	selectedMails: Mail[]
-	actionableMails: () => Promise<readonly Mail[]>
 	primaryMailViewerViewModel?: MailViewerViewModel
 	actionableMailViewerViewModel?: MailViewerViewModel
 	selectNone?: () => void
+	actionableMails: () => Promise<readonly IdTuple[]>
 }
 
 // Note: this is only used for non-mobile views. Please also update MobileMailMultiselectionActionBar or MobileMailActionBar
@@ -104,7 +104,7 @@ export class MailViewerActions implements Component<MailViewerToolbarAttrs> {
 		})
 	}
 
-	private renderLabelButton(mailModel: MailModel, mails: readonly Mail[], actionableMails: () => Promise<readonly Mail[]>): Children {
+	private renderLabelButton(mailModel: MailModel, mails: readonly Mail[], actionableMails: () => Promise<readonly IdTuple[]>): Children {
 		return m(IconButton, {
 			title: "assignLabel_action",
 			icon: Icons.Label,
@@ -188,7 +188,7 @@ export class MailViewerActions implements Component<MailViewerToolbarAttrs> {
 		})
 	}
 
-	private renderMoreButton(viewModel: MailViewerViewModel | undefined, actionableMails: () => Promise<readonly Mail[]>): Children {
+	private renderMoreButton(viewModel: MailViewerViewModel | undefined, actionableMails: () => Promise<readonly IdTuple[]>): Children {
 		let actions: DropdownButtonAttrs[] = []
 
 		if (viewModel) {

src/mail-app/mail/view/MailViewerUtils.ts

@@ -1,5 +1,5 @@
 import { Keys, MailReportType, MailState, ReplyType, SYSTEM_GROUP_MAIL_ADDRESS } from "../../../common/api/common/TutanotaConstants"
-import { assertNotNull, neverNull, ofClass } from "@tutao/tutanota-utils"
+import { $Promisable, assertNotNull, groupByAndMap, neverNull, ofClass, promiseMap } from "@tutao/tutanota-utils"
 import { InfoLink, lang } from "../../../common/misc/LanguageViewModel"
 import { Dialog, DialogType } from "../../../common/gui/base/Dialog"
 import m from "mithril"
@@ -20,21 +20,21 @@ import { ExternalLink } from "../../../common/gui/base/ExternalLink.js"
 import { SourceCodeViewer } from "./SourceCodeViewer.js"
 import { getMailAddressDisplayText, hasValidEncryptionAuthForTeamOrSystemMail } from "../../../common/mailFunctionality/SharedMailUtils.js"
 import { mailLocator } from "../../mailLocator.js"
-import { Mail, MailDetails } from "../../../common/api/entities/tutanota/TypeRefs.js"
+import { Mail, MailDetails, MailTypeRef } from "../../../common/api/entities/tutanota/TypeRefs.js"
 import { getDisplayedSender } from "../../../common/api/common/CommonMailUtils.js"
 import { MailFacade } from "../../../common/api/worker/facades/lazy/MailFacade.js"
 
 import { ListFilter } from "../../../common/misc/ListModel.js"
-import { isApp, isDesktop } from "../../../common/api/common/Env.js"
+import { isDesktop } from "../../../common/api/common/Env.js"
 import { isDraft } from "../model/MailChecks.js"
-import { MailModel } from "../model/MailModel"
 import { DialogHeaderBarAttrs } from "../../../common/gui/base/DialogHeaderBar"
-import { IconButton } from "../../../common/gui/base/IconButton"
 import { exportMails } from "../export/Exporter"
 import stream from "mithril/stream"
+import Stream from "mithril/stream"
 import { ExpanderButton, ExpanderPanel } from "../../../common/gui/base/Expander"
 import { ColumnWidth, Table } from "../../../common/gui/base/Table"
-import { MailViewerToolbarAttrs } from "./MailViewerToolbar"
+import { elementIdPart, listIdPart } from "../../../common/api/common/utils/EntityUtils"
+import { OperationHandle } from "../../../common/api/main/OperationProgressTracker"
 
 export async function showHeaderDialog(headersPromise: Promise<string | null>) {
 	let state: { state: "loading" } | { state: "loaded"; headers: string | null } = { state: "loading" }
@@ -128,7 +128,7 @@ export async function showSourceDialog(rawHtml: string) {
 	return Dialog.viewerDialog("emailSourceCode_title", SourceCodeViewer, { rawHtml })
 }
 
-export function exportAction(actionableMails: () => Promise<readonly Mail[]>): DropdownButtonAttrs {
+export function exportAction(actionableMails: () => Promise<readonly IdTuple[]>): DropdownButtonAttrs {
 	const operation = locator.operationProgressTracker.startNewOperation()
 	const ac = new AbortController()
 	const headerBarAttrs: DialogHeaderBarAttrs = {
@@ -145,25 +145,52 @@ export function exportAction(actionableMails: () => Promise<readonly Mail[]>): D
 	return {
 		label: "export_action",
 		click: () => {
-			actionableMails().then((mails) => {
+			// We are doing a little backflip here to start showing progress while we still determine the number of
+			// mails to export.
+			const numberOfMailsStream = stream<number>()
+			numberOfMailsStream.map(m.redraw)
 			showProgressDialog(
-					lang.getTranslation("mailExportProgress_msg", {
-						"{current}": Math.round((operation.progress() / 100) * mails.length).toFixed(0),
-						"{total}": mails.length,
-					}),
-					exportMails(mails, locator.mailFacade, locator.entityClient, locator.fileController, locator.cryptoFacade, operation.id, ac.signal)
-						.then((result) => handleExportEmailsResult(result.failed))
-						.finally(operation.done),
+				() => {
+					const numberOfMails = numberOfMailsStream()
+					if (isNaN(numberOfMails)) {
+						return lang.getTranslation("mailExportProgress_msg", {
+							"{current}": 0,
+							"{total}": "?",
+						})
+					} else {
+						return lang.getTranslation("mailExportProgress_msg", {
+							"{current}": Math.round((operation.progress() / 100) * numberOfMails).toFixed(0),
+							"{total}": numberOfMails,
+						})
+					}
+				},
+				doExport(actionableMails, numberOfMailsStream, operation, ac),
 				operation.progress,
 				true,
 				headerBarAttrs,
 			)
-			})
 		},
 		icon: Icons.Export,
 	}
 }
 
+async function doExport(
+	actionableMails: () => $Promisable<readonly IdTuple[]>,
+	numberOfMailsStream: Stream<number>,
+	operation: OperationHandle,
+	ac: AbortController,
+) {
+	const mailIdsToLoad = await actionableMails()
+	numberOfMailsStream(mailIdsToLoad.length)
+	const mailIdsPerList = groupByAndMap(mailIdsToLoad, listIdPart, elementIdPart)
+	const mails = (
+		await promiseMap(mailIdsPerList, ([listId, elementIds]) => locator.entityClient.loadMultiple(MailTypeRef, listId, elementIds), { concurrency: 2 })
+	).flat()
+	return exportMails(mails, locator.mailFacade, locator.entityClient, locator.fileController, locator.cryptoFacade, operation.id, ac.signal)
+		.then((result) => handleExportEmailsResult(result.failed))
+		.finally(operation.done)
+}
+
 function handleExportEmailsResult(mailList: Mail[]) {
 	if (mailList && mailList.length > 0) {
 		const lines = mailList.map((mail) => ({
@@ -210,7 +237,7 @@ function handleExportEmailsResult(mailList: Mail[]) {
 	}
 }
 
-export function multipleMailViewerMoreActions(viewModel: MailViewerViewModel, actionableMails: () => Promise<readonly Mail[]>): Array<DropdownButtonAttrs> {
+export function multipleMailViewerMoreActions(viewModel: MailViewerViewModel, actionableMails: () => Promise<readonly IdTuple[]>): Array<DropdownButtonAttrs> {
 	const moreButtons: Array<DropdownButtonAttrs> = []
 
 	if (viewModel.isUnread()) {

src/mail-app/mail/view/MobileMailActionBar.ts

@@ -15,7 +15,7 @@ import { Mail } from "../../../common/api/entities/tutanota/TypeRefs"
 
 export interface MobileMailActionBarAttrs {
 	viewModel: MailViewerViewModel
-	actionableMails: () => Promise<readonly Mail[]>
+	actionableMails: () => Promise<readonly IdTuple[]>
 }
 
 export class MobileMailActionBar implements Component<MobileMailActionBarAttrs> {

src/mail-app/mail/view/MobileMailMultiselectionActionBar.ts

@@ -15,7 +15,7 @@ export interface MobileMailMultiselectionActionBarAttrs {
 	mailModel: MailModel
 	mailboxModel: MailboxModel
 	selectNone: () => unknown
-	actionableMails: () => Promise<readonly Mail[]>
+	actionableMails: () => Promise<readonly IdTuple[]>
 }
 
 // Note: The MailViewerToolbar is the counterpart for this on non-mobile views. Please update there too if needed

src/mail-app/search/view/SearchView.ts

@@ -409,7 +409,7 @@ export class SearchView extends BaseTopLevelView implements TopLevelView<SearchV
 					selectedMails: selectedMails,
 					// note on actionApplyMails: in search view, conversations are not grouped in the list and individual
 					//    mails are always shown. So the action applies only to the selected mails
-					actionableMails: async () => selectedMails,
+					actionableMails: async () => selectedMails.map((m) => m._id),
 					selectNone: () => this.searchViewModel.listModel.selectNone(),
 				})
 				return m(BackgroundColumnLayout, {
@@ -447,7 +447,7 @@ export class SearchView extends BaseTopLevelView implements TopLevelView<SearchV
 					selectedMails: [conversationViewModel.primaryMail],
 					// note on actionApplyMails: in search view, conversations are not grouped in the list and individual
 					//    mails are always shown. So the action applies only to the shown mail
-					actionableMails: async () => [conversationViewModel.primaryMail],
+					actionableMails: async () => [conversationViewModel.primaryMail._id],
 					actionableMailViewerViewModel: conversationViewModel.primaryViewModel(),
 				})
 				return m(BackgroundColumnLayout, {
@@ -610,7 +610,7 @@ export class SearchView extends BaseTopLevelView implements TopLevelView<SearchV
 				viewModel: this.searchViewModel.conversationViewModel?.primaryViewModel(),
 				// note on actionApplyMails: in search view, conversations are not grouped in the list and individual
 				//    mails are always shown. So the action applies only to the shown mail
-				actionableMails: async () => [assertNotNull(this.searchViewModel.conversationViewModel).primaryViewModel().mail],
+				actionableMails: async () => [assertNotNull(this.searchViewModel.conversationViewModel).primaryViewModel().mail._id],
 			})
 		} else if (!isInMultiselect && this.viewSlider.focusedColumn === this.resultDetailsColumn) {
 			if (getCurrentSearchMode() === SearchCategoryTypes.contact) {
@@ -672,7 +672,7 @@ export class SearchView extends BaseTopLevelView implements TopLevelView<SearchV
 					mailboxModel: locator.mailboxModel,
 					// note on actionApplyMails: in search view, conversations are not grouped in the list and individual
 					//    mails are always shown. So the action applies only to the selected mails
-					actionableMails: async () => this.searchViewModel.getSelectedMails(),
+					actionableMails: async () => this.searchViewModel.getSelectedMails().map((m) => m._id),
 				})
 			} else if (this.viewSlider.focusedColumn === this.resultListColumn) {
 				return m(
@@ -1033,7 +1033,10 @@ export class SearchView extends BaseTopLevelView implements TopLevelView<SearchV
 		let selectedMails = this.searchViewModel.getSelectedMails()
 
 		if (selectedMails.length > 0) {
-			mailLocator.mailModel.markMails(selectedMails, !selectedMails[0].unread)
+			mailLocator.mailModel.markMails(
+				selectedMails.map((m) => m._id),
+				!selectedMails[0].unread,
+			)
 		}
 	}
 

test/tests/mail/MailModelTest.ts

@@ -79,28 +79,11 @@ o.spec("MailModelTest", function () {
 	})
 
 	o("markMails", async function () {
-		const mails = [
-			createTestEntity(MailTypeRef, {
-				_id: ["mailbag id1", "mail id1"],
-			}),
-			createTestEntity(MailTypeRef, {
-				_id: ["mailbag id2", "mail id2"],
-			}),
-			createTestEntity(MailTypeRef, {
-				_id: ["mailbag id3", "mail id3"],
-			}),
-		]
-		await model.markMails(mails, true)
-		verify(
-			mailFacade.markMails(
-				[
-					["mailbag id1", "mail id1"],
-					["mailbag id2", "mail id2"],
-					["mailbag id3", "mail id3"],
-				],
-				true,
-			),
-		)
+		const mailId1: IdTuple = ["mailbag id1", "mail id1"]
+		const mailId2: IdTuple = ["mailbag id2", "mail id2"]
+		const mailId3: IdTuple = ["mailbag id3", "mail id3"]
+		await model.markMails([mailId1, mailId2, mailId3], true)
+		verify(mailFacade.markMails([mailId1, mailId2, mailId3], true))
 	})
 
 	function makeUpdate(arg: { instanceListId: string; instanceId: Id; operation: OperationType }): EntityUpdateData {