Desktop, Mobile, Cli: Fix sync eligible conflicts not being synced, if the conflict is for a note in a share (#16726)

Co-authored-by: Laurent Cozic <laurent22@users.noreply.github.com>

packages/lib/models/Folder.sharing.test.ts

@@ -7,6 +7,7 @@ import Resource from '../models/Resource';
 import { FolderEntity, NoteEntity, ResourceEntity } from '../services/database/types';
 import ResourceService from '../services/ResourceService';
 import { StateShare } from '../services/share/reducer';
+import ItemChange from './ItemChange';
 
 const testImagePath = `${supportDir}/photo.jpg`;
 
@@ -333,6 +334,22 @@ describe('models/Folder.sharing', () => {
 		expect(note4.share_id).toBe('');
 	}));
 
+	it('should clear the share ID from conflict notes', async () => {
+		const folder = await Folder.save({ title: 'Shared folder', share_id: 'abcd1234' });
+		const note = await Note.save({ title: 'Note', parent_id: folder.id, share_id: folder.share_id });
+		const newConflictNote = await Note.createConflictNote(note, ItemChange.SOURCE_SYNC);
+		const existingConflictNote = await Note.createConflictNote(note, ItemChange.SOURCE_SYNC);
+		await Note.save({ id: existingConflictNote.id, share_id: folder.share_id });
+
+		expect(newConflictNote.share_id).toBe('');
+		expect((await Note.load(existingConflictNote.id)).share_id).toBe(folder.share_id);
+
+		await Folder.updateAllShareIds(resourceService(), []);
+
+		expect((await Note.load(newConflictNote.id)).share_id).toBe('');
+		expect((await Note.load(existingConflictNote.id)).share_id).toBe('');
+	});
+
 	it('should remove the share ID when a note is moved in or out of shared folder', (async () => {
 		const folder1 = await createFolderTree('', [
 			{

packages/lib/models/Folder.ts

@@ -581,13 +581,17 @@ export default class Folder extends BaseItem {
 	}
 
 	public static async updateNoteShareIds() {
-		// Find all the notes where the share_id is not the same as the
-		// parent share_id because we only need to update those.
+		// Find notes whose share_id differs from the desired value. Conflict
+		// notes must remain outside the share so they can sync only between the
+		// current user's devices.
 		const rows = await this.db().selectAll(`
-			SELECT notes.id, folders.share_id, notes.parent_id
+			SELECT
+				notes.id,
+				CASE WHEN notes.is_conflict = 1 THEN '' ELSE folders.share_id END AS share_id,
+				notes.parent_id
 			FROM notes
 			LEFT JOIN folders ON notes.parent_id = folders.id
-			WHERE notes.share_id != folders.share_id
+			WHERE notes.share_id != CASE WHEN notes.is_conflict = 1 THEN '' ELSE folders.share_id END
 		`);
 
 		logger.debug('updateNoteShareIds: notes to update:', rows.length);

packages/lib/services/ReportService.test.ts

@@ -120,17 +120,25 @@ describe('ReportService', () => {
 			is_conflict: 1,
 			deleted_time: Date.now(),
 		});
-		await Note.save({
-			title: 'Shared conflict',
+		const conflictWithStaleShareId = await Note.save({
+			title: 'Conflict with stale share ID',
 			is_conflict: 1,
 			conflict_original_id: originalNote.id,
 			share_id: 'share-id',
 		});
 
+		// While the conflict has a share ID, it is excluded from sync. This prevents
+		// it from being sent to share recipients or rejected by a read-only share.
+		const statusBeforeShareIdUpdate = await new ReportService().syncStatus(syncTargetId());
+		expect(statusBeforeShareIdUpdate.items.Note).toEqual({ total: 2, synced: 0 });
+
 		await synchronizerStart();
 
+		// Synchronisation clears the stale share ID, keeping the conflict private to
+		// the current account while allowing it to sync between that account's devices.
+		expect((await Note.load(conflictWithStaleShareId.id)).share_id).toBe('');
 		const status = await new ReportService().syncStatus(syncTargetId());
-		expect(status.items.Note).toEqual({ total: 2, synced: 2 });
+		expect(status.items.Note).toEqual({ total: 3, synced: 3 });
 		expect(status.conflicted.total).toBe(3);
 		expect(status.total.synced).toBe(status.total.total);
 	});