Send notification to users upon receiving sms messages

ash/common/system/chromeos/network/sms_observer_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.

[James Cook, 2017/02/10 17:36:38]

2017 unless this is mostly a move

[yiyix, 2017/02/13 23:34:00]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/common/system/chromeos/network/sms_observer.h"

[James Cook, 2017/02/10 17:36:38]

nit: blank line below

[yiyix, 2017/02/13 23:34:00]

Done.

+#include "ash/common/shelf/shelf_widget.h"
+#include "ash/common/shelf/wm_shelf.h"
+#include "ash/public/interfaces/vpn_list.mojom.h"
+#include "ash/test/ash_test_base.h"
+#include "base/macros.h"
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification.h"
+#include "ui/message_center/notification_list.h"
+
+namespace ash {
+namespace test {
+
+class SmsObserverTest : public AshTestBase {
+ public:
+  SmsObserverTest() {}
+  ~SmsObserverTest() override {}
+
+  SmsObserver* GetSmsObserver(ShelfWidget* shelf_widget) {
+    return shelf_widget->sms_observer_.get();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SmsObserverTest);
+};
+
+std::unique_ptr<base::DictionaryValue> CreateMessage(

[James Cook, 2017/02/10 17:36:38]

put in anonymous namespace

[yiyix, 2017/02/13 23:34:00]

Done. Is it true that unless it's class specific helper function and mentioned
in the .h file, we always want to include it in the anonymous namespace?

[James Cook, 2017/02/14 02:04:38]

Yes. Unless you need to access it from outside the .cc file, or it's part of the
class, it should be in an anonymous namespace. This helps prevent polluting the
global namespace and it also makes things a little bit easier on the linker (it
doesn't have as many symbols to resolve).

https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Stat...

+    const char* kDefaultNumber,

[James Cook, 2017/02/10 17:36:38]

default_number (and below)

[yiyix, 2017/02/13 23:34:00]

Done.

+    const char* kDefaultMessage,
+    const char* kDefaultTimestamp) {
+  std::unique_ptr<base::DictionaryValue> sms(new base::DictionaryValue());

[James Cook, 2017/02/10 17:36:38]

prefer MakeUnique

[yiyix, 2017/02/13 23:34:00]

Done.

+  if (kDefaultNumber)
+    sms->SetString("number", kDefaultNumber);
+  if (kDefaultMessage)
+    sms->SetString("text", kDefaultMessage);
+  if (kDefaultTimestamp)
+    sms->SetString("timestamp", kDefaultMessage);
+  return sms;
+}
+
+// Verify if notification is received after receiving a sms message with
+// number and content.
+TEST_F(SmsObserverTest, SendTextMessage) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+
+  sms_observer->MessageReceived(*CreateMessage("000-000-0000",

[James Cook, 2017/02/10 17:36:38]

nit: put result of CreateMessage in a local var. It's hard to reason about
memory management if the unique_ptr is a temporary

[yiyix, 2017/02/13 23:34:00]

Done.

+                                               "FakeSMSClient: Test Message.",
+                                               "Fri Jun  8 13:26:04 EDT 2016"));
+
+  const message_center::NotificationList::Notifications notifications =
+      message_center::MessageCenter::Get()->GetVisibleNotifications();
+  EXPECT_EQ(1u, notifications.size());
+
+  EXPECT_EQ((*notifications.begin())->title(),

[James Cook, 2017/02/10 17:36:38]

nit: revert EXPECT_EQ parameters so expected value is first (and below)

[yiyix, 2017/02/13 23:34:00]

Done.

+            base::ASCIIToUTF16("000-000-0000"));
+  EXPECT_EQ((*notifications.begin())->message(),
+            base::ASCIIToUTF16("FakeSMSClient: Test Message."));
+  message_center::MessageCenter::Get()->RemoveAllNotifications(
+      false /* by_user */, message_center::MessageCenter::RemoveType::ALL);
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());

[James Cook, 2017/02/10 17:36:38]

optional: using message_center::MessageCenter if it makes wrapping better

[yiyix, 2017/02/13 23:34:00]

Done.

+}
+
+// Verify if no notification is received if phone number is missing in sms
+// message.
+TEST_F(SmsObserverTest, TextMessageMissingNumber) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+  sms_observer->MessageReceived(*CreateMessage(
+      nullptr, "FakeSMSClient: Test Message.", "Fri Jun  8 13:26:04 EDT 2016"));
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+}
+
+// Verify if no notification is received if text body is empty in sms message.
+TEST_F(SmsObserverTest, TextMessageEmptyText) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+  sms_observer->MessageReceived(
+      *CreateMessage("000-000-0000", "", "Fri Jun  8 13:26:04 EDT 2016"));
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+}
+
+// Verify if no notification is received if the text is missing in sms message.
+TEST_F(SmsObserverTest, TextMessageMissingText) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+  sms_observer->MessageReceived(
+      *CreateMessage("000-000-0000", nullptr, "Fri Jun  8 13:26:04 EDT 2016"));
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+}
+
+// Verify if 2 notification received after receiving 2 sms messages from the
+// same number.
+TEST_F(SmsObserverTest, MultipleTextMessages) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_EQ(
+      0u,
+      message_center::MessageCenter::Get()->GetVisibleNotifications().size());
+
+  sms_observer->MessageReceived(*CreateMessage("000-000-0000", "first message",
+                                               "Fri Jun  8 13:26:04 EDT 2016"));
+
+  sms_observer->MessageReceived(*CreateMessage("000-000-0000", "second message",
+                                               "Fri Jun  8 13:26:04 EDT 2016"));
+  const message_center::NotificationList::Notifications notifications =
+      message_center::MessageCenter::Get()->GetVisibleNotifications();
+  EXPECT_EQ(2u, notifications.size());
+
+  for (message_center::Notification* iter : notifications) {
+    if (iter->id().find("chrome://network/sms1") != std::string::npos) {
+      EXPECT_EQ(iter->title(), base::ASCIIToUTF16("000-000-0000"));
+      EXPECT_EQ(iter->message(), base::ASCIIToUTF16("first message"));
+    } else if (iter->id().find("chrome://network/sms2") != std::string::npos) {
+      EXPECT_EQ(iter->title(), base::ASCIIToUTF16("000-000-0000"));
+      EXPECT_EQ(iter->message(), base::ASCIIToUTF16("second message"));
+    } else {
+      ASSERT_TRUE(false);
+    }
+  }
+}
+
+}  // namespace test
+}  // namespace ash