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.

Sign up for GitHub

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

Jump to bottom

Implement cache and TaskGroup #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Implement cache and TaskGroup #23

wants to merge 3 commits into from

Conversation

chtitux
Copy link
Contributor

@chtitux chtitux commented Nov 3, 2019

  • Implement a cache so we don't have to fetch all tasks on every call, fix Add a cache to speed up usage #22
  • Browse Assigments recursively so we handle TaskGroup (and any other forms of tree if it's ever allowed by Dovico), fix Implement TaskGroup #21
  • Improve formatter so we have a nicer display per day

It's a big refactoring on how we manage entries but I hope it'll be easier if there are multiple levels of folders.

@chtitux chtitux requested a review from misdoro November 3, 2019 10:33
@chtitux
Copy link
Contributor Author

chtitux commented Nov 3, 2019

@squiggleuk : would you mind to test this PR to see if it solves your issue please?

$ git clone -b cache-task-group https://github.com/chtitux/dovico.git
$ cd dovico/
$ bundle exec bin/dovico --tasks
$ dovico --show --start 2019-10-01 --end 2019-10-07

Or just git fetch and switch to cache-task-group branch with git checkout cache-task-group, it should do the job.

I have implemented a local cache so the first run will be long, but next ones should be quicker.

Thank you for testing!

@geftactics
Copy link

I get errors with this PR... AssignmentID C210 unsupported

~/ext/dovico [cache-task-group]
10:01 $ bundle exec bin/dovico --tasks

== List of available projects ==
bundler: failed to load command: bin/dovico (bin/dovico)
RuntimeError: AssignmentID C210 unsupported
  /Users/gkendal/ext/dovico/lib/dovico/model/assignment.rb:52:in `class_for'
  /Users/gkendal/ext/dovico/lib/dovico/model/assignment.rb:32:in `parse'
  /Users/gkendal/ext/dovico/lib/dovico/model/assignment.rb:61:in `block in fetch_assignments'
  /Users/gkendal/ext/dovico/lib/dovico/model/assignment.rb:60:in `map'
  /Users/gkendal/ext/dovico/lib/dovico/model/assignment.rb:60:in `fetch_assignments'
  /Users/gkendal/ext/dovico/lib/dovico/model/assignment.rb:78:in `fetch_all'
  /Users/gkendal/ext/dovico/lib/dovico/model/assignments.rb:21:in `load'
  /Users/gkendal/ext/dovico/lib/dovico/app.rb:105:in `assignments'
  /Users/gkendal/ext/dovico/lib/dovico/app.rb:120:in `display_tasks'
  /Users/gkendal/ext/dovico/lib/dovico/app.rb:70:in `run'
  bin/dovico:4:in `<top (required)>'

@chtitux
Copy link
Contributor Author

chtitux commented Nov 4, 2019

AssignmentID C210 unsupported

At least, we have a new kind of error 😅

The "Client" assigment was not included but it seems it's used in your Dovico setup.
I have added it (it's very simple now, it's just an empty class) and pushed the code.

Could you git pull my branch & test again please?

@geftactics
Copy link

geftactics commented Nov 5, 2019 •
edited
Loading

We're making progress! Good output from --tasks

Example:

Client #207 Company Ops Support
  Project #3827 Dept 1 Ops Support
    Task #249 -Delivery

  Project #3829 Dept 2 Ops Support
    Task #249 -Delivery

  Project #3830 Dept 3 Ops Support
    Task #100 Architecture
    Task #103 Programme Management
    Task #104 Project Management
    Task #106 Incident Management
    Task #249 -Delivery

Also --show seems to be working too, but with odd output formatting... Some kind of [ . xxxxxxxxx] that seems to grow with each record?

I can now fill and submit time records!!! Thanks :D

@chtitux
Copy link
Contributor Author

chtitux commented Nov 5, 2019

dovico-list

The entries are grouped by date and in the same day, the × of a line starts where it ends on the previous one.

Feel free to give your feedback or post a screenshot if you have a bug in the formatting!

I'll probably make a new release once this PR would have been merged.

@geftactics geftactics mentioned this pull request Nov 5, 2019
Closed
Théophile Helleboid added 2 commits November 6, 2019 15:21
Improve management of list of Assignments
1f60f8a 
- Implement a cache so we don't have to fetch all tasks on every call
- Browse Assigments recursively so we handle TaskGroup (and any other
forms of tree if it's ever allowed by Dovico)
- Improve formatter so we have a nicer display per day
Fix missing myself id in clear task
f27e4d2
misdoro
misdoro reviewed Dec 9, 2019
View reviewed changes
lib/dovico/model/assignments.rb

attr_reader :assignments, :myself

CACHE_FILE = "#{APP_DIRECTORY}/assignments.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

File.join(APP_DIRECTORY, 'assgnments.json') will be more portable I think

lib/dovico/model/assignments.rb

@assignments = Assignment.unserialize(json["assignments"]) unless json["assignments"].nil?
@myself = Employee.unserialize(json["myself"]) unless json["myself"].nil?
rescue JSON::ParserError, CacheError => e
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 wdyt of logging something to stderr on CacheError exceptions?

lib/dovico/model/task_group.rb
@@ -0,0 +1,4 @@
module Dovico
class TaskGroup < Assignment
Copy link
Contributor

Choose a reason for hiding this comment

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

May it make sense to group various assignment types in Dovico::Assignment:: ?
Like Dovico::Assignment::TaskGroup, Dovico::Assignment::Task ?

module Dovico
  class Client < Assignment

is a bit confusing, I'm thinking rather of an API client then of an assignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@misdoro misdoro misdoro left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a cache to speed up usage Implement TaskGroup
3 participants
@chtitux @geftactics @misdoro