From d7d87f84ee0909f50201e0f13f1d1798bec87585 Mon Sep 17 00:00:00 2001 From: Tim Su Date: Tue, 5 Oct 2010 11:24:10 -0700 Subject: [PATCH] First pass at writing the rearrangling logic --- .../astrid/gtasks/GtasksListActivity.java | 3 +- .../astrid/gtasks/GtasksOrderAction.java | 61 ----- .../astrid/gtasks/GtasksTaskListUpdater.java | 213 +++++++----------- ...ionTest.java => GtasksTaskMovingTest.java} | 138 ++++++++---- 4 files changed, 179 insertions(+), 236 deletions(-) delete mode 100644 astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksOrderAction.java rename tests/src/com/todoroo/astrid/gtasks/{GtasksOrderActionTest.java => GtasksTaskMovingTest.java} (71%) diff --git a/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksListActivity.java b/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksListActivity.java index 3572042c7..59d3012f5 100644 --- a/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksListActivity.java +++ b/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksListActivity.java @@ -54,7 +54,8 @@ public class GtasksListActivity extends DraggableTaskListActivity { @Override public void drop(int from, int to) { long targetTaskId = taskAdapter.getItemId(from); - gtasksTaskListUpdater.move(listId, targetTaskId, to - from); + long destinationTaskId = taskAdapter.getItemId(to); + gtasksTaskListUpdater.moveTo(listId, targetTaskId, destinationTaskId); loadTaskListContent(true); } }; diff --git a/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksOrderAction.java b/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksOrderAction.java deleted file mode 100644 index d1e88c7e3..000000000 --- a/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksOrderAction.java +++ /dev/null @@ -1,61 +0,0 @@ -package com.todoroo.astrid.gtasks; - -import android.content.BroadcastReceiver; -import android.content.Context; -import android.content.Intent; - -import com.todoroo.andlib.service.Autowired; -import com.todoroo.andlib.service.ContextManager; -import com.todoroo.andlib.service.DependencyInjectionService; -import com.todoroo.astrid.api.AstridApiConstants; -import com.todoroo.astrid.data.Metadata; -import com.todoroo.astrid.utility.Flags; - -/** - * Context Menu actions for changing the order of a task - * @author Tim Su - * - */ -abstract public class GtasksOrderAction extends BroadcastReceiver { - - @Autowired private GtasksMetadataService gtasksMetadataService; - @Autowired private GtasksTaskListUpdater gtasksTaskListUpdater; - - abstract int getDelta(); - - @Override - public void onReceive(Context context, Intent intent) { - ContextManager.setContext(context); - DependencyInjectionService.getInstance().inject(this); - - long taskId = intent.getLongExtra(AstridApiConstants.EXTRAS_TASK_ID, -1); - if(taskId == -1) - return; - Metadata metadata = gtasksMetadataService.getTaskMetadata(taskId); - if(metadata == null) - return; - - metadata = gtasksMetadataService.getTaskMetadata(taskId); - - String listId = metadata.getValue(GtasksMetadata.LIST_ID); - gtasksTaskListUpdater.move(listId, taskId, getDelta()); - gtasksTaskListUpdater.correctMetadataForList(listId); - - Flags.set(Flags.REFRESH); - } - - public static class GtasksMoveUpAction extends GtasksOrderAction { - @Override - public int getDelta() { - return -1; - } - } - - public static class GtasksMoveDownAction extends GtasksOrderAction { - @Override - public int getDelta() { - return 1; - } - } - -} diff --git a/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksTaskListUpdater.java b/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksTaskListUpdater.java index 30a463343..fabc0cb4d 100644 --- a/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksTaskListUpdater.java +++ b/astrid/plugin-src/com/todoroo/astrid/gtasks/GtasksTaskListUpdater.java @@ -1,9 +1,10 @@ package com.todoroo.astrid.gtasks; +import java.util.ArrayList; import java.util.HashMap; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import android.text.TextUtils; @@ -36,22 +37,7 @@ public class GtasksTaskListUpdater { DependencyInjectionService.getInstance().inject(this); } - // --- used during normal ui operations - - public void debugPrint(String listId) { - StoreObject list = gtasksListService.getList(listId); - if(list == GtasksListService.LIST_NOT_FOUND_OBJECT) - return; - - iterateThroughList(list, new ListIterator() { - public void processTask(long taskId, Metadata metadata) { - System.err.format("%d: %d, indent:%d, parent:%d\n", taskId, //$NON-NLS-1$ - metadata.getValue(GtasksMetadata.ORDER), - metadata.getValue(GtasksMetadata.INDENT), - metadata.getValue(GtasksMetadata.PARENT_TASK)); - } - }); - } + // --- task indenting /** * Indent a task and all its children @@ -106,142 +92,117 @@ public class GtasksTaskListUpdater { }); } + // --- task moving + + private static class Node { + public long taskId; + public Node parent; + public ArrayList children = new ArrayList(); + + public Node(long taskId, Node parent) { + this.taskId = taskId; + this.parent = parent; + } + } + /** - * Move a task and all its children. - *

- * if moving up and first task in list or moving down and last, - * indents to same as task that we swapped with. - * - * @param delta # of positions to move + * Move a task and all its children to the position right above + * taskIdToMoveto. Will change the indent level to match taskIdToMoveTo. * + * @param newTaskId task we will move above. if -1, moves to end of list */ - public void move(String listId, final long targetTaskId, final int delta) { + public void moveTo(String listId, final long targetTaskId, final long moveBeforeTaskId) { StoreObject list = gtasksListService.getList(listId); if(list == GtasksListService.LIST_NOT_FOUND_OBJECT) return; - long taskToSwap = -1; - if(delta == -1) { - // use sibling / parent map to figure out prior task - updateParentSiblingMapsFor(list); - if(siblings.containsKey(targetTaskId) && siblings.get(targetTaskId) != -1L) - taskToSwap = siblings.get(targetTaskId); - else if(parents.containsKey(targetTaskId) && parents.get(targetTaskId) != -1L) - taskToSwap = parents.get(targetTaskId); + Node root = buildTreeModel(list); + Node target = findNode(root, targetTaskId); + + target.parent.children.remove(target); + + if(moveBeforeTaskId == -1) { + root.children.add(target); } else { - // walk through to find the next task - Filter filter = GtasksFilterExposer.filterFromList(list); - TodorooCursor cursor = PluginServices.getTaskService().fetchFiltered(filter.sqlQuery, null, Task.ID); - try { - int targetIndent = -1; - for(cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) { - long taskId = cursor.getLong(0); - - if(targetIndent != -1) { - Metadata metadata = gtasksMetadataService.getTaskMetadata(taskId); - if(metadata.getValue(GtasksMetadata.INDENT) <= targetIndent) { - taskToSwap = taskId; - break; - } - } else if(taskId == targetTaskId) { - Metadata metadata = gtasksMetadataService.getTaskMetadata(taskId); - targetIndent = metadata.getValue(GtasksMetadata.INDENT); - } - } - } finally { - cursor.close(); - } + Node sibling = findNode(root, moveBeforeTaskId); + int index = sibling.parent.children.indexOf(sibling); + sibling.parent.children.add(index, sibling); } - if(taskToSwap == -1L) - return; + traverseTreeAndWriteValues(root, new AtomicInteger(0), 0, new Task()); + } - if(delta == -1) { - moveUp(list, targetTaskId, taskToSwap); - } else { - // adjust indent of target task to task to swap - Metadata targetTask = gtasksMetadataService.getTaskMetadata(targetTaskId); - Metadata nextTask = gtasksMetadataService.getTaskMetadata(taskToSwap); - int targetIndent = targetTask.getValue(GtasksMetadata.INDENT); - int nextIndent = nextTask.getValue(GtasksMetadata.INDENT); - if(targetIndent != nextIndent) - indent(listId, targetTaskId, nextIndent - targetIndent); - moveUp(list, taskToSwap, targetTaskId); + private void traverseTreeAndWriteValues(Node node, AtomicInteger order, int indent, Task taskContainer) { + if(node.taskId != -1) { + Metadata metadata = gtasksMetadataService.getTaskMetadata(node.taskId); + metadata.setValue(GtasksMetadata.ORDER, order.getAndIncrement()); + metadata.setValue(GtasksMetadata.INDENT, indent); + if(PluginServices.getMetadataService().save(metadata)) + updateModifiedDate(taskContainer, node.taskId); + } + + for(Node child : node.children) { + traverseTreeAndWriteValues(child, order, indent + 1, taskContainer); } } - private void moveUp(StoreObject list, final long targetTaskId, final long priorTaskId) { - final AtomicInteger priorTaskOrder = new AtomicInteger(-1); - final AtomicInteger priorTaskIndent = new AtomicInteger(-1); - final AtomicInteger targetTaskOrder = new AtomicInteger(0); - final AtomicInteger targetTaskIndent = new AtomicInteger(-1); - final AtomicInteger tasksToMove = new AtomicInteger(1); - final AtomicBoolean finished = new AtomicBoolean(false); + private Node findNode(Node node, long taskId) { + if(node.taskId == taskId) + return node; + for(Node child : node.children) { + Node found = findNode(child, taskId); + if(found != null) + return found; + } + return null; + } + + private Node buildTreeModel(StoreObject list) { + final Node root = new Node(-1, null); + final AtomicInteger previoustIndent = new AtomicInteger(0); + final AtomicReference currentNode = new AtomicReference(root); - // step 1. calculate tasks to move iterateThroughList(list, new ListIterator() { @Override public void processTask(long taskId, Metadata metadata) { - if(finished.get() && priorTaskOrder.get() != -1) - return; - - if(taskId == priorTaskId) { - priorTaskIndent.set(metadata.getValue(GtasksMetadata.INDENT)); - priorTaskOrder.set(metadata.getValue(GtasksMetadata.ORDER)); - } else if(targetTaskId == taskId) { - targetTaskIndent.set(metadata.getValue(GtasksMetadata.INDENT)); - targetTaskOrder.set(metadata.getValue(GtasksMetadata.ORDER)); - } else if(targetTaskIndent.get() > -1) { - // found first task that is not beneath target - if(metadata.getValue(GtasksMetadata.INDENT) <= targetTaskIndent.get()) - finished.set(true); - else - tasksToMove.incrementAndGet(); + int indent = metadata.getValue(GtasksMetadata.INDENT); + + int previousIndentValue = previoustIndent.get(); + if(indent == previousIndentValue) { // sibling + Node parent = currentNode.get().parent; + currentNode.set(new Node(taskId, parent)); + parent.children.add(currentNode.get()); + } else if(indent > previousIndentValue) { // child + Node parent = currentNode.get(); + currentNode.set(new Node(taskId, parent)); + parent.children.add(currentNode.get()); + } else { // in a different tree + Node node = currentNode.get().parent; + for(int i = indent; i < previousIndentValue; i++) + node = node.parent; + currentNode.set(new Node(taskId, node)); + node.children.add(currentNode.get()); } } }); + return root; + } - final AtomicBoolean priorFound = new AtomicBoolean(false); - final AtomicBoolean targetFound = new AtomicBoolean(false); - final Task taskContainer = new Task(); - finished.set(false); + // --- utility + + public void debugPrint(String listId) { + StoreObject list = gtasksListService.getList(listId); + if(list == GtasksListService.LIST_NOT_FOUND_OBJECT) + return; - // step 2. swap the order of prior and our tasks iterateThroughList(list, new ListIterator() { - @Override public void processTask(long taskId, Metadata metadata) { - if(finished.get()) - return; - - if(targetTaskId == taskId) - targetFound.set(true); - else if(taskId == priorTaskId) - priorFound.set(true); - - if(targetFound.get()) { - if(targetTaskId != taskId && metadata.getValue(GtasksMetadata.INDENT) <= targetTaskIndent.get()) - finished.set(true); - else { - int newOrder = metadata.getValue(GtasksMetadata.ORDER) - - targetTaskOrder.get() + priorTaskOrder.get(); - int newIndent = metadata.getValue(GtasksMetadata.INDENT) - - targetTaskIndent.get() + priorTaskIndent.get(); - - metadata.setValue(GtasksMetadata.ORDER, newOrder); - metadata.setValue(GtasksMetadata.INDENT, newIndent); - PluginServices.getMetadataService().save(metadata); - updateModifiedDate(taskContainer, taskId); - } - } else if(priorFound.get()) { - int newOrder = metadata.getValue(GtasksMetadata.ORDER) + - tasksToMove.get(); - metadata.setValue(GtasksMetadata.ORDER, newOrder); - PluginServices.getMetadataService().save(metadata); - updateModifiedDate(taskContainer, taskId); - } + System.err.format("%d: %d, indent:%d, parent:%d\n", taskId, //$NON-NLS-1$ + metadata.getValue(GtasksMetadata.ORDER), + metadata.getValue(GtasksMetadata.INDENT), + metadata.getValue(GtasksMetadata.PARENT_TASK)); } }); - } private void updateModifiedDate(Task taskContainer, long taskId) { diff --git a/tests/src/com/todoroo/astrid/gtasks/GtasksOrderActionTest.java b/tests/src/com/todoroo/astrid/gtasks/GtasksTaskMovingTest.java similarity index 71% rename from tests/src/com/todoroo/astrid/gtasks/GtasksOrderActionTest.java rename to tests/src/com/todoroo/astrid/gtasks/GtasksTaskMovingTest.java index 5232ad9c0..fd21a58b6 100644 --- a/tests/src/com/todoroo/astrid/gtasks/GtasksOrderActionTest.java +++ b/tests/src/com/todoroo/astrid/gtasks/GtasksTaskMovingTest.java @@ -1,47 +1,43 @@ package com.todoroo.astrid.gtasks; -import android.content.BroadcastReceiver; -import android.content.Intent; - import com.todoroo.andlib.service.Autowired; -import com.todoroo.astrid.api.AstridApiConstants; import com.todoroo.astrid.core.PluginServices; import com.todoroo.astrid.data.Metadata; import com.todoroo.astrid.data.Task; -import com.todoroo.astrid.gtasks.GtasksOrderAction.GtasksMoveDownAction; -import com.todoroo.astrid.gtasks.GtasksOrderAction.GtasksMoveUpAction; import com.todoroo.astrid.test.DatabaseTestCase; import com.todoroo.gtasks.GoogleTaskListInfo; -public class GtasksOrderActionTest extends DatabaseTestCase { +public class GtasksTaskMovingTest extends DatabaseTestCase { @Autowired private GtasksListService gtasksListService; @Autowired private GtasksMetadataService gtasksMetadataService; + @Autowired private GtasksTaskListUpdater gtasksTaskListUpdater; private Task A, B, C, D, E, F; - public void testMoveUpFromListTop() { - givenTasksABCDEF(); - - whenTrigger(A, new GtasksMoveUpAction()); - - thenExpectMetadataIndentAndOrder(A, 0, 0); - thenExpectMetadataIndentAndOrder(B, 1, 1); - } + /* Starting State: + * + * A + * B + * C + * D + * E + * F + */ public void testMoveDownFromListBottom() { givenTasksABCDEF(); - whenTrigger(F, new GtasksMoveDownAction()); + whenTriggerMove(F, null); thenExpectMetadataIndentAndOrder(E, 4, 0); thenExpectMetadataIndentAndOrder(F, 5, 0); } - public void testMoveDownSimple() { + public void testMoveDownToListbottom() { givenTasksABCDEF(); - whenTrigger(E, new GtasksMoveDownAction()); + whenTriggerMove(E, null); thenExpectMetadataIndentAndOrder(E, 5, 0); thenExpectMetadataIndentAndOrder(F, 4, 00); @@ -50,7 +46,7 @@ public class GtasksOrderActionTest extends DatabaseTestCase { public void testMoveUpSimple() { givenTasksABCDEF(); - whenTrigger(F, new GtasksMoveUpAction()); + whenTriggerMove(F, E); thenExpectMetadataIndentAndOrder(E, 5, 0); thenExpectMetadataIndentAndOrder(F, 4, 00); @@ -59,7 +55,7 @@ public class GtasksOrderActionTest extends DatabaseTestCase { public void testMoveWithSubtasks() { givenTasksABCDEF(); - whenTrigger(C, new GtasksMoveUpAction()); + whenTriggerMove(C, B); /* * A @@ -72,44 +68,39 @@ public class GtasksOrderActionTest extends DatabaseTestCase { thenExpectMetadataIndentAndOrder(B, 3, 1); thenExpectMetadataIndentAndOrder(C, 1, 1); thenExpectMetadataIndentAndOrder(D, 2, 2); - - whenTrigger(C, new GtasksMoveDownAction()); - - thenExpectOriginalIndentAndOrder(); } public void testMoveThroughSubtasks() { givenTasksABCDEF(); - whenTrigger(B, new GtasksMoveDownAction()); + whenTriggerMove(B, E); /* * A * C * D - * B + * B + * E */ thenExpectMetadataIndentAndOrder(A, 0, 0); - thenExpectMetadataIndentAndOrder(B, 3, 1); + thenExpectMetadataIndentAndOrder(B, 3, 0); thenExpectMetadataIndentAndOrder(C, 1, 1); thenExpectMetadataIndentAndOrder(D, 2, 2); - - whenTrigger(B, new GtasksMoveUpAction()); - - thenExpectOriginalIndentAndOrder(); } public void testMoveUpAboveParent() { givenTasksABCDEF(); - whenTrigger(B, new GtasksMoveUpAction()); + whenTriggerMove(B, A); /* * B * A * C * D + * E + * F */ thenExpectMetadataIndentAndOrder(A, 1, 0); @@ -117,10 +108,10 @@ public class GtasksOrderActionTest extends DatabaseTestCase { thenExpectMetadataIndentAndOrder(C, 2, 1); } - public void testMoveDownFromChildren() { + public void testMoveDownWithChildren() { givenTasksABCDEF(); - whenTrigger(C, new GtasksMoveDownAction()); + whenTriggerMove(C, F); /* * A @@ -128,6 +119,7 @@ public class GtasksOrderActionTest extends DatabaseTestCase { * E * C * D + * F */ thenExpectMetadataIndentAndOrder(A, 0, 0); @@ -140,7 +132,7 @@ public class GtasksOrderActionTest extends DatabaseTestCase { public void testMoveDownIndentingTwice() { givenTasksABCDEF(); - whenTrigger(D, new GtasksMoveDownAction()); + whenTriggerMove(D, F); /* * A @@ -157,13 +149,72 @@ public class GtasksOrderActionTest extends DatabaseTestCase { thenExpectMetadataIndentAndOrder(E, 3, 0); } + public void testMoveUpMultiple() { + givenTasksABCDEF(); + + whenTriggerMove(C, A); + + /* + * C + * D + * A + * B + */ + + thenExpectMetadataIndentAndOrder(A, 2, 0); + thenExpectMetadataIndentAndOrder(B, 3, 1); + thenExpectMetadataIndentAndOrder(C, 0, 0); + thenExpectMetadataIndentAndOrder(D, 1, 1); + } + + public void testMoveUpIntoSublist() { + givenTasksABCDEF(); + + whenTriggerMove(F, D); + + /* + * A + * B + * C + * F + * D + */ + + thenExpectMetadataIndentAndOrder(A, 0, 0); + thenExpectMetadataIndentAndOrder(B, 1, 1); + thenExpectMetadataIndentAndOrder(C, 2, 1); + thenExpectMetadataIndentAndOrder(D, 4, 2); + thenExpectMetadataIndentAndOrder(E, 5, 0); + thenExpectMetadataIndentAndOrder(F, 3, 2); + } + + public void testMoveDownMultiple() { + givenTasksABCDEF(); + + whenTriggerMove(B, F); + + /* + * A + * C + * D + * E + * B + */ + + thenExpectMetadataIndentAndOrder(A, 0, 0); + thenExpectMetadataIndentAndOrder(B, 4, 0); + thenExpectMetadataIndentAndOrder(C, 1, 1); + thenExpectMetadataIndentAndOrder(D, 2, 2); + thenExpectMetadataIndentAndOrder(E, 3, 0); + thenExpectMetadataIndentAndOrder(F, 5, 0); + } + // --- helpers - private void whenTrigger(Task task, BroadcastReceiver action) { - Intent intent = new Intent(AstridApiConstants.ACTION_TASK_CONTEXT_MENU); - intent.putExtra(AstridApiConstants.EXTRAS_TASK_ID, task.getId()); - action.onReceive(getContext(), intent); + /** moveTo = null => move to end */ + private void whenTriggerMove(Task target, Task moveTo) { + gtasksTaskListUpdater.moveTo("1", target.getId(), moveTo == null ? -1 : moveTo.getId()); } private void thenExpectMetadataIndentAndOrder(Task task, int order, int indent) { @@ -173,15 +224,6 @@ public class GtasksOrderActionTest extends DatabaseTestCase { assertEquals("indentation", indent, (int)metadata.getValue(GtasksMetadata.INDENT)); } - private void thenExpectOriginalIndentAndOrder() { - thenExpectMetadataIndentAndOrder(A, 0, 0); - thenExpectMetadataIndentAndOrder(B, 1, 1); - thenExpectMetadataIndentAndOrder(C, 2, 1); - thenExpectMetadataIndentAndOrder(D, 3, 2); - thenExpectMetadataIndentAndOrder(E, 4, 0); - thenExpectMetadataIndentAndOrder(F, 5, 0); - } - @Override protected void setUp() throws Exception { super.setUp();