Address first round of review feedback for thread list view model migration

This commit is contained in:
Torsten Grote
2021-01-26 16:56:43 -03:00
parent e5d78a858d
commit 239c4a27ad
7 changed files with 38 additions and 48 deletions

View File

@@ -103,16 +103,16 @@ public class ForumActivity extends
// Handle presses on the action bar items // Handle presses on the action bar items
int itemId = item.getItemId(); int itemId = item.getItemId();
if (itemId == R.id.action_forum_share) { if (itemId == R.id.action_forum_share) {
Intent i2 = new Intent(this, ShareForumActivity.class); Intent i = new Intent(this, ShareForumActivity.class);
i2.setFlags(FLAG_ACTIVITY_CLEAR_TOP); i.setFlags(FLAG_ACTIVITY_CLEAR_TOP);
i2.putExtra(GROUP_ID, groupId.getBytes()); i.putExtra(GROUP_ID, groupId.getBytes());
startActivityForResult(i2, REQUEST_SHARE_FORUM); startActivityForResult(i, REQUEST_SHARE_FORUM);
return true; return true;
} else if (itemId == R.id.action_forum_sharing_status) { } else if (itemId == R.id.action_forum_sharing_status) {
Intent i3 = new Intent(this, ForumSharingStatusActivity.class); Intent i = new Intent(this, ForumSharingStatusActivity.class);
i3.setFlags(FLAG_ACTIVITY_CLEAR_TOP); i.setFlags(FLAG_ACTIVITY_CLEAR_TOP);
i3.putExtra(GROUP_ID, groupId.getBytes()); i.putExtra(GROUP_ID, groupId.getBytes());
startActivity(i3); startActivity(i);
return true; return true;
} else if (itemId == R.id.action_forum_delete) { } else if (itemId == R.id.action_forum_delete) {
showUnsubscribeDialog(); showUnsubscribeDialog();

View File

@@ -25,6 +25,7 @@ import androidx.lifecycle.ViewModelProvider;
import static android.view.View.GONE; import static android.view.View.GONE;
import static android.view.View.VISIBLE; import static android.view.View.VISIBLE;
import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull;
import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_GROUP_INVITE; import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_GROUP_INVITE;
import static org.briarproject.briar.android.util.UiUtils.observeOnce; import static org.briarproject.briar.android.util.UiUtils.observeOnce;
import static org.briarproject.briar.api.privategroup.PrivateGroupConstants.MAX_GROUP_POST_TEXT_LENGTH; import static org.briarproject.briar.api.privategroup.PrivateGroupConstants.MAX_GROUP_POST_TEXT_LENGTH;
@@ -110,26 +111,26 @@ public class GroupActivity extends
startActivity(i); startActivity(i);
return true; return true;
} else if (itemId == R.id.action_group_reveal) { } else if (itemId == R.id.action_group_reveal) {
if (viewModel.isCreator().getValue()) if (requireNonNull(viewModel.isCreator().getValue()))
throw new IllegalStateException(); throw new IllegalStateException();
Intent i = new Intent(this, RevealContactsActivity.class); Intent i = new Intent(this, RevealContactsActivity.class);
i.putExtra(GROUP_ID, groupId.getBytes()); i.putExtra(GROUP_ID, groupId.getBytes());
startActivity(i); startActivity(i);
return true; return true;
} else if (itemId == R.id.action_group_invite) { } else if (itemId == R.id.action_group_invite) {
if (!viewModel.isCreator().getValue()) if (!requireNonNull(viewModel.isCreator().getValue()))
throw new IllegalStateException(); throw new IllegalStateException();
Intent i = new Intent(this, GroupInviteActivity.class); Intent i = new Intent(this, GroupInviteActivity.class);
i.putExtra(GROUP_ID, groupId.getBytes()); i.putExtra(GROUP_ID, groupId.getBytes());
startActivityForResult(i, REQUEST_GROUP_INVITE); startActivityForResult(i, REQUEST_GROUP_INVITE);
return true; return true;
} else if (itemId == R.id.action_group_leave) { } else if (itemId == R.id.action_group_leave) {
if (viewModel.isCreator().getValue()) if (requireNonNull(viewModel.isCreator().getValue()))
throw new IllegalStateException(); throw new IllegalStateException();
showLeaveGroupDialog(); showLeaveGroupDialog();
return true; return true;
} else if (itemId == R.id.action_group_dissolve) { } else if (itemId == R.id.action_group_dissolve) {
if (!viewModel.isCreator().getValue()) if (!requireNonNull(viewModel.isCreator().getValue()))
throw new IllegalStateException(); throw new IllegalStateException();
showDissolveGroupDialog(); showDissolveGroupDialog();
return true; return true;
@@ -152,7 +153,8 @@ public class GroupActivity extends
@Override @Override
public void onReplyClick(GroupMessageItem item) { public void onReplyClick(GroupMessageItem item) {
if (!viewModel.isDissolved().getValue()) super.onReplyClick(item); Boolean isDissolved = viewModel.isDissolved().getValue();
if (isDissolved != null && !isDissolved) super.onReplyClick(item);
} }
private void setGroupEnabled(boolean enabled) { private void setGroupEnabled(boolean enabled) {
@@ -195,7 +197,7 @@ public class GroupActivity extends
viewModel.deletePrivateGroup(); viewModel.deletePrivateGroup();
} }
public void onGroupDissolved() { private void onGroupDissolved() {
AlertDialog.Builder builder = AlertDialog.Builder builder =
new AlertDialog.Builder(this, R.style.BriarDialogTheme); new AlertDialog.Builder(this, R.style.BriarDialogTheme);
builder.setTitle(getString(R.string.groups_dissolved_dialog_title)); builder.setTitle(getString(R.string.groups_dissolved_dialog_title));

View File

@@ -104,6 +104,9 @@ class GroupViewModel extends ThreadListViewModel<GroupMessageItem> {
LOG.info("Group message received, adding..."); LOG.info("Group message received, adding...");
GroupMessageItem item = buildItem(g.getHeader(), g.getText()); GroupMessageItem item = buildItem(g.getHeader(), g.getText());
addItem(item); addItem(item);
// In case the join message comes from the creator,
// we need to reload the sharing contacts
// in case it was delayed and the sharing count is wrong (#850).
if (item instanceof JoinMessageItem && if (item instanceof JoinMessageItem &&
(((JoinMessageItem) item).isInitial())) { (((JoinMessageItem) item).isInitial())) {
loadSharingContacts(); loadSharingContacts();

View File

@@ -16,7 +16,7 @@ class JoinMessageItem extends GroupMessageItem {
JoinMessageItem(JoinMessageHeader h, String text) { JoinMessageItem(JoinMessageHeader h, String text) {
super(h, text); super(h, text);
this.isInitial = h.isInitial(); isInitial = h.isInitial();
} }
@Override @Override

View File

@@ -4,8 +4,7 @@ import android.view.LayoutInflater;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.MessageId;
import org.briarproject.briar.R; import org.briarproject.briar.R;
import org.briarproject.briar.android.util.ItemReturningAdapter; import org.briarproject.briar.android.util.ItemReturningAdapter;
@@ -21,8 +20,7 @@ import androidx.recyclerview.widget.ListAdapter;
import static androidx.recyclerview.widget.RecyclerView.NO_POSITION; import static androidx.recyclerview.widget.RecyclerView.NO_POSITION;
@UiThread @UiThread
@MethodsNotNullByDefault @NotNullByDefault
@ParametersNotNullByDefault
public class ThreadItemAdapter<I extends ThreadItem> public class ThreadItemAdapter<I extends ThreadItem>
extends ListAdapter<I, BaseThreadItemViewHolder<I>> extends ListAdapter<I, BaseThreadItemViewHolder<I>>
implements ItemReturningAdapter<I> { implements ItemReturningAdapter<I> {
@@ -41,7 +39,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
@Override @Override
public boolean areContentsTheSame(I a, I b) { public boolean areContentsTheSame(I a, I b) {
return a.isHighlighted() == b.isHighlighted() && return a.isHighlighted() == b.isHighlighted() &&
a.isRead() && b.isRead(); a.isRead() == b.isRead();
} }
}); });
this.listener = listener; this.listener = listener;
@@ -63,7 +61,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
ui.bind(item, listener); ui.bind(item, listener);
} }
public int findItemPosition(MessageId id) { int findItemPosition(MessageId id) {
for (int i = 0; i < getItemCount(); i++) { for (int i = 0; i < getItemCount(); i++) {
if (id.equals(getItem(i).getId())) return i; if (id.equals(getItem(i).getId())) return i;
} }
@@ -91,8 +89,7 @@ public class ThreadItemAdapter<I extends ThreadItem>
@Nullable @Nullable
I getHighlightedItem() { I getHighlightedItem() {
for (int i = 0; i < getItemCount(); i++) { for (I item : getCurrentList()) {
I item = getItem(i);
if (item.isHighlighted()) return item; if (item.isHighlighted()) return item;
} }
return null; return null;

View File

@@ -67,7 +67,6 @@ public abstract class ThreadListViewModel<I extends ThreadItem>
private final MessageTracker messageTracker; private final MessageTracker messageTracker;
private final EventBus eventBus; private final EventBus eventBus;
@DatabaseExecutor
private final MessageTree<I> messageTree = new MessageTreeImpl<>(); private final MessageTree<I> messageTree = new MessageTreeImpl<>();
private final MutableLiveData<LiveResult<List<I>>> items = private final MutableLiveData<LiveResult<List<I>>> items =
new MutableLiveData<>(); new MutableLiveData<>();
@@ -126,12 +125,12 @@ public abstract class ThreadListViewModel<I extends ThreadItem>
protected abstract void clearNotifications(); protected abstract void clearNotifications();
public void blockAndClearNotifications() { void blockAndClearNotifications() {
notificationManager.blockNotification(groupId); notificationManager.blockNotification(groupId);
clearNotifications(); clearNotifications();
} }
public void unblockNotifications() { void unblockNotifications() {
notificationManager.unblockNotification(groupId); notificationManager.unblockNotification(groupId);
} }
@@ -221,24 +220,26 @@ public abstract class ThreadListViewModel<I extends ThreadItem>
PostHeader header) throws DbException; PostHeader header) throws DbException;
@UiThread @UiThread
public void setReplyId(@Nullable MessageId id) { void setReplyId(@Nullable MessageId id) {
replyId = id; replyId = id;
} }
@UiThread @UiThread
@Nullable @Nullable
public MessageId getReplyId() { MessageId getReplyId() {
return replyId; return replyId;
} }
void storeMessageId(@Nullable MessageId messageId) { void storeMessageId(@Nullable MessageId messageId) {
if (messageId != null) runOnDbThread(() -> { if (messageId != null) {
try { runOnDbThread(() -> {
messageTracker.storeMessageId(groupId, messageId); try {
} catch (DbException e) { messageTracker.storeMessageId(groupId, messageId);
logException(LOG, WARNING, e); } catch (DbException e) {
} logException(LOG, WARNING, e);
}); }
});
}
} }
protected abstract void markItemRead(I item); protected abstract void markItemRead(I item);

View File

@@ -356,19 +356,6 @@ public class PrivateGroupManagerIntegrationTest
} }
} }
@Test
public void testJoinMessages() throws Exception {
addGroup();
Collection<GroupMessageHeader> headers0 =
groupManager0.getHeaders(groupId0);
assertEquals(2, headers0.size());
Collection<GroupMessageHeader> headers1 =
groupManager1.getHeaders(groupId0);
assertEquals(2, headers1.size());
}
@Test @Test
public void testRevealingRelationships() throws Exception { public void testRevealingRelationships() throws Exception {
addGroup(); addGroup();