Merge branch 'rename-delete-tag' of git://192.168.10.106/astrid

README.md

@@ -18,7 +18,7 @@ Getting Started With Development
 
 3. Use **git** to clone your forked repository (see Github's instructions if you need help). Follow the [Github Line Ending Help](http://help.github.com/dealing-with-lineendings/)
 
-4. Clone the [astridApi](http://github.com/todoroo/astridApi) github project. This is an (Android Library Project)[http://developer.android.com/guide/developing/eclipse-adt.html#libraryProject] and so you will need ADT version 0.9.7+. Put the astridApi folder in the same root level folder as the astrid folder.
+4. Clone the [astridApi](http://github.com/todoroo/astridApi) github project. This is an [Android Library Project](http://developer.android.com/guide/developing/eclipse-adt.html#libraryProject) and so you will need ADT version 0.9.7+. Put the astridApi folder in the same root level folder as the astrid folder.
 
 4. Open up **eclipse** and import the *astrid*, *astridApi*, and *astrid-tests* projects. There should be no compilation errors. If there are, check the Android page of Eclipse Project Properties to verify the astridApi project was found.
 

astrid/AndroidManifest.xml

@@ -196,6 +196,8 @@
     </receiver>
     <activity android:name="com.todoroo.astrid.core.CustomFilterActivity" />
     <activity android:name="com.todoroo.astrid.core.CustomFilterExposer$DeleteActivity" />
+    <activity android:name="com.todoroo.astrid.tags.TagFilterExposer$DeleteTagActivity" />
+    <activity android:name="com.todoroo.astrid.tags.TagFilterExposer$RenameTagActivity" />
 
     <!-- alarms -->
     <receiver android:name="com.todoroo.astrid.alarms.AlarmTaskRepeatListener">

astrid/plugin-src/com/todoroo/astrid/tags/TagFilterExposer.java

@@ -7,17 +7,21 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.TreeSet;
 
-import android.content.BroadcastReceiver;
+import android.app.Activity;
-import android.content.ContentValues;
+import android.content.*;
-import android.content.Context;
-import android.content.Intent;
 import android.content.res.Resources;
 import android.graphics.Color;
 import android.graphics.drawable.BitmapDrawable;
 
+import android.os.Bundle;
+import android.widget.EditText;
+import android.widget.Toast;
 import com.timsu.astrid.R;
+import com.todoroo.andlib.service.Autowired;
+import com.todoroo.andlib.service.DependencyInjectionService;
 import com.todoroo.andlib.sql.Criterion;
 import com.todoroo.andlib.sql.QueryTemplate;
+import com.todoroo.andlib.utility.DialogUtilities;
 import com.todoroo.astrid.api.AstridApiConstants;
 import com.todoroo.astrid.api.Filter;
 import com.todoroo.astrid.api.FilterCategory;
@@ -35,6 +39,8 @@ import com.todoroo.astrid.tags.TagService.Tag;
  */
 public class TagFilterExposer extends BroadcastReceiver {
 
+    private static final String TAG = "tag";
+
     private TagService tagService;
 
     private Filter filterFromTag(Context context, Tag tag, Criterion criterion) {
@@ -51,18 +57,24 @@ public class TagFilterExposer extends BroadcastReceiver {
         if(tag.count == 0)
             filter.color = Color.GRAY;
 
-//        filters[0].contextMenuLabels = new String[] {
+        filter.contextMenuLabels = new String[] {
-//            "Rename Tag",
+            context.getString(R.string.tag_cm_rename),
-//            "Delete Tag"
+            context.getString(R.string.tag_cm_delete)
-//        };
+        };
-//        filters[0].contextMenuIntents = new Intent[] {
+        filter.contextMenuIntents = new Intent[] {
-//                new Intent(),
+                newTagIntent(context, RenameTagActivity.class, tag),
-//                new Intent()
+                newTagIntent(context, DeleteTagActivity.class, tag)
-//        };
+        };
 
         return filter;
     }
 
+    private Intent newTagIntent(Context context, Class<? extends Activity> activity, Tag tag) {
+        Intent ret = new Intent(context, activity);
+        ret.putExtra(TAG, tag.tag);
+        return ret;
+    }
+
     @Override
     public void onReceive(Context context, Intent intent) {
         tagService = TagService.getInstance();
@@ -81,15 +93,17 @@ public class TagFilterExposer extends BroadcastReceiver {
             @Override
             public int compare(Tag a, Tag b) {
                 if(a.count == b.count)
-                    return a.tag.toLowerCase().compareTo(b.tag.toLowerCase());
+                    return a.tag.compareTo(b.tag);
                 return b.count - a.count;
             }
         });
         for(Tag tag : tags) {
             if(!actives.containsKey(tag.tag))
                 tag.count = 0;
-            else
+            else {
+                // will decrease tag.count is there are tasks with this tag which are not activeAndVisible but also have not been deleted
                 tag.count = actives.get(tag.tag);
+            }
             sortedTagSet.add(tag);
         }
 
@@ -122,4 +136,115 @@ public class TagFilterExposer extends BroadcastReceiver {
         context.sendBroadcast(broadcastIntent, AstridApiConstants.PERMISSION_READ);
     }
 
+    public abstract static class TagActivity extends Activity {
+
+        protected String tag;
+
+        @Autowired
+        public TagService tagService;
+
+        protected TagActivity() {
+            DependencyInjectionService.getInstance().inject(this);
+        }
+
+        @Override
+        protected void onCreate(Bundle savedInstanceState) {
+            super.onCreate(savedInstanceState);
+            setTheme(android.R.style.Theme_Dialog);
+
+            tag = getIntent().getStringExtra(TAG);
+            if(tag == null) {
+                finish();
+                return;
+            }
+
+            DependencyInjectionService.getInstance().inject(this); // why?
+
+            showDialog();
+        }
+
+        protected DialogInterface.OnClickListener getOkListener() {
+            return new DialogInterface.OnClickListener() {
+                @Override
+                public void onClick(DialogInterface dialog, int which) {
+                    try {
+                        if (ok()) {
+                            setResult(RESULT_OK);
+                        } else {
+                            toastNoChanges();
+                            setResult(RESULT_CANCELED);
+                        }
+                    } finally {
+                        finish();
+                    }
+                }
+            };
+        }
+
+        protected DialogInterface.OnClickListener getCancelListener() {
+            return new DialogInterface.OnClickListener() {
+                @Override
+                public void onClick(DialogInterface dialog, int which) {
+                    try {
+                        toastNoChanges();
+                    } finally {
+                        setResult(RESULT_CANCELED);
+                        finish();
+                    }
+                }
+
+            };
+        }
+
+        private void toastNoChanges() {
+            Toast.makeText(this, R.string.TEA_no_tags_modified,
+                        Toast.LENGTH_SHORT).show();
+        }
+
+        protected abstract void showDialog();
+
+        protected abstract boolean ok();
+    }
+
+    public static class DeleteTagActivity extends TagActivity {
+
+        @Override
+        protected void showDialog() {
+            DialogUtilities.okCancelDialog(this, getString(R.string.DLG_delete_this_tag_question, tag), getOkListener(), getCancelListener());
+        }
+
+        @Override
+        protected boolean ok() {
+            int deleted = tagService.delete(tag);
+            Toast.makeText(this, getString(R.string.TEA_tags_deleted, tag, deleted),
+                    Toast.LENGTH_SHORT).show();
+            return true;
+        }
+
+    }
+
+    public static class RenameTagActivity extends TagActivity {
+
+        private EditText editor;
+
+        @Override
+        protected void showDialog() {
+            editor = new EditText(this); // not sure why this can't be done in the RenameTagActivity constructor.
+            DialogUtilities.viewDialog(this, getString(R.string.DLG_rename_this_tag_header, tag), editor, getOkListener(), getCancelListener());
+        }
+
+        @Override
+        protected boolean ok() { // this interface is not going to work well with the dialog that says "Are you sure?"
+            String text = editor.getText().toString();
+            if (text == null || text.length() == 0) {
+                return false;
+            } else {
+                int renamed = tagService.rename(tag, text);
+                Toast.makeText(this, getString(R.string.TEA_tags_renamed, tag, text, renamed),
+                        Toast.LENGTH_SHORT).show();
+                return true;
+            }
+        }
+    }
+
 }

astrid/plugin-src/com/todoroo/astrid/tags/TagService.java

@@ -8,11 +8,7 @@ import com.todoroo.andlib.data.Property.StringProperty;
 import com.todoroo.andlib.data.TodorooCursor;
 import com.todoroo.andlib.service.Autowired;
 import com.todoroo.andlib.service.DependencyInjectionService;
-import com.todoroo.andlib.sql.Criterion;
+import com.todoroo.andlib.sql.*;
-import com.todoroo.andlib.sql.Join;
-import com.todoroo.andlib.sql.Order;
-import com.todoroo.andlib.sql.Query;
-import com.todoroo.andlib.sql.QueryTemplate;
 import com.todoroo.astrid.core.PluginServices;
 import com.todoroo.astrid.dao.MetadataDao;
 import com.todoroo.astrid.dao.MetadataDao.MetadataCriteria;
@@ -20,6 +16,8 @@ import com.todoroo.astrid.dao.TaskDao.TaskCriteria;
 import com.todoroo.astrid.data.Metadata;
 import com.todoroo.astrid.data.Task;
 import com.todoroo.astrid.service.MetadataService;
+import com.todoroo.astrid.service.TaskService;
+import com.todoroo.astrid.utility.Flags;
 
 /**
  * Provides operations for working with tags
@@ -53,7 +51,10 @@ public final class TagService {
     @Autowired
     private MetadataDao metadataDao;
 
-    private TagService() {
+    @Autowired
+    private TaskService taskService;
+
+    public TagService() {
         DependencyInjectionService.getInstance().inject(this);
     }
 
@@ -92,12 +93,16 @@ public final class TagService {
          */
         public QueryTemplate queryTemplate(Criterion criterion) {
             return new QueryTemplate().join(Join.inner(Metadata.TABLE,
-                    Task.ID.eq(Metadata.TASK))).where(Criterion.and(
+                    Task.ID.eq(Metadata.TASK))).where(tagEq(tag, criterion));
-                            MetadataCriteria.withKey(KEY), TAG.eq(tag),
-                            criterion));
         }
+
     }
 
+    private static Criterion tagEq(String tag, Criterion additionalCriterion) {
+        return Criterion.and(
+                MetadataCriteria.withKey(KEY), TAG.eq(tag),
+                additionalCriterion);
+    }
     public QueryTemplate untaggedTemplate() {
         return new QueryTemplate().where(Criterion.and(
                 Criterion.not(Task.ID.in(Query.select(Metadata.TASK).from(Metadata.TABLE).where(MetadataCriteria.withKey(KEY)))),
@@ -195,4 +200,35 @@ public final class TagService {
 
         return service.synchronizeMetadata(taskId, metadata, Metadata.KEY.eq(KEY)) > 0;
     }
+
+    public int delete(String tag) {
+        invalidateTaskCache(tag);
+        return PluginServices.getMetadataService().deleteWhere(tagEq(tag, Criterion.all));
+    }
+
+    public int rename(String oldTag, String newTag) {
+        // First remove newTag from all tasks that have both oldTag and newTag.
+        MetadataService metadataService = PluginServices.getMetadataService();
+        metadataService.deleteWhere(
+                Criterion.and(
+                        Metadata.VALUE1.eq(newTag),
+                        Metadata.TASK.in(rowsWithTag(oldTag, Metadata.TASK))));
+
+        // Then rename all instances of oldTag to newTag.
+        Metadata metadata = new Metadata();
+        metadata.setValue(TAG, newTag);
+        int ret = metadataService.update(tagEq(oldTag, Criterion.all), metadata);
+        invalidateTaskCache(newTag);
+        return ret;
+    }
+
+    private Query rowsWithTag(String tag, Field... projections) {
+        return Query.select(projections).from(Metadata.TABLE).where(Metadata.VALUE1.eq(tag));
+    }
+
+    private void invalidateTaskCache(String tag) {
+        taskService.clearDetails(Task.ID.in(rowsWithTag(tag, Task.ID)));
+        Flags.set(Flags.REFRESH);
+    }
+
 }

astrid/res/values/strings-core.xml

@@ -321,7 +321,7 @@
   <!-- Preference: Task List Show Notes Description (disabled) -->
   <string name="EPr_showNotes_desc_disabled">Notes will be displayed when you tap a task</string>  
   <!-- Preference: Task List Show Notes Description (enabled) -->
-  <string name="EPr_showNotes_desc_enabled">Notes will always displayed</string>  
+  <string name="EPr_showNotes_desc_enabled">Notes will always be displayed</string>
   
   <!-- Preference Category: Defaults Title -->
   <string name="EPr_defaults_header">New Task Defaults</string>

astrid/res/values/strings-tags.xml

@@ -29,4 +29,25 @@
   <!-- %s => tag name -->
   <string name="tag_FEx_name">Tagged \'%s\'</string>
 
+  <!-- context menu option to rename a tag -->
+  <string name="tag_cm_rename">Rename Tag</string>
+
+  <!-- context menu option to delete a tag -->
+  <string name="tag_cm_delete">Delete Tag</string>
+
+  <!-- Dialog to confirm deletion of a tag -->
+  <string name="DLG_delete_this_tag_question">Delete this tag: %s? (No tasks will be deleted.)</string>
+
+  <!-- Dialog to rename tag -->
+  <string name="DLG_rename_this_tag_header">Rename the tag %s to:</string>
+
+  <!-- Toast notification that no changes have been made -->
+  <string name="TEA_no_tags_modified">No changes made</string>
+
+  <!-- Toast notification that a tag has been deleted -->
+  <string name="TEA_tags_deleted">Tag %s removed from %d tasks</string>
+
+  <!-- Toast notification that a tag has been renamed -->
+  <string name="TEA_tags_renamed">Replaced %s with %s on %d tasks</string>
+
 </resources>

astrid/src/com/todoroo/astrid/adapter/FilterAdapter.java

@@ -5,6 +5,8 @@ package com.todoroo.astrid.adapter;
 
 import java.util.ArrayList;
 import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
 import android.app.Activity;
 import android.content.BroadcastReceiver;
@@ -71,11 +73,14 @@ public class FilterAdapter extends BaseExpandableListAdapter {
     /** whether to skip Filters that launch intents instead of being real filters */
     private final boolean skipIntentFilters;
 
-    /** queue for loading list sizes */
+    // Previous solution involved a queue of filters and a filterSizeLoadingThread. The filterSizeLoadingThread had
-    private final LinkedBlockingQueue<Filter> filterQueue = new LinkedBlockingQueue<Filter>();
+    // a few problems: how to make sure that the thread is resumed when the controlling activity is resumed, and
-
+    // how to make sure that the the filterQueue does not accumulate filters without being processed. I am replacing
-    /** thread for loading list sizes */
+    // both the queue and a the thread with a thread pool, which will shut itself off after a second if it has
-    private Thread filterSizeLoadingThread = null;
+    // nothing to do (corePoolSize == 0, which makes it available for garbage collection), and will wake itself up
+    // if new filters are queued (obviously it cannot be garbage collected if it is possible for new filters to
+    // be added).
+    private ThreadPoolExecutor filterExecutor = new ThreadPoolExecutor(0, 1, 1, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>());
 
     public FilterAdapter(Activity activity, ExpandableListView listView,
             int rowLayout, boolean skipIntentFilters) {
@@ -95,34 +100,26 @@ public class FilterAdapter extends BaseExpandableListAdapter {
         activity.getWindowManager().getDefaultDisplay().getMetrics(metrics);
         listView.setGroupIndicator(
                 activity.getResources().getDrawable(R.drawable.expander_group));
-
-        startFilterSizeLoadingThread();
     }
 
-    private void startFilterSizeLoadingThread() {
+    private void offerFilter(final Filter filter) {
-        filterSizeLoadingThread = new Thread() {
+        filterExecutor.submit(new Runnable() {
             @Override
             public void run() {
-                while(true) {
+                try {
-                    try {
+                    int size = taskService.countTasks(filter);
-                        Filter filter = filterQueue.take();
+                    filter.listingTitle = filter.listingTitle + (" (" + //$NON-NLS-1$
-                        int size = taskService.countTasks(filter);
+                        size + ")"); //$NON-NLS-1$
-                        filter.listingTitle = filter.listingTitle + (" (" + //$NON-NLS-1$
+                    activity.runOnUiThread(new Runnable() {
-                            size + ")"); //$NON-NLS-1$
+                        public void run() {
-                        activity.runOnUiThread(new Runnable() {
+                            notifyDataSetInvalidated();
-                            public void run() {
+                        }
-                                notifyDataSetInvalidated();
+                    });
-                            }
+                } catch (Exception e) {
-                        });
+                    Log.e("astrid-filter-adapter", "Error loading filter size", e); //$NON-NLS-1$ //$NON-NLS-2$
-                    } catch (InterruptedException e) {
-                        break;
-                    } catch (Exception e) {
-                        Log.e("astrid-filter-adapter", "Error loading filter size", e); //$NON-NLS-1$ //$NON-NLS-2$
-                    }
                 }
             }
-        };
+        });
-        filterSizeLoadingThread.start();
     }
 
     public boolean hasStableIds() {
@@ -134,10 +131,10 @@ public class FilterAdapter extends BaseExpandableListAdapter {
 
         // load sizes
         if(item instanceof Filter) {
-            filterQueue.offer((Filter) item);
+            offerFilter((Filter)item);
         } else if(item instanceof FilterCategory) {
             for(Filter filter : ((FilterCategory)item).children)
-                filterQueue.offer(filter);
+                offerFilter(filter);
         }
     }
 
@@ -367,8 +364,6 @@ public class FilterAdapter extends BaseExpandableListAdapter {
      */
     public void unregisterRecevier() {
         activity.unregisterReceiver(filterReceiver);
-        if(filterSizeLoadingThread != null)
-            filterSizeLoadingThread.interrupt();
     }
 
     /**

astrid/src/com/todoroo/astrid/service/AstridDependencyInjector.java

@@ -14,6 +14,7 @@ import com.todoroo.astrid.dao.TaskDao;
 import com.todoroo.astrid.gtasks.GtasksListService;
 import com.todoroo.astrid.gtasks.GtasksMetadataService;
 import com.todoroo.astrid.gtasks.GtasksPreferenceService;
+import com.todoroo.astrid.tags.TagService;
 import com.todoroo.astrid.utility.Constants;
 
 /**
@@ -69,6 +70,9 @@ public class AstridDependencyInjector extends AbstractDependencyInjector {
         injectables.put("gtasksListService", GtasksListService.class);
         injectables.put("gtasksMetadataService", GtasksMetadataService.class);
 
+        // com.todoroo.astrid.tags
+        injectables.put("tagService", TagService.class);
+
         // these make reference to fields defined above
         injectables.put("errorReporters", new ErrorReporter[] {
                 new AndroidLogReporter(),

astrid/src/com/todoroo/astrid/service/MetadataService.java

@@ -62,8 +62,17 @@ public class MetadataService {
      * Delete from metadata table where rows match a certain condition
      * @param where
      */
-    public void deleteWhere(Criterion where) {
+    public int deleteWhere(Criterion where) {
-        metadataDao.deleteWhere(where);
+        return metadataDao.deleteWhere(where);
+    }
+
+    /**
+     * Delete from metadata table where rows match a certain condition
+     * @param where predicate for which rows to update
+     * @param metadata values to set
+     */
+    public int update(Criterion where, Metadata metadata) {
+        return metadataDao.update(where, metadata);
     }
 
     /**