Repository: GothenburgBitFactory/timewarrior (cpp)

Issue:
[TI-101] Keeps tracking renamed and stopped task
_Antanas B. on 2018-01-11T09:25:17Z says:_ 

 How to reproduce:
```
$ task add dummy
Created task 6.
$ task 6 start
[...]
$ timew
Tracking dummy
Started 2018-01-11T11:18:24
Current 39
Total 0:00:15
$ task 6 mod foo
Modifying task 6 'foo'.
Modified 1 task.
$ task 6 stop
The current interval does not have the 'foo' tag.
Stopping task 4a6c05ca-1396-4a3b-acc5-d04f8afe3a3f 'foo'.
Stopped 1 task.
$ timew
Tracking dummy
[...]
```


Discussion:
Migrated metadata:
```
Created: 2018-01-11T09:25:17Z
Modified: 2018-01-13T08:46:22Z
```
_Thomas Lauf on 2018-01-13T08:46:22Z says:_ 

 Seems to me as if the on-modify hook does not convey the switch of project to TimeWarrior. Do you have a dump of the two JSONs for the old and new task?

Patch:
diff --git a/ext/on-modify.timewarrior b/ext/on-modify.timewarrior
--- a/ext/on-modify.timewarrior
+++ b/ext/on-modify.timewarrior
@@ -42,6 +42,18 @@ old = json.loads(sys.stdin.readline())
 new = json.loads(sys.stdin.readline())
 print(json.dumps(new))
 
+def extract_timew_tags_from(json_obj):
+    # Extract attributes for use as tags.
+    tags = [json_obj['description']]
+
+    if 'project' in json_obj:
+        tags.append(json_obj['project'])
+
+    if 'tags' in json_obj:
+        tags.extend(json_obj['tags'])
+
+    return ' '.join(['"{0}"'.format(tag) for tag in tags]).encode('utf-8').strip()
+
 start_or_stop = ''
 
 # Started task.
@@ -53,16 +65,15 @@ elif 'start' not in new and 'start' in old:
     start_or_stop = 'stop'
 
 if start_or_stop:
-    # Extract attributes for use as tags.
-    tags = [new['description']]
-
-    if 'project' in new:
-        tags.append(new['project'])
+    combined = extract_timew_tags_from(new)
 
-    if 'tags' in new:
-        tags.extend(new['tags'])
+    subprocess.call(shlex.split('timew {0} {1} :yes'.format(start_or_stop, combined)))
 
-    combined = ' '.join(['"%s"' % tag for tag in tags]).encode('utf-8').strip()
+# Modifications to task other than start/stop
+elif 'start' in new and 'start' in old:
+    old_combined = extract_timew_tags_from(old)
+    new_combined = extract_timew_tags_from(new)
 
-    command = 'timew {0} {1} :yes'.format(start_or_stop, combined)
-    subprocess.call(shlex.split(command))
+    if old_combined != new_combined:
+        subprocess.call(shlex.split('timew untag @1 {0} :yes'.format(old_combined)))
+        subprocess.call(shlex.split('timew tag @1 {0} :yes'.format(new_combined)))


Test patch:
diff --git a/test/test_on-modify.timewarrior.t.py b/test/test_on-modify.timewarrior.t.py
--- a/test/test_on-modify.timewarrior.t.py
+++ b/test/test_on-modify.timewarrior.t.py
@@ -110,14 +110,76 @@ def test_hook_should_process_edit(self):
         self.assertEqual('{"status": "pending", "entry": "20180316T231125Z", "uuid": "e11d7d19-20a7-47bb-999e-6554a70ea094", "description": "FooBar", "modified": "20180316T231055Z"}\n', out)
         self.assertEqual('', err)
 
-    def test_hook_should_process_modify(self):
-        """on-modify hook should process 'task modify'"""
+    def test_hook_should_process_modify_desc(self):
+        """on-modify hook should process 'task modify' for changing description"""
+
+        self.t("start dummy")
         out, err = self.process.communicate(input="""\
 {"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","start":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534"}
 {"description":"foo","entry":"20180317T092629Z","modified":"20180317T092629Z","start":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534"}
 """)
 
-        self.assertEqual('{"status": "pending", "uuid": "3422d76c-c087-4ecd-9c62-1246b078e534", "modified": "20180317T092629Z", "start": "20180317T092629Z", "entry": "20180317T092629Z", "description": "foo"}\n', out)
+        self.assertRegexpMatches(out, """\
+Removed dummy from @1
+Added foo to @1
+\{"status": "pending", "uuid": "3422d76c-c087-4ecd-9c62-1246b078e534", "modified": "20180317T092629Z", "start": "20180317T092629Z", "entry": "20180317T092629Z", "description": "foo"\}
+""")
+        self.assertEqual('', err)
+
+    def test_hook_should_process_modify_tags(self):
+        """on-modify hook should process 'task modify' for changing tags"""
+
+        self.t("start dummy tag1 tag2")
+        out, err = self.process.communicate(input="""\
+{"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","start":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534","tags":["tag1","tag2"]}
+{"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","start":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534","tags":["tag3","tag4"]}
+""")
+
+        self.assertRegexpMatches(out, """\
+Removed dummy tag1 tag2 from @1
+Added dummy tag3 tag4 to @1
+\{"status": "pending", "uuid": "3422d76c-c087-4ecd-9c62-1246b078e534", "tags": \["tag3", "tag4"\], "modified": "20180317T092629Z", "start": "20180317T092629Z", "entry": "20180317T092629Z", "description": "dummy"\}
+""")
+        self.assertEqual('', err)
+
+    def test_hook_should_process_modify_project(self):
+        """on-modify hook should process 'task modify' for changing project"""
+
+        self.t("start dummy proj1")
+        out, err = self.process.communicate(input="""\
+{"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","start":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534","project":"proj1"}
+{"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","start":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534","project":"proj2"}
+""")
+
+        self.assertRegexpMatches(out, """\
+Removed dummy proj1 from @1
+Added dummy proj2 to @1
+\{"status": "pending", "uuid": "3422d76c-c087-4ecd-9c62-1246b078e534", "modified": "20180317T092629Z", "project": "proj2", "start": "20180317T092629Z", "entry": "20180317T092629Z", "description": "dummy"\}
+""")
+        self.assertEqual('', err)
+
+    def test_hook_should_process_modify_not_tracked(self):
+        """on-modify hook should process 'task modify' for tasks which are not time-tracked"""
+        out, err = self.process.communicate(input="""\
+{"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534","project":"proj1"}
+{"description":"dummy","entry":"20180317T092629Z","modified":"20180317T092629Z","status":"pending","uuid":"3422d76c-c087-4ecd-9c62-1246b078e534","project":"proj2"}
+""")
+
+        self.assertEqual('{"status": "pending", "uuid": "3422d76c-c087-4ecd-9c62-1246b078e534", "modified": "20180317T092629Z", "project": "proj2", "entry": "20180317T092629Z", "description": "dummy"}\n', out)
+        self.assertEqual('', err)
+
+    # This is needed to test the case for task which are time-tracked and modify is called for changes other than project/tag/description
+    def test_hook_should_process_modify_due_date(self):
+        """on-modify hook should process 'task modify' for time-tracked task for changing annotations"""
+
+        self.t("start dummy")
+        out, err = self.process.communicate(input="""\
+{"status": "pending", "description": "dummy", "modified": "20180513T073915Z", "due": "20180514T000000Z", "start": "20180513T073915Z", "entry": "20180513T054634Z", "uuid": "09e9620e-8906-47ad-92c5-305894534aac"}
+{"status": "pending", "description": "dummy", "modified": "20180513T073915Z", "due": "20180512T000000Z", "start": "20180513T073915Z", "entry": "20180513T054634Z", "uuid": "09e9620e-8906-47ad-92c5-305894534aac"}
+""")
+
+        self.assertEqual('{"status": "pending", "description": "dummy", "due": "20180512T000000Z", "modified": "20180513T073915Z", "start": "20180513T073915Z", "entry": "20180513T054634Z", "uuid": "09e9620e-8906-47ad-92c5-305894534aac"}\n', out)
+
         self.assertEqual('', err)
 
     def test_hook_should_process_prepend(self):