-
-
Notifications
You must be signed in to change notification settings - Fork 88
feat: add support for viewing completed tasks #391
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
base: master
Are you sure you want to change the base?
Conversation
|
resolves #233 |
|
yay looking forward to this! |
|
@jamiebrynes7 could you take a look? |
jamiebrynes7
left a comment
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.
This is really awesome! I mostly have questions around how this is exposed to end-users and trying to resolve some of the confusion that this is introducing inside the query syntax
| debug({ | ||
| msg: "Sending Todoist API request", | ||
| context: Object.fromEntries(queryParams.entries()), | ||
| }); |
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.
Can we debug log the RequestParams that gets passed into fetcher.fetch like what happens in do?
| } | ||
|
|
||
| private getProjectById(projectId: string): Project | undefined { | ||
| return this.projects.byId(projectId); |
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.
Shall we include the makeUnknownProject call here like with section/labels?
| export type Task = { | ||
| id: TaskId; | ||
| createdAt: string; | ||
| createdAt?: string; |
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.
We are using the completed task's completed_at for this, so I think this doesn't need to be made optional
| }; | ||
|
|
||
| const querySchema = z.object({ | ||
| const baseQuerySchema = z.object({ |
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.
I think it would be cleaner and clearer if the completed tasks were modelled as a new kind of query block with their own dedicated query schema.
To query for active tasks
```todoist
description: "Today's tasks"
filter: "today"
```
To query for completed tasks (note we can also drop the prefix as its now redundant!):
```todoist-completed
limit: 100
since: 2025-02-01
```
Rather than trying to encode the combinations in the schema here, having two independent schemas (which can still share common elements where applicable!) seems like the best way forward. It also resolves ambiguities like:
completedLimit/completedSince/completedUntilare meaningless iffilteris providedsorting/groupBydo not look like they are supported for completed tasks
| import { z } from "zod"; | ||
|
|
||
| type ErrorTree = string | { msg: string; children: ErrorTree[] }; | ||
| const MIN_COMPLETED_TASKS_AUTOREFRESH = 9; |
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.
This looks like a call to the completed tasks API endpoint counts as a 'full sync' operation:
For each user, you can make a maximum of 1000 partial sync requests within a 15 minute period.
For each user, you can make a maximum of 100 full sync requests within a 15 minute period.
Is this being conservative or have you verified which kind this request counts as?
Its also worth noting the utility of this is somewhat lessened since a user can have any number of blocks rendered at once
|
This feature will be a clutch for periodic reviews! Anyway, can I contribute to this via development? |
|
To everyone involved, thanks to all who are working on this 🫡 —that's something I've always wanted. 🙏 It's one thing to know what still needs to be done, but the list is often long, even when you narrow your perspective. What personally gives me the strength to do more of what I set out to do when I procrastinate is being able to clearly see what I've already accomplished, because that's something we often forget. I combine this with Habitica as additional gamification to not only feel a sense of completion but also receive direct feedback. Simply being aware of the often large list of things I've done motivates me a lot. Ideally, this would be grouped in a hierarchical project view so it can be integrated into the daily note. Together with personal notes and other items, it would fit in perfectly. |


This PR adds basic support for viewing completed tasks. Users can now:
viewCompleted: trueparametercompletedSinceandcompletedUntilcompletedLimit