diff --git a/app/src/androidTest/java/org/tasks/data/ManualGoogleTaskQueryTest.kt b/app/src/androidTest/java/org/tasks/data/ManualGoogleTaskQueryTest.kt index 2f93b25e2..b1b8b0007 100644 --- a/app/src/androidTest/java/org/tasks/data/ManualGoogleTaskQueryTest.kt +++ b/app/src/androidTest/java/org/tasks/data/ManualGoogleTaskQueryTest.kt @@ -6,6 +6,7 @@ import com.todoroo.astrid.api.GtasksFilter import com.todoroo.astrid.dao.TaskDao import com.todoroo.astrid.helper.UUIDHelper import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -31,13 +32,11 @@ class ManualGoogleTaskQueryTest : InjectingTestCase() { @Inject lateinit var googleTaskDao: GoogleTaskDao @Inject lateinit var taskDao: TaskDao @Inject lateinit var preferences: Preferences - private lateinit var filter: GtasksFilter + private val filter: GtasksFilter = GtasksFilter(newGoogleTaskList(with(REMOTE_ID, "1234"))) @Before override fun setUp() { super.setUp() - filter = GtasksFilter(newGoogleTaskList(with(REMOTE_ID, "1234"))) - filter.setFilterQueryOverride(GtasksFilter.toManualOrder(filter.getSqlQuery())) preferences.clear() preferences.setBoolean(R.string.p_manual_sort, true) } @@ -88,6 +87,17 @@ class ManualGoogleTaskQueryTest : InjectingTestCase() { assertEquals(1, subtasks[2].secondarySort) } + @Test + fun ignoreDisableSubtasksPreference() { + preferences.setBoolean(R.string.p_disable_subtasks, true) + newTask(1, 0, 0) + newTask(2, 0, 1) + + val parent = query()[0] + + assertTrue(parent.hasChildren()) + } + private fun newTask(id: Long, order: Long, parent: Long = 0) { taskDao.insert(TaskMaker.newTask(with(ID, id), with(UUID, UUIDHelper.newUUID()))) googleTaskDao.insert(newGoogleTask(with(LIST, filter.list.remoteId), with(TASK, id), with(PARENT, parent), with(ORDER, order))) diff --git a/app/src/main/java/com/todoroo/astrid/adapter/GoogleTaskManualSortAdapter.kt b/app/src/main/java/com/todoroo/astrid/adapter/GoogleTaskManualSortAdapter.kt index 77b809e70..0182ac70c 100644 --- a/app/src/main/java/com/todoroo/astrid/adapter/GoogleTaskManualSortAdapter.kt +++ b/app/src/main/java/com/todoroo/astrid/adapter/GoogleTaskManualSortAdapter.kt @@ -8,23 +8,20 @@ import org.tasks.data.TaskContainer open class GoogleTaskManualSortAdapter internal constructor(val taskDao: TaskDao, val googleTaskDao: GoogleTaskDao) : TaskAdapter() { override fun canMove(source: TaskContainer, from: Int, target: TaskContainer, to: Int): Boolean { - if (!source.hasChildren() || to <= 0 || to >= count - 1) { - return true - } - return if (from < to) { - if (target.hasChildren()) { - return false + return if (!source.hasChildren() || to <= 0 || to >= count - 1) { + true + } else if (from < to) { + when { + target.hasChildren() -> false + target.hasParent() -> !getTask(to + 1).hasParent() + else -> true } - if (target.hasParent()) { - target.isLastSubtask - } else true } else { - if (target.hasChildren()) { - return true + when { + target.hasChildren() -> true + target.hasParent() -> target.parent == source.id && target.secondarySort == 0L + else -> true } - if (target.hasParent()) { - target.parent == source.id && target.secondarySort == 0L - } else true } } diff --git a/app/src/main/java/com/todoroo/astrid/adapter/TaskAdapterProvider.java b/app/src/main/java/com/todoroo/astrid/adapter/TaskAdapterProvider.java index b0e038eb1..c88ffb297 100644 --- a/app/src/main/java/com/todoroo/astrid/adapter/TaskAdapterProvider.java +++ b/app/src/main/java/com/todoroo/astrid/adapter/TaskAdapterProvider.java @@ -72,7 +72,9 @@ public class TaskAdapterProvider { GtasksFilter gtasksFilter = (GtasksFilter) filter; GoogleTaskList list = gtasksListService.getList(gtasksFilter.getStoreId()); if (list != null) { - return createGoogleTaskAdapter(gtasksFilter); + return preferences.isManualSort() + ? new GoogleTaskManualSortAdapter(taskDao, googleTaskDao) + : new GoogleTaskAdapter(taskDao, googleTaskDao, preferences.addGoogleTasksToTop()); } } else if (filter instanceof CaldavFilter) { CaldavFilter caldavFilter = (CaldavFilter) filter; @@ -102,14 +104,6 @@ public class TaskAdapterProvider { return new AstridTaskAdapter(list, filter, updater, taskDao); } - private TaskAdapter createGoogleTaskAdapter(GtasksFilter filter) { - if (preferences.isManualSort()) { - filter.setFilterQueryOverride(GtasksFilter.toManualOrder(filter.getSqlQuery())); - return new GoogleTaskManualSortAdapter(taskDao, googleTaskDao); - } - return new GoogleTaskAdapter(taskDao, googleTaskDao, preferences.addGoogleTasksToTop()); - } - private TaskAdapter createManualFilterTaskAdapter(Filter filter) { String filterId = null; String prefId = null; diff --git a/app/src/main/java/com/todoroo/astrid/api/GtasksFilter.java b/app/src/main/java/com/todoroo/astrid/api/GtasksFilter.java index 5f6445c23..6baf09525 100644 --- a/app/src/main/java/com/todoroo/astrid/api/GtasksFilter.java +++ b/app/src/main/java/com/todoroo/astrid/api/GtasksFilter.java @@ -49,28 +49,6 @@ public class GtasksFilter extends Filter { icon = list.getIcon(); } - public static String toManualOrder(String query) { - query = - query.replace( - "WHERE", - "JOIN (SELECT 0 as indent, google_tasks.*, COUNT(c.gt_id) AS children, 0 AS siblings, google_tasks.gt_order AS primary_sort, NULL AS secondary_sort" - + " FROM google_tasks" - + " LEFT JOIN google_tasks AS c ON c.gt_parent = google_tasks.gt_task" - + " WHERE google_tasks.gt_parent = 0 GROUP BY google_tasks.gt_task" - + " UNION SELECT 1 as indent, c.*, 0 AS children, COUNT(s.gt_id) AS siblings, p.gt_order AS primary_sort, c.gt_order AS secondary_sort" - + " FROM google_tasks AS c" - + " INNER JOIN google_tasks AS p ON c.gt_parent = p.gt_task" - + " INNER JOIN tasks ON c.gt_parent = tasks._id" - + " LEFT JOIN google_tasks AS s ON s.gt_parent = p.gt_task" - + " WHERE c.gt_parent > 0 AND ((tasks.completed=0) AND (tasks.deleted=0)" - + " AND tasks.collapsed = 0" - + " AND (tasks.hideUntil<(strftime('%s','now')*1000)))" - + " GROUP BY c.gt_task) as g2 ON g2.gt_id = google_tasks.gt_id WHERE"); - query = query.replaceAll("ORDER BY .*", ""); - query = query + "ORDER BY primary_sort ASC, secondary_sort ASC"; - return query; - } - private static QueryTemplate getQueryTemplate(GoogleTaskList list) { return new QueryTemplate() .join(Join.left(GoogleTask.TABLE, Task.ID.eq(GoogleTask.TASK))) diff --git a/app/src/main/java/com/todoroo/astrid/core/SortHelper.java b/app/src/main/java/com/todoroo/astrid/core/SortHelper.java index f3c197173..719f2c08d 100644 --- a/app/src/main/java/com/todoroo/astrid/core/SortHelper.java +++ b/app/src/main/java/com/todoroo/astrid/core/SortHelper.java @@ -29,6 +29,7 @@ public class SortHelper { public static final int SORT_IMPORTANCE = 3; public static final int SORT_MODIFIED = 4; public static final int SORT_CREATED = 5; + public static final int SORT_GTASKS = 6; private static final String ADJUSTED_DUE_DATE = "(CASE WHEN (dueDate / 1000) % 60 > 0 THEN dueDate ELSE (dueDate + 43140000) END)"; @@ -135,6 +136,9 @@ public class SortHelper { case SORT_CREATED: select = "tasks.created AS sort_created"; break; + case SORT_GTASKS: + select = "google_tasks.gt_order AS sort_manual"; + break; default: select ="(CASE WHEN (tasks.dueDate=0) " + // if no due date @@ -150,9 +154,9 @@ public class SortHelper { return select; } - public static Order orderForSortTypeRecursive(Preferences preferences) { + public static Order orderForSortTypeRecursive(int sortMode, boolean reverse) { Order order; - switch (preferences.getSortMode()) { + switch (sortMode) { case SORT_ALPHA: order = Order.asc("sort_title"); break; @@ -168,14 +172,17 @@ public class SortHelper { case SORT_CREATED: order = Order.desc("sort_created"); break; + case SORT_GTASKS: + order = Order.asc("sort_manual"); + break; default: order = Order.asc("sort_smart"); } - if (preferences.getSortMode() != SORT_ALPHA) { + if (sortMode != SORT_ALPHA) { order.addSecondaryExpression(Order.asc("sort_title")); } - if (preferences.isReverseSort()) { + if (reverse) { order = order.reverse(); } diff --git a/app/src/main/java/com/todoroo/astrid/subtasks/SubtasksHelper.java b/app/src/main/java/com/todoroo/astrid/subtasks/SubtasksHelper.java index 964f7d284..c0e596d5c 100644 --- a/app/src/main/java/com/todoroo/astrid/subtasks/SubtasksHelper.java +++ b/app/src/main/java/com/todoroo/astrid/subtasks/SubtasksHelper.java @@ -4,6 +4,7 @@ import static org.tasks.Strings.isNullOrEmpty; import static org.tasks.db.QueryUtils.showHidden; import android.content.Context; +import androidx.annotation.NonNull; import com.todoroo.astrid.api.Filter; import com.todoroo.astrid.api.GtasksFilter; import com.todoroo.astrid.core.BuiltInFilterExposer; @@ -125,31 +126,28 @@ public class SubtasksHelper { return map; } - public boolean shouldUseSubtasksFragmentForFilter(Filter filter) { - return preferences.isManualSort() && filter != null && filter.supportsManualSort(); + public boolean shouldUseSubtasksFragmentForFilter(@NonNull Filter filter) { + return filter.supportsManualSort() + && preferences.isManualSort() + && !(filter instanceof GtasksFilter); } public String applySubtasksToWidgetFilter(Filter filter, String query) { if (shouldUseSubtasksFragmentForFilter(filter)) { - - if (filter instanceof GtasksFilter) { - query = GtasksFilter.toManualOrder(query); - } else { - TagData tagData = tagDataDao.getTagByName(filter.listingTitle); - TaskListMetadata tlm = null; - if (tagData != null) { - tlm = taskListMetadataDao.fetchByTagOrFilter(tagData.getRemoteId()); - } else if (BuiltInFilterExposer.isInbox(context, filter)) { - tlm = taskListMetadataDao.fetchByTagOrFilter(TaskListMetadata.FILTER_ID_ALL); - } else if (BuiltInFilterExposer.isTodayFilter(context, filter)) { - tlm = taskListMetadataDao.fetchByTagOrFilter(TaskListMetadata.FILTER_ID_TODAY); - } - - query = query.replaceAll("ORDER BY .*", ""); - query = query + String.format(" ORDER BY %s", getOrderString(tagData, tlm)); - query = showHidden(query); + TagData tagData = tagDataDao.getTagByName(filter.listingTitle); + TaskListMetadata tlm = null; + if (tagData != null) { + tlm = taskListMetadataDao.fetchByTagOrFilter(tagData.getRemoteId()); + } else if (BuiltInFilterExposer.isInbox(context, filter)) { + tlm = taskListMetadataDao.fetchByTagOrFilter(TaskListMetadata.FILTER_ID_ALL); + } else if (BuiltInFilterExposer.isTodayFilter(context, filter)) { + tlm = taskListMetadataDao.fetchByTagOrFilter(TaskListMetadata.FILTER_ID_TODAY); } + query = query.replaceAll("ORDER BY .*", ""); + query = query + String.format(" ORDER BY %s", getOrderString(tagData, tlm)); + query = showHidden(query); + filter.setFilterQueryOverride(query); } return query; diff --git a/app/src/main/java/org/tasks/data/TaskContainer.java b/app/src/main/java/org/tasks/data/TaskContainer.java index 53499cae3..883622af2 100644 --- a/app/src/main/java/org/tasks/data/TaskContainer.java +++ b/app/src/main/java/org/tasks/data/TaskContainer.java @@ -11,7 +11,6 @@ public class TaskContainer { @Embedded public Location location; public String tags; public int children; - public int siblings; public long primarySort; public long secondarySort; public int indent; @@ -96,7 +95,6 @@ public class TaskContainer { } TaskContainer that = (TaskContainer) o; return children == that.children - && siblings == that.siblings && primarySort == that.primarySort && secondarySort == that.secondarySort && indent == that.indent @@ -111,7 +109,7 @@ public class TaskContainer { @Override public int hashCode() { return Objects - .hash(task, googletask, caldavTask, location, tags, children, siblings, primarySort, + .hash(task, googletask, caldavTask, location, tags, children, primarySort, secondarySort, indent, targetIndent); } @@ -131,8 +129,6 @@ public class TaskContainer { + '\'' + ", children=" + children - + ", siblings=" - + siblings + ", primarySort=" + primarySort + ", secondarySort=" @@ -174,10 +170,6 @@ public class TaskContainer { return children > 0; } - public boolean isLastSubtask() { - return secondarySort == siblings - 1; - } - public SubsetGoogleTask getGoogleTask() { return googletask; } diff --git a/app/src/main/java/org/tasks/data/TaskListQuery.java b/app/src/main/java/org/tasks/data/TaskListQuery.java index 11d17e126..0d4646312 100644 --- a/app/src/main/java/org/tasks/data/TaskListQuery.java +++ b/app/src/main/java/org/tasks/data/TaskListQuery.java @@ -50,7 +50,6 @@ public class TaskListQuery { private static final Field PLACE = field("places.*"); private static final Field CALDAV = field(CALDAV_METADATA_JOIN + ".*"); private static final Field CHILDREN = field("children"); - private static final Field SIBLINGS = field("siblings"); private static final Field PRIMARY_SORT = field("primary_sort").as("primarySort"); private static final Field SECONDARY_SORT = field("secondary_sort").as("secondarySort"); private static final Field INDENT = field("indent"); @@ -71,13 +70,15 @@ public class TaskListQuery { private static final List FIELDS = ImmutableList.of(TASKS, GTASK, CALDAV, GEOFENCE, PLACE); public static List getQuery( - Preferences preferences, - com.todoroo.astrid.api.Filter filter, - SubtaskInfo subtasks) { - if (filter.supportSubtasks() - && subtasks.usesSubtasks() - && preferences.showSubtasks() - && !(preferences.isManualSort() && filter.supportsManualSort())) { + Preferences preferences, com.todoroo.astrid.api.Filter filter, SubtaskInfo subtasks) { + + if (filter.supportsManualSort() && preferences.isManualSort()) { + return subtasks.usesSubtasks() && filter instanceof GtasksFilter + ? getRecursiveQuery(filter, preferences, subtasks) + : getNonRecursiveQuery(filter, preferences); + } + + if (filter.supportSubtasks() && subtasks.usesSubtasks() && preferences.showSubtasks()) { return getRecursiveQuery(filter, preferences, subtasks); } else { return getNonRecursiveQuery(filter, preferences); @@ -92,6 +93,8 @@ public class TaskListQuery { fields.add(TAG_QUERY); fields.add(INDENT); fields.add(CHILDREN); + fields.add(PRIMARY_SORT); + fields.add(SECONDARY_SORT); String joinedQuery = Join.inner(RECURSIVE, Task.ID.eq(RECURSIVE_TASK)) @@ -153,20 +156,33 @@ public class TaskListQuery { } joinedQuery += where; - String sortSelect = SortHelper.orderSelectForSortTypeRecursive(preferences.getSortMode()); + boolean manualSort = preferences.isManualSort(); + boolean manualGtasks = manualSort && filter instanceof GtasksFilter; + int sortMode; + if (manualGtasks) { + sortMode = SortHelper.SORT_GTASKS; + } else { + sortMode = preferences.getSortMode(); + } + boolean reverseSort = preferences.isReverseSort() && sortMode != SortHelper.SORT_GTASKS; + String sortSelect = SortHelper.orderSelectForSortTypeRecursive(sortMode); String withClause = "CREATE TEMPORARY TABLE `recursive_tasks` AS\n" - + "WITH RECURSIVE recursive_tasks (task, parent, collapsed, hidden, indent, title, sortField) AS (\n" + + "WITH RECURSIVE recursive_tasks (task, parent, collapsed, hidden, indent, title, sortField, primary_sort, secondary_sort) AS (\n" + " SELECT tasks._id, 0 as parent, tasks.collapsed as collapsed, 0 as hidden, 0 AS sort_indent, UPPER(tasks.title) AS sort_title, " + sortSelect + + (manualGtasks ? ", google_tasks.gt_order as primary_sort" : ", NULL as primary_sort") + + ", NULL as secondary_sort" + " FROM tasks\n" + parentQuery + "\nUNION ALL SELECT tasks._id, recursive_tasks.task as parent, tasks.collapsed as collapsed, CASE WHEN recursive_tasks.collapsed > 0 OR recursive_tasks.hidden > 0 THEN 1 ELSE 0 END as hidden, recursive_tasks.indent+1 AS sort_indent, UPPER(tasks.title) AS sort_title, " + sortSelect + + ", recursive_tasks.primary_sort as primary_sort" + + (manualGtasks ? ", google_tasks.gt_order as secondary_sort" : ", NULL as secondary_sort") + " FROM tasks\n" + subtaskQuery + "\nORDER BY sort_indent DESC, " - + SortHelper.orderForSortTypeRecursive(preferences) + + SortHelper.orderForSortTypeRecursive(sortMode, reverseSort) + ") SELECT * FROM recursive_tasks"; return newArrayList( @@ -184,13 +200,6 @@ public class TaskListQuery { List fields = new ArrayList<>(FIELDS); fields.add(TAGS); - if (filter instanceof GtasksFilter && preferences.isManualSort()) { - fields.add(INDENT); - fields.add(CHILDREN); - fields.add(SIBLINGS); - fields.add(PRIMARY_SORT); - fields.add(SECONDARY_SORT); - } // TODO: For now, we'll modify the query to join and include the things like tag data here. // Eventually, we might consider restructuring things so that this query is constructed // elsewhere.