Skip to content

Improve group update descriptions #5416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Improve group update descriptions #5416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f6f08d
Improve group update descriptions
cascheberg Apr 5, 2016
a00a9a8
add sender parameter
cascheberg Nov 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,8 @@
<!-- MessageRecord -->
<string name="MessageRecord_message_encrypted_with_a_legacy_protocol_version_that_is_no_longer_supported">Received a message encrypted using an old version of Signal that is no longer supported. Please ask the sender to update to the most recent version and resend the message.</string>
<string name="MessageRecord_left_group">You have left the group.</string>
<string name="MessageRecord_updated_group">Updated the group.</string>
<string name="MessageRecord_you_updated_group">You updated the group.</string>
<string name="MessageRecord_s_updated_group">%s updated the group.</string>
<string name="MessageRecord_s_called_you">%s called you</string>
<string name="MessageRecord_called_s">Called %s</string>
<string name="MessageRecord_missed_call_from">Missed call from %s</string>
Expand Down Expand Up @@ -569,6 +570,7 @@
<string name="SmsMessageRecord_duplicate_message">Duplicate message.</string>

<!-- ThreadRecord -->
<string name="ThreadRecord_group_updated">Group updated</string>
<string name="ThreadRecord_left_the_group">Left the group</string>
<string name="ThreadRecord_secure_session_reset">Secure session reset.</string>
<string name="ThreadRecord_draft">Draft:</string>
Expand Down Expand Up @@ -876,7 +878,6 @@
<item quantity="one">%1$s joined the group.</item>
<item quantity="other">%1$s joined the group.</item>
</plurals>
<string name="GroupUtil_group_updated">Group updated.</string>
<string name="GroupUtil_group_name_is_now">Group name is now \'%1$s\'.</string>

<!-- prompt_passphrase_activity -->
Expand Down
6 changes: 3 additions & 3 deletions src/org/thoughtcrime/securesms/database/model/MessageRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.database.MmsSmsColumns;
import org.thoughtcrime.securesms.database.SmsDatabase;
import org.thoughtcrime.securesms.database.documents.NetworkFailure;
import org.thoughtcrime.securesms.database.documents.IdentityKeyMismatch;
import org.thoughtcrime.securesms.database.documents.NetworkFailure;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.Recipients;
import org.thoughtcrime.securesms.util.ExpirationUtil;
Expand Down Expand Up @@ -93,9 +93,9 @@ public boolean isAsymmetricEncryption() {
@Override
public SpannableString getDisplayBody() {
if (isGroupUpdate() && isOutgoing()) {
return emphasisAdded(context.getString(R.string.MessageRecord_updated_group));
return emphasisAdded(context.getString(R.string.MessageRecord_you_updated_group));
} else if (isGroupUpdate()) {
return emphasisAdded(GroupUtil.getDescription(context, getBody().getBody()).toString());
return emphasisAdded(GroupUtil.getDescription(context, getBody().getBody()).toString(getIndividualRecipient()));
} else if (isGroupQuit() && isOutgoing()) {
return emphasisAdded(context.getString(R.string.MessageRecord_left_group));
} else if (isGroupQuit()) {
Expand Down
4 changes: 1 addition & 3 deletions src/org/thoughtcrime/securesms/database/model/ThreadRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@
import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.database.MmsSmsColumns;
import org.thoughtcrime.securesms.database.SmsDatabase;
import org.thoughtcrime.securesms.database.ThreadDatabase;
import org.thoughtcrime.securesms.recipients.Recipients;
import org.thoughtcrime.securesms.util.ExpirationUtil;
import org.thoughtcrime.securesms.util.GroupUtil;

/**
* The message record model which represents thread heading messages.
Expand Down Expand Up @@ -75,7 +73,7 @@ public SpannableString getDisplayBody() {
if (SmsDatabase.Types.isDecryptInProgressType(type)) {
return emphasisAdded(context.getString(R.string.MessageDisplayHelper_decrypting_please_wait));
} else if (isGroupUpdate()) {
return emphasisAdded(GroupUtil.getDescription(context, getBody().getBody()).toString());
return emphasisAdded(context.getString(R.string.ThreadRecord_group_updated));
} else if (isGroupQuit()) {
return emphasisAdded(context.getString(R.string.ThreadRecord_left_the_group));
} else if (isKeyExchange()) {
Expand Down
23 changes: 12 additions & 11 deletions src/org/thoughtcrime/securesms/util/GroupUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.util.Log;

import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientFactory;
import org.thoughtcrime.securesms.recipients.Recipients;

Expand Down Expand Up @@ -61,33 +62,33 @@ public GroupDescription(@NonNull Context context, @Nullable GroupContext groupCo
if (groupContext == null || groupContext.getMembersList().isEmpty()) {
this.members = null;
} else {
this.members = RecipientFactory.getRecipientsFromString(context, Util.join(groupContext.getMembersList(), ", "), true);
this.members = RecipientFactory.getRecipientsFromStrings(context, groupContext.getMembersList(), true);
}
}

public String toString() {
public String toString(Recipient sender) {
StringBuilder description = new StringBuilder();
description.append(context.getString(R.string.MessageRecord_s_updated_group, sender.toShortString()));

if (groupContext == null) {
return context.getString(R.string.GroupUtil_group_updated);
return description.toString();
}

StringBuilder description = new StringBuilder();
String title = groupContext.getName();
String title = groupContext.getName();

if (members != null) {
description.append("\n");
description.append(context.getResources().getQuantityString(R.plurals.GroupUtil_joined_the_group,
members.getRecipientsList().size(), members.toShortString()));
}

if (title != null && !title.trim().isEmpty()) {
if (description.length() > 0) description.append(" ");
if (members != null) description.append(" ");
else description.append("\n");
description.append(context.getString(R.string.GroupUtil_group_name_is_now, title));
}

if (description.length() > 0) {
return description.toString();
} else {
return context.getString(R.string.GroupUtil_group_updated);
}
return description.toString();
Copy link
Contributor

@moxie0 moxie0 Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is designed to be idempotent, and is called from several places. Instead of making it dependent on some external functionality (since it can now return an empty string), it seems both safer and cleaner to consider making your changes in this method.

Copy link
Contributor Author

@cascheberg cascheberg Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this PR in a followup commit. There is currently only that one caller, and it now passes the sender as a method parameter. Please let me know if you have suggestions other than these two approaches.

}

public void addListener(Recipients.RecipientsModifiedListener listener) {
Expand Down