From 80a15006344e76359849d5c174bfa18299d321d9 Mon Sep 17 00:00:00 2001
From: Tao Klerks <tao@klerks.biz>
Date: Sun, 12 Mar 2023 16:16:45 +0100
Subject: [PATCH] Desktop: Fixes #7741: With Custom Sort, new notes appear at
 bottom and later randomly "pop" to the top (#7765)

---
 packages/lib/models/Note.ts                   | 11 +++++++---
 .../lib/models/Note_CustomSortOrder.test.js   | 20 ++++++++++++-------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/packages/lib/models/Note.ts b/packages/lib/models/Note.ts
index ab5223698..9f1bcd7f9 100644
--- a/packages/lib/models/Note.ts
+++ b/packages/lib/models/Note.ts
@@ -874,16 +874,21 @@ export default class Note extends BaseItem {
 				if (note.id === noteIds[0] && index === i) return defer();
 			}
 
-			// If some of the target notes have order = 0, set the order field to user_created_time
-			// (historically, all notes had the order field set to 0)
+			// If some of the target notes have order = 0, set the order field to a reasonable
+			// value to avoid moving the note. Using "smallest value / 2" (vs, for example,
+			// subtracting a constant) ensures items remain in their current position at the
+			// end, without ever reaching 0.
 			let hasSetOrder = false;
+			let previousOrder = 0;
 			for (let i = 0; i < notes.length; i++) {
 				const note = notes[i];
 				if (!note.order) {
-					const updatedNote = await this.updateNoteOrder_(note, note.user_created_time);
+					const newOrder = previousOrder ? previousOrder / 2 : note.user_created_time;
+					const updatedNote = await this.updateNoteOrder_(note, newOrder);
 					notes[i] = updatedNote;
 					hasSetOrder = true;
 				}
+				previousOrder = notes[i].order;
 			}
 
 			if (hasSetOrder) notes = await getSortedNotes(folderId);
diff --git a/packages/lib/models/Note_CustomSortOrder.test.js b/packages/lib/models/Note_CustomSortOrder.test.js
index 1dfa5ccb1..26b18da4c 100644
--- a/packages/lib/models/Note_CustomSortOrder.test.js
+++ b/packages/lib/models/Note_CustomSortOrder.test.js
@@ -24,6 +24,9 @@ describe('models/Note_CustomSortOrder', () => {
 		// (which normally is the creation timestamp). So if the user tries to move notes
 		// in the middle of other notes with order = 0, the order of all these notes is
 		// initialised at this point.
+		// Also check that the order-0 notes stay in their previous position when their
+		// order values are initialized. There was at one point a bug where they popped to
+		// the top.
 
 		const folder1 = await Folder.save({});
 		const folder2 = await Folder.save({});
@@ -32,6 +35,7 @@ describe('models/Note_CustomSortOrder', () => {
 		notes1.push(await Note.save({ order: 0, parent_id: folder1.id })); await time.msleep(2);
 		notes1.push(await Note.save({ order: 0, parent_id: folder1.id })); await time.msleep(2);
 		notes1.push(await Note.save({ order: 0, parent_id: folder1.id })); await time.msleep(2);
+		notes1.push(await Note.save({ order: 1000, parent_id: folder1.id })); await time.msleep(2);
 
 		const notes2 = [];
 		notes2.push(await Note.save({ parent_id: folder2.id })); await time.msleep(2);
@@ -45,12 +49,13 @@ describe('models/Note_CustomSortOrder', () => {
 			};
 		}
 
-		await Note.insertNotesAt(folder1.id, notes2.map(n => n.id), 1);
+		await Note.insertNotesAt(folder1.id, notes2.map(n => n.id), 2);
 
 		const newNotes1 = [
 			await Note.load(notes1[0].id),
 			await Note.load(notes1[1].id),
 			await Note.load(notes1[2].id),
+			await Note.load(notes1[3].id),
 		];
 
 		// Check that timestamps haven't changed - moving a note should not change the user timestamps
@@ -64,12 +69,13 @@ describe('models/Note_CustomSortOrder', () => {
 			order: Note.customOrderByColumns(),
 		});
 
-		expect(sortedNotes.length).toBe(5);
-		expect(sortedNotes[0].id).toBe(notes1[2].id);
-		expect(sortedNotes[1].id).toBe(notes2[0].id);
-		expect(sortedNotes[2].id).toBe(notes2[1].id);
-		expect(sortedNotes[3].id).toBe(notes1[1].id);
-		expect(sortedNotes[4].id).toBe(notes1[0].id);
+		expect(sortedNotes.length).toBe(6);
+		expect(sortedNotes[0].id).toBe(notes1[3].id);
+		expect(sortedNotes[1].id).toBe(notes1[2].id);
+		expect(sortedNotes[2].id).toBe(notes2[0].id);
+		expect(sortedNotes[3].id).toBe(notes2[1].id);
+		expect(sortedNotes[4].id).toBe(notes1[1].id);
+		expect(sortedNotes[5].id).toBe(notes1[0].id);
 	}));
 
 	it('should bump system but not user updated time when changing sort value', (async () => {