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

feat: add created date to task #1723

Merged

Conversation

vanadium23
Copy link
Contributor

@vanadium23 vanadium23 commented Mar 5, 2023

  • Add field createdDate to task
  • Add option (false by default) to add created on every new task.
  • Update tests

It will be practical to query old tasks using dataview or maybe later add ability to obsidian-tasks Query.

Description

Motivation and Context

Query old tasks, based on field shorthands in dataview: https://blacksmithgu.github.io/obsidian-dataview/annotation/metadata-tasks/#field-shorthands

How has this been tested?

Place new main.js to my vault, and try to edit and add tasks. On newly added, created field apperae

Screenshots (if appropriate)

image

Types of changes

Changes visible to users:

  • Bug fix (prefix: fix - non-breaking change which fixes an issue)
  • New feature (prefix: feat - non-breaking change which adds functionality)
  • Breaking change (prefix: feat!! or fix!! - fix or feature that would cause existing functionality to not work as expected)
  • Documentation (prefix: docs - improvements to any documentation content)
  • Sample vault (prefix: vault - improvements to the Tasks-Demo sample vault)

Internal changes:

  • Refactor (prefix: refactor - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
  • Tests (prefix: test - additions and improvements to unit tests and the smoke tests)
  • Infrastructure (prefix: chore - examples include GitHub Actions, issue templates)

Checklist

Terms

* Add field created, based on field shorthands in dataview: https://blacksmithgu.github.io/obsidian-dataview/annotation/metadata-tasks/#field-shorthands
* Add option (false by default) to add created on every new task.
* Update tests

It will be practical to query old tasks using dataview or maybe later
add ability to obsidian-tasks Query.
@claremacrae claremacrae added the scope: task dates and times Requests for enhancements to types and formats of dates and times associated with tasks label Mar 5, 2023
@claremacrae
Copy link
Collaborator

Hello @vanadium23, Thank you very much indeed for offering this. I know it will be popular with many.

Just to join up the info, it's the first step of implementing #98.

I see you checked the box saying that you had added tests, but I'm not seeing them in the changed files. Are there perhaps some other changes not committed or pushed?

Note to self: this will need to be merged fairly soon, though, as @kedestin is shortly going to be working on a pull request that will read conflicts with this work.

Copy link
Collaborator

@claremacrae claremacrae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much. If I had implemented this, I would have felt it necessary to do other steps, like adding searching.

But I like your approach of first enabling the gathering of data in users' tasks, and then the searching can indeed be added as a separate step.

I've added some comments on the new code, for your consideration.

The thing that's missing is any new tests. From the checkboxes in the PR it sounds like you have done that already, so I'll hold off making any suggestions for unit tests until you have had a chance to reply to my other comment about that...

src/Config/SettingsTab.ts Outdated Show resolved Hide resolved
src/Config/SettingsTab.ts Show resolved Hide resolved
src/Commands/CreateOrEditTaskParser.ts Outdated Show resolved Hide resolved
src/Task.ts Outdated Show resolved Hide resolved
src/Task.ts Outdated Show resolved Hide resolved
tests/TestingTools/TaskBuilder.ts Outdated Show resolved Hide resolved
src/Commands/CreateOrEditTaskParser.ts Outdated Show resolved Hide resolved
src/TaskLayout.ts Show resolved Hide resolved
src/TaskModal.ts Outdated Show resolved Hide resolved
@vanadium23 vanadium23 marked this pull request as draft March 5, 2023 12:08
@vanadium23
Copy link
Contributor Author

@claremacrae, I tick unit test checkbox because has edited TaskBuilder.test.ts file. If you have any suggestions for unit tests I will try to implement in the PR.

@claremacrae
Copy link
Collaborator

claremacrae commented Mar 5, 2023

@claremacrae, I tick unit test checkbox because has edited TaskBuilder.test.ts file. If you have any suggestions for unit tests I will try to implement in the PR.

Thank you.

Tests worth adding

  • You taught the Task class to read a new symbol, ➕, so there should be a test somewhere that confirms that a ➕ value is read correctly.
    • An easy place to add that would be this test - by adding a created date to the input line, and then added expect() statements to confirm that the value was read correctly.

About the tests

If you would like to know more about the tests, like how to run them, then the following links may help.

I wrote them this morning, reasonably hastily, so if you see anything that doesn't make sense, please do let me know. Thank you.

@claremacrae
Copy link
Collaborator

claremacrae commented Mar 5, 2023

I had a bit of a think about the things that would be necessary for a first release of this - and what things could come later.

Action Docs
Needed for first release
✅ Add ➕ field to Task - and tests dates - Tasks User Guide - Obsidian Publish
✅ Update recurring tasks for addition of ➕ - presumably add today's date in new recurring task - it doesn't make sense to copy over the original creation date recurring-tasks - Tasks User Guide - Obsidian Publish
✅ Add ➕ to the tooltip shown in short mode layout - Tasks User Guide - Obsidian Publish
✅ Add show/hide created instruction layout - Tasks User Guide - Obsidian Publish
Review the docs (#1740)
OK to add later on
(#1743) Add ➕ to auto-suggest - and maybe auto add today's date when new task is created? auto-suggest - Tasks User Guide - Obsidian Publish
✅ (#1742) Show the created date in the Create or Edit Task modal create-or-edit-task - Tasks User Guide - Obsidian Publish
✅ (#1741) Add all the filters (has created date, no created date, created before/after ) filters - Tasks User Guide - Obsidian Publish
✅ (#1741) Add group by created grouping - Tasks User Guide - Obsidian Publish
✅ (#1741) Add sort by created sorting - Tasks User Guide - Obsidian Publish
✅ Add all the above to quick reference index - Tasks User Guide - Obsidian Publish

@claremacrae
Copy link
Collaborator

I think the big thing is how this interacts with the Recurrence code.

If I have this task:

- [ ] #task Phone Fred 🔁 every day ➕ 2023-03-01 📅 2024-03-01

And then I complete it, and get the new occurrence:

- [ ] #task Phone Fred 🔁 every day ➕ 2023-03-01 📅 2024-03-02
- [x] #task Phone Fred 🔁 every day ➕ 2023-03-01 📅 2024-03-01 ✅ 2023-03-05

Notice how the ➕symbol carries over the same date to the new instance.

I think it would be more logical for the new task to have the date that the line was created.

So I think the above should be:

- [ ] #task Phone Fred 🔁 every day ➕ 2023-03-05 📅 2024-03-02
- [x] #task Phone Fred 🔁 every day ➕ 2023-03-01 📅 2024-03-01 ✅ 2023-03-05

@vanadium23 I have got a lot of Tasks work in progress at the moment, and don't have the capacity to take on more work right now. So is this something that you would be willing to work on please?

If so, I could give you some pointers to the relevant bits of code.

@vanadium23
Copy link
Contributor Author

Yeah, I'll try to tackle on all comments. Appreciate for your great review.

@vanadium23
Copy link
Contributor Author

I mark a PR as ready for review within Needed for first release:

  • Add ➕ field to Task - and tests
  • Update recurring tasks for addition of ➕ - presumably add today's date in new recurring task - it doesn't make sense to copy over the original creation date
  • Add ➕ to the tooltip shown in short mode
  • Add show/hide created instruction

@vanadium23 vanadium23 marked this pull request as ready for review March 7, 2023 17:36
Copy link
Collaborator

@claremacrae claremacrae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is great. I wasn't expecting you to pick up all those things, and it's really helpful that you did.

I did manual testing of all the new features.

Other than something I noticed in the wording in the settings dialog, I think that the code in src/ is all looking good now.

I don't know how much more energy you have for this PR. I would understand if you wanted to put it down.

But so that they are not forgotten, I add some other comments around:

  • where possible, adding tests for the new features added today
  • document the changes, for users - especially documenting what is not yet implemented.

If you are up to doing those things, that's great.

But if not, that's absolutely fine and after the Settings wording is updated, I would be happy to merge what has been done so far, and I or someone else will pick up the tests and the docs.

tests/Query.test.ts Show resolved Hide resolved
src/Config/SettingsTab.ts Outdated Show resolved Hide resolved
docs/queries/layout.md Show resolved Hide resolved
src/TaskLineRenderer.ts Show resolved Hide resolved
src/Task.ts Show resolved Hide resolved
Copy link
Collaborator

@claremacrae claremacrae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent - thank you very much!

src/TaskLineRenderer.ts Show resolved Hide resolved
docs/queries/layout.md Show resolved Hide resolved
src/Config/SettingsTab.ts Outdated Show resolved Hide resolved
src/Commands/CreateOrEditTaskParser.ts Outdated Show resolved Hide resolved
@claremacrae claremacrae changed the title feat: add createdDate to task feat: add created date to task Mar 8, 2023
@claremacrae claremacrae merged commit 94d4f69 into obsidian-tasks-group:main Mar 8, 2023
@claremacrae
Copy link
Collaborator

Hi @vanadium23, very many thanks for implementing this, and responding to the feedback so thoroughly. It will be much appreciated by users, I know, and I think I will use it too. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: task dates and times Requests for enhancements to types and formats of dates and times associated with tasks type: enhancement New feature or request
Projects
Status: 🎉 Released
Development

Successfully merging this pull request may close these issues.

None yet

2 participants