Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7ab83089 authored by Tony Mak's avatar Tony Mak
Browse files

Do not generate smart suggestions if the last message is from local user

Also, instead of using Person.equals, introduce arePersonEqual. It is
because Person.equals is doing == check on the icon, which may return
false even the icons are actually the same.

And fixed a bug in handling sender == null in Message.
According to the javadoc of addMessage, person == null means the message
is from the local user.

Test: atest SmartActionsHelperTest

BUG: 123996228
Change-Id: I0ab24d53796d4b124c23cacf3df25cd8705d4fb3
parent 8e6dc3b7
Loading
Loading
Loading
Loading
+16 −4
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Deque;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

public class SmartActionsHelper {
@@ -120,6 +121,12 @@ public class SmartActionsHelper {
        if (messages.isEmpty()) {
            return Collections.emptyList();
        }
        // Do not generate smart actions if the last message is from the local user.
        ConversationActions.Message lastMessage = messages.get(messages.size() - 1);
        if (arePersonsEqual(
                ConversationActions.Message.PERSON_USER_SELF, lastMessage.getAuthor())) {
            return Collections.emptyList();
        }

        TextClassifier.EntityConfig.Builder typeConfigBuilder =
                new TextClassifier.EntityConfig.Builder();
@@ -312,13 +319,12 @@ public class SmartActionsHelper {
            if (message == null) {
                continue;
            }
            // As per the javadoc of Notification.addMessage, null means local user.
            Person senderPerson = message.getSenderPerson();
            // Skip encoding once the sender is missing as it is important to distinguish
            // local user and remote user when generating replies.
            if (senderPerson == null) {
                break;
                senderPerson = localUser;
            }
            Person author = localUser != null && localUser.equals(senderPerson)
            Person author = localUser != null && arePersonsEqual(localUser, senderPerson)
                    ? ConversationActions.Message.PERSON_USER_SELF : senderPerson;
            extractMessages.push(new ConversationActions.Message.Builder(author)
                    .setText(message.getText())
@@ -333,6 +339,12 @@ public class SmartActionsHelper {
        return new ArrayList<>(extractMessages);
    }

    private static boolean arePersonsEqual(@NonNull Person left, @NonNull Person right) {
        return Objects.equals(left.getKey(), right.getKey())
                && Objects.equals(left.getName(), right.getName())
                && Objects.equals(left.getUri(), right.getUri());
    }

    static class SmartSuggestions {
        public final ArrayList<CharSequence> replies;
        public final ArrayList<Notification.Action> actions;
+33 −4
Original line number Diff line number Diff line
@@ -222,28 +222,57 @@ public class SmartActionsHelperTest {

        List<ConversationActions.Message> messages =
                runSuggestAndCaptureRequest().getConversation();
        assertThat(messages).hasSize(3);
        assertThat(messages).hasSize(4);

        ConversationActions.Message secondMessage = messages.get(0);
        ConversationActions.Message firstMessage = messages.get(0);
        MessageSubject.assertThat(firstMessage).hasText("firstMessage");
        MessageSubject.assertThat(firstMessage)
                .hasPerson(ConversationActions.Message.PERSON_USER_SELF);
        MessageSubject.assertThat(firstMessage)
                .hasReferenceTime(createZonedDateTimeFromMsUtc(1000));

        ConversationActions.Message secondMessage = messages.get(1);
        MessageSubject.assertThat(secondMessage).hasText("secondMessage");
        MessageSubject.assertThat(secondMessage)
                .hasPerson(ConversationActions.Message.PERSON_USER_SELF);
        MessageSubject.assertThat(secondMessage)
                .hasReferenceTime(createZonedDateTimeFromMsUtc(2000));

        ConversationActions.Message thirdMessage = messages.get(1);
        ConversationActions.Message thirdMessage = messages.get(2);
        MessageSubject.assertThat(thirdMessage).hasText("thirdMessage");
        MessageSubject.assertThat(thirdMessage).hasPerson(userA);
        MessageSubject.assertThat(thirdMessage)
                .hasReferenceTime(createZonedDateTimeFromMsUtc(3000));

        ConversationActions.Message fourthMessage = messages.get(2);
        ConversationActions.Message fourthMessage = messages.get(3);
        MessageSubject.assertThat(fourthMessage).hasText("fourthMessage");
        MessageSubject.assertThat(fourthMessage).hasPerson(userB);
        MessageSubject.assertThat(fourthMessage)
                .hasReferenceTime(createZonedDateTimeFromMsUtc(4000));
    }

    @Test
    public void testSuggest_lastMessageLocalUser() {
        Person me = new Person.Builder().setName("Me").build();
        Person userA = new Person.Builder().setName("A").build();
        Notification.MessagingStyle style =
                new Notification.MessagingStyle(me)
                        .addMessage("firstMessage", 1000, userA)
                        .addMessage("secondMessage", 2000, me);
        Notification notification =
                mNotificationBuilder
                        .setContentText("You have two new messages")
                        .setStyle(style)
                        .setActions(createReplyAction())
                        .build();
        when(mStatusBarNotification.getNotification()).thenReturn(notification);

        mSmartActionsHelper.suggest(createNotificationEntry());

        verify(mTextClassifier, never())
                .suggestConversationActions(any(ConversationActions.Request.class));
    }

    @Test
    public void testSuggest_messageStyle_noPerson() {
        Person me = new Person.Builder().setName("Me").build();