From 7f88f507df99b29c5b695647e603ae3394ab4803 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 14 Nov 2020 12:37:18 +0000 Subject: [PATCH] All: Fixes #3997: Display note count for conflict folder, and display notes even if they are completed to-dos --- .eslintignore | 6 ++ .gitignore | 6 ++ .../{models_Folder.js => models_Folder.ts} | 67 ++++++++++--------- .../tests/{models_Note.js => models_Note.ts} | 17 ++--- packages/lib/models/Folder.js | 21 ++++-- packages/lib/models/Note.js | 12 +++- packages/lib/shim.ts | 4 +- 7 files changed, 81 insertions(+), 52 deletions(-) rename packages/app-cli/tests/{models_Folder.js => models_Folder.ts} (79%) rename packages/app-cli/tests/{models_Note.js => models_Note.ts} (94%) diff --git a/.eslintignore b/.eslintignore index 90dae9f86a..eaf7be4866 100644 --- a/.eslintignore +++ b/.eslintignore @@ -166,6 +166,12 @@ packages/app-cli/tests/MdToHtml.js.map packages/app-cli/tests/fsDriver.d.ts packages/app-cli/tests/fsDriver.js packages/app-cli/tests/fsDriver.js.map +packages/app-cli/tests/models_Folder.d.ts +packages/app-cli/tests/models_Folder.js +packages/app-cli/tests/models_Folder.js.map +packages/app-cli/tests/models_Note.d.ts +packages/app-cli/tests/models_Note.js +packages/app-cli/tests/models_Note.js.map packages/app-cli/tests/models_Setting.d.ts packages/app-cli/tests/models_Setting.js packages/app-cli/tests/models_Setting.js.map diff --git a/.gitignore b/.gitignore index 51435a597e..6921593bc7 100644 --- a/.gitignore +++ b/.gitignore @@ -158,6 +158,12 @@ packages/app-cli/tests/MdToHtml.js.map packages/app-cli/tests/fsDriver.d.ts packages/app-cli/tests/fsDriver.js packages/app-cli/tests/fsDriver.js.map +packages/app-cli/tests/models_Folder.d.ts +packages/app-cli/tests/models_Folder.js +packages/app-cli/tests/models_Folder.js.map +packages/app-cli/tests/models_Note.d.ts +packages/app-cli/tests/models_Note.js +packages/app-cli/tests/models_Note.js.map packages/app-cli/tests/models_Setting.d.ts packages/app-cli/tests/models_Setting.js packages/app-cli/tests/models_Setting.js.map diff --git a/packages/app-cli/tests/models_Folder.js b/packages/app-cli/tests/models_Folder.ts similarity index 79% rename from packages/app-cli/tests/models_Folder.js rename to packages/app-cli/tests/models_Folder.ts index 1e7f31e0d3..0a81d8b4d0 100644 --- a/packages/app-cli/tests/models_Folder.js +++ b/packages/app-cli/tests/models_Folder.ts @@ -1,12 +1,7 @@ -/* eslint-disable no-unused-vars */ - - -const time = require('@joplin/lib/time').default; -const { createNTestNotes, asyncTest, fileContentEqual, setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('./test-utils.js'); +import { FolderEntity } from '@joplin/lib/services/database/types'; +const { createNTestNotes, asyncTest, setupDatabaseAndSynchronizer, sleep, switchClient, checkThrowAsync } = require('./test-utils.js'); const Folder = require('@joplin/lib/models/Folder.js'); const Note = require('@joplin/lib/models/Note.js'); -const BaseModel = require('@joplin/lib/BaseModel').default; -const shim = require('@joplin/lib/shim').default; process.on('unhandledRejection', (reason, p) => { console.log('Unhandled Rejection at models_Folder: Promise', p, 'reason:', reason); @@ -74,7 +69,7 @@ describe('models_Folder', function() { expect(folders[1].id).toBe(f3.id); expect(folders[2].id).toBe(f1.id); - const n2 = await Note.save({ title: 'note1', parent_id: f1.id }); + await Note.save({ title: 'note1', parent_id: f1.id }); folders = await Folder.orderByLastModified(await Folder.all(), 'desc'); expect(folders[0].id).toBe(f1.id); @@ -108,7 +103,7 @@ describe('models_Folder', function() { expect(folders[1].id).toBe(f3.id); expect(folders[2].id).toBe(f2.id); - const n2 = await Note.save({ title: 'note2', parent_id: f2.id }); + await Note.save({ title: 'note2', parent_id: f2.id }); folders = await Folder.orderByLastModified(await Folder.all(), 'desc'); expect(folders[0].id).toBe(f2.id); @@ -123,7 +118,7 @@ describe('models_Folder', function() { expect(folders[2].id).toBe(f2.id); const f4 = await Folder.save({ title: 'folder4', parent_id: f1.id }); await sleep(0.1); - const n3 = await Note.save({ title: 'note3', parent_id: f4.id }); + await Note.save({ title: 'note3', parent_id: f4.id }); folders = await Folder.orderByLastModified(await Folder.all(), 'desc'); expect(folders.length).toBe(4); @@ -139,21 +134,33 @@ describe('models_Folder', function() { const f3 = await Folder.save({ title: 'folder3', parent_id: f2.id }); const f4 = await Folder.save({ title: 'folder4' }); - const n1 = await Note.save({ title: 'note1', parent_id: f3.id }); - const n2 = await Note.save({ title: 'note1', parent_id: f3.id }); - const n3 = await Note.save({ title: 'note1', parent_id: f1.id }); + await Note.save({ title: 'note1', parent_id: f3.id }); + await Note.save({ title: 'note1', parent_id: f3.id }); + await Note.save({ title: 'note1', parent_id: f1.id }); + await Note.save({ title: 'conflicted', parent_id: f1.id, is_conflict: 1 }); - const folders = await Folder.all(); - await Folder.addNoteCounts(folders); + { + const folders = await Folder.all({ includeConflictFolder: false }); + await Folder.addNoteCounts(folders); + const foldersById: any = {}; + folders.forEach((f: FolderEntity) => { foldersById[f.id] = f; }); - const foldersById = {}; - folders.forEach((f) => { foldersById[f.id] = f; }); + expect(folders.length).toBe(4); + expect(foldersById[f1.id].note_count).toBe(3); + expect(foldersById[f2.id].note_count).toBe(2); + expect(foldersById[f3.id].note_count).toBe(2); + expect(foldersById[f4.id].note_count).toBe(0); + } - expect(folders.length).toBe(4); - expect(foldersById[f1.id].note_count).toBe(3); - expect(foldersById[f2.id].note_count).toBe(2); - expect(foldersById[f3.id].note_count).toBe(2); - expect(foldersById[f4.id].note_count).toBe(0); + { + const folders = await Folder.all({ includeConflictFolder: true }); + await Folder.addNoteCounts(folders); + const foldersById: any = {}; + folders.forEach((f: FolderEntity) => { foldersById[f.id] = f; }); + + expect(folders.length).toBe(5); + expect(foldersById[Folder.conflictFolderId()].note_count).toBe(1); + } })); it('should not count completed to-dos', asyncTest(async () => { @@ -163,18 +170,18 @@ describe('models_Folder', function() { const f3 = await Folder.save({ title: 'folder3', parent_id: f2.id }); const f4 = await Folder.save({ title: 'folder4' }); - const n1 = await Note.save({ title: 'note1', parent_id: f3.id }); - const n2 = await Note.save({ title: 'note2', parent_id: f3.id }); - const n3 = await Note.save({ title: 'note3', parent_id: f1.id }); - const n4 = await Note.save({ title: 'note4', parent_id: f3.id, is_todo: true, todo_completed: 0 }); - const n5 = await Note.save({ title: 'note5', parent_id: f3.id, is_todo: true, todo_completed: 999 }); - const n6 = await Note.save({ title: 'note6', parent_id: f3.id, is_todo: true, todo_completed: 999 }); + await Note.save({ title: 'note1', parent_id: f3.id }); + await Note.save({ title: 'note2', parent_id: f3.id }); + await Note.save({ title: 'note3', parent_id: f1.id }); + await Note.save({ title: 'note4', parent_id: f3.id, is_todo: true, todo_completed: 0 }); + await Note.save({ title: 'note5', parent_id: f3.id, is_todo: true, todo_completed: 999 }); + await Note.save({ title: 'note6', parent_id: f3.id, is_todo: true, todo_completed: 999 }); const folders = await Folder.all(); await Folder.addNoteCounts(folders, false); - const foldersById = {}; - folders.forEach((f) => { foldersById[f.id] = f; }); + const foldersById: any = {}; + folders.forEach((f: FolderEntity) => { foldersById[f.id] = f; }); expect(folders.length).toBe(4); expect(foldersById[f1.id].note_count).toBe(4); diff --git a/packages/app-cli/tests/models_Note.js b/packages/app-cli/tests/models_Note.ts similarity index 94% rename from packages/app-cli/tests/models_Note.js rename to packages/app-cli/tests/models_Note.ts index 5d8c9745ae..429c185913 100644 --- a/packages/app-cli/tests/models_Note.js +++ b/packages/app-cli/tests/models_Note.ts @@ -1,14 +1,10 @@ -/* eslint-disable no-unused-vars */ - - -const time = require('@joplin/lib/time').default; -const { sortedIds, createNTestNotes, asyncTest, fileContentEqual, setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('./test-utils.js'); +import Setting from '@joplin/lib/models/Setting'; +import BaseModel from '@joplin/lib/BaseModel'; +import shim from '@joplin/lib/shim'; +const { sortedIds, createNTestNotes, asyncTest, setupDatabaseAndSynchronizer, switchClient, checkThrowAsync } = require('./test-utils.js'); const Folder = require('@joplin/lib/models/Folder.js'); const Note = require('@joplin/lib/models/Note.js'); -const Setting = require('@joplin/lib/models/Setting').default; -const BaseModel = require('@joplin/lib/BaseModel').default; const ArrayUtils = require('@joplin/lib/ArrayUtils.js'); -const shim = require('@joplin/lib/shim').default; process.on('unhandledRejection', (reason, p) => { console.log('Unhandled Rejection at models_Note: Promise', p, 'reason:', reason); @@ -108,9 +104,7 @@ describe('models_Note', function() { for (let i = 0; i < testCases.length; i++) { const t = testCases[i]; - const input = t[0]; - const expectedTitle = t[1]; - const expectedBody = t[1]; + const input: any = t[0]; const note1 = await Note.save(input); const serialized = await Note.serialize(note1); @@ -226,7 +220,6 @@ describe('models_Note', function() { const note1 = await Note.save({ title: 'note1' }); const t1 = r1.updated_time; const t2 = r2.updated_time; - const t3 = r3.updated_time; const testCases = [ [ diff --git a/packages/lib/models/Folder.js b/packages/lib/models/Folder.js index 2b95b99497..61d4095288 100644 --- a/packages/lib/models/Folder.js +++ b/packages/lib/models/Folder.js @@ -107,16 +107,25 @@ class Folder extends BaseItem { // Note: this only calculates the overall number of nodes for this folder and all its descendants static async addNoteCounts(folders, includeCompletedTodos = true) { const foldersById = {}; - folders.forEach((f) => { + for (const f of folders) { foldersById[f.id] = f; - f.note_count = 0; - }); - const where = !includeCompletedTodos ? 'WHERE (notes.is_todo = 0 OR notes.todo_completed = 0)' : ''; + if (this.conflictFolderId() === f.id) { + f.note_count = await Note.conflictedCount(); + } else { + f.note_count = 0; + } + } - const sql = `SELECT folders.id as folder_id, count(notes.parent_id) as note_count + const where = ['is_conflict = 0']; + if (!includeCompletedTodos) where.push('(notes.is_todo = 0 OR notes.todo_completed = 0)'); + + const sql = ` + SELECT folders.id as folder_id, count(notes.parent_id) as note_count FROM folders LEFT JOIN notes ON notes.parent_id = folders.id - ${where} GROUP BY folders.id`; + WHERE ${where.join(' AND ')} + GROUP BY folders.id + `; const noteCounts = await this.db().selectAll(sql); noteCounts.forEach((noteCount) => { diff --git a/packages/lib/models/Note.js b/packages/lib/models/Note.js index 1c687f40e3..847b680ce2 100644 --- a/packages/lib/models/Note.js +++ b/packages/lib/models/Note.js @@ -306,8 +306,8 @@ class Note extends BaseItem { } static async previews(parentId, options = null) { - // Note: ordering logic must be duplicated in sortNotes(), which - // is used to sort already loaded notes. + // Note: ordering logic must be duplicated in sortNotes(), which is used + // to sort already loaded notes. if (!options) options = {}; if (!('order' in options)) options.order = [{ by: 'user_updated_time', dir: 'DESC' }, { by: 'user_created_time', dir: 'DESC' }, { by: 'title', dir: 'DESC' }, { by: 'id', dir: 'DESC' }]; @@ -317,7 +317,13 @@ class Note extends BaseItem { if (!options.uncompletedTodosOnTop) options.uncompletedTodosOnTop = false; if (!('showCompletedTodos' in options)) options.showCompletedTodos = true; - if (parentId == BaseItem.getClass('Folder').conflictFolderId()) { + const Folder = BaseItem.getClass('Folder'); + + // Conflicts are always displayed regardless of options, since otherwise + // it's confusing to have conflicts but with an empty conflict folder. + if (parentId === Folder.conflictFolderId()) options.showCompletedTodos = true; + + if (parentId == Folder.conflictFolderId()) { options.conditions.push('is_conflict = 1'); } else { options.conditions.push('is_conflict = 0'); diff --git a/packages/lib/shim.ts b/packages/lib/shim.ts index 5cd4b77ee6..f8163b7564 100644 --- a/packages/lib/shim.ts +++ b/packages/lib/shim.ts @@ -1,3 +1,5 @@ +import { ResourceEntity } from './services/database/types'; + let isTestingEnv_ = false; let react_: any = null; @@ -148,7 +150,7 @@ const shim = { throw new Error('Not implemented'); }, - createResourceFromPath: async (_filePath: string, _defaultProps: any = null, _options: any = null) => { + createResourceFromPath: async (_filePath: string, _defaultProps: any = null, _options: any = null): Promise => { throw new Error('Not implemented'); },