hcl resume should be idempotent

[aaronjensen]

currently if i send resume for one task twice in a row it will start the timer and then stop it. Resume does not imply toggle.

[zenhob]

Good catch!

Here is the offending line, for future reference: https://github.com/zenhob/hcl/blob/master/lib/hcl/commands.rb#L174

[zenhob]

Um I think stop has the same bug:

https://github.com/zenhob/hcl/blob/master/lib/hcl/commands.rb#L130

[charneykaye]

I have some time coming up to work on hcl. Top of my list is this issue (I think), or perhaps broader work to ensure that timer resume picks up any already-used-today timers.

[zenhob]

So it turns out that this is a limitation of the underlying timesheet API provided by Harvest.

Instead of attempting to work around this (or change APIs, which is a larger scope than I wish to tackle), I plan to deprecate stop and resume since they have a misleading name, and replace them both with something like toggle that has the same functionality while also properly describing what it does.

I'll plan to roll out a minor release with the new command and optional deprecation of the old commands, with a longer term plan to release a version with mandatory deprecation, concluding with the removal the deprecated commands for version 1.0 (if that ever happens)

[aaronjensen]

Will there still be a way to stop the current timer that doesn't require knowing which timer is running and won't start a timer if there's nothing running?

[zenhob]

this is a great question! it turns out that the stop command already checks for a running timer so I left it in place. I've added a toggle command that works mostly like resume that will be in the next release. Good catch, thank you

[aaronjensen]

Okay, coming around to trying this out today and it doesn't actually solve my problem. It gives a better name to resume, but it still fails to give me an idempotent resume timer.

I want to be able to call hcl XXX 1234 5678 twice in a row and have the result be that timer 1234 5678 is started once.

If I use toggle, the second call will stop it.

What I want is just like toggle, but it won't stop:

    def resume *args
      ident = get_ident args
      entry = if ident
          task_ids = get_task_ids ident, args
          DayEntry.last_by_task http, *task_ids
        else
          DayEntry.last(http)
        end
      if entry
        unless entry.running?
          "Starting #{entry} (at #{current_time})"
          entry.toggle http
        end
      else
        fail "No timer found."
      end
    end

As it is now, I have to call stop and then call toggle and the harvest api isn't exactly fast, so reducing the calls would be great.

[charneykaye]

Progress!

Charney Kaye

On Thu, Jun 25, 2020 at 1:24 PM Aaron Jensen notifications@github.com wrote:

Okay, coming around to trying this out today and it doesn't actually solve my problem. It gives a better name to resume, but it still fails to give me an idempotent resume timer.

I want to be able to call hcl XXX 1234 5678 twice in a row and have the result be that timer 1234 5678 is started once.

If I use toggle, the second call will stop it.

What I want is just like toggle, but it won't stop:

def resume *args
  ident = get_ident args
  entry = if ident
      task_ids = get_task_ids ident, args
      DayEntry.last_by_task http, *task_ids
    else
      DayEntry.last(http)
    end
  if entry
    unless entry.running?
      "Starting #{entry} (at #{current_time})"
      entry.toggle http
    end
  else
    fail "No timer found."
  end
end

As it is now, I have to call stop and then call toggle and the harvest api isn't exactly fast, so reducing the calls would be great.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/zenhob/hcl/issues/68#issuecomment-649798678, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBP5BGSGF26BW7U3FYFI63RYOW7LANCNFSM4BIGONJQ .