fix: retain manually added Done & Cancelled dates in modal

[ilandikov]

Types of changes

Changes visible to users:

• Bug fix (prefix: fix - non-breaking change which fixes an issue)

Internal changes:

• Tests (prefix: test - additions and improvements to unit tests and the smoke tests)

Description

• Done/Cancelled date have to be kept if they are already present in the respective date editor while the status is set to Done/Cancelled respectively.

Don't overwrite a user-entered Done date if changing status to DONE
Don't overwrite a user-entered Cancelled date if changing status to CANCELLED

Motivation and Context

• Fixes Edit Task modal: switching status to Done or Cancelled changes the existing Done or Cancelled date #3085

How has this been tested?

• new unit tests added
• manual test in demo vault

Checklist

• My code follows the code style of this project and passes yarn run lint.
• My change has adequate Unit Test coverage.

Terms

• My contribution follows this project's contributing guide
• I agree to follow this project's Code of Conduct

[ilandikov]

Sorry for late minute force push, just a comment update

[claremacrae]

I think I'm missing something: how do the tests verify the reported 'Steps to reproduce' are fixed?

1. Open Edit Task modal on a task that is not Done status
2. Set Done date to any value other than today's
3. Change status to Done

If I understand the tests correctly, they confirm that editing a task that already has a Done date does not change the Done date, which is a different thing.

What have I misunderstood?

[claremacrae]

Thanks for looking at this.

[ilandikov]

I think I'm missing something: how do the tests verify the reported 'Steps to reproduce' are fixed?

1. Open Edit Task modal on a task that is not Done status
2. Set Done date to any value other than today's
3. Change status to Done

If I understand the tests correctly, they confirm that editing a task that already has a Done date does not change the Done date, which is a different thing.

What have I misunderstood?

As mentioned in a different comment, in today's implementation these are same, but may change in future, so I will try to create new tests that show this behaviour.

[claremacrae]

I am approving this becauseI I really don't think it's worth spending any more time on it.

But it's become really quite complex code to understand and maintain now, I feel.

[ilandikov]

1 New issue

The issue is due to the TODO in the test. Let me know if you wish to remove the todo or the whole refernce =)

[claremacrae]

1 New issue

The issue is due to the TODO in the test. Let me know if you wish to remove the todo or the whole refernce =)

It's OK, I've told SonarCloud to ignore it...

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[claremacrae]

Thank you!