Fix tag picker issues

* Fix NPE when there are duplicate tags
* New tags survive rotation
pull/437/head
Alex Baker 10 years ago
parent cf5676976c
commit 51152c5522

@ -7,6 +7,8 @@ package com.todoroo.astrid.tags;
import android.text.TextUtils; import android.text.TextUtils;
import com.google.common.base.Function;
import com.google.common.collect.Iterables;
import com.todoroo.andlib.data.Callback; import com.todoroo.andlib.data.Callback;
import com.todoroo.andlib.data.Property.CountProperty; import com.todoroo.andlib.data.Property.CountProperty;
import com.todoroo.andlib.sql.Criterion; import com.todoroo.andlib.sql.Criterion;
@ -27,6 +29,9 @@ import java.util.List;
import javax.inject.Inject; import javax.inject.Inject;
import javax.inject.Singleton; import javax.inject.Singleton;
import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Lists.transform;
/** /**
* Provides operations for working with tags * Provides operations for working with tags
* *
@ -82,6 +87,34 @@ public final class TagService {
return tagDataDao.getByUuid(uuid, TagData.PROPERTIES); return tagDataDao.getByUuid(uuid, TagData.PROPERTIES);
} }
public List<TagData> getTagDataForTask(String uuid) {
List<Metadata> tags = metadataDao.toList(Query.select(TaskToTagMetadata.TAG_UUID).where(
Criterion.and(
MetadataCriteria.withKey(TaskToTagMetadata.KEY),
Metadata.DELETION_DATE.eq(0),
TaskToTagMetadata.TASK_UUID.eq(uuid))));
return newArrayList(Iterables.transform(tags, new Function<Metadata, TagData>() {
@Override
public TagData apply(Metadata metadata) {
return tagFromUUID(metadata.getValue(TaskToTagMetadata.TAG_UUID));
}
}));
}
public ArrayList<TagData> getTagDataForTask(long taskId) {
List<Metadata> tags = metadataDao.toList(Query.select(TaskToTagMetadata.TAG_UUID).where(
Criterion.and(
MetadataCriteria.withKey(TaskToTagMetadata.KEY),
Metadata.DELETION_DATE.eq(0),
MetadataCriteria.byTask(taskId))));
return newArrayList(transform(tags, new Function<Metadata, TagData>() {
@Override
public TagData apply(Metadata metadata) {
return tagFromUUID(metadata.getValue(TaskToTagMetadata.TAG_UUID));
}
}));
}
public ArrayList<String> getTagNames(long taskId) { public ArrayList<String> getTagNames(long taskId) {
Query query = Query.select(TaskToTagMetadata.TAG_NAME, TaskToTagMetadata.TAG_UUID).where( Query query = Query.select(TaskToTagMetadata.TAG_NAME, TaskToTagMetadata.TAG_UUID).where(
Criterion.and( Criterion.and(

@ -35,17 +35,16 @@ import android.widget.TextView.OnEditorActionListener;
import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Ordering; import com.google.common.collect.Ordering;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.todoroo.andlib.sql.Criterion; import com.todoroo.andlib.sql.Criterion;
import com.todoroo.andlib.sql.Query;
import com.todoroo.andlib.utility.DateUtilities; import com.todoroo.andlib.utility.DateUtilities;
import com.todoroo.astrid.dao.MetadataDao; import com.todoroo.astrid.dao.MetadataDao;
import com.todoroo.astrid.dao.TagDataDao; import com.todoroo.astrid.dao.TagDataDao;
import com.todoroo.astrid.data.Metadata; import com.todoroo.astrid.data.Metadata;
import com.todoroo.astrid.data.RemoteModel;
import com.todoroo.astrid.data.TagData; import com.todoroo.astrid.data.TagData;
import com.todoroo.astrid.data.Task; import com.todoroo.astrid.data.Task;
import com.todoroo.astrid.utility.Flags; import com.todoroo.astrid.utility.Flags;
@ -58,7 +57,6 @@ import org.tasks.themes.ThemeColor;
import org.tasks.ui.TaskEditControlFragment; import org.tasks.ui.TaskEditControlFragment;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
@ -68,7 +66,6 @@ import butterknife.BindView;
import butterknife.OnClick; import butterknife.OnClick;
import static com.google.common.base.Predicates.notNull; import static com.google.common.base.Predicates.notNull;
import static com.google.common.collect.Iterables.transform;
import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Sets.difference; import static com.google.common.collect.Sets.difference;
import static com.google.common.collect.Sets.filter; import static com.google.common.collect.Sets.filter;
@ -86,7 +83,8 @@ public final class TagsControlSet extends TaskEditControlFragment {
private static final char NO_BREAK_SPACE = '\u00a0'; private static final char NO_BREAK_SPACE = '\u00a0';
private static final char HAIR_SPACE = '\u200a'; private static final char HAIR_SPACE = '\u200a';
private static final String EXTRA_TAGS = "extra_tags"; private static final String EXTRA_NEW_TAGS = "extra_new_tags";
private static final String EXTRA_SELECTED_TAGS = "extra_selected_tags";
@Inject MetadataDao metadataDao; @Inject MetadataDao metadataDao;
@Inject TagDataDao tagDataDao; @Inject TagDataDao tagDataDao;
@ -97,12 +95,12 @@ public final class TagsControlSet extends TaskEditControlFragment {
@BindView(R.id.display_row_edit) TextView tagsDisplay; @BindView(R.id.display_row_edit) TextView tagsDisplay;
private long taskId; private long taskId;
private ArrayList<String> allTagNames; private LinearLayout newTagLayout;
private LinearLayout newTags; private ListView tagListView;
private ListView selectedTags;
private View dialogView; private View dialogView;
private AlertDialog dialog; private AlertDialog dialog;
private ArrayList<String> tagList; private List<TagData> allTags;
private ArrayList<TagData> selectedTags;
private final Ordering<TagData> orderByName = new Ordering<TagData>() { private final Ordering<TagData> orderByName = new Ordering<TagData>() {
@Override @Override
@ -131,7 +129,6 @@ public final class TagsControlSet extends TaskEditControlFragment {
} }
private CharSequence buildTagString() { private CharSequence buildTagString() {
Set<TagData> selectedTags = getSelectedTags(false);
List<TagData> sortedTagData = orderByName.sortedCopy(selectedTags); List<TagData> sortedTagData = orderByName.sortedCopy(selectedTags);
List<SpannableString> tagStrings = Lists.transform(sortedTagData, tagToString(Float.MAX_VALUE)); List<SpannableString> tagStrings = Lists.transform(sortedTagData, tagToString(Float.MAX_VALUE));
SpannableStringBuilder builder = new SpannableStringBuilder(); SpannableStringBuilder builder = new SpannableStringBuilder();
@ -148,22 +145,19 @@ public final class TagsControlSet extends TaskEditControlFragment {
@Override @Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
View view = super.onCreateView(inflater, container, savedInstanceState); View view = super.onCreateView(inflater, container, savedInstanceState);
ArrayList<String> newTags;
if (savedInstanceState != null) { if (savedInstanceState != null) {
tagList = savedInstanceState.getStringArrayList(EXTRA_TAGS); selectedTags = savedInstanceState.getParcelableArrayList(EXTRA_SELECTED_TAGS);
newTags = savedInstanceState.getStringArrayList(EXTRA_NEW_TAGS);
} else { } else {
tagList = tagService.getTagNames(taskId); selectedTags = tagService.getTagDataForTask(taskId);
newTags = newArrayList();
} }
final List<TagData> allTags = tagService.getTagList(); allTags = tagService.getTagList();
allTagNames = newArrayList(ImmutableSet.copyOf(transform(allTags, new Function<TagData, String>() {
@Override
public String apply(TagData tagData) {
return tagData.getName();
}
})));
dialogView = inflater.inflate(R.layout.control_set_tag_list, null); dialogView = inflater.inflate(R.layout.control_set_tag_list, null);
newTags = (LinearLayout) dialogView.findViewById(R.id.newTags); newTagLayout = (LinearLayout) dialogView.findViewById(R.id.newTags);
selectedTags = (ListView) dialogView.findViewById(R.id.existingTags); tagListView = (ListView) dialogView.findViewById(R.id.existingTags);
ArrayAdapter<TagData> adapter = new ArrayAdapter<TagData>(getActivity(), R.layout.simple_list_item_multiple_choice_themed, allTags) { tagListView.setAdapter(new ArrayAdapter<TagData>(getActivity(), R.layout.simple_list_item_multiple_choice_themed, allTags) {
@Override @Override
public View getView(int position, View convertView, ViewGroup parent) { public View getView(int position, View convertView, ViewGroup parent) {
CheckedTextView view = (CheckedTextView) super.getView(position, convertView, parent); CheckedTextView view = (CheckedTextView) super.getView(position, convertView, parent);
@ -176,10 +170,12 @@ public final class TagsControlSet extends TaskEditControlFragment {
view.setCompoundDrawablesWithIntrinsicBounds(drawable, null, null, null); view.setCompoundDrawablesWithIntrinsicBounds(drawable, null, null, null);
return view; return view;
} }
}; });
selectedTags.setAdapter(adapter); for (String newTag : newTags) {
addTag(newTag);
}
addTag(""); addTag("");
for (String tag : tagList) { for (TagData tag : selectedTags) {
setTagSelected(tag); setTagSelected(tag);
} }
refreshDisplayView(); refreshDisplayView();
@ -190,7 +186,8 @@ public final class TagsControlSet extends TaskEditControlFragment {
public void onSaveInstanceState(Bundle outState) { public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState); super.onSaveInstanceState(outState);
outState.putStringArrayList(EXTRA_TAGS, tagList); outState.putParcelableArrayList(EXTRA_SELECTED_TAGS, selectedTags);
outState.putStringArrayList(EXTRA_NEW_TAGS, getNewTags());
} }
@Override @Override
@ -225,51 +222,68 @@ public final class TagsControlSet extends TaskEditControlFragment {
.setOnDismissListener(new DialogInterface.OnDismissListener() { .setOnDismissListener(new DialogInterface.OnDismissListener() {
@Override @Override
public void onDismiss(DialogInterface dialogInterface) { public void onDismiss(DialogInterface dialogInterface) {
tagList = getTagList();
refreshDisplayView(); refreshDisplayView();
} }
}) })
.create(); .create();
} }
private void setTagSelected(String tag) { private void setTagSelected(TagData tag) {
int index = allTagNames.indexOf(tag); int index = allTags.indexOf(tag);
if (index >= 0) { if (index >= 0) {
selectedTags.setItemChecked(index, true); tagListView.setItemChecked(index, true);
} else {
allTagNames.add(tag);
((ArrayAdapter<String>)selectedTags.getAdapter()).notifyDataSetChanged();
} }
} }
private ArrayList<String> getTagList() { private boolean isSelected(List<TagData> selected, final String name) {
Set<String> tags = new LinkedHashSet<>(); return Iterables.any(selected, new Predicate<TagData>() {
for(int i = 0; i < selectedTags.getAdapter().getCount(); i++) { @Override
if (selectedTags.isItemChecked(i)) { public boolean apply(TagData input) {
tags.add(allTagNames.get(i)); return name.equalsIgnoreCase(input.getName());
}
});
}
private ArrayList<TagData> getSelectedTags() {
ArrayList<TagData> tags = new ArrayList<>();
for(int i = 0; i < tagListView.getAdapter().getCount(); i++) {
if (tagListView.isItemChecked(i)) {
tags.add(allTags.get(i));
} }
} }
for (int i = newTags.getChildCount() - 1 ; i >= 0 ; i--) { for (int i = newTagLayout.getChildCount() - 1; i >= 0 ; i--) {
TextView tagName = (TextView) newTags.getChildAt(i).findViewById(R.id.text1); TextView tagName = (TextView) newTagLayout.getChildAt(i).findViewById(R.id.text1);
final String text = tagName.getText().toString(); final String text = tagName.getText().toString();
if (Strings.isNullOrEmpty(text)) { if (Strings.isNullOrEmpty(text)) {
continue; continue;
} }
TagData tagByName = tagDataDao.getTagByName(text, TagData.PROPERTIES); TagData tagByName = tagDataDao.getTagByName(text, TagData.PROPERTIES);
if (tagByName != null) { if (tagByName != null) {
setTagSelected(tagByName.getName()); if (!isSelected(tags, text)) {
tags.add(tagByName.getName()); setTagSelected(tagByName);
newTags.removeViewAt(i); tags.add(tagByName);
} else if (!Iterables.any(tags, new Predicate<String>() {
@Override
public boolean apply(String input) {
return text.equalsIgnoreCase(input);
} }
})) { newTagLayout.removeViewAt(i);
tags.add(text); } else if (!isSelected(tags, text)) {
TagData newTag = new TagData();
newTag.setName(text);
tags.add(newTag);
}
}
return tags;
}
private ArrayList<String> getNewTags() {
ArrayList<String> tags = new ArrayList<>();
for (int i = newTagLayout.getChildCount() - 1 ; i >= 0 ; i--) {
TextView textView = (TextView) newTagLayout.getChildAt(i).findViewById(R.id.text1);
String tagName = textView.getText().toString();
if (Strings.isNullOrEmpty(tagName)) {
continue;
} }
tags.add(tagName);
} }
return newArrayList(tags); return tags;
} }
/** Adds a tag to the tag field */ /** Adds a tag to the tag field */
@ -278,8 +292,8 @@ public final class TagsControlSet extends TaskEditControlFragment {
// check if already exists // check if already exists
TextView lastText; TextView lastText;
for(int i = 0; i < newTags.getChildCount(); i++) { for(int i = 0; i < newTagLayout.getChildCount(); i++) {
View view = newTags.getChildAt(i); View view = newTagLayout.getChildAt(i);
lastText = (TextView) view.findViewById(R.id.text1); lastText = (TextView) view.findViewById(R.id.text1);
if(lastText.getText().equals(tagName)) { if(lastText.getText().equals(tagName)) {
return; return;
@ -288,7 +302,7 @@ public final class TagsControlSet extends TaskEditControlFragment {
final View tagItem; final View tagItem;
tagItem = inflater.inflate(R.layout.tag_edit_row, null); tagItem = inflater.inflate(R.layout.tag_edit_row, null);
newTags.addView(tagItem); newTagLayout.addView(tagItem);
if(tagName == null) { if(tagName == null) {
tagName = ""; //$NON-NLS-1$ tagName = ""; //$NON-NLS-1$
} }
@ -310,7 +324,7 @@ public final class TagsControlSet extends TaskEditControlFragment {
@Override @Override
public void onTextChanged(CharSequence s, int start, int before, public void onTextChanged(CharSequence s, int start, int before,
int count) { int count) {
if(count > 0 && newTags.getChildAt(newTags.getChildCount()-1) == if(count > 0 && newTagLayout.getChildAt(newTagLayout.getChildCount()-1) ==
tagItem) { tagItem) {
addTag(""); //$NON-NLS-1$ addTag(""); //$NON-NLS-1$
} }
@ -338,8 +352,8 @@ public final class TagsControlSet extends TaskEditControlFragment {
return; return;
} }
if(newTags.getChildCount() > 1) { if(newTagLayout.getChildCount() > 1) {
newTags.removeView(tagItem); newTagLayout.removeView(tagItem);
} else { } else {
textView.setText(""); //$NON-NLS-1$ textView.setText(""); //$NON-NLS-1$
} }
@ -351,10 +365,10 @@ public final class TagsControlSet extends TaskEditControlFragment {
* Get tags container last text view. might be null * Get tags container last text view. might be null
*/ */
private TextView getLastTextView() { private TextView getLastTextView() {
if(newTags.getChildCount() == 0) { if(newTagLayout.getChildCount() == 0) {
return null; return null;
} }
View lastItem = newTags.getChildAt(newTags.getChildCount()-1); View lastItem = newTagLayout.getChildAt(newTagLayout.getChildCount()-1);
return (TextView) lastItem.findViewById(R.id.text1); return (TextView) lastItem.findViewById(R.id.text1);
} }
@ -370,10 +384,13 @@ public final class TagsControlSet extends TaskEditControlFragment {
@Override @Override
public boolean hasChanges(Task original) { public boolean hasChanges(Task original) {
return !getExistingTags(original.getUUID()).equals(getSelectedTags(false)); Set<TagData> existingSet = newHashSet(tagService.getTagDataForTask(original.getUUID()));
Set<TagData> selectedSet = newHashSet(selectedTags);
return !existingSet.equals(selectedSet);
} }
protected void refreshDisplayView() { protected void refreshDisplayView() {
selectedTags = getSelectedTags();
CharSequence tagString = buildTagString(); CharSequence tagString = buildTagString();
if (TextUtils.isEmpty(tagString)) { if (TextUtils.isEmpty(tagString)) {
tagsDisplay.setText(R.string.tag_FEx_untagged); tagsDisplay.setText(R.string.tag_FEx_untagged);
@ -382,46 +399,19 @@ public final class TagsControlSet extends TaskEditControlFragment {
} }
} }
private Set<TagData> getSelectedTags(final boolean createMissingTags) {
return newHashSet(transform(tagList, new Function<String, TagData>() {
@Override
public TagData apply(String tagName) {
TagData tagData = tagDataDao.getTagByName(tagName, TagData.PROPERTIES);
if (tagData == null) {
// create missing tags
tagData = new TagData();
tagData.setName(tagName);
if (createMissingTags) {
tagDataDao.persist(tagData);
}
}
return tagData;
}
}));
}
private Set<TagData> getExistingTags(String taskUuid) {
Query query = Query.select(Metadata.PROPERTIES).where(
Criterion.and(
TaskToTagMetadata.TASK_UUID.eq(taskUuid),
Metadata.DELETION_DATE.eq(0))
);
return newHashSet(transform(metadataDao.toList(query), new Function<Metadata, TagData>() {
@Override
public TagData apply(Metadata metadata) {
return tagDataDao.getByUuid(metadata.getValue(TaskToTagMetadata.TAG_UUID));
}
}));
}
/** /**
* Save the given array of tags into the database * Save the given array of tags into the database
*/ */
private boolean synchronizeTags(long taskId, String taskUuid) { private boolean synchronizeTags(long taskId, String taskUuid) {
Set<TagData> existingTags = getExistingTags(taskUuid); for (TagData tagData : selectedTags) {
Set<TagData> selectedTags = getSelectedTags(true); if (RemoteModel.NO_UUID.equals(tagData.getUuid())) {
Sets.SetView<TagData> added = difference(selectedTags, existingTags); tagDataDao.persist(tagData);
Sets.SetView<TagData> removed = difference(existingTags, selectedTags); }
}
Set<TagData> existingHash = newHashSet(tagService.getTagDataForTask(taskUuid));
Set<TagData> selectedHash = newHashSet(selectedTags);
Sets.SetView<TagData> added = difference(selectedHash, existingHash);
Sets.SetView<TagData> removed = difference(existingHash, selectedHash);
deleteLinks(taskId, taskUuid, filter(removed, notNull())); deleteLinks(taskId, taskUuid, filter(removed, notNull()));
for (TagData tagData : added) { for (TagData tagData : added) {
Metadata newLink = TaskToTagMetadata.newTagMetadata(taskId, taskUuid, tagData.getName(), tagData.getUuid()); Metadata newLink = TaskToTagMetadata.newTagMetadata(taskId, taskUuid, tagData.getName(), tagData.getUuid());

Loading…
Cancel
Save