mirror of
https://github.com/laurent22/joplin.git
synced 2026-10-25 02:47:28 +00:00
Desktop, Mobile: Fixes #14647: Fix inconsistent note order upon note creation, when custom order is set (#14656)
This commit is contained in:
1 parent
d744db5063
commit
5fdd648954
7 files changed
+92
-9
No files matched your search
@@ -494,6 +494,7 @@ packages/app-desktop/gui/WindowCommandsAndDialogs/commands/index.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/linkToNote.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/moveToFolder.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newFolder.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newNote.test.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newNote.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newSubFolder.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newTodo.js
|
||||
|
||||
@@ -467,6 +467,7 @@ packages/app-desktop/gui/WindowCommandsAndDialogs/commands/index.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/linkToNote.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/moveToFolder.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newFolder.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newNote.test.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newNote.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newSubFolder.js
|
||||
packages/app-desktop/gui/WindowCommandsAndDialogs/commands/newTodo.js
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { runtime } from './newNote';
|
||||
import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils';
|
||||
import Note from '@joplin/lib/models/Note';
|
||||
import Folder from '@joplin/lib/models/Folder';
|
||||
import Setting from '@joplin/lib/models/Setting';
|
||||
|
||||
describe('newNote', () => {
|
||||
beforeEach(async () => {
|
||||
await setupDatabaseAndSynchronizer(1);
|
||||
await switchClient(1);
|
||||
});
|
||||
test.each([
|
||||
[null, null],
|
||||
['order', true],
|
||||
['order', false],
|
||||
])('should create a new note', async (sortOrderField: string, sortOrderReverse: boolean) => {
|
||||
// The command needs an active folder ID.
|
||||
const activeFolder = await Folder.save({ title: 'folder' });
|
||||
const initialNote = await Note.save({ title: 'test', parent_id: activeFolder.id });
|
||||
Setting.setValue('activeFolderId', activeFolder.id);
|
||||
Setting.setValue('notes.sortOrder.field', sortOrderField);
|
||||
Setting.setValue('notes.sortOrder.reverse', sortOrderReverse);
|
||||
|
||||
await runtime().execute(null, 'test note', true);
|
||||
|
||||
// Correct note should have been created
|
||||
const newNote = (await Note.loadByField('body', 'test note'));
|
||||
expect(newNote.body).toEqual('test note');
|
||||
expect(newNote.parent_id).toEqual(activeFolder.id);
|
||||
if (sortOrderField === 'order' && !!sortOrderReverse) {
|
||||
expect(newNote.order).toBeGreaterThanOrEqual(initialNote.order + Note.defaultIntevalBetweenNotes);
|
||||
} else if (sortOrderField === 'order' && !sortOrderReverse) {
|
||||
expect(newNote.order).toBeLessThanOrEqual(initialNote.order - Note.defaultIntevalBetweenNotes);
|
||||
} else {
|
||||
expect(newNote.order).toEqual(0);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { utils, CommandRuntime, CommandDeclaration, CommandContext } from '@jopl
|
||||
import { _ } from '@joplin/lib/locale';
|
||||
import Note from '@joplin/lib/models/Note';
|
||||
import Folder from '@joplin/lib/models/Folder';
|
||||
import Setting from '@joplin/lib/models/Setting';
|
||||
|
||||
export const newNoteEnabledConditions = 'oneFolderSelected && selectedFolderIsValid && !inConflictFolder && !folderIsReadOnly && !folderIsTrash';
|
||||
|
||||
@@ -19,9 +20,17 @@ export const runtime = (): CommandRuntime => {
|
||||
|
||||
const defaultValues = Note.previewFieldsWithDefaultValues({ includeTimestamps: false });
|
||||
|
||||
let newNote = { ...defaultValues, parent_id: folder.id,
|
||||
let order;
|
||||
if (Setting.value('notes.sortOrder.field') === 'order') {
|
||||
order = await Note.getNextOrderValue(folder.id);
|
||||
}
|
||||
|
||||
let newNote = {
|
||||
...defaultValues, parent_id: folder.id,
|
||||
is_todo: isTodo ? 1 : 0,
|
||||
body: body };
|
||||
body: body,
|
||||
...(order !== undefined ? { order } : {}),
|
||||
};
|
||||
|
||||
newNote = await Note.save(newNote, { provisional: true });
|
||||
|
||||
|
||||
@@ -10,20 +10,36 @@ describe('newNote', () => {
|
||||
await setupDatabaseAndSynchronizer(1);
|
||||
await switchClient(1);
|
||||
});
|
||||
test('should create and navigate to a new note', async () => {
|
||||
test.each([
|
||||
[null, null],
|
||||
['order', true],
|
||||
['order', false],
|
||||
])('should create and navigate to a new note', async (sortOrderField: string, sortOrderReverse: boolean) => {
|
||||
const dispatchMock = jest.fn();
|
||||
NavService.dispatch = dispatchMock;
|
||||
|
||||
// The command needs an active folder ID.
|
||||
const activeFolder = await Folder.save({ title: 'folder' });
|
||||
const initialNote = await Note.save({ title: 'test', parent_id: activeFolder.id });
|
||||
Setting.setValue('activeFolderId', activeFolder.id);
|
||||
Setting.setValue('notes.sortOrder.field', sortOrderField);
|
||||
Setting.setValue('notes.sortOrder.reverse', sortOrderReverse);
|
||||
|
||||
await runtime().execute(null, 'test note', true);
|
||||
expect(dispatchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Correct note should have been created
|
||||
const noteId = dispatchMock.mock.lastCall[0].noteId;
|
||||
expect(await Note.load(noteId)).toMatchObject({ body: 'test note', parent_id: activeFolder.id });
|
||||
const newNote = await Note.load(noteId);
|
||||
expect(newNote.body).toEqual('test note');
|
||||
expect(newNote.parent_id).toEqual(activeFolder.id);
|
||||
if (sortOrderField === 'order' && !!sortOrderReverse) {
|
||||
expect(newNote.order).toBeGreaterThanOrEqual(initialNote.order + Note.defaultIntevalBetweenNotes);
|
||||
} else if (sortOrderField === 'order' && !sortOrderReverse) {
|
||||
expect(newNote.order).toBeLessThanOrEqual(initialNote.order - Note.defaultIntevalBetweenNotes);
|
||||
} else {
|
||||
expect(newNote.order).toBeLessThan(initialNote.order + Note.defaultIntevalBetweenNotes);
|
||||
}
|
||||
|
||||
// Should have tried to navigate to the note.
|
||||
expect(dispatchMock.mock.lastCall).toMatchObject([
|
||||
|
||||
@@ -3,6 +3,7 @@ import Logger from '@joplin/utils/Logger';
|
||||
import goToNote, { GotoNoteOptions } from './util/goToNote';
|
||||
import Note from '@joplin/lib/models/Note';
|
||||
import Folder from '@joplin/lib/models/Folder';
|
||||
import Setting from '@joplin/lib/models/Setting';
|
||||
|
||||
const logger = Logger.create('newNoteCommand');
|
||||
|
||||
@@ -19,10 +20,16 @@ export const runtime = (): CommandRuntime => {
|
||||
return;
|
||||
}
|
||||
|
||||
let order;
|
||||
if (Setting.value('notes.sortOrder.field') === 'order') {
|
||||
order = await Note.getNextOrderValue(folder.id);
|
||||
}
|
||||
|
||||
const note = await Note.save({
|
||||
body,
|
||||
parent_id: folder.id,
|
||||
is_todo: todo ? 1 : 0,
|
||||
...(order !== undefined ? { order } : {}),
|
||||
}, { provisional: true });
|
||||
|
||||
logger.info(`Navigating to note ${note.id}`);
|
||||
|
||||
@@ -46,6 +46,7 @@ export interface PreviewsOptions {
|
||||
|
||||
export default class Note extends BaseItem {
|
||||
|
||||
public static defaultIntevalBetweenNotes = 60 * 60 * 1000;
|
||||
public static updateGeolocationEnabled_ = true;
|
||||
private static geolocationUpdating_ = false;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
|
||||
@@ -1125,19 +1126,18 @@ export default class Note extends BaseItem {
|
||||
// and the increment between the order values of each inserted notes.
|
||||
let newOrder = 0;
|
||||
let intervalBetweenNotes = 0;
|
||||
const defaultIntevalBetweenNotes = 60 * 60 * 1000;
|
||||
|
||||
if (!relevantExistingNoteCount) { // If there's no (relevant) notes in the target notebook
|
||||
newOrder = Date.now();
|
||||
intervalBetweenNotes = defaultIntevalBetweenNotes;
|
||||
intervalBetweenNotes = this.defaultIntevalBetweenNotes;
|
||||
} else if (index > lastRelevantNoteIndex) { // Insert at the end (of relevant group)
|
||||
intervalBetweenNotes = notes[lastRelevantNoteIndex].order / (noteIds.length + 1);
|
||||
newOrder = notes[lastRelevantNoteIndex].order - intervalBetweenNotes;
|
||||
} else if (index <= firstRelevantNoteIndex) { // Insert at the beginning (of relevant group)
|
||||
const firstNoteOrder = notes[firstRelevantNoteIndex].order;
|
||||
if (firstNoteOrder >= Date.now()) {
|
||||
intervalBetweenNotes = defaultIntevalBetweenNotes;
|
||||
newOrder = firstNoteOrder + defaultIntevalBetweenNotes;
|
||||
intervalBetweenNotes = this.defaultIntevalBetweenNotes;
|
||||
newOrder = firstNoteOrder + this.defaultIntevalBetweenNotes;
|
||||
} else {
|
||||
intervalBetweenNotes = (Date.now() - firstNoteOrder) / (noteIds.length + 1);
|
||||
newOrder = firstNoteOrder + intervalBetweenNotes * noteIds.length;
|
||||
@@ -1151,7 +1151,7 @@ export default class Note extends BaseItem {
|
||||
for (let i = index; i >= 0; i--) {
|
||||
const n = notes[i];
|
||||
if (n.order <= previousOrder) {
|
||||
const o = previousOrder + defaultIntevalBetweenNotes;
|
||||
const o = previousOrder + this.defaultIntevalBetweenNotes;
|
||||
const updatedNote = await this.updateNoteOrder_(n, o);
|
||||
notes[i] = { ...n, ...updatedNote };
|
||||
previousOrder = o;
|
||||
@@ -1203,4 +1203,15 @@ export default class Note extends BaseItem {
|
||||
conflictNote.conflict_original_id = sourceNote.id;
|
||||
return await Note.save(conflictNote, { autoTimestamp: false, changeSource: changeSource });
|
||||
}
|
||||
|
||||
public static async getNextOrderValue(folderId: string) {
|
||||
const reverse = Setting.value('notes.sortOrder.reverse');
|
||||
if (reverse) {
|
||||
const folder = await this.modelSelectOne('SELECT MAX(`order`) as `order` FROM notes WHERE parent_id = ?', [folderId]);
|
||||
return Number(folder.order ?? 0) + this.defaultIntevalBetweenNotes;
|
||||
} else {
|
||||
const folder = await this.modelSelectOne('SELECT MIN(`order`) as `order` FROM notes WHERE parent_id = ?', [folderId]);
|
||||
return Number(folder.order ?? 0) - this.defaultIntevalBetweenNotes;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user