Logic around expand step improved. Now work items are added to exapnd…

… step directly

WhiskWork.Core/Properties/AssemblyInfo.cs

@@ -1,10 +1,6 @@
 using System.Reflection;
-using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 
-// General Information about an assembly is controlled through the following 
-// set of attributes. Change these attribute values to modify the information
-// associated with an assembly.
 [assembly: AssemblyTitle("WhiskWork.Core")]
 [assembly: AssemblyDescription("")]
 [assembly: AssemblyConfiguration("")]

WhiskWork.Core/WorkItem.cs

@@ -164,13 +164,9 @@ public WorkItem ReplacesClasses(IEnumerable<string> newClasses)
 
         public WorkItem UpdatePropertiesAndOrdinalFrom(WorkItem item)
         {
-            var modifiedProperties = new NameValueCollection(_properties);
             var modifiedOrdinal = _ordinal;
 
-            foreach (var key in item.Properties.AllKeys)
-            {
-                modifiedProperties[key] = item.Properties[key];
-            }
+            var modifiedProperties = GetModifiedProperties(item.Properties);
 
             if(item.HasOrdinal)
             {
@@ -180,18 +176,35 @@ public WorkItem UpdatePropertiesAndOrdinalFrom(WorkItem item)
             return new WorkItem(Id, Path, Classes, Status, ParentId, modifiedOrdinal, modifiedProperties);
         }
 
+
         public WorkItem UpdateProperties(WorkItemProperties properties)
+        {
+            var modifiedProperties = GetModifiedProperties(properties);
+
+            return new WorkItem(Id, Path, Classes, Status, ParentId, _ordinal, modifiedProperties);
+        }
+
+        private NameValueCollection GetModifiedProperties(WorkItemProperties propertyUpdate)
         {
             var modifiedProperties = new NameValueCollection(_properties);
 
-            foreach (var key in properties.AllKeys)
+            foreach (var key in propertyUpdate.AllKeys)
             {
-                modifiedProperties[key] = properties[key];
+                var newValue = propertyUpdate[key];
+
+                if (string.IsNullOrEmpty(newValue))
+                {
+                    modifiedProperties.Remove(key);
+                }
+                else
+                {
+                    modifiedProperties[key] = newValue;
+                }
             }
-
-            return new WorkItem(Id, Path, Classes, Status, ParentId, _ordinal, modifiedProperties);
+            return modifiedProperties;
         }
 
+
         public WorkItem UpdateProperties(NameValueCollection properties)
         {
             var modifiedProperties = new NameValueCollection(_properties);

WhiskWork.Core/WorkItemCreator.cs

@@ -26,10 +26,7 @@ public void CreateWorkItem(WorkItem newWorkItem)
             WorkStep transientStep;
             if (WorkStepRepository.IsWithinTransientStep(leafStep, out transientStep))
             {
-                var workItems = WorkItemRepository.GetWorkItems(transientStep.Path);
-                Debug.Assert(workItems.Count() == 1);
-
-                var parentItem = workItems.ElementAt(0);
+                WorkItem parentItem = GetTransientParentWorkItem(transientStep);
                 WorkItemRepository.UpdateWorkItem(parentItem.UpdateStatus(WorkItemStatus.ExpandLocked));
 
                 newWorkItem = newWorkItem.MoveTo(leafStep).UpdateParent(parentItem);
@@ -54,6 +51,12 @@ public void CreateWorkItem(WorkItem newWorkItem)
 
             WorkItemRepository.CreateWorkItem(newWorkItem);
         }
-
+
+        private WorkItem GetTransientParentWorkItem(WorkStep transientStep)
+        {
+            var workItemId = transientStep.Path.Split(WorkStep.Separator).Last();
+
+            return WorkItemRepository.GetWorkItem(workItemId);
+        }
     }
 }

WhiskWork.Core/WorkItemMover.cs

@@ -32,7 +32,7 @@ public void MoveWorkItem(WorkItem workItem, WorkStep toStep)
 
             ThrowIfMovingExpandLockedWorkItem(transition);
 
-            ThrowIfMovingFromTransientStepToParallelStep(transition);
+            ThrowIfMovingFromExpandStepToParallelStep(transition);
 
             transition = CreateTransitionIfMovingToWithinParallelStep(transition);
 
@@ -42,7 +42,7 @@ public void MoveWorkItem(WorkItem workItem, WorkStep toStep)
 
             transition = CreateTransitionIfMovingToExpandStep(transition);
 
-            transition = CleanUpIfMovingFromTransientStep(transition);
+            transition = CleanUpIfMovingFromExpandStep(transition);
 
             transition = AttemptMergeIfMovingChildOfParallelledWorkItem(transition);
 
@@ -68,18 +68,18 @@ private void ThrowIfMovingExpandLockedWorkItem(WorkItemTransition transition)
             }
         }
 
-        private void ThrowIfMovingFromTransientStepToParallelStep(WorkItemTransition transition)
+        private void ThrowIfMovingFromExpandStepToParallelStep(WorkItemTransition transition)
         {
-            WorkStep transientStep;
+            WorkStep expandStep;
 
-            var isInTransientStep = WorkStepRepository.IsInTransientStep(transition.WorkItem, out transientStep);
+            var isInExpandStep = WorkStepRepository.IsInExpandStep(transition.WorkItem, out expandStep);
 
             WorkStep parallelStepRoot;
             var isWithinParallelStep = WorkStepRepository.IsWithinParallelStep(transition.WorkStep, out parallelStepRoot);
 
-            if (isInTransientStep && isWithinParallelStep)
+            if (isInExpandStep && isWithinParallelStep)
             {
-                throw new InvalidOperationException("Cannot move directly from transient step to parallelstep");
+                throw new InvalidOperationException("Cannot move directly from expand step to parallelstep");
             }
         }
 
@@ -120,15 +120,12 @@ private WorkItemTransition CreateTransitionIfMovingToExpandStep(WorkItemTransiti
         {
             if (WorkStepRepository.IsExpandStep(transition.WorkStep))
             {
-                var stepToMoveTo = CreateTransientWorkSteps(transition.WorkItem, transition.WorkStep);
-                var workItemToMove = transition.WorkItem.AddClass(stepToMoveTo.WorkItemClass);
-
-                transition = new WorkItemTransition(workItemToMove, stepToMoveTo);
+                CreateTransientWorkSteps(transition.WorkItem, transition.WorkStep);
             }
             return transition;
         }
 
-        private WorkItemTransition CleanUpIfMovingFromTransientStep(WorkItemTransition transition)
+        private WorkItemTransition CleanUpIfMovingFromExpandStep(WorkItemTransition transition)
         {
             var remover = new WorkItemRemover(WorkStepRepository, WorkItemRepository);
 
@@ -192,7 +189,7 @@ private WorkStep CreateTransientWorkSteps(WorkItem item, WorkStep expandStep)
         {
             Debug.Assert(expandStep.Type == WorkStepType.Expand);
 
-            var transientRootPath = WorkStep.CombinePath(expandStep.Path, item.Id);
+            var transientRootPath = ExpandedWorkStep.GetTransientPath(expandStep, item);
 
             CreateTransientWorkStepsRecursively(transientRootPath, expandStep, item.Id);
 
@@ -258,7 +255,7 @@ private bool IsChildOfExpandedWorkItem(WorkItem item)
             var parent = WorkItemRepository.GetWorkItem(item.ParentId);
             var workStep = WorkStepRepository.GetWorkStep(parent.Path);
 
-            return workStep.Type == WorkStepType.Transient;
+            return workStep.Type == WorkStepType.Expand;
         }
 
 

WhiskWork.Core/WorkItemRemover.cs

@@ -11,12 +11,15 @@ public WorkItemRemover(IWorkStepRepository workStepRepository, IWorkItemReposito
 
         public WorkItem CleanUpIfInTransientStep(WorkItem workItemToMove)
         {
-            WorkStep transientStep;
-            if (WorkStepRepository.IsInTransientStep(workItemToMove, out transientStep))
+            WorkStep expandStep;
+            if (WorkStepRepository.IsInExpandStep(workItemToMove, out expandStep))
             {
+                var transientStepPath = ExpandedWorkStep.GetTransientPath(expandStep, workItemToMove);
+                var transientStep = WorkStepRepository.GetWorkStep(transientStepPath);
+
                 DeleteChildWorkItems(workItemToMove);
                 WorkStepRepository.DeleteWorkStepsRecursively(transientStep);
-                workItemToMove = workItemToMove.RemoveClass(transientStep.WorkItemClass);
+                //workItemToMove = workItemToMove.RemoveClass(transientStep.WorkItemClass);
             }
             return workItemToMove;
         }

WhiskWork.Core/WorkStep.cs

@@ -5,10 +5,19 @@
 
 namespace WhiskWork.Core
 {
-    public class WorkStep
+    public static class ExpandedWorkStep
     {
-        public const string Separator = "/";
+        public static string GetTransientPath(WorkStep expandedWorkStep, WorkItem workItem)
+        {
+            return WorkStep.CombinePath(expandedWorkStep.Path, workItem.Id);
+        }
+    }
 
+    public class WorkStep
+    {
+        public const char Separator = '/';
+        private static readonly string _separator = new string(new [] {Separator});
+
         public WorkStep(string path, string parentPath, int ordinal, WorkStepType workStepType, string workItemClass) : this(path,parentPath,ordinal,workStepType,workItemClass,null, true)
         {
         }
@@ -41,16 +50,16 @@ public static WorkStep Root
         {
             get
             {
-                return new WorkStep(Separator, null, 0, WorkStepType.Normal, null,null,false);
+                return new WorkStep(_separator, null, 0, WorkStepType.Normal, null,null,false);
             }
         }
 
         public static string CombinePath(string path1, string path2)
         {
-            path1 = path1.EndsWith(Separator) ? path1.Remove(path1.Length - 1, 1) : path1;
-            path2 = path2.StartsWith(Separator) ? path2.Remove(0, 1) : path2;
+            path1 = path1.EndsWith(_separator) ? path1.Remove(path1.Length - 1, 1) : path1;
+            path2 = path2.StartsWith(_separator) ? path2.Remove(0, 1) : path2;
 
-            var result = path1 + Separator + path2;
+            var result = path1 + _separator + path2;
             return result;
         }
 
@@ -112,7 +121,7 @@ public override string ToString()
 
         private static void ThrowIfParentPathIsNotProperSubPathOfPath(string path, string parentPath)
         {
-            var separator = parentPath == Root.Path ? string.Empty : Separator;
+            var separator = parentPath == Root.Path ? string.Empty : _separator;
 
             var regex = new Regex(parentPath + separator + @"[a-z,A-Z,0-9,\-]+$");
             if (!regex.IsMatch(path))

WhiskWork.Core/WorkStepRepositoryExtensions.cs

@@ -71,19 +71,21 @@ public static bool IsWithinParallelStep(this IWorkStepRepository workStepReposit
             return TryLocateFirstAncestorStepOfType(workStepRepository, workStep, WorkStepType.Parallel, out parallelStepRoot);
         }
 
-        public static bool IsInTransientStep(this IWorkStepRepository workStepRepository, WorkItem workItem, out WorkStep transientStep)
+
+        public static bool IsInExpandStep(this IWorkStepRepository workStepRepository, WorkItem workItem, out WorkStep expandStep)
         {
-            transientStep = null;
-            bool isInTransientStep = workStepRepository.GetWorkStep(workItem.Path).Type == WorkStepType.Transient;
+            expandStep = null;
+            bool isInExpandStep = workStepRepository.GetWorkStep(workItem.Path).Type == WorkStepType.Expand;
 
-            if (isInTransientStep)
+            if (isInExpandStep)
             {
-                transientStep = workStepRepository.GetWorkStep(workItem.Path);
+                expandStep = workStepRepository.GetWorkStep(workItem.Path);
             }
 
-            return isInTransientStep;
+            return isInExpandStep;
         }
 
+
         public static bool IsExpandStep(this IWorkStepRepository workStepRepository, WorkStep step)
         {
             return step.Type == WorkStepType.Expand;

WhiskWork.UnitTest/ExpandWorkStepsTest.cs

@@ -48,12 +48,12 @@ public void ShouldNotCreateWorkItemInExpandStep()
         }
 
         [TestMethod]
-        public void ShouldMoveToTransientStepWhenEnteringExpandStep()
+        public void ShouldMoveToExpandStep()
         {
             _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
             _wp.UpdateWorkItem(WorkItem.New("cr1", "/development", new NameValueCollection()));
 
-            Assert.AreEqual("/development/inprocess/cr1", _workItemRepository.GetWorkItem("cr1").Path);
+            Assert.AreEqual("/development/inprocess", _workItemRepository.GetWorkItem("cr1").Path);
         }
 
         [TestMethod]
@@ -151,11 +151,11 @@ public void ShouldBeAbleToDeleteChildOfExpandedWorkItem()
         }
 
         [TestMethod]
-        public void ShouldMoveUnlockedExpandedWorkItemFromTransientStep()
+        public void ShouldMoveUnlockedExpandedWorkItemFromExpandStep()
         {
             _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
             _wp.UpdateWorkItem(WorkItem.New("cr1", "/development/inprocess", new NameValueCollection()));
-            Assert.AreEqual("/development/inprocess/cr1", _workItemRepository.GetWorkItem("cr1").Path);
+            Assert.AreEqual("/development/inprocess", _workItemRepository.GetWorkItem("cr1").Path);
 
             _wp.UpdateWorkItem(WorkItem.New("cr1", "/development/done", new NameValueCollection()));
             Assert.AreEqual("/development/done", _workItemRepository.GetWorkItem("cr1").Path);
@@ -170,10 +170,7 @@ public void ShouldNotMoveLockedExpandedWorkItemFromExpandStep()
             Assert.AreEqual(WorkItemStatus.ExpandLocked, _workItemRepository.GetWorkItem("cr1").Status);
 
             AssertUtils.AssertThrows<InvalidOperationException>(
-                () =>
-                    {
-                        _wp.UpdateWorkItem(WorkItem.New("cr1", "/development/done", new NameValueCollection()));
-                    });
+                () => _wp.UpdateWorkItem(WorkItem.New("cr1", "/development/done", new NameValueCollection())));
         }
 
         [TestMethod]
@@ -193,7 +190,7 @@ public void ShouldAlsoDeleteChildrenWhenDeletingExpandedWorkItem()
         }
 
         [TestMethod]
-        public void ShouldDeleteChildStepsWhenMovingFromTransientExpandStep()
+        public void ShouldDeleteChildStepsWhenMovingFromExpandStep()
         {
             _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
             _wp.UpdateWorkItem(WorkItem.New("cr1", "/development", new NameValueCollection()));
@@ -243,21 +240,6 @@ public void ShouldSetWorkItemClassOnTransientSteps()
                             _workflowRepository.GetWorkStep("/development/inprocess/cr1/tasks/done").WorkItemClass);
         }
 
-        [TestMethod]
-        public void ShouldNotMoveOtherWorkItemToTransientStep()
-        {
-            _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
-            _wp.UpdateWorkItem(WorkItem.New("cr1", "/development", new NameValueCollection()));
-            Assert.AreEqual("/development/inprocess/cr1", _workItemRepository.GetWorkItem("cr1").Path);
-
-            _wp.CreateWorkItem(WorkItem.New("cr2","/analysis"));
-            AssertUtils.AssertThrows<InvalidOperationException>(
-                () =>
-                    {
-                        _wp.UpdateWorkItem(WorkItem.New("cr2", "/development/inprocess/cr1", new NameValueCollection()));
-                    });
-        }
-
 
         [TestMethod]
         public void ShouldGiveChildOfExpandedItemCorrectClasses()
@@ -299,7 +281,7 @@ public void ShouldNotCreateWorkItemsBelowExpandStep()
         }
 
         [TestMethod]
-        public void ShouldNotMoveChildOfWorkItemInTransientStepDirectlyToExpandStep()
+        public void ShouldNotMoveChildOfWorkItemInExapndStepDirectlyToExpandStep()
         {
             _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
             _wp.UpdateWorkItem(WorkItem.New("cr1", "/development", new NameValueCollection()));
@@ -314,7 +296,7 @@ public void ShouldNotMoveChildOfWorkItemInTransientStepDirectlyToExpandStep()
         }
 
         [TestMethod]
-        public void ShouldNotMoveChildOfWorkItemInTransientStepToStepUnderExpandStep()
+        public void ShouldNotMoveChildOfWorkItemInExpandStepToStepUnderExpandStep()
         {
             _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
             _wp.UpdateWorkItem(WorkItem.New("cr1", "/development", new NameValueCollection()));
@@ -373,17 +355,6 @@ public void ShouldSetExpandLockWhenASingleDoneChildIsMovedToNotEndStep()
             Assert.AreEqual(WorkItemStatus.ExpandLocked, _workItemRepository.GetWorkItem("cr1").Status);
         }
 
-        [TestMethod]
-        public void ShouldRemoveTransientStepClassWhenMovedFromTransientStep()
-        {
-            _wp.CreateWorkItem(WorkItem.New("cr1","/analysis"));
-            _wp.UpdateWorkItem(WorkItem.New("cr1", "/development", new NameValueCollection()));
-
-            Assert.IsTrue(_workItemRepository.GetWorkItem("cr1").Classes.SetEquals("cr", "cr-cr1"));
-
-            _wp.UpdateWorkItem(WorkItem.New("cr1", "/done", new NameValueCollection()));
-            Assert.IsTrue(_workItemRepository.GetWorkItem("cr1").Classes.SetEquals("cr"));
-        }
 
         [TestMethod]
         public void ShouldDeleteChildItemsWhenWorkItemIsMovedOutOfTransientStep()