From 64f73b44d951f6f7137e4e648ab9c172d1fc550d Mon Sep 17 00:00:00 2001
From: "Dustin J. Mitchell" <djmitche@google.com>
Date: Sun, 13 Oct 2024 11:49:06 -0400
Subject: [PATCH] Issue warnings instead of errors for 'weird' tasks

---
 src/Task.cpp               | 36 ++++++++++++++++++++----------------
 src/commands/CmdImport.cpp |  3 +++
 test/import.test.py        |  4 ++--
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/Task.cpp b/src/Task.cpp
index 6474c4384..df06addf0 100644
--- a/src/Task.cpp
+++ b/src/Task.cpp
@@ -1412,9 +1412,10 @@ void Task::substitute(const std::string& from, const std::string& to, const std:
 // The purpose of Task::validate is three-fold:
 //   1) To provide missing attributes where possible
 //   2) To provide suitable warnings about odd states
-//   3) To generate errors when the inconsistencies are not fixable
-//   4) To update status depending on other attributes
+//   3) To update status depending on other attributes
 //
+// As required by TaskChampion, no combination of properties and values is an
+// error. This function will try to make sensible defaults and resolve inconsistencies.
 // Critically, note that despite the name this is not a read-only function.
 //
 void Task::validate(bool applyDefault /* = true */) {
@@ -1428,7 +1429,10 @@ void Task::validate(bool applyDefault /* = true */) {
     Lexer lex(uid);
     std::string token;
     Lexer::Type type;
-    if (!lex.isUUID(token, type, true)) throw format("Not a valid UUID '{1}'.", uid);
+    // `uuid` is not a property in the TaskChampion model, so an invalid UUID is
+    // actually an error.
+    if (!lex.isUUID(token, type, true))
+      throw format("Not a valid UUID '{1}'.", uid);
   } else
     set("uuid", uuid());
 
@@ -1543,27 +1547,27 @@ void Task::validate(bool applyDefault /* = true */) {
   validate_before("scheduled", "due");
   validate_before("scheduled", "end");
 
-  // 3) To generate errors when the inconsistencies are not fixable
+  if (!has("description") || get("description") == "")
+    Context::getContext().footnote(format("Warning: task has no description."));
 
-  // There is no fixing a missing description.
-  if (!has("description"))
-    throw std::string("A task must have a description.");
-  else if (get("description") == "")
-    throw std::string("Cannot add a task that is blank.");
-
-  // Cannot have a recur frequency with no due date - when would it recur?
-  if (has("recur") && (!has("due") || get("due") == ""))
-    throw std::string("A recurring task must also have a 'due' date.");
+  // Cannot have an old-style recur frequency with no due date - when would it recur?
+  if (has("recur") && (!has("due") || get("due") == "")) {
+    Context::getContext().footnote(format("Warning: recurring task has no due date."));
+    remove("recur");
+  }
 
-  // Recur durations must be valid.
+  // Old-style recur durations must be valid.
   if (has("recur")) {
     std::string value = get("recur");
     if (value != "") {
       Duration p;
       std::string::size_type i = 0;
-      if (!p.parse(value, i))
+      if (!p.parse(value, i)) {
         // TODO Ideal location to map unsupported old recurrence periods to supported values.
-        throw format("The recurrence value '{1}' is not valid.", value);
+        Context::getContext().footnote(
+            format("Warning: The recurrence value '{1}' is not valid.", value));
+        remove("recur");
+      }
     }
   }
 }
diff --git a/src/commands/CmdImport.cpp b/src/commands/CmdImport.cpp
index 8c7aa3486..31a2e141a 100644
--- a/src/commands/CmdImport.cpp
+++ b/src/commands/CmdImport.cpp
@@ -174,6 +174,9 @@ void CmdImport::importSingleTask(json::object* obj) {
   // Parse the whole thing, validate the data.
   Task task(obj);
 
+  // An empty task is probably not intentional - at least a UUID should be included.
+  if (task.is_empty()) throw format("Cannot import an empty task.");
+
   auto hasGeneratedEntry = not task.has("entry");
   auto hasExplicitEnd = task.has("end");
 
diff --git a/test/import.test.py b/test/import.test.py
index 7771345a3..1c709c23d 100755
--- a/test/import.test.py
+++ b/test/import.test.py
@@ -284,10 +284,10 @@ def setUp(self):
         self.t = Task()
 
     def test_import_empty_json(self):
-        """Verify empty JSON is caught"""
+        """Verify empty JSON is ignored"""
         j = "{}"
         code, out, err = self.t.runError("import", input=j)
-        self.assertIn("A task must have a description.", err)
+        self.assertIn("Cannot import an empty task.", err)
 
     def test_import_invalid_uuid(self):
         """Verify invalid UUID is caught"""