From 628e6bd6f6586798d1e585e49aaace73cf4cb775 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Mon, 17 Aug 2026 10:52:31 -0700 Subject: [PATCH] Desktop: Fixes #16167: Fix conflict created when syncing a just-published note (#16182) --- .gitignore | 1 + .ignore.eslint | 1 + packages/lib/Synchronizer.ts | 11 ++ packages/lib/models/Folder.publishing.test.ts | 104 ++++++++++++++++++ packages/lib/models/Folder.ts | 38 ++----- packages/lib/models/Note.ts | 33 ++++++ 6 files changed, 160 insertions(+), 28 deletions(-) create mode 100644 packages/lib/models/Folder.publishing.test.ts diff --git a/.gitignore b/.gitignore index d3833db577..89030ff91c 100644 --- a/.gitignore +++ b/.gitignore @@ -1478,6 +1478,7 @@ packages/lib/models/Alarm.js packages/lib/models/BaseItem.test.js packages/lib/models/BaseItem.js packages/lib/models/ConflictNoteState.js +packages/lib/models/Folder.publishing.test.js packages/lib/models/Folder.sharing.test.js packages/lib/models/Folder.test.js packages/lib/models/Folder.js diff --git a/.ignore.eslint b/.ignore.eslint index ba3064d0ce..28ab0ed10e 100644 --- a/.ignore.eslint +++ b/.ignore.eslint @@ -1503,6 +1503,7 @@ packages/lib/models/Alarm.js packages/lib/models/BaseItem.test.js packages/lib/models/BaseItem.js packages/lib/models/ConflictNoteState.js +packages/lib/models/Folder.publishing.test.js packages/lib/models/Folder.sharing.test.js packages/lib/models/Folder.test.js packages/lib/models/Folder.js diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index 03eac85d15..7bea0dd83b 100644 --- a/packages/lib/Synchronizer.ts +++ b/packages/lib/Synchronizer.ts @@ -1248,6 +1248,17 @@ export default class Synchronizer { } } + try { + // Update published/unpublished status after the main sync to avoid conflicts. + // See https://github.com/laurent22/joplin/issues/16167. + if (!hasCaughtError && !this.cancelling()) { + await Note.updatePublishedNotes(this.shareService_ ? this.shareService_.shares : []); + } + } catch (error) { + logger.error('Failed to save note publication status', error); + this.progressReport_.errors.push(error); + } + if (syncLock) { this.lockHandler().stopAutoLockRefresh(syncLock); await this.lockHandler().releaseLock(LockType.Sync, this.lockClientType(), this.clientId_); diff --git a/packages/lib/models/Folder.publishing.test.ts b/packages/lib/models/Folder.publishing.test.ts new file mode 100644 index 0000000000..4856de8ba2 --- /dev/null +++ b/packages/lib/models/Folder.publishing.test.ts @@ -0,0 +1,104 @@ +import { setupDatabaseAndSynchronizer, switchClient, resourceService, createFolderTree } from '../testing/test-utils'; +import Folder from '../models/Folder'; +import { ShareType, StateShare } from '../services/share/reducer'; +import Note from './Note'; + +const publishedFolderShareState = (folderId: string): StateShare => ({ + id: `share-${folderId}`, + type: ShareType.PublishedFolder, + folder_id: folderId, + note_id: '', + master_key_id: '', +}); + +type ItemSlice = { title: string }; + +const expectPublished = async (items: ItemSlice[], published = true) => { + for (let item of items) { + item = (await Folder.loadByTitle(item.title)) ?? await Note.loadByTitle(item.title); + expect(item).toMatchObject({ + title: item.title, + is_shared: published ? 1 : 0, + }); + } +}; + +const expectUnpublished = (items: ItemSlice[]) => expectPublished(items, false); + +describe('models/Folder.publishing', () => { + + beforeEach(async () => { + await setupDatabaseAndSynchronizer(1); + await switchClient(1); + }); + + it('should set is_shared=1 on descendants of a published folder', async () => { + const root = await createFolderTree('', [ + { + title: 'root', + children: [ + { + title: 'sub-folder 1', + children: [], + }, + { + title: 'sub-folder 2', + children: [ + { title: 'sub-sub-folder', children: [] }, + { title: 'published note 1' }, + { title: 'published note 2' }, + ], + }, + { title: 'published note 3' }, + ], + }, + { + title: 'unpublished folder', + children: [ + { title: 'unpublished note' }, + ], + }, + ]); + + const shareState: StateShare[] = [ + publishedFolderShareState(root.id), + ]; + + await Folder.updateAllShareIds( + resourceService(), + shareState, + ); + + // After the first round, only folders should have is_shared=1 + await expectPublished([ + 'root', + 'sub-folder 1', + 'sub-folder 2', + 'sub-sub-folder', + ].map(title => ({ title }))); + await expectUnpublished([ + 'published note 1', + 'published note 2', + 'published note 3', + 'unpublished folder', + 'unpublished note', + ].map(title => ({ title }))); + + // Should update published notes when calling Note.updateNotePublicationStatus + await Note.updatePublishedNotes(shareState); + + await expectPublished([ + 'root', + 'sub-folder 1', + 'sub-folder 2', + 'sub-sub-folder', + 'published note 1', + 'published note 2', + 'published note 3', + ].map(title => ({ title }))); + await expectUnpublished([ + 'unpublished folder', + 'unpublished note', + ].map(title => ({ title }))); + }); +}); diff --git a/packages/lib/models/Folder.ts b/packages/lib/models/Folder.ts index b83953372b..929c3d24d5 100644 --- a/packages/lib/models/Folder.ts +++ b/packages/lib/models/Folder.ts @@ -781,19 +781,24 @@ export default class Folder extends BaseItem { public static async updateAllShareIds(resourceService: ResourceService, activeShares: StateShare[]) { await this.updateFolderShareIds(activeShares); await this.updateNoteShareIds(); + await this.updateResourceShareIds(resourceService); + // Don't update note publication status here: Doing so can cause conflicts if updateAllShareIds + // is called just before sync + await this.updateFolderPublishStatus_(activeShares); + } + + private static async updateFolderPublishStatus_(activeShares: StateShare[]) { const publishedFolderRootIds = activeShares .filter(share => share.type === ShareType.PublishedFolder && !!share.folder_id) .map(share => share.folder_id); - const directlyPublishedNoteIds = activeShares - .filter(share => share.type === ShareType.Note && !!share.note_id) - .map(share => share.note_id); const publishedFolderIds = unique(publishedFolderRootIds.concat( - ...(await Promise.all(publishedFolderRootIds.map(id => this.allChildrenFolders(id)))).map(folders => folders.map(f => f.id)), + (await Promise.all( + publishedFolderRootIds.map(id => this.allChildrenFolders(id)), + )).flatMap(folders => folders.map(f => f.id)), )); const publishedFolderIdSet = new Set(publishedFolderIds); - const directlyPublishedNoteIdSet = new Set(directlyPublishedNoteIds); if (publishedFolderIds.length) { for (const folder of await this.all({ fields: ['id', 'is_shared'] })) { @@ -801,29 +806,6 @@ export default class Folder extends BaseItem { await this.updateShareStatus({ ...folder, type_: BaseModel.TYPE_FOLDER }, true); } } - - const noteIsSharedSql = [ - directlyPublishedNoteIds.length ? `id IN (${this.escapeIdsForSql(directlyPublishedNoteIds)})` : '', - publishedFolderIds.length ? `parent_id IN (${this.escapeIdsForSql(publishedFolderIds)})` : '', - ].filter(v => !!v).join(' OR '); - - let notesToUpdate: NoteEntity[] = []; - if (noteIsSharedSql) { - notesToUpdate = await this.db().selectAll(` - SELECT id, parent_id, is_shared - FROM notes - WHERE is_shared = 0 AND (${noteIsSharedSql}) - `); - } - - for (const note of notesToUpdate) { - await this.updateShareStatus( - { ...note, type_: BaseModel.TYPE_NOTE }, - directlyPublishedNoteIdSet.has(note.id) || publishedFolderIdSet.has(note.parent_id), - ); - } - - await this.updateResourceShareIds(resourceService); } // Clear the "share_id" property for the items that are associated with a diff --git a/packages/lib/models/Note.ts b/packages/lib/models/Note.ts index 3fada4368b..1fa69d118b 100644 --- a/packages/lib/models/Note.ts +++ b/packages/lib/models/Note.ts @@ -29,6 +29,7 @@ import { ALL_NOTES_FILTER_ID } from '../reserved-ids'; import NoteLockNote from '../services/noteLock/NoteLockNote'; import isNoteLockEnabled from '../services/noteLock/isNoteLockEnabled'; import isItemId from './utils/isItemId'; +import { ShareType, StateShare } from '../services/share/reducer'; export interface PreviewsOrder { by: string; @@ -599,6 +600,38 @@ export default class Note extends BaseItem { return this.modelSelectAll('SELECT * FROM notes WHERE is_conflict = 0'); } + public static async updatePublishedNotes(activeShares: StateShare[]) { + const directlyPublishedNoteIds = activeShares + .filter(share => share.type === ShareType.Note && !!share.note_id) + .map(share => share.note_id); + + const loadUnpublishedWithDirectShare = async (): Promise => { + if (directlyPublishedNoteIds.length === 0) return []; + + return await this.db().selectAll(` + SELECT id, parent_id, is_shared + FROM notes + WHERE is_shared = 0 AND id IN (${this.escapeIdsForSql(directlyPublishedNoteIds)}) + `); + }; + const unpublishedNotesInPublishedFolders: NoteEntity[] = await this.db().selectAll(` + SELECT notes.id, notes.parent_id, notes.is_shared + FROM notes + JOIN folders ON notes.parent_id = folders.id + WHERE notes.is_shared = 0 AND folders.is_shared = 1 + AND notes.is_conflict = 0 + AND notes.deleted_time = 0 + `); + + const notesToPublish = unpublishedNotesInPublishedFolders.concat(await loadUnpublishedWithDirectShare()); + for (const note of notesToPublish) { + await this.updateShareStatus( + { ...note, type_: BaseModel.TYPE_NOTE }, + true, + ); + } + } + public static async updateGeolocation(noteId: string): Promise { if (!Setting.value('trackLocation')) return null; if (!Note.updateGeolocationEnabled_) return null;