Fix broken canCreateEntity logic

If loading status is `Done`, then it is always OK to insert an entity. Also rename to canInsertEntity as it is not creating an entity but inserting one into the list. Lastly add tests for CREATE as they were missing. Fixes #8277 Co-authored-by: hrb-hub <[email protected]> Co-authored-by: charlag <[email protected]>
tutao · Jan 16, 2025 · 6d5bcac · 6d5bcac
1 parent 4adbb51
commit 6d5bcac
Show file tree

Hide file tree

Showing 2 changed files with 138 additions and 18 deletions.
diff --git a/src/common/misc/ListElementListModel.ts b/src/common/misc/ListElementListModel.ts
@@ -41,7 +41,7 @@ export class ListElementListModel<ElementType extends ListElement> {
 			// Wait for any pending loading
 			return this.listModel.waitLoad(() => {
 				if (operation === OperationType.CREATE) {
-					if (this.canCreateEntity(entity)) {
+					if (this.canInsertEntity(entity)) {
 						this.listModel.insertLoadedItem(entity)
 					}
 				} else if (operation === OperationType.UPDATE) {
@@ -54,9 +54,9 @@ export class ListElementListModel<ElementType extends ListElement> {
 		}
 	}
 
-	private canCreateEntity(entity: ElementType): boolean {
-		if (this.state.loadingStatus !== ListLoadingState.Done) {
-			return false
+	private canInsertEntity(entity: ElementType): boolean {
+		if (this.state.loadingStatus === ListLoadingState.Done) {
+			return true
 		}
 
 		// new element is in the loaded range or newer than the first element

diff --git a/test/tests/misc/ListElementListModelTest.ts b/test/tests/misc/ListElementListModelTest.ts
@@ -7,6 +7,7 @@ import { OperationType } from "../../../src/common/api/common/TutanotaConstants.
 import { createTestEntity } from "../TestUtils.js"
 import { ListAutoSelectBehavior } from "../../../src/common/misc/DeviceConfig.js"
 import { ListElementListModel, ListElementListModelConfig } from "../../../src/common/misc/ListElementListModel"
+import { ConnectionError } from "../../../src/common/api/common/error/RestError"
 
 o.spec("ListElementListModel", function () {
 	const listId = "listId"
@@ -174,18 +175,22 @@ o.spec("ListElementListModel", function () {
 	})
 
 	o.spec("Updating items", function () {
+		function loadsElement(element: KnowledgeBaseEntry): (listId: Id, elementId: Id) => Promise<KnowledgeBaseEntry | null> {
+			return async (_listId: Id, elementId: Id): Promise<KnowledgeBaseEntry | null> => {
+				if (elementId === getElementId(element)) {
+					return element
+				} else {
+					throw new Error("noop")
+				}
+			}
+		}
+
 		o("update for item with id sorting updates item", async function () {
 			const updatedItemD = createTestEntity(KnowledgeBaseEntryTypeRef, { ...itemD, title: "AA" })
 
 			const newConfig: ListElementListModelConfig<KnowledgeBaseEntry> = {
 				...defaultListConfig,
-				async loadSingle(_listId: Id, elementId: Id): Promise<KnowledgeBaseEntry | null> {
-					if (elementId === getElementId(itemD)) {
-						return updatedItemD
-					} else {
-						throw new Error("noop")
-					}
-				},
+				loadSingle: loadsElement(updatedItemD),
 			}
 
 			listModel = new ListElementListModel<KnowledgeBaseEntry>(newConfig)
@@ -201,13 +206,7 @@ o.spec("ListElementListModel", function () {
 
 			const newConfig: ListElementListModelConfig<KnowledgeBaseEntry> = {
 				...defaultListConfig,
-				async loadSingle(_listId: Id, elementId: Id): Promise<KnowledgeBaseEntry | null> {
-					if (elementId === getElementId(itemD)) {
-						return updatedItemD
-					} else {
-						throw new Error("noop")
-					}
-				},
+				loadSingle: loadsElement(updatedItemD),
 				sortCompare: (e1, e2) => {
 					return e1.title.localeCompare(e2.title)
 				},
@@ -220,5 +219,126 @@ o.spec("ListElementListModel", function () {
 
 			o(listModel.state.items).deepEquals([itemA, updatedItemD, itemB, itemC])
 		})
+
+		o("create loading done", async function () {
+			const itemE = createTestEntity(KnowledgeBaseEntryTypeRef, {
+				_id: [listId, "e"],
+				title: "e",
+			})
+
+			let somePromise: DeferredObject<ListFetchResult<KnowledgeBaseEntry>> = defer()
+
+			const newConfig: ListElementListModelConfig<KnowledgeBaseEntry> = {
+				...defaultListConfig,
+				fetch: () => {
+					return somePromise.promise
+				},
+				loadSingle: loadsElement(itemE),
+			}
+
+			listModel = new ListElementListModel<KnowledgeBaseEntry>(newConfig)
+
+			listModel.loadInitial()
+
+			const received = listModel.entityEventReceived(getListId(itemE), getElementId(itemE), OperationType.CREATE)
+			somePromise.resolve({
+				items: [],
+				complete: true,
+			})
+
+			await received
+
+			o(listModel.state.items).deepEquals([itemE])
+		})
+
+		o("when receive create event while empty list and not loaded completely it will not insert the item", async function () {
+			const itemE = createTestEntity(KnowledgeBaseEntryTypeRef, {
+				_id: [listId, "e"],
+				title: "e",
+			})
+
+			let somePromise: DeferredObject<ListFetchResult<KnowledgeBaseEntry>> = defer()
+
+			const newConfig: ListElementListModelConfig<KnowledgeBaseEntry> = {
+				...defaultListConfig,
+				fetch: () => {
+					return somePromise.promise
+				},
+				loadSingle: loadsElement(itemE),
+			}
+
+			listModel = new ListElementListModel<KnowledgeBaseEntry>(newConfig)
+
+			listModel.loadInitial()
+
+			const received = listModel.entityEventReceived(getListId(itemE), getElementId(itemE), OperationType.CREATE)
+			somePromise.resolve({
+				items: [],
+				complete: false,
+			})
+
+			await received
+
+			o(listModel.state.items).deepEquals([])
+		})
+
+		o("when receive create event while empty list and error it does not insert", async function () {
+			const itemE = createTestEntity(KnowledgeBaseEntryTypeRef, {
+				_id: [listId, "e"],
+				title: "e",
+			})
+
+			let somePromise: DeferredObject<ListFetchResult<KnowledgeBaseEntry>> = defer()
+
+			const newConfig: ListElementListModelConfig<KnowledgeBaseEntry> = {
+				...defaultListConfig,
+				fetch: () => {
+					return somePromise.promise
+				},
+				loadSingle: loadsElement(itemE),
+			}
+
+			listModel = new ListElementListModel<KnowledgeBaseEntry>(newConfig)
+
+			listModel.loadInitial()
+
+			const received = listModel.entityEventReceived(getListId(itemE), getElementId(itemE), OperationType.CREATE)
+			somePromise.reject(new ConnectionError("test"))
+
+			await received
+
+			o(listModel.state.items).deepEquals([])
+		})
+
+		o("when receive create event and out of range", async function () {
+			const itemE = createTestEntity(KnowledgeBaseEntryTypeRef, {
+				_id: [listId, "e"],
+				title: "e",
+			})
+
+			let somePromise: DeferredObject<ListFetchResult<KnowledgeBaseEntry>> = defer()
+
+			const newConfig: ListElementListModelConfig<KnowledgeBaseEntry> = {
+				...defaultListConfig,
+				fetch: () => {
+					return somePromise.promise
+				},
+				loadSingle: loadsElement(itemE),
+			}
+
+			listModel = new ListElementListModel<KnowledgeBaseEntry>(newConfig)
+
+			listModel.loadInitial()
+
+			const received = listModel.entityEventReceived(getListId(itemE), getElementId(itemE), OperationType.CREATE)
+			somePromise.resolve({
+				items: [itemA, itemB, itemC, itemD],
+				complete: false,
+			})
+
+			await received
+
+			o(listModel.state.items).deepEquals([itemA, itemB, itemC, itemD])
+		})
 	})
 })