From 6f9d2ae273120a93dd3aa2fdf14508d186ddce00 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Sun, 20 Dec 2015 23:36:23 -0600 Subject: [PATCH] Prevent duplicate tag creation Closes #163 --- .../astrid/actfm/TagSettingsActivity.java | 65 ++++--- .../todoroo/astrid/tags/TagsControlSet.java | 158 +++++++++--------- src/main/res/layout/tag_settings_activity.xml | 13 ++ src/main/res/values/strings.xml | 1 + 4 files changed, 136 insertions(+), 101 deletions(-) diff --git a/src/main/java/com/todoroo/astrid/actfm/TagSettingsActivity.java b/src/main/java/com/todoroo/astrid/actfm/TagSettingsActivity.java index ac34a207c..ef2b39d3a 100644 --- a/src/main/java/com/todoroo/astrid/actfm/TagSettingsActivity.java +++ b/src/main/java/com/todoroo/astrid/actfm/TagSettingsActivity.java @@ -13,10 +13,14 @@ import android.os.Bundle; import android.support.v4.graphics.drawable.DrawableCompat; import android.support.v7.app.ActionBar; import android.support.v7.widget.Toolbar; +import android.text.Editable; +import android.text.TextWatcher; import android.view.Menu; import android.view.MenuItem; +import android.view.View; import android.view.inputmethod.InputMethodManager; import android.widget.EditText; +import android.widget.TextView; import com.todoroo.andlib.sql.Criterion; import com.todoroo.andlib.utility.AndroidUtilities; @@ -58,6 +62,7 @@ public class TagSettingsActivity extends InjectingAppCompatActivity { @Bind(R.id.tag_name) EditText tagName; @Bind(R.id.toolbar) Toolbar toolbar; + @Bind(R.id.tag_error) TextView tagError; @Override protected void onCreate(Bundle savedInstanceState) { @@ -90,38 +95,54 @@ public class TagSettingsActivity extends InjectingAppCompatActivity { tagName.setText(autopopulateName); getIntent().removeExtra(TOKEN_AUTOPOPULATE_NAME); } + + tagName.addTextChangedListener(new TextWatcher() { + @Override + public void beforeTextChanged(CharSequence s, int start, int count, int after) { + } + + @Override + public void onTextChanged(CharSequence s, int start, int before, int count) { + } + + @Override + public void afterTextChanged(Editable s) { + tagError.setVisibility(clashes() ? View.VISIBLE : View.GONE); + } + }); + } + + private String getNewName() { + return tagName.getText().toString().trim(); + } + + private boolean clashes() { + String newName = getNewName(); + TagData existing = tagDataDao.getTagByName(newName, TagData.PROPERTIES); + return existing != null && tagData.getId() != existing.getId(); } private void save() { String oldName = tagData.getName(); - String newName = tagName.getText().toString().trim(); + String newName = getNewName(); if (isEmpty(newName)) { return; } - boolean nameChanged = !oldName.equals(newName); - if (nameChanged) { - if (oldName.equalsIgnoreCase(newName)) { // Change the capitalization of a list manually - tagData.setName(newName); - tagService.rename(tagData.getUuid(), newName); - } else { // Rename list--check for existing name - newName = tagService.getTagWithCase(newName); - tagName.setText(newName); - if (!newName.equals(oldName)) { - tagData.setName(newName); - tagService.rename(tagData.getUuid(), newName); - } - } + if (clashes()) { + return; + } + if (isNewTag) { + tagData.setName(newName); tagDataDao.persist(tagData); - - if (isNewTag) { - setResult(RESULT_OK, new Intent().putExtra(TOKEN_NEW_FILTER, - TagFilterExposer.filterFromTagData(TagSettingsActivity.this, tagData))); - } else { - setResult(RESULT_OK, new Intent(AstridApiConstants.BROADCAST_EVENT_TAG_RENAMED).putExtra(TagViewFragment.EXTRA_TAG_UUID, tagData.getUuid())); - } + setResult(RESULT_OK, new Intent().putExtra(TOKEN_NEW_FILTER, TagFilterExposer.filterFromTagData(TagSettingsActivity.this, tagData))); + } else if (!oldName.equals(newName)) { + tagData.setName(newName); + tagService.rename(tagData.getUuid(), newName); + tagDataDao.persist(tagData); + setResult(RESULT_OK, new Intent(AstridApiConstants.BROADCAST_EVENT_TAG_RENAMED).putExtra(TagViewFragment.EXTRA_TAG_UUID, tagData.getUuid())); } finish(); @@ -185,7 +206,7 @@ public class TagSettingsActivity extends InjectingAppCompatActivity { } private void discard() { - String tagName = this.tagName.getText().toString().trim(); + String tagName = getNewName(); if ((isNewTag && isEmpty(tagName)) || (!isNewTag && tagData.getName().equals(tagName))) { finish(); diff --git a/src/main/java/com/todoroo/astrid/tags/TagsControlSet.java b/src/main/java/com/todoroo/astrid/tags/TagsControlSet.java index c231a0bc0..67c71e6c2 100644 --- a/src/main/java/com/todoroo/astrid/tags/TagsControlSet.java +++ b/src/main/java/com/todoroo/astrid/tags/TagsControlSet.java @@ -20,8 +20,12 @@ import android.widget.ListView; import android.widget.TextView; import android.widget.TextView.OnEditorActionListener; +import com.google.api.client.repackaged.com.google.common.base.Strings; +import com.google.common.base.Function; +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.todoroo.andlib.data.AbstractModel; -import com.todoroo.andlib.data.Callback; import com.todoroo.andlib.sql.Criterion; import com.todoroo.andlib.sql.Query; import com.todoroo.andlib.utility.DateUtilities; @@ -38,12 +42,18 @@ import org.tasks.dialogs.DialogBuilder; import org.tasks.preferences.ActivityPreferences; import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import javax.annotation.Nullable; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Lists.transform; +import static com.google.common.collect.Sets.difference; +import static com.google.common.collect.Sets.newHashSet; + /** * Control set to manage adding and removing tags * @@ -61,7 +71,6 @@ public final class TagsControlSet extends PopupControlSet { private LinearLayout newTags; private ListView selectedTags; private boolean populated = false; - private HashMap tagIndices; //private final LinearLayout tagsContainer; private final TextView tagsDisplay; @@ -78,34 +87,12 @@ public final class TagsControlSet extends PopupControlSet { tagsDisplay = (TextView) getView().findViewById(R.id.display_row_edit); } - private TagData[] getTagArray() { - List tagList = tagService.getTagList(); - return tagList.toArray(new TagData[tagList.size()]); - } - - private HashMap buildTagIndices(ArrayList tagNames) { - HashMap indices = new HashMap<>(); - for (int i = 0; i < tagNames.size(); i++) { - indices.put(tagNames.get(i), i); - } - return indices; - } - - private ArrayList getTagNames(TagData[] tags) { - ArrayList names = new ArrayList<>(); - for (TagData tag : tags) { - if (!names.contains(tag.getName())) { - names.add(tag.getName()); - } - } - return names; - } - private String buildTagString() { StringBuilder builder = new StringBuilder(); - LinkedHashSet tags = getTagSet(); - for (String tag : tags) { + List tagList = getTagList(); + Collections.sort(tagList); + for (String tag : tagList) { if (tag.trim().length() == 0) { continue; } @@ -118,41 +105,50 @@ public final class TagsControlSet extends PopupControlSet { return builder.toString(); } - private void setTagSelected(String tag) { - Integer index = tagIndices.get(tag); - if (index != null) { + int index = allTagNames.indexOf(tag); + if (index >= 0) { selectedTags.setItemChecked(index, true); } else { allTagNames.add(tag); - tagIndices.put(tag, allTagNames.size() - 1); ((ArrayAdapter)selectedTags.getAdapter()).notifyDataSetChanged(); } } - private LinkedHashSet getTagSet() { - LinkedHashSet tags = new LinkedHashSet<>(); + private List getTagList() { + Set tags = new LinkedHashSet<>(); if (initialized) { for(int i = 0; i < selectedTags.getAdapter().getCount(); i++) { if (selectedTags.isItemChecked(i)) { tags.add(allTagNames.get(i)); } } - - for(int i = 0; i < newTags.getChildCount(); i++) { + for (int i = newTags.getChildCount() - 1 ; i >= 0 ; i--) { TextView tagName = (TextView) newTags.getChildAt(i).findViewById(R.id.text1); - if(tagName.getText().length() == 0) { + final String text = tagName.getText().toString(); + if (Strings.isNullOrEmpty(text)) { continue; } - - tags.add(tagName.getText().toString()); + TagData tagByName = tagDataDao.getTagByName(text, TagData.PROPERTIES); + if (tagByName != null) { + setTagSelected(tagByName.getName()); + tags.add(tagByName.getName()); + newTags.removeViewAt(i); + } else if (!Iterables.any(tags, new Predicate() { + @Override + public boolean apply(@Nullable String input) { + return text.equalsIgnoreCase(input); + } + })) { + tags.add(text); + } } } else { if (model.getTransitory(TRANSITORY_TAGS) != null) { - return (LinkedHashSet) model.getTransitory(TRANSITORY_TAGS); + return (List) model.getTransitory(TRANSITORY_TAGS); } } - return tags; + return newArrayList(tags); } /** Adds a tag to the tag field */ @@ -245,7 +241,7 @@ public final class TagsControlSet extends PopupControlSet { public void readFromTask(Task task) { super.readFromTask(task); if(model.getId() != AbstractModel.NO_ID) { - model.putTransitory(TRANSITORY_TAGS, new LinkedHashSet<>(tagService.getTagNames(model.getId()))); + model.putTransitory(TRANSITORY_TAGS, tagService.getTagNames(model.getId())); refreshDisplayView(); } } @@ -271,7 +267,7 @@ public final class TagsControlSet extends PopupControlSet { } private void selectTagsFromModel() { - LinkedHashSet tags = (LinkedHashSet) model.getTransitory(TRANSITORY_TAGS); + List tags = (List) model.getTransitory(TRANSITORY_TAGS); if (tags != null) { for (String tag : tags) { setTagSelected(tag); @@ -281,16 +277,18 @@ public final class TagsControlSet extends PopupControlSet { @Override protected void afterInflate() { - TagData[] allTags = getTagArray(); - allTagNames = getTagNames(allTags); - tagIndices = buildTagIndices(allTagNames); + allTagNames = newArrayList(ImmutableSet.copyOf(transform(tagService.getTagList(), new Function() { + @Override + public String apply(TagData tagData) { + return tagData.getName(); + } + }))); selectedTags = (ListView) getDialogView().findViewById(R.id.existingTags); selectedTags.setAdapter(new ArrayAdapter<>(activity, R.layout.simple_list_item_multiple_choice_themed, allTagNames)); selectedTags.setChoiceMode(ListView.CHOICE_MODE_MULTIPLE); - - this.newTags = (LinearLayout) getDialogView().findViewById(R.id.newTags); + newTags = (LinearLayout) getDialogView().findViewById(R.id.newTags); } @Override @@ -300,9 +298,7 @@ public final class TagsControlSet extends PopupControlSet { return; } - LinkedHashSet tags = getTagSet(); - - synchronizeTags(task.getId(), task.getUUID(), tags); + synchronizeTags(task.getId(), task.getUUID()); Flags.set(Flags.TAGS_CHANGED); task.setModificationDate(DateUtilities.now()); } @@ -322,57 +318,61 @@ public final class TagsControlSet extends PopupControlSet { /** * Save the given array of tags into the database */ - private void synchronizeTags(long taskId, String taskUuid, Set tags) { + private void synchronizeTags(long taskId, String taskUuid) { Query query = Query.select(Metadata.PROPERTIES).where( Criterion.and( TaskToTagMetadata.TASK_UUID.eq(taskUuid), Metadata.DELETION_DATE.eq(0)) ); - final HashSet existingLinks = new HashSet<>(); - metadataDao.query(query, new Callback() { + Set existingTagIds = newHashSet(transform(metadataDao.toList(query), new Function() { @Override - public void apply(Metadata link) { - existingLinks.add(link.getValue(TaskToTagMetadata.TAG_UUID)); + public String apply(Metadata tagData) { + return tagData.getValue(TaskToTagMetadata.TAG_UUID); } - }); - + })); + List tags = getTagList(); + // create missing tags for (String tag : tags) { - if (tag.trim().length() == 0) { - continue; - } - TagData tagData = tagDataDao.getTagByName(tag, TagData.NAME, TagData.UUID); + TagData tagData = tagDataDao.getTagByName(tag, TagData.ID); if (tagData == null) { tagData = new TagData(); tagData.setName(tag); tagDataDao.persist(tagData); } - if (existingLinks.contains(tagData.getUUID())) { - existingLinks.remove(tagData.getUUID()); - } else { - Metadata newLink = TaskToTagMetadata.newTagMetadata(taskId, taskUuid, tag, tagData.getUUID()); + } + List selectedTags = newArrayList(transform(tags, new Function() { + @Override + public TagData apply(String tag) { + return tagDataDao.getTagByName(tag, TagData.PROPERTIES); + } + })); + deleteLinks(taskId, taskUuid, difference(existingTagIds, newHashSet(Iterables.transform(selectedTags, new Function() { + @Override + public String apply(TagData tagData) { + return tagData.getUUID(); + } + })))); + for (TagData tagData : selectedTags) { + if (!existingTagIds.contains(tagData.getUUID())) { + Metadata newLink = TaskToTagMetadata.newTagMetadata(taskId, taskUuid, tagData.getName(), tagData.getUUID()); metadataDao.createNew(newLink); } } - - // Mark as deleted links that don't exist anymore - deleteLinks(taskId, taskUuid, existingLinks.toArray(new String[existingLinks.size()])); } /** * Delete all links between the specified task and the list of tags */ - private void deleteLinks(long taskId, String taskUuid, String[] tagUuids) { + private void deleteLinks(long taskId, String taskUuid, Iterable tagUuids) { Metadata deleteTemplate = new Metadata(); deleteTemplate.setTask(taskId); // Need this for recording changes in outstanding table deleteTemplate.setDeletionDate(DateUtilities.now()); - if (tagUuids != null) { - for (String uuid : tagUuids) { - // TODO: Right now this is in a loop because each deleteTemplate needs the individual tagUuid in order to record - // the outstanding entry correctly. If possible, this should be improved to a single query - deleteTemplate.setValue(TaskToTagMetadata.TAG_UUID, uuid); // Need this for recording changes in outstanding table - metadataDao.update(Criterion.and(MetadataDao.MetadataCriteria.withKey(TaskToTagMetadata.KEY), Metadata.DELETION_DATE.eq(0), - TaskToTagMetadata.TASK_UUID.eq(taskUuid), TaskToTagMetadata.TAG_UUID.eq(uuid)), deleteTemplate); - } + for (String uuid : tagUuids) { + // TODO: Right now this is in a loop because each deleteTemplate needs the individual tagUuid in order to record + // the outstanding entry correctly. If possible, this should be improved to a single query + deleteTemplate.setValue(TaskToTagMetadata.TAG_UUID, uuid); // Need this for recording changes in outstanding table + metadataDao.update(Criterion.and(MetadataDao.MetadataCriteria.withKey(TaskToTagMetadata.KEY), Metadata.DELETION_DATE.eq(0), + TaskToTagMetadata.TASK_UUID.eq(taskUuid), TaskToTagMetadata.TAG_UUID.eq(uuid)), deleteTemplate); } } } diff --git a/src/main/res/layout/tag_settings_activity.xml b/src/main/res/layout/tag_settings_activity.xml index 27e9694cd..5163d1b54 100644 --- a/src/main/res/layout/tag_settings_activity.xml +++ b/src/main/res/layout/tag_settings_activity.xml @@ -45,6 +45,19 @@ android:textColor="?attr/asTextColorHint" android:textColorHint="?attr/asTextColorHint" android:textSize="15sp" /> + + diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index 33b59771c..2a51eb85d 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -137,6 +137,7 @@ Send anonymous usage statistics and crash reports to help improve Tasks. No personal data will be collected. Anonymous usage statistics are collected Opt-out + Tag already exists