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

Add pagination support for completed items and projects #9

Merged
merged 1 commit into from Feb 16, 2021
Merged

Add pagination support for completed items and projects #9

merged 1 commit into from Feb 16, 2021

Conversation

dltn
Copy link
Contributor

@dltn dltn commented Dec 19, 2020

Hey @darekkay, this tool is wonderful! Thanks for open sourcing this.

Currently, the export only contains 30 each of completed items and projects (the API default limit):

% cat ~/Downloads/todoist.json | jq .completed.items | jq length
30

% cat ~/Downloads/todoist.json | jq .completed.projects | jq length
30

This PR adds pagination support so all previously completed items / projects are exported:

image

% cat ~/Downloads/todoist.json | jq .completed.items | jq length
2702

% cat ~/Downloads/todoist.json | jq .completed.projects | jq length
220

@dltn dltn marked this pull request as ready for review December 19, 2020 22:51
@darekkay darekkay mentioned this pull request Dec 22, 2020
@darekkay
Copy link
Owner

Hey @dltn
thanks for your contribution! The change itself looks good and works like a charm.

However, I've got over 10.000 archived tasks, and it takes almost 1 minute for the tool to finish. I believe, some people will have even more tasks. Additionally, the tool doesn't have any loading feedback when using the download button (it does work when clicking the direct link, though). I've created #10 to track this issue, as it's not directly related to this PR.

My proposal is to add a disclaimer, e.g. "Exporting large amounts of archived tasks may take some time".

Some users might not care about archived tasks, so an additional improvement would be to add a checkbox like "Download all archived tasks (this will take more time to complete)" - default = "off" for better performance.

Last but not least, did you find any information about the default quota? Todoist might not be happy about getting so many requests per download (50+ requests for me).

@dltn
Copy link
Contributor Author

dltn commented Jan 1, 2021

Wow, that's a while! I'll address the feedback and test larger archives. The limit is 50 requests per minute, so about ~10K tasks/minute at 200 items per request. Behavior is likely undefined if we exceed that - so we'd better to add a timeout to be safe.

@dltn
Copy link
Contributor Author

dltn commented Jan 26, 2021

(Haven't forgotten about this, just busy in the new year. Looping back soon.)

@darekkay
Copy link
Owner

(Haven't forgotten about this, just busy in the new year. Looping back soon.)

There's no hurry :)

@dltn
Copy link
Contributor Author

dltn commented Feb 16, 2021

Added optional checkbox (default off) control and disclaimers:

demo of checkbox

I learned the way format is currently passed through the OAuth state parameter is non-standard. In our case, this complicates passing more than one piece of state (before: format, after: format & archive option).

This PR encodes the archive option in the existing "format string" – which isn't ideal, but minimizes the changes needed to keep compatibility. I don't want to bloat this PR further, so I created #11 to track this.

Copy link
Owner

@darekkay darekkay left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a lot! 😊🚀

@darekkay darekkay merged commit 5902ca9 into darekkay:master Feb 16, 2021
@darekkay
Copy link
Owner

I've deployed the change to production

@dltn dltn mentioned this pull request Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants