From 0f3ec582e98fff87988b829d704e1152f52d8d1f Mon Sep 17 00:00:00 2001 From: Ben Sima Date: Fri, 28 Nov 2025 04:01:07 -0500 Subject: Make task description a required field on create All tests pass. The changes are complete: **Summary of changes:** 1. **Omni/Task/Core.hs**: Changed `taskDescription` from `Maybe Text` to 2. **Omni/Task.hs**: Made `--description` required on CLI `create` (pani 3. **Omni/Task/RaceTest.hs**: Updated test to provide descriptions Task-Id: t-165 --- Omni/Task.hs | 82 +++++++++++++++++++++++++-------------------------- Omni/Task/Core.hs | 18 +++++------ Omni/Task/RaceTest.hs | 4 +-- 3 files changed, 51 insertions(+), 53 deletions(-) (limited to 'Omni') diff --git a/Omni/Task.hs b/Omni/Task.hs index c078a3e..5e0595b 100644 --- a/Omni/Task.hs +++ b/Omni/Task.hs @@ -180,8 +180,8 @@ move' args pure <| Just nsPath description <- case Cli.getArg args (Cli.longOption "description") of - Nothing -> pure Nothing - Just d -> pure <| Just (T.pack d) + Nothing -> panic "--description is required for task create" + Just d -> pure (T.pack d) createdTask <- createTask title taskType parent namespace priority deps description if isJsonMode args @@ -245,7 +245,7 @@ move' args taskNamespace = case maybeNamespace of Nothing -> taskNamespace task; Just ns -> Just ns, taskStatus = fromMaybe (taskStatus task) maybeStatus, taskPriority = fromMaybe (taskPriority task) maybePriority, - taskDescription = case maybeDesc of Nothing -> taskDescription task; Just d -> Just d, + taskDescription = fromMaybe (taskDescription task) maybeDesc, taskDependencies = fromMaybe (taskDependencies task) maybeDeps } @@ -439,60 +439,60 @@ unitTests = initTaskDb True Test.@?= True, Test.unit "can create task" <| do - task <- createTask "Test task" WorkTask Nothing Nothing P2 [] Nothing + task <- createTask "Test task" WorkTask Nothing Nothing P2 [] "Test description" taskTitle task Test.@?= "Test task" taskType task Test.@?= WorkTask taskStatus task Test.@?= Open taskPriority task Test.@?= P2 null (taskDependencies task) Test.@?= True, Test.unit "can create human task" <| do - task <- createTask "Human Task" HumanTask Nothing Nothing P2 [] Nothing + task <- createTask "Human Task" HumanTask Nothing Nothing P2 [] "Human task description" taskType task Test.@?= HumanTask, Test.unit "ready tasks exclude human tasks" <| do - task <- createTask "Human Task" HumanTask Nothing Nothing P2 [] Nothing + task <- createTask "Human Task" HumanTask Nothing Nothing P2 [] "Human task" ready <- getReadyTasks (taskId task `notElem` map taskId ready) Test.@?= True, Test.unit "ready tasks exclude draft tasks" <| do - task <- createTask "Draft Task" WorkTask Nothing Nothing P2 [] Nothing + task <- createTask "Draft Task" WorkTask Nothing Nothing P2 [] "Draft description" updateTaskStatus (taskId task) Draft [] ready <- getReadyTasks (taskId task `notElem` map taskId ready) Test.@?= True, Test.unit "can create task with description" <| do - task <- createTask "Test task" WorkTask Nothing Nothing P2 [] (Just "My description") - taskDescription task Test.@?= Just "My description", + task <- createTask "Test task" WorkTask Nothing Nothing P2 [] "My description" + taskDescription task Test.@?= "My description", Test.unit "can list tasks" <| do - _ <- createTask "Test task for list" WorkTask Nothing Nothing P2 [] Nothing + _ <- createTask "Test task for list" WorkTask Nothing Nothing P2 [] "List test" tasks <- listTasks Nothing Nothing Nothing Nothing not (null tasks) Test.@?= True, Test.unit "ready tasks exclude blocked ones" <| do - task1 <- createTask "First task" WorkTask Nothing Nothing P2 [] Nothing + task1 <- createTask "First task" WorkTask Nothing Nothing P2 [] "First description" let blockingDep = Dependency {depId = taskId task1, depType = Blocks} - task2 <- createTask "Blocked task" WorkTask Nothing Nothing P2 [blockingDep] Nothing + task2 <- createTask "Blocked task" WorkTask Nothing Nothing P2 [blockingDep] "Blocked description" ready <- getReadyTasks (taskId task1 `elem` map taskId ready) Test.@?= True (taskId task2 `notElem` map taskId ready) Test.@?= True, Test.unit "discovered-from dependencies don't block" <| do - task1 <- createTask "Original task" WorkTask Nothing Nothing P2 [] Nothing + task1 <- createTask "Original task" WorkTask Nothing Nothing P2 [] "Original" let discDep = Dependency {depId = taskId task1, depType = DiscoveredFrom} - task2 <- createTask "Discovered work" WorkTask Nothing Nothing P2 [discDep] Nothing + task2 <- createTask "Discovered work" WorkTask Nothing Nothing P2 [discDep] "Discovered" ready <- getReadyTasks -- Both should be ready since DiscoveredFrom doesn't block (taskId task1 `elem` map taskId ready) Test.@?= True (taskId task2 `elem` map taskId ready) Test.@?= True, Test.unit "related dependencies don't block" <| do - task1 <- createTask "Task A" WorkTask Nothing Nothing P2 [] Nothing + task1 <- createTask "Task A" WorkTask Nothing Nothing P2 [] "Task A description" let relDep = Dependency {depId = taskId task1, depType = Related} - task2 <- createTask "Task B" WorkTask Nothing Nothing P2 [relDep] Nothing + task2 <- createTask "Task B" WorkTask Nothing Nothing P2 [relDep] "Task B description" ready <- getReadyTasks -- Both should be ready since Related doesn't block (taskId task1 `elem` map taskId ready) Test.@?= True (taskId task2 `elem` map taskId ready) Test.@?= True, Test.unit "ready tasks exclude epics" <| do - epic <- createTask "Epic task" Epic Nothing Nothing P2 [] Nothing + epic <- createTask "Epic task" Epic Nothing Nothing P2 [] "Epic description" ready <- getReadyTasks (taskId epic `notElem` map taskId ready) Test.@?= True, Test.unit "ready tasks exclude tasks needing intervention (retry >= 3)" <| do - task <- createTask "Failing task" WorkTask Nothing Nothing P2 [] Nothing + task <- createTask "Failing task" WorkTask Nothing Nothing P2 [] "Failing description" ready1 <- getReadyTasks (taskId task `elem` map taskId ready1) Test.@?= True setRetryContext @@ -507,26 +507,26 @@ unitTests = ready2 <- getReadyTasks (taskId task `notElem` map taskId ready2) Test.@?= True, Test.unit "child task gets sequential ID" <| do - parent <- createTask "Parent" Epic Nothing Nothing P2 [] Nothing - child1 <- createTask "Child 1" WorkTask (Just (taskId parent)) Nothing P2 [] Nothing - child2 <- createTask "Child 2" WorkTask (Just (taskId parent)) Nothing P2 [] Nothing + parent <- createTask "Parent" Epic Nothing Nothing P2 [] "Parent epic" + child1 <- createTask "Child 1" WorkTask (Just (taskId parent)) Nothing P2 [] "Child 1 description" + child2 <- createTask "Child 2" WorkTask (Just (taskId parent)) Nothing P2 [] "Child 2 description" taskId child1 Test.@?= taskId parent <> ".1" taskId child2 Test.@?= taskId parent <> ".2", Test.unit "grandchild task gets sequential ID" <| do - parent <- createTask "Grandparent" Epic Nothing Nothing P2 [] Nothing - child <- createTask "Parent" Epic (Just (taskId parent)) Nothing P2 [] Nothing - grandchild <- createTask "Grandchild" WorkTask (Just (taskId child)) Nothing P2 [] Nothing + parent <- createTask "Grandparent" Epic Nothing Nothing P2 [] "Grandparent epic" + child <- createTask "Parent" Epic (Just (taskId parent)) Nothing P2 [] "Parent epic" + grandchild <- createTask "Grandchild" WorkTask (Just (taskId child)) Nothing P2 [] "Grandchild task" taskId grandchild Test.@?= taskId parent <> ".1.1", Test.unit "siblings of grandchild task get sequential ID" <| do - parent <- createTask "Grandparent" Epic Nothing Nothing P2 [] Nothing - child <- createTask "Parent" Epic (Just (taskId parent)) Nothing P2 [] Nothing - grandchild1 <- createTask "Grandchild 1" WorkTask (Just (taskId child)) Nothing P2 [] Nothing - grandchild2 <- createTask "Grandchild 2" WorkTask (Just (taskId child)) Nothing P2 [] Nothing + parent <- createTask "Grandparent" Epic Nothing Nothing P2 [] "Grandparent" + child <- createTask "Parent" Epic (Just (taskId parent)) Nothing P2 [] "Parent" + grandchild1 <- createTask "Grandchild 1" WorkTask (Just (taskId child)) Nothing P2 [] "Grandchild 1" + grandchild2 <- createTask "Grandchild 2" WorkTask (Just (taskId child)) Nothing P2 [] "Grandchild 2" taskId grandchild1 Test.@?= taskId parent <> ".1.1" taskId grandchild2 Test.@?= taskId parent <> ".1.2", Test.unit "child ID generation skips gaps" <| do - parent <- createTask "Parent with gaps" Epic Nothing Nothing P2 [] Nothing - child1 <- createTask "Child 1" WorkTask (Just (taskId parent)) Nothing P2 [] Nothing + parent <- createTask "Parent with gaps" Epic Nothing Nothing P2 [] "Parent with gaps" + child1 <- createTask "Child 1" WorkTask (Just (taskId parent)) Nothing P2 [] "Child 1" -- Manually create a task with .3 suffix to simulate a gap (or deleted task) let child3Id = taskId parent <> ".3" child3 = @@ -541,15 +541,15 @@ unitTests = taskDependencies = [], taskCreatedAt = taskCreatedAt child1, taskUpdatedAt = taskUpdatedAt child1, - taskDescription = Nothing + taskDescription = "Child 3" } saveTask child3 -- Create a new child, it should get .4, not .2 - child4 <- createTask "Child 4" WorkTask (Just (taskId parent)) Nothing P2 [] Nothing + child4 <- createTask "Child 4" WorkTask (Just (taskId parent)) Nothing P2 [] "Child 4" taskId child4 Test.@?= taskId parent <> ".4", Test.unit "can edit task" <| do - task <- createTask "Original Title" WorkTask Nothing Nothing P2 [] Nothing + task <- createTask "Original Title" WorkTask Nothing Nothing P2 [] "Original" let modifyFn t = t {taskTitle = "New Title", taskPriority = P0} updated <- editTask (taskId task) modifyFn taskTitle updated Test.@?= "New Title" @@ -562,7 +562,7 @@ unitTests = taskTitle reloaded Test.@?= "New Title" taskPriority reloaded Test.@?= P0, Test.unit "task lookup is case insensitive" <| do - task <- createTask "Case sensitive" WorkTask Nothing Nothing P2 [] Nothing + task <- createTask "Case sensitive" WorkTask Nothing Nothing P2 [] "Case sensitive description" let tid = taskId task upperTid = T.toUpper tid tasks <- loadTasks @@ -575,19 +575,19 @@ unitTests = validNs = Namespace.fromHaskellModule ns Namespace.toPath validNs Test.@?= "Omni/Task.hs", Test.unit "generated IDs are lowercase" <| do - task <- createTask "Lowercase check" WorkTask Nothing Nothing P2 [] Nothing + task <- createTask "Lowercase check" WorkTask Nothing Nothing P2 [] "Lowercase description" let tid = taskId task tid Test.@?= T.toLower tid -- check it matches regex for base36 (t-[0-9a-z]+) let isLowerBase36 = T.all (\c -> c `elem` ['0' .. '9'] ++ ['a' .. 'z'] || c == 't' || c == '-') tid isLowerBase36 Test.@?= True, Test.unit "dependencies are case insensitive" <| do - task1 <- createTask "Blocker" WorkTask Nothing Nothing P2 [] Nothing + task1 <- createTask "Blocker" WorkTask Nothing Nothing P2 [] "Blocker description" let tid1 = taskId task1 -- Use uppercase ID for dependency upperTid1 = T.toUpper tid1 dep = Dependency {depId = upperTid1, depType = Blocks} - task2 <- createTask "Blocked" WorkTask Nothing Nothing P2 [dep] Nothing + task2 <- createTask "Blocked" WorkTask Nothing Nothing P2 [dep] "Blocked description" -- task1 is Open, so task2 should NOT be ready ready <- getReadyTasks @@ -601,7 +601,7 @@ unitTests = Test.unit "can create task with lowercase ID" <| do -- This verifies that lowercase IDs are accepted and not rejected let lowerId = "t-lowercase" - let task = Task lowerId "Lower" WorkTask Nothing Nothing Open P2 [] Nothing (read "2025-01-01 00:00:00 UTC") (read "2025-01-01 00:00:00 UTC") + let task = Task lowerId "Lower" WorkTask Nothing Nothing Open P2 [] "Lower description" (read "2025-01-01 00:00:00 UTC") (read "2025-01-01 00:00:00 UTC") saveTask task tasks <- loadTasks case findTask lowerId tasks of @@ -609,7 +609,7 @@ unitTests = Nothing -> Test.assertFailure "Should find task with lowercase ID", Test.unit "generateId produces valid ID" <| do tid <- generateId - let task = Task tid "Auto" WorkTask Nothing Nothing Open P2 [] Nothing (read "2025-01-01 00:00:00 UTC") (read "2025-01-01 00:00:00 UTC") + let task = Task tid "Auto" WorkTask Nothing Nothing Open P2 [] "Auto description" (read "2025-01-01 00:00:00 UTC") (read "2025-01-01 00:00:00 UTC") saveTask task tasks <- loadTasks case findTask tid tasks of @@ -633,7 +633,7 @@ unitTests = Test.unit "lowercase ID does not clash with existing uppercase ID" <| do -- Setup: Create task with Uppercase ID let upperId = "t-UPPER" - let task1 = Task upperId "Upper Task" WorkTask Nothing Nothing Open P2 [] Nothing (read "2025-01-01 00:00:00 UTC") (read "2025-01-01 00:00:00 UTC") + let task1 = Task upperId "Upper Task" WorkTask Nothing Nothing Open P2 [] "Upper description" (read "2025-01-01 00:00:00 UTC") (read "2025-01-01 00:00:00 UTC") saveTask task1 -- Action: Try to create task with Lowercase ID (same letters) @@ -644,7 +644,7 @@ unitTests = -- treating them as the same is dangerous. let lowerId = "t-upper" - let task2 = Task lowerId "Lower Task" WorkTask Nothing Nothing Open P2 [] Nothing (read "2025-01-01 00:00:01 UTC") (read "2025-01-01 00:00:01 UTC") + let task2 = Task lowerId "Lower Task" WorkTask Nothing Nothing Open P2 [] "Lower description" (read "2025-01-01 00:00:01 UTC") (read "2025-01-01 00:00:01 UTC") saveTask task2 tasks <- loadTasks diff --git a/Omni/Task/Core.hs b/Omni/Task/Core.hs index 6d69834..18f80e2 100644 --- a/Omni/Task/Core.hs +++ b/Omni/Task/Core.hs @@ -35,7 +35,7 @@ data Task = Task taskStatus :: Status, taskPriority :: Priority, -- Priority level (0-4) taskDependencies :: [Dependency], -- List of dependencies with types - taskDescription :: Maybe Text, -- Optional detailed description + taskDescription :: Text, -- Required description taskCreatedAt :: UTCTime, taskUpdatedAt :: UTCTime } @@ -251,7 +251,7 @@ instance SQL.FromRow Task where <*> SQL.field <*> SQL.field <*> SQL.field - <*> SQL.field + <*> (fromMaybe "" SQL.field <*> SQL.field @@ -598,7 +598,7 @@ saveTask task = task -- Create a new task -createTask :: Text -> TaskType -> Maybe Text -> Maybe Text -> Priority -> [Dependency] -> Maybe Text -> IO Task +createTask :: Text -> TaskType -> Maybe Text -> Maybe Text -> Priority -> [Dependency] -> Text -> IO Task createTask title taskType parent namespace priority deps description = withTaskLock <| do let parent' = fmap normalizeId parent @@ -957,13 +957,11 @@ showTaskDetailed t = do putText "Dependencies:" traverse_ printDependency (taskDependencies t) - case taskDescription t of - Nothing -> pure () - Just desc -> do - putText "" - putText "Description:" - let indented = T.unlines <| map (" " <>) (T.lines desc) - putText indented + unless (T.null (taskDescription t)) <| do + putText "" + putText "Description:" + let indented = T.unlines <| map (" " <>) (T.lines (taskDescription t)) + putText indented putText "" where diff --git a/Omni/Task/RaceTest.hs b/Omni/Task/RaceTest.hs index 007046f..78410a4 100644 --- a/Omni/Task/RaceTest.hs +++ b/Omni/Task/RaceTest.hs @@ -28,7 +28,7 @@ raceTest = initTaskDb -- Create a parent epic - parent <- createTask "Parent Epic" Epic Nothing Nothing P2 [] Nothing + parent <- createTask "Parent Epic" Epic Nothing Nothing P2 [] "Parent Epic description" let parentId = taskId parent -- Create multiple children concurrently @@ -39,7 +39,7 @@ raceTest = -- Run concurrent creations children <- mapConcurrently - (\i -> createTask ("Child " <> tshow i) WorkTask (Just parentId) Nothing P2 [] Nothing) + (\i -> createTask ("Child " <> tshow i) WorkTask (Just parentId) Nothing P2 [] ("Child " <> tshow i <> " description")) indices -- Check for duplicates in generated IDs -- cgit v1.2.3