prevent tags from being separated inbetween

[felixschurk]

I experienced that tags such as +next where separated as +n e x t and thus the time tracking integration was not properly working.
The PR just adds the brackets around the parsed output from task warrior and thus is able to iterate over the list as a whole and not over the independent characters.

[lauft]

@felixschurk Thanks for the contribution, however, there are some issues with the test suite that need to be addressed before merging.

Also, I was not able to reproduce your issue with a test, unfortunately. Can you provide more information on that?
Here's a hook you can install to dump the input for the on-modified hook:

#!/usr/bin/env bash

# Input:
# - line of JSON for the original task
# - line of JSON for the modified task, the diff being the modification
read original_task
read modified_task

# Dump the data
echo "${original_task}" > /tmp/on-modify.dump
echo "${modified_task}" >> /tmp/on-modify.dump

# Create output:
echo "${modified_task}"
echo 'on-modify.dump'

exit 0

[felixschurk]

@lauft
Thanks for the script the output is as following:

$ task add testtask +new
Created task 78.

$ task 78 start
Starting task ea8efca9 'testtask'.
Started 1 task.
on-modify.dump
Tracking e n testtask
  Started 2023-12-01T09:36:56
  Current                  56
  Total               0:00:00
You have more urgent tasks.

where the corresponding dump with the original script then looks like

{"description":"testtask","entry":"20231201T083449Z","modified":"20231201T083558Z","status":"pending","uuid":"ea8efca9-97d3-42e5-8cf3-dfff34a30924","tags":["new"]}
{"description":"testtask","entry":"20231201T083449Z","modified":"20231201T083558Z","status":"pending","uuid":"ea8efca9-97d3-42e5-8cf3-dfff34a30924","tags":"new","start":"20231201T083656Z"}

when adding another tag such as +test the dump looks like

{"description":"testtask","entry":"20231201T083449Z","modified":"20231201T084520Z","status":"pending","uuid":"ea8efca9-97d3-42e5-8cf3-dfff34a30924","tags":["new","test"]}
{"description":"testtask","entry":"20231201T083449Z","modified":"20231201T084520Z","status":"pending","uuid":"ea8efca9-97d3-42e5-8cf3-dfff34a30924","tags":"test,new","start":"20231201T084525Z"}

[felixschurk]

tbabej/taskpirate#19
#20
GothenburgBitFactory/tasklib#77

I now also stumbled that this is not a new issue, so the links are mainly for reference.

[lauft]

@felixschurk Is the on-modify.timewarrior hook the only hook installed? I am a bit puzzled that the value for tags is a list in the original task JSON, but a comma-concatenated string in the modified one...

[felixschurk]

@lauft
For having the on-modify.timewarrior working I also have on-modify-pirate installed.

Hooks
     System: Enabled
   Location: /home/felix/.task/hooks
     Active: on-add-pirate         (executable)
             on-modify-pirate      (executable)
             on-modify.timewarrior (executable)
   Inactive: on-modify.dump        (not executable)

where the pirate hooks do come from https://github.com/tbabej/taskpirate, also for the on-add I use https://github.com/tbabej/task.default-date-time.

So my hooks folder looks like

$ tree -L 2
.
├── default-time
│   ├── LICENCE
│   ├── pirate_add_default_time.py
│   ├── pirate_mod_default_time.py -> pirate_add_default_time.py
│   ├── __pycache__
│   └── README.md
├── on-add-pirate
├── on-modify.dump
├── on-modify-pirate
└── on-modify.timewarrior

But to your question, at least when they are all working as I guess, when I modify a task only the on-modify.timewarrior should get used.

[lauft]

@felixschurk Thanks for your reply and the references.
That confirms that tasklib converting the tag list from list to string in the JSON is the cause.
However, instead of simply switching to string, I would rather propose adding a workaround for this, preserving the expected behavior as well, see the following patch

diff --git a/on_modify.py b/on_modify.py
--- a/on_modify.py	(revision 360fc2292a5eab46cd5c9bdd4cc791b6f9ea5e9a)
+++ b/on_modify.py	(date 1702157404269)
@@ -51,7 +51,13 @@
         tags.append(json_obj['project'])
 
     if 'tags' in json_obj:
-        tags.extend(json_obj['tags'])
+        if type(json_obj['tags']) is str:
+            # Usage of tasklib (e.g. in taskpirate) converts the tag list into a string
+            # If this is the case, convert it back into a list first
+            # See https://github.com/tbabej/taskpirate/issues/11
+            tags.extend(json_obj['tags'].split(','))
+        else:
+            tags.extend(json_obj['tags'])
 
     return tags
 

Of course, we should add tests for this as well: 🙂

diff --git a/test/test_on-modify_unit.py b/test/test_on-modify_unit.py
--- a/test/test_on-modify_unit.py	(revision 360fc2292a5eab46cd5c9bdd4cc791b6f9ea5e9a)
+++ b/test/test_on-modify_unit.py	(date 1702156540207)
@@ -341,6 +341,66 @@
     verify(subprocess).call(['timew', 'start', 'Foo', ':yes'])
 
 
[email protected]("teardown")
+def test_hook_should_process_start_issue34_with_multiple_tags():
+    """on-modify hook should process 'task start' (issue #34) with multiple tags"""
+
+    when(subprocess).call(...)
+    on_modify.main(
+        json.loads(
+            '''{
+            "description": "Foo",
+            "entry": "20190820T203842Z",
+            "modified": "20190820T203842Z",
+            "status": "pending",
+            "tags": ["abc", "xyz"],
+            "uuid": "16af44c5-57d2-43bf-97ed-cf2e541d927f"
+            }'''),
+        json.loads(
+            '''{
+            "description": "Foo",
+            "entry": "20190820T203842Z",
+            "modified": "20190820T203842Z",
+            "start": "20190820T203842Z",
+            "status": "pending",
+            "tags": "abc,xyz",
+            "uuid": "16af44c5-57d2-43bf-97ed-cf2e541d927f"
+            }''')
+    )
+
+    verify(subprocess).call(['timew', 'start', 'Foo', 'abc', 'xyz', ':yes'])
+
+
[email protected]("teardown")
+def test_hook_should_process_start_issue34_with_single_tags():
+    """on-modify hook should process 'task start' (issue #34) with single tags"""
+
+    when(subprocess).call(...)
+    on_modify.main(
+        json.loads(
+            '''{
+            "description": "Foo",
+            "entry": "20190820T203842Z",
+            "modified": "20190820T203842Z",
+            "status": "pending",
+            "tags": ["abc"],
+            "uuid": "16af44c5-57d2-43bf-97ed-cf2e541d927f"
+            }'''),
+        json.loads(
+            '''{
+            "description": "Foo",
+            "entry": "20190820T203842Z",
+            "modified": "20190820T203842Z",
+            "start": "20190820T203842Z",
+            "status": "pending",
+            "tags": "abc",
+            "uuid": "16af44c5-57d2-43bf-97ed-cf2e541d927f"
+            }''')
+    )
+
+    verify(subprocess).call(['timew', 'start', 'Foo', 'abc', ':yes'])
+
+
 @pytest.mark.usefixtures("teardown")
 def test_hook_should_process_stop():
     """on-modify hook should process 'task stop'"""

At least until tbabej/taskpirate#19 and GothenburgBitFactory/tasklib#77 are resolved.

[felixschurk]

@lauft

Yes, that seems like the proper way of doing and dealing with it.
I never worked with Pytest before, therefore I was unable to add there a proper unit test.

Should I incorporate the changes from your last comment to update that PR, or should we delete this PR and you create one with the changes?

[lauft]

@felixschurk Feel free to incorporate those changes 👍🏻
Thank you.

[felixschurk]

@lauft
I am quite inexperienced when it comes to python, pytest in combination with docker, so this might be a simple question.

How can I locally set up the test and run them, based on the docker-compose.yml file?
Or otherwise, which modules would I need to have on my local machine in order to run the tests? I saw that in the Dockerfile it revers to Debian, and I am on Fedora. Could this be a problem?

This is mainly as I would like to verify that it is running properly, before I push it online.

[lauft]

@felixschurk To run the unit tests locally, do

python3 -m venv venv                         # create virtual enviroment
venv/bin/pip install pytest mockito          # install requirements
export PYTHONPATH=.                          # set PYTHONPATH such it finds 'on_modify.py'
venv/bin/pytest test/test_on-modify_unit.py  # run the tests

If you want to run the end-2-end test as well, you will have to create a GitHub token first.
Go to your profile -> settings -> developer settings -> personal access tokens, and create a classic token with permission read:packages. With that authenticate against the GitHub Container Registry:

docker -u <username> ghcr.io

After that you can run an end-2-end test with the following command:

REGISTRY=ghcr.io OWNER=gothenburgbitfactory docker-compose run task-stable-timew-stable

Although you could also let GitHub actions take over this part. 🙂

[felixschurk]

Thank you a lot for the detailed explanation, I managed to set it locally up and now also the GitHub actions are happy.