-
-
Notifications
You must be signed in to change notification settings - Fork 244
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
feat: add created date to task #1723
Conversation
* 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.
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. |
There was a problem hiding this 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...
@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
About the testsIf 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.
|
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.
|
I think the big thing is how this interacts with the Recurrence code. If I have this task:
And then I complete it, and get the new occurrence:
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:
@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. |
Yeah, I'll try to tackle on all comments. Appreciate for your great review. |
* move tab with created date above completed * make consisted createdDate position through project * remove debug console
I mark a PR as ready for review within
|
There was a problem hiding this 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.
There was a problem hiding this 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!
in quick reference table.
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. 🎉 |
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)
Types of changes
Changes visible to users:
fix
- non-breaking change which fixes an issue)feat
- non-breaking change which adds functionality)feat!!
orfix!!
- fix or feature that would cause existing functionality to not work as expected)docs
- improvements to any documentation content)vault
- improvements to the Tasks-Demo sample vault)Internal changes:
refactor
- non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)test
- additions and improvements to unit tests and the smoke tests)chore
- examples include GitHub Actions, issue templates)Checklist
yarn run lint
.Terms