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

wip research ability to create simple report of done tasks #390

Closed
wants to merge 1 commit into from

Conversation

titoOdUA
Copy link
Collaborator

I have started exploring of the ability to create simple reports discussed here #355 (comment)

@titoOdUA
Copy link
Collaborator Author

it is just a rough draft, I don't have much time for now. I will have time on Friday/Saturday.

@alanvardy how do you like completed_at field in tasks? it is in UTC by default and private, we can get it through getter which will convert it to the user's timezone.

Also, I see that we have very similar functions in the task list

    if has_flag(matches.clone(), "scheduled") {
        projects::scheduled_items(&config, &project)
    } else if has_flag(matches.clone(), "done-yesterday") {
        projects::completed_items(&config, &project)
    } else {
        projects::all_items(&config, &project)
    }

scheduled_items, completed_items, all_items
I think that we can extract all of it in a separate file/module that would have some enum of available variants. We can have one functions that make requests based on the provided variant of the enum. And maybe it could receive some closure as a parameter for filtering purposes. (maybe our task list can transform into some module for reporting/filtering)

@alanvardy how do you like the idea?

@titoOdUA titoOdUA requested a review from alanvardy May 31, 2023 19:29
@alanvardy
Copy link
Owner

it is just a rough draft, I don't have much time for now. I will have time on Friday/Saturday.

👍

@alanvardy how do you like completed_at field in tasks? it is in UTC by default and private, we can get it through getter which will convert it to the user's timezone.

I like it!

Also, I see that we have very similar functions in the task list

    if has_flag(matches.clone(), "scheduled") {
        projects::scheduled_items(&config, &project)
    } else if has_flag(matches.clone(), "done-yesterday") {
        projects::completed_items(&config, &project)
    } else {
        projects::all_items(&config, &project)
    }

scheduled_items, completed_items, all_items I think that we can extract all of it in a separate file/module that would have some enum of available variants. We can have one functions that make requests based on the provided variant of the enum. And maybe it could receive some closure as a parameter for filtering purposes. (maybe our task list can transform into some module for reporting/filtering)

@alanvardy how do you like the idea?

Yes there is lots of duplicate code in there, I'm interested in seeing what you come up with!

Copy link
Owner

@alanvardy alanvardy left a comment

Choose a reason for hiding this comment

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

So far so good, I'm interested in using this feature when it is done

@@ -9,6 +10,7 @@ use std::cmp::Reverse;
use std::fmt::Display;

use crate::config::Config;
use crate::time::timezone_from_str;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
use crate::time::timezone_from_str;
use crate::time;

@@ -253,6 +256,13 @@ impl Item {
Ok(DateTimeInfo::DateTime { .. })
)
}

pub fn get_completed_at(&self, config: &Config) -> DateTime<Tz> {
let tz = timezone_from_str(&config.timezone);
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
let tz = timezone_from_str(&config.timezone);
let tz = time::timezone_from_str(&config.timezone);

@titoOdUA
Copy link
Collaborator Author

titoOdUA commented Jun 6, 2023

#411
new PR for this (there were too many merge conflicts when I decided to update the branch, wasn't worth fixing)

@titoOdUA titoOdUA closed this Jun 6, 2023
@alanvardy
Copy link
Owner

Sorry about that, I'm hoping that by splitting out into more modules we can conflict less in the future. There's a buncha spaghetti code in there!

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.

2 participants