Skip to content

Commit

Permalink
Desktop: Fixes #4411: Count tags based on showCompletedTodos setting (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
JackGruber authored May 27, 2021
1 parent 907ac7c commit 2b28641
Show file tree
Hide file tree
Showing 5 changed files with 285 additions and 2 deletions.
54 changes: 54 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,60 @@ packages/app-cli/tests/HtmlToMd.js.map
packages/app-cli/tests/MdToHtml.d.ts
packages/app-cli/tests/MdToHtml.js
packages/app-cli/tests/MdToHtml.js.map
packages/app-cli/tests/Synchronizer.basics.d.ts
packages/app-cli/tests/Synchronizer.basics.js
packages/app-cli/tests/Synchronizer.basics.js.map
packages/app-cli/tests/Synchronizer.conflicts.d.ts
packages/app-cli/tests/Synchronizer.conflicts.js
packages/app-cli/tests/Synchronizer.conflicts.js.map
packages/app-cli/tests/Synchronizer.e2ee.d.ts
packages/app-cli/tests/Synchronizer.e2ee.js
packages/app-cli/tests/Synchronizer.e2ee.js.map
packages/app-cli/tests/Synchronizer.resources.d.ts
packages/app-cli/tests/Synchronizer.resources.js
packages/app-cli/tests/Synchronizer.resources.js.map
packages/app-cli/tests/Synchronizer.revisions.d.ts
packages/app-cli/tests/Synchronizer.revisions.js
packages/app-cli/tests/Synchronizer.revisions.js.map
packages/app-cli/tests/Synchronizer.sharing.d.ts
packages/app-cli/tests/Synchronizer.sharing.js
packages/app-cli/tests/Synchronizer.sharing.js.map
packages/app-cli/tests/Synchronizer.tags.d.ts
packages/app-cli/tests/Synchronizer.tags.js
packages/app-cli/tests/Synchronizer.tags.js.map
packages/app-cli/tests/Synchronizer.tools.d.ts
packages/app-cli/tests/Synchronizer.tools.js
packages/app-cli/tests/Synchronizer.tools.js.map
packages/app-cli/tests/dateTimeFormats.d.ts
packages/app-cli/tests/dateTimeFormats.js
packages/app-cli/tests/dateTimeFormats.js.map
packages/app-cli/tests/file-api-driver.d.ts
packages/app-cli/tests/file-api-driver.js
packages/app-cli/tests/file-api-driver.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/htmlUtils.d.ts
packages/app-cli/tests/htmlUtils.js
packages/app-cli/tests/htmlUtils.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_Folder.sharing.d.ts
packages/app-cli/tests/models_Folder.sharing.js
packages/app-cli/tests/models_Folder.sharing.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
packages/app-cli/tests/models_Tag.d.ts
packages/app-cli/tests/models_Tag.js
packages/app-cli/tests/models_Tag.js.map
packages/app-cli/tests/registry.d.ts
packages/app-cli/tests/registry.js
packages/app-cli/tests/registry.js.map
packages/app-cli/tests/MdToMd.d.ts
packages/app-cli/tests/MdToMd.js
packages/app-cli/tests/MdToMd.js.map
Expand Down
54 changes: 54 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,60 @@ packages/app-cli/tests/HtmlToMd.js.map
packages/app-cli/tests/MdToHtml.d.ts
packages/app-cli/tests/MdToHtml.js
packages/app-cli/tests/MdToHtml.js.map
packages/app-cli/tests/Synchronizer.basics.d.ts
packages/app-cli/tests/Synchronizer.basics.js
packages/app-cli/tests/Synchronizer.basics.js.map
packages/app-cli/tests/Synchronizer.conflicts.d.ts
packages/app-cli/tests/Synchronizer.conflicts.js
packages/app-cli/tests/Synchronizer.conflicts.js.map
packages/app-cli/tests/Synchronizer.e2ee.d.ts
packages/app-cli/tests/Synchronizer.e2ee.js
packages/app-cli/tests/Synchronizer.e2ee.js.map
packages/app-cli/tests/Synchronizer.resources.d.ts
packages/app-cli/tests/Synchronizer.resources.js
packages/app-cli/tests/Synchronizer.resources.js.map
packages/app-cli/tests/Synchronizer.revisions.d.ts
packages/app-cli/tests/Synchronizer.revisions.js
packages/app-cli/tests/Synchronizer.revisions.js.map
packages/app-cli/tests/Synchronizer.sharing.d.ts
packages/app-cli/tests/Synchronizer.sharing.js
packages/app-cli/tests/Synchronizer.sharing.js.map
packages/app-cli/tests/Synchronizer.tags.d.ts
packages/app-cli/tests/Synchronizer.tags.js
packages/app-cli/tests/Synchronizer.tags.js.map
packages/app-cli/tests/Synchronizer.tools.d.ts
packages/app-cli/tests/Synchronizer.tools.js
packages/app-cli/tests/Synchronizer.tools.js.map
packages/app-cli/tests/dateTimeFormats.d.ts
packages/app-cli/tests/dateTimeFormats.js
packages/app-cli/tests/dateTimeFormats.js.map
packages/app-cli/tests/file-api-driver.d.ts
packages/app-cli/tests/file-api-driver.js
packages/app-cli/tests/file-api-driver.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/htmlUtils.d.ts
packages/app-cli/tests/htmlUtils.js
packages/app-cli/tests/htmlUtils.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_Folder.sharing.d.ts
packages/app-cli/tests/models_Folder.sharing.js
packages/app-cli/tests/models_Folder.sharing.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
packages/app-cli/tests/models_Tag.d.ts
packages/app-cli/tests/models_Tag.js
packages/app-cli/tests/models_Tag.js.map
packages/app-cli/tests/registry.d.ts
packages/app-cli/tests/registry.js
packages/app-cli/tests/registry.js.map
packages/app-cli/tests/MdToMd.d.ts
packages/app-cli/tests/MdToMd.js
packages/app-cli/tests/MdToMd.js.map
Expand Down
159 changes: 159 additions & 0 deletions packages/app-cli/tests/models_Tag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { setupDatabaseAndSynchronizer, switchClient, checkThrowAsync } from './test-utils.js';
import Folder from '@joplin/lib/models/Folder';
import Note from '@joplin/lib/models/Note';
import Tag from '@joplin/lib/models/Tag';

describe('models_Tag', function() {

beforeEach(async (done) => {
await setupDatabaseAndSynchronizer(1);
await switchClient(1);
done();
});

it('should add tags by title', (async () => {
const folder1 = await Folder.save({ title: 'folder1' });
const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });

await Tag.setNoteTagsByTitles(note1.id, ['un', 'deux']);

const noteTags = await Tag.tagsByNoteId(note1.id);
expect(noteTags.length).toBe(2);
}));

it('should not allow renaming tag to existing tag names', (async () => {
const folder1 = await Folder.save({ title: 'folder1' });
const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });

await Tag.setNoteTagsByTitles(note1.id, ['un', 'deux']);

const tagUn = await Tag.loadByTitle('un');
const hasThrown = await checkThrowAsync(async () => await Tag.save({ id: tagUn.id, title: 'deux' }, { userSideValidation: true }));

expect(hasThrown).toBe(true);
}));

it('should not return tags without notes', (async () => {
const folder1 = await Folder.save({ title: 'folder1' });
const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
await Tag.setNoteTagsByTitles(note1.id, ['un']);

let tags = await Tag.allWithNotes();
expect(tags.length).toBe(1);

await Note.delete(note1.id);

tags = await Tag.allWithNotes();
expect(tags.length).toBe(0);
}));

it('should return tags with note counts', (async () => {
const folder1 = await Folder.save({ title: 'folder1' });
const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
const note2 = await Note.save({ title: 'ma 2nd note', parent_id: folder1.id });
const todo1 = await Note.save({ title: 'todo 1', parent_id: folder1.id, is_todo: 1, todo_completed: 1590085027710 });
await Tag.setNoteTagsByTitles(note1.id, ['un']);
await Tag.setNoteTagsByTitles(note2.id, ['un']);
await Tag.setNoteTagsByTitles(todo1.id, ['un']);

let tags = await Tag.allWithNotes();
expect(tags.length).toBe(1);
expect(tags[0].note_count).toBe(3);
expect(tags[0].todo_completed_count).toBe(1);

await Note.delete(todo1.id);

tags = await Tag.allWithNotes();
expect(tags.length).toBe(1);
expect(tags[0].note_count).toBe(2);

await Note.delete(note1.id);

tags = await Tag.allWithNotes();
expect(tags.length).toBe(1);
expect(tags[0].note_count).toBe(1);

await Note.delete(note2.id);

tags = await Tag.allWithNotes();
expect(tags.length).toBe(0);
}));

it('should load individual tags with note count', (async () => {
const folder1 = await Folder.save({ title: 'folder1' });
const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
const note2 = await Note.save({ title: 'ma 2nd note', parent_id: folder1.id });
const todo1 = await Note.save({ title: 'todo 2', parent_id: folder1.id, is_todo: 1, todo_completed: 1590085027710 });
const todo2 = await Note.save({ title: 'todo 2', parent_id: folder1.id, is_todo: 1 });
const tag = await Tag.save({ title: 'mytag' });
await Tag.addNote(tag.id, note1.id);

let tagWithCount = await Tag.loadWithCount(tag.id);
expect(tagWithCount.note_count).toBe(1);

await Tag.addNote(tag.id, note2.id);
tagWithCount = await Tag.loadWithCount(tag.id);
expect(tagWithCount.note_count).toBe(2);

await Tag.addNote(tag.id, todo1.id);
await Tag.addNote(tag.id, todo2.id);
tagWithCount = await Tag.loadWithCount(tag.id);
expect(tagWithCount.note_count).toBe(4);
expect(tagWithCount.todo_completed_count).toBe(1);
}));

it('should get common tags for set of notes', (async () => {
const folder1 = await Folder.save({ title: 'folder1' });
const taga = await Tag.save({ title: 'mytaga' });
const tagb = await Tag.save({ title: 'mytagb' });
const tagc = await Tag.save({ title: 'mytagc' });
await Tag.save({ title: 'mytagd' });

const note0 = await Note.save({ title: 'ma note 0', parent_id: folder1.id });
const note1 = await Note.save({ title: 'ma note 1', parent_id: folder1.id });
const note2 = await Note.save({ title: 'ma note 2', parent_id: folder1.id });
const note3 = await Note.save({ title: 'ma note 3', parent_id: folder1.id });

await Tag.addNote(taga.id, note1.id);

await Tag.addNote(taga.id, note2.id);
await Tag.addNote(tagb.id, note2.id);

await Tag.addNote(taga.id, note3.id);
await Tag.addNote(tagb.id, note3.id);
await Tag.addNote(tagc.id, note3.id);

let commonTags = await Tag.commonTagsByNoteIds(null);
expect(commonTags.length).toBe(0);

commonTags = await Tag.commonTagsByNoteIds(undefined);
expect(commonTags.length).toBe(0);

commonTags = await Tag.commonTagsByNoteIds([]);
expect(commonTags.length).toBe(0);

commonTags = await Tag.commonTagsByNoteIds([note0.id, note1.id, note2.id, note3.id]);
let commonTagIds = commonTags.map(t => t.id);
expect(commonTagIds.length).toBe(0);

commonTags = await Tag.commonTagsByNoteIds([note1.id, note2.id, note3.id]);
commonTagIds = commonTags.map(t => t.id);
expect(commonTagIds.length).toBe(1);
expect(commonTagIds.includes(taga.id)).toBe(true);

commonTags = await Tag.commonTagsByNoteIds([note2.id, note3.id]);
commonTagIds = commonTags.map(t => t.id);
expect(commonTagIds.length).toBe(2);
expect(commonTagIds.includes(taga.id)).toBe(true);
expect(commonTagIds.includes(tagb.id)).toBe(true);

commonTags = await Tag.commonTagsByNoteIds([note3.id]);

commonTagIds = commonTags.map(t => t.id);
expect(commonTags.length).toBe(3);
expect(commonTagIds.includes(taga.id)).toBe(true);
expect(commonTagIds.includes(tagb.id)).toBe(true);
expect(commonTagIds.includes(tagc.id)).toBe(true);
}));

});
6 changes: 5 additions & 1 deletion packages/app-desktop/gui/Sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,11 @@ class SidebarComponent extends React.Component<Props, State> {

renderTag(tag: any, selected: boolean) {
const anchorRef = this.anchorItemRef('tag', tag.id);
const noteCount = Setting.value('showNoteCounts') ? this.renderNoteCount(tag.note_count) : '';
let noteCount = null;
if (Setting.value('showNoteCounts')) {
if (Setting.value('showCompletedTodos')) noteCount = this.renderNoteCount(tag.note_count);
else noteCount = this.renderNoteCount(tag.note_count - tag.todo_completed_count);
}

return (
<StyledListItem selected={selected}
Expand Down
14 changes: 13 additions & 1 deletion packages/lib/JoplinDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ export default class JoplinDatabase extends Database {
// must be set in the synchronizer too.

// Note: v16 and v17 don't do anything. They were used to debug an issue.
const existingDatabaseVersions = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37];
const existingDatabaseVersions = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38];

let currentVersionIndex = existingDatabaseVersions.indexOf(fromVersion);

Expand Down Expand Up @@ -876,6 +876,18 @@ export default class JoplinDatabase extends Database {
queries.push('ALTER TABLE resources ADD COLUMN share_id TEXT NOT NULL DEFAULT ""');
}

if (targetVersion == 38) {
queries.push('DROP VIEW tags_with_note_count');
queries.push(`CREATE VIEW tags_with_note_count AS
SELECT tags.id as id, tags.title as title, tags.created_time as created_time, tags.updated_time as updated_time, COUNT(notes.id) as note_count,
SUM(CASE WHEN notes.todo_completed > 0 THEN 1 ELSE 0 END) AS todo_completed_count
FROM tags
LEFT JOIN note_tags nt on nt.tag_id = tags.id
LEFT JOIN notes on notes.id = nt.note_id
WHERE notes.id IS NOT NULL
GROUP BY tags.id`);
}

const updateVersionQuery = { sql: 'UPDATE version SET version = ?', params: [targetVersion] };

queries.push(updateVersionQuery);
Expand Down

0 comments on commit 2b28641

Please sign in to comment.