Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 855165 - Migrate to new Notification API and close notifications #15307

Merged
merged 1 commit into from Jan 16, 2014
Merged

Bug 855165 - Migrate to new Notification API and close notifications #15307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions apps/sms/js/activity_handler.js
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@

/*global Utils, MessageManager, Compose, OptionMenu, NotificationHelper,
Attachment, Template, Notify, BlackList, Threads, SMIL, Contacts,
ThreadUI */
ThreadUI, Notification */
/*exported ActivityHandler */

'use strict';
Expand Down Expand Up @@ -421,14 +421,23 @@ var ActivityHandler = {
sender = contact[0].name[0];
}

var continueWithNotification =
function ah_continueWithNotification(sender, body) {
var options = {
icon: iconURL,
body: body,
tag: 'threadId:' + threadId
};
var notification = new Notification(sender, options);
notification.addEventListener('click', goToMessage.bind(this));
releaseWakeLock();
};

if (message.type === 'sms') {
NotificationHelper.send(sender, message.body, iconURL,
goToMessage);
releaseWakeLock();
continueWithNotification(sender, message.body);
} else { // mms
getTitleFromMms(function textCallback(text) {
NotificationHelper.send(sender, text, iconURL, goToMessage);
releaseWakeLock();
continueWithNotification(sender, text);
});
}
});
Expand Down
17 changes: 16 additions & 1 deletion apps/sms/js/message_manager.js
View file
Open in desktop
Expand Up @@ -3,7 +3,8 @@

/*global ThreadListUI, ThreadUI, Threads, SMIL, MozSmsFilter, Compose,
Utils, LinkActionHandler, Contacts, Attachment, GroupView,
ReportView, Utils, LinkActionHandler, Contacts, Attachment, Drafts */
ReportView, Utils, LinkActionHandler, Contacts, Attachment, Drafts,
Notification */

/*exported MessageManager */

Expand Down Expand Up @@ -358,6 +359,20 @@ var MessageManager = {

ThreadListUI.mark(threadId, 'read');

var targetTag = 'threadId:' + threadId;
Notification.get({tag: targetTag})
Copy link
Contributor

Choose a reason for hiding this comment

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

Re: my previous question about the string value: is this why the tag is a string?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is untested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you mean that it is still lacking unit tests, it's right, that even why I did not yet put any review flag: it's not finished.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed some test for this.

.then(
function onSuccess(notifications) {
for (var i = 0; i < notifications.length; i++) {
notifications[i].close();
}
},
function onError(reason) {
console.error('Notification.get(tag: ' + targetTag + '): ' +
reason);
}
);

// Update Header
ThreadUI.updateHeaderData(function headerUpdated() {
if (willSlide) {
Expand Down
35 changes: 25 additions & 10 deletions apps/sms/test/unit/activity_handler_test.js
View file
Open in desktop
@@ -1,8 +1,8 @@
/*global Notify, Compose, mocha, MocksHelper, ActivityHandler, Contacts,
MessageManager, Attachment, ThreadUI */
/*global MockNavigatormozSetMessageHandler, MockNavigatormozApps,
MockNavigatorWakeLock, MockNotificationHelper, MockOptionMenu,
Mockalert, MockMessages, MockNavigatorSettings, MockL10n,
MockNavigatorWakeLock, MockOptionMenu, Mockalert,
MockMessages, MockNavigatorSettings, MockL10n,
MockNavigatormozMobileMessage */

'use strict';
Expand All @@ -13,6 +13,7 @@ requireApp(
'sms/shared/test/unit/mocks/mock_navigator_moz_set_message_handler.js'
);
requireApp('sms/shared/test/unit/mocks/mock_navigator_wake_lock.js');
requireApp('sms/shared/test/unit/mocks/mock_notification.js');
requireApp('sms/shared/test/unit/mocks/mock_notification_helper.js');
requireApp('sms/shared/test/unit/mocks/mock_navigator_moz_apps.js');
requireApp('sms/shared/test/unit/mocks/mock_navigator_moz_settings.js');
Expand Down Expand Up @@ -42,6 +43,7 @@ var mocksHelperForActivityHandler = new MocksHelper([
'Compose',
'Contacts',
'MessageManager',
'Notification',
'NotificationHelper',
'OptionMenu',
'SettingsURL',
Expand Down Expand Up @@ -171,21 +173,27 @@ suite('ActivityHandler', function() {

suite('contact retrieved (after getSelf)', function() {
var contactName = '<&>';
var sendSpy;
setup(function() {
sendSpy = this.sinon.spy(window, 'Notification');
this.sinon.stub(Contacts, 'findByPhoneNumber')
.callsArgWith(1, [{name: [contactName]}]);
MockNavigatormozApps.mTriggerLastRequestSuccess();
});

test('passes contact name in plain text', function() {
assert.equal(MockNotificationHelper.mTitle, contactName);
sinon.assert.called(sendSpy);
var notification = sendSpy.firstCall.thisValue;
assert.equal(notification.title, contactName);
});
});

suite('contact without name (after getSelf)', function() {
var phoneNumber = '+1111111111';
var oldSender;
var sendSpy;
setup(function() {
sendSpy = this.sinon.spy(window, 'Notification');
oldSender = message.sender;
message.sender = phoneNumber;
this.sinon.stub(Contacts, 'findByPhoneNumber')
Expand All @@ -202,22 +210,26 @@ suite('ActivityHandler', function() {


test('phone in notification title when contact without name', function() {
assert.equal(MockNotificationHelper.mTitle, phoneNumber);
sinon.assert.called(sendSpy);
var notification = sendSpy.firstCall.thisValue;
assert.equal(notification.title, phoneNumber);
});
});

suite('after getSelf', function() {
var sendSpy;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're keeping the iconURL code, these tests need to be kept as well.

setup(function() {
sendSpy = this.sinon.spy(window, 'Notification');
MockNavigatormozApps.mTriggerLastRequestSuccess();
});

test('a notification is sent', function() {
assert.equal(MockNotificationHelper.mBody, message.body);

sinon.assert.called(sendSpy);
var notification = sendSpy.firstCall.thisValue;
assert.equal(notification.body, message.body);
var expectedicon = 'sms?threadId=' + message.threadId + '&number=' +
message.sender + '&id=' + message.id;

assert.equal(MockNotificationHelper.mIcon, expectedicon);
assert.equal(notification.icon, expectedicon);
});

test('the lock is released', function() {
Expand All @@ -226,9 +238,10 @@ suite('ActivityHandler', function() {

suite('click on the notification', function() {
setup(function() {
assert.ok(MockNotificationHelper.mClickCB);
var notification = sendSpy.firstCall.thisValue;
assert.ok(notification.mEvents.click);
this.sinon.stub(ActivityHandler, 'handleMessageNotification');
MockNotificationHelper.mClickCB();
notification.mEvents.click();
});

test('launches the app', function() {
Expand Down Expand Up @@ -289,6 +302,7 @@ suite('ActivityHandler', function() {
title: title,
body: body,
imageURL: 'url?id=' + messageId + '&threadId=' + threadId,
tag: 'threadId:' + threadId,
clicked: true
};

Expand Down Expand Up @@ -317,6 +331,7 @@ suite('ActivityHandler', function() {
body: body,
imageURL: 'url?id=' + messageId + '&threadId=' + threadId +
'&type=class0',
tag: 'threadId:' + threadId,
clicked: true
};

Expand Down
26 changes: 25 additions & 1 deletion apps/sms/test/unit/message_manager_test.js
View file
Open in desktop
Expand Up @@ -2,13 +2,14 @@
MockL10n, MockContact, loadBodyHTML, MozSmsFilter,
ThreadListUI, MockThreads, MockMessages, Threads, Compose,
GroupView, ReportView, ThreadListUI, MockThreads, MockMessages,
Threads, Compose, Drafts, Draft */
Threads, Compose, Drafts, Draft, MockNotification, Notification */

'use strict';

requireApp('sms/js/utils.js');
requireApp('sms/js/time_headers.js');

requireApp('sms/shared/test/unit/mocks/mock_notification.js');

requireApp('sms/test/unit/mock_attachment.js');
requireApp('sms/test/unit/mock_async_storage.js');
Expand Down Expand Up @@ -41,6 +42,7 @@ var mocksHelperForMessageManager = new MocksHelper([
'Drafts',
'LinkActionHandler',
'MozSmsFilter',
'Notification',
'LinkActionHandler',
'GroupView',
'ReportView',
Expand Down Expand Up @@ -541,6 +543,7 @@ suite('message_manager.js >', function() {
});

suite('onHashChange', function() {
var notificationGetStub;
setup(function() {
this.sinon.spy(document.activeElement, 'blur');
MessageManager.threadMessages = document.createElement('div');
Expand All @@ -554,6 +557,16 @@ suite('message_manager.js >', function() {
this.sinon.spy(ReportView, 'reset');
this.sinon.spy(MessageManager, 'handleActivity');
this.sinon.stub(MessageManager, 'slide');
notificationGetStub = function notificationGet(options) {
return {
then: function(onSuccess, onError, onProgress) {
onSuccess([
new Notification('123456789', options)
]);
}
};
};
this.sinon.stub(Notification, 'get', notificationGetStub);
MessageManager.onHashChange();
});

Expand Down Expand Up @@ -641,12 +654,14 @@ suite('message_manager.js >', function() {
});

suite('> Switch to #thread=100', function() {
var closeSpy;
setup(function() {
// reset states
MessageManager.threadMessages.classList.add('new');
MessageManager.slide.reset();
ThreadUI.updateHeaderData.reset();

closeSpy = this.sinon.spy(MockNotification.prototype, 'close');
this.threadId = MockThreads.currentId = 100;
window.location.hash = '#thread=' + this.threadId;
MessageManager.onHashChange();
Expand All @@ -664,6 +679,15 @@ suite('message_manager.js >', function() {
ThreadListUI.mark.calledWith(this.threadId, 'read')
);
});
test('calls Notification.get() on correct tag', function() {
assert.ok(
Notification.get.calledWith(
{tag: 'threadId:' + this.threadId})
);
});
test('calls Notification.close()', function() {
sinon.assert.calledOnce(closeSpy);
});
test('calls updateHeaderData', function() {
assert.ok(
ThreadUI.updateHeaderData.called
Expand Down
25 changes: 25 additions & 0 deletions shared/test/unit/mocks/mock_notification.js
View file
Open in desktop
@@ -0,0 +1,25 @@
'use strict';

function MockNotification(title, options) {
this.id = options.id || 0;
this.title = title;
this.icon = options.icon || undefined;
this.body = options.body || undefined;
this.tag = options.tag || undefined;
this.mEvents = {};
}

MockNotification.prototype.close = function() {
// nothing to do
};

MockNotification.prototype.addEventListener =
function mockNotification_addEventListener(evt, callback) {
this.mEvents[evt] = callback;
};

MockNotification.get = function mockNotification_get(options) {
return {
then: function() {}
};
};