obsidian-tasks icon indicating copy to clipboard operation
obsidian-tasks copied to clipboard

Data in `Task` and `TasksDate` are accidentally mutable - dates can be edited unintentionally

Open claremacrae opened this issue 8 months ago • 4 comments

Please check that this issue hasn't been reported before.

  • [X] I searched previous Bug Reports didn't find any similar reports.

Expected Behavior

That a user should be able to do task.due.moment?.startOf('week') or similar in group by function, and not modify the stored date in the task.

Cause: https://momentjscom.readthedocs.io/en/latest/moment/03-manipulating/03-start-of/

Mutates the original moment by setting it to the start of a unit of time.

There may be other methods of Moment that mutate the stored data.

I suspect that this may be the underlying cause of:

  • https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2785
  • https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2789

Based on the tests below, I expect that doing so would modify the stored data in Task, even though Task is designed to be immutable.

Also, that these tests should pass:

describe('immutability', () => {
    it.failing('should not be possible to edit a date Moment after Task creation', () => {
        // TODO Make Task's use of Moment immutable - always return a clone of the stored Moment.
        //      https://momentjscom.readthedocs.io/en/latest/moment/01-parsing/12-moment-clone/

        const inputDate = '2024-02-28 12:34';
        const task = new Task({ ...new TaskBuilder().build(), dueDate: moment(inputDate) });

        const parsedDate = '2024-02-28T12:34:00.000Z';
        expect(task.dueDate).toEqualMoment(moment(parsedDate));

        task.dueDate?.startOf('day');
        // TODO This fails, giving '2024-02-28T00:00:00.000Z', as the startOf call edits the stored date.
        //      See https://www.geeksforgeeks.org/moment-js-moment-startof-method/.
        expect(task.dueDate).toEqualMoment(moment(parsedDate));
    });

    it.failing('should not be possible to edit a date TasksDate after Task creation', () => {
        // TODO Make TasksDate objects immutable - always return a clone of the stored Moment.
        //      https://momentjscom.readthedocs.io/en/latest/moment/01-parsing/12-moment-clone/

        const inputDate = '2024-02-28 12:34';
        const task = new Task({ ...new TaskBuilder().build(), dueDate: moment(inputDate) });

        const due: TasksDate = task.due;
        const parsedDate = '2024-02-28T12:34:00.000Z';
        expect(due.moment).toEqualMoment(moment(parsedDate));

        due.moment?.startOf('day');
        // TODO This fails, giving '2024-02-28T00:00:00.000Z', as the startOf call edits the stored date.
        //      See https://www.geeksforgeeks.org/moment-js-moment-startof-method/.
        expect(due.moment).toEqualMoment(moment(parsedDate));
    });
});

Current behaviour

The above tests fail, and so are marked as .failing.

Steps to reproduce

See the tests above, which I will commit.

Which Operating Systems are you using?

  • [ ] Android
  • [ ] iPhone/iPad
  • [ ] Linux
  • [X] macOS
  • [ ] Windows

Obsidian Version

1.6.1

Tasks Plugin Version

7.3.0

Checks

  • [ ] I have tried it with all other plugins disabled and the error still occurs

Possible solution

The code should always clone moment objects before returning them.

See https://momentjscom.readthedocs.io/en/latest/moment/01-parsing/12-moment-clone/

claremacrae avatar May 30 '24 10:05 claremacrae