Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: retain manually added Done & Cancelled dates in modal #3087

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
ea30347
test: . inline variable
ilandikov Sep 19, 2024
2d2b3c2
test: - make test task descriptions self-explanatory
ilandikov Sep 19, 2024
d735a72
test: - add tests showing current behaviour
ilandikov Sep 19, 2024
60738be
test: - add missing test on cancelled date
ilandikov Sep 19, 2024
52c4a79
fix: - keep done & cancelled date if they are present when status cha…
ilandikov Sep 19, 2024
ce5b3de
test: - update test descriptions
ilandikov Sep 20, 2024
5d7b558
test: - add test showing current behaviour
ilandikov Sep 20, 2024
e0ea373
refactor: - access done date field as a dictionary
ilandikov Sep 21, 2024
e50688a
refactor: - access doneDate date field as a dictionary
ilandikov Sep 21, 2024
e63817a
refactor: . extract variables
ilandikov Sep 21, 2024
e62a2df
refactor: . extract appleSauce()
ilandikov Sep 21, 2024
451bde5
refactor: . inline variables
ilandikov Sep 21, 2024
9f2e535
refactor: . extract variable
ilandikov Sep 21, 2024
1cb3803
refactor: - split if into two ifs
ilandikov Sep 21, 2024
14bcf14
refactor: - set the correct value explicitly
ilandikov Sep 21, 2024
8b8f03f
refactor: . rename function and add jsdoc
ilandikov Sep 21, 2024
47de533
refactor: - reuse setStatusRelatedDate()
ilandikov Sep 21, 2024
fd8c461
test: . rename variable
ilandikov Sep 21, 2024
a94e4d6
test: . extract variables
ilandikov Sep 21, 2024
42b2f91
test: - use toEqual() matcher
ilandikov Sep 21, 2024
3de8c50
test: . extract testDateInputAndStatusChange()
ilandikov Sep 21, 2024
4d52017
test: - convert it() to it.each()
ilandikov Sep 21, 2024
b3628f9
test: - add another test case
ilandikov Sep 21, 2024
9cf76cd
test: . document the tester
ilandikov Sep 21, 2024
395f088
test: . update test description
ilandikov Sep 21, 2024
cfa30a6
refactor: - replace setStatusRelatedDate() with simpler implementation
ilandikov Sep 21, 2024
5deae66
refactor: . flip &&s
ilandikov Sep 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions src/ui/StatusEditor.svelte
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<script lang="ts">
import type { TasksDate } from '../DateTime/TasksDate';
import type { Status } from '../Statuses/Status';
import type { Task } from '../Task/Task';
import type { EditableTask } from './EditableTask';
Expand All @@ -11,6 +12,22 @@

let statusSymbol = task.status.symbol;

function setStatusRelatedDate(currentValue: string, isInStatus: boolean, editedValue: TasksDate) {
const dateFieldIsEmpty = currentValue === '';

if (isInStatus && dateFieldIsEmpty) {
// the date field is empty and the status was set (set the date from the task with the applied status)
return editedValue.formatAsDate();
}

if (!isInStatus && !dateFieldIsEmpty) {
// the date field is not empty but another status was set (clean the date field)
return '';
}

return currentValue;
}

const _onStatusChange = () => {
// Use statusSymbol to find the status to save to editableTask.status
const selectedStatus: Status | undefined = statusOptions.find((s) => s.symbol === statusSymbol);
Expand All @@ -26,9 +43,17 @@
const taskWithEditedStatusApplied = task.handleNewStatus(selectedStatus).pop();

if (taskWithEditedStatusApplied) {
// Update the doneDate field, in case changing the status changed the value:
editableTask.doneDate = taskWithEditedStatusApplied.done.formatAsDate();
editableTask.cancelledDate = taskWithEditedStatusApplied.cancelled.formatAsDate();
editableTask.doneDate = setStatusRelatedDate(
editableTask.doneDate,
selectedStatus.isCompleted(),
taskWithEditedStatusApplied.done,
);

editableTask.cancelledDate = setStatusRelatedDate(
editableTask.cancelledDate,
selectedStatus.isCancelled(),
taskWithEditedStatusApplied.cancelled,
);
}
};
</script>
Expand Down
153 changes: 145 additions & 8 deletions tests/ui/EditTask.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import moment from 'moment';
import { taskFromLine } from '../../src/Commands/CreateOrEditTaskParser';
import { GlobalFilter } from '../../src/Config/GlobalFilter';
import { resetSettings, updateSettings } from '../../src/Config/Settings';
import { StatusRegistry } from '../../src/Statuses/StatusRegistry';
import { DateFallback } from '../../src/DateTime/DateFallback';
import { StatusRegistry } from '../../src/Statuses/StatusRegistry';
import type { Task } from '../../src/Task/Task';
import EditTask from '../../src/ui/EditTask.svelte';
import { verifyWithFileExtension } from '../TestingTools/ApprovalTestHelpers';
Expand Down Expand Up @@ -158,6 +158,35 @@ async function renderTaskModalAndChangeStatus(line: string, newStatusSymbol: str
return { waitForClose, container, submit };
}

/**
* Simulate the behaviour of:
* - clicking on a line in Obsidian,
* - opening the Edit task modal,
* - editing a field,
* - changing the status,
* - and clicking Apply.
* @param line
* @param elementId - specifying the field to edit
* @param newValue - the new value for the field
* @param newStatusSymbol - new Status symbol value
*/
async function renderChangeDateAndStatus(line: string, elementId: string, newValue: string, newStatusSymbol: string) {
const task = taskFromLine({ line: line, path: '' });
const { waitForClose, onSubmit } = constructSerialisingOnSubmit(task);
const { result, container } = renderAndCheckModal(task, onSubmit);

const inputElement = getAndCheckRenderedElement<HTMLInputElement>(container, elementId);
await editInputElement(inputElement, newValue);

const statusSelector = getAndCheckRenderedElement<HTMLSelectElement>(container, 'status-type');
await fireEvent.change(statusSelector, {
target: { value: newStatusSymbol },
});

const submit = getAndCheckApplyButton(result);
return { waitForClose, container, submit };
}

function getElementValue(container: HTMLElement, elementId: string) {
const element = getAndCheckRenderedElement<HTMLInputElement>(container, elementId);
return element.value;
Expand Down Expand Up @@ -334,34 +363,142 @@ describe('Task editing', () => {
resetSettings();
});

const line = '- [ ] simple';
it('should change status to Done and add doneDate', async () => {
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(line, 'x');
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
'- [ ] expecting done date to be added',
'x',
);
expect(getElementValue(container, 'done')).toEqual(today);

submit.click();
expect(await waitForClose).toMatchInlineSnapshot('"- [x] simple ✅ 2024-02-29"');
expect(await waitForClose).toMatchInlineSnapshot('"- [x] expecting done date to be added ✅ 2024-02-29"');
});

it('should change status to Done and keep doneDate', async () => {
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
'- [ ] expecting done date to be kept ✅ 2024-09-19',
claremacrae marked this conversation as resolved.
Show resolved Hide resolved
'x',
);
expect(getElementValue(container, 'done')).toEqual('2024-09-19');

submit.click();
expect(await waitForClose).toMatchInlineSnapshot('"- [x] expecting done date to be kept ✅ 2024-09-19"');
});

it('should change status to Todo and remove doneDate', async () => {
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
'- [x] simple ✅ 2024-02-29',
'- [x] expecting done date to be removed ✅ 2024-02-29',
' ',
);
expect(getElementValue(container, 'done')).toEqual('');

submit.click();
expect(await waitForClose).toMatchInlineSnapshot('"- [ ] simple"');
expect(await waitForClose).toMatchInlineSnapshot('"- [ ] expecting done date to be removed"');
});

it('should change status to Cancelled and add cancelledDate', async () => {
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(line, '-');
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
'- [ ] expecting cancelled date to be added',
'-',
);
expect(getElementValue(container, 'cancelled')).toEqual(today);

submit.click();
expect(await waitForClose).toMatchInlineSnapshot('"- [-] simple ❌ 2024-02-29"');
expect(await waitForClose).toMatchInlineSnapshot(
'"- [-] expecting cancelled date to be added ❌ 2024-02-29"',
);
});

it('should change status to Cancelled and keep cancelledDate', async () => {
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
'- [ ] expecting cancelled date to be kept ❌ 2024-09-20',
'-',
);
expect(getElementValue(container, 'cancelled')).toEqual('2024-09-20');

submit.click();
expect(await waitForClose).toMatchInlineSnapshot(
'"- [-] expecting cancelled date to be kept ❌ 2024-02-29"',
);
});

it('should change status to Todo and remove cancelledDate', async () => {
const { waitForClose, container, submit } = await renderTaskModalAndChangeStatus(
'- [-] expecting cancelled date to be removed ❌ 2024-02-29',
' ',
);
expect(getElementValue(container, 'cancelled')).toEqual('');

submit.click();
expect(await waitForClose).toMatchInlineSnapshot('"- [ ] expecting cancelled date to be removed"');
});

/**
* Test opening task modal for a given line, changing a date to a value, changing the status,
* clicking Apply, verifying the final line.
*
* @param line
* @param dateElementToChange
* @param dateValue
* @param newStatusSymbol
* @param expectedTaskAfterEdits
*/
async function testDateInputAndStatusChange(
line: string,
dateElementToChange: string,
dateValue: string,
newStatusSymbol: string,
expectedTaskAfterEdits: string,
) {
const { waitForClose, container, submit } = await renderChangeDateAndStatus(
line,
dateElementToChange,
dateValue,
newStatusSymbol,
);

expect(getElementValue(container, dateElementToChange)).toEqual(dateValue);

submit.click();
expect(await waitForClose).toEqual(expectedTaskAfterEdits);
}

it.each([
[
'- [ ] input done date, change status to done and expect the date to be kept',
'done',
'2024-09-20',
'x',
'- [x] input done date, change status to done and expect the date to be kept ✅ 2024-09-20',
],
[
'- [ ] input cancelled date, change status to cancelled and expect the date to be kept',
'cancelled',
'2024-09-21',
'-',
// TODO the difference between the date in the modal and the saved one is a bug:
// https://github.com/obsidian-tasks-group/obsidian-tasks/issues/3089
'- [-] input cancelled date, change status to cancelled and expect the date to be kept ❌ 2024-02-29',
],
])(
'for "%s" task, change %s date to %s and status to %s',
async (
line: string,
dateElementToChange: string,
dateValue: string,
newStatusSymbol: string,
expectedTaskAfterEdits: string,
) => {
await testDateInputAndStatusChange(
line,
dateElementToChange,
dateValue,
newStatusSymbol,
expectedTaskAfterEdits,
);
},
);

it('should create new instance of recurring task, with doneDate set to today', async () => {
updateSettings({ recurrenceOnNextLine: false });
const { waitForClose, submit } = await renderTaskModalAndChangeStatus(
Expand Down