diff --git a/reviewboard/accounts/evolutions/__init__.py b/reviewboard/accounts/evolutions/__init__.py index 50c80eea5d339ae0a4ad3fb57ce5df729fe8835b..ad796a353de383eda955afcdde2abbe5f8020c23 100644 --- a/reviewboard/accounts/evolutions/__init__.py +++ b/reviewboard/accounts/evolutions/__init__.py @@ -13,4 +13,5 @@ SEQUENCE = [ 'profile_should_send_own_updates', 'profile_default_use_rich_text', 'reviewrequestvisit_visibility', + 'profile_settings', ] diff --git a/reviewboard/accounts/evolutions/profile_settings.py b/reviewboard/accounts/evolutions/profile_settings.py new file mode 100644 index 0000000000000000000000000000000000000000..e493d71f8a79a2cab53632c203dec1c7f1f44cc6 --- /dev/null +++ b/reviewboard/accounts/evolutions/profile_settings.py @@ -0,0 +1,9 @@ +from __future__ import unicode_literals + +from django_evolution.mutations import AddField +from djblets.db.fields import JSONField + + +MUTATIONS = [ + AddField('Profile', 'settings', JSONField, null=True), +] diff --git a/reviewboard/accounts/forms/pages.py b/reviewboard/accounts/forms/pages.py index edc092223625cadf19a41a7246fa66356e7bf6b7..c2ec95cef276ce233bc5865651c255242761026b 100644 --- a/reviewboard/accounts/forms/pages.py +++ b/reviewboard/accounts/forms/pages.py @@ -55,6 +55,10 @@ class AccountSettingsForm(AccountPageForm): label=_('Get e-mail notifications for my own activity'), required=False) + enable_desktop_notifications = forms.BooleanField( + label=_('Show desktop notifications'), + required=False) + default_use_rich_text = forms.BooleanField( label=_('Always use Markdown for text fields'), required=False) @@ -65,6 +69,8 @@ class AccountSettingsForm(AccountPageForm): 'open_an_issue': self.profile.open_an_issue, 'should_send_email': self.profile.should_send_email, 'should_send_own_updates': self.profile.should_send_own_updates, + 'enable_desktop_notifications': + self.profile.should_enable_desktop_notifications, 'syntax_highlighting': self.profile.syntax_highlighting, 'timezone': self.profile.timezone, 'default_use_rich_text': self.profile.should_use_rich_text, @@ -85,6 +91,8 @@ class AccountSettingsForm(AccountPageForm): self.profile.should_send_email = self.cleaned_data['should_send_email'] self.profile.should_send_own_updates = \ self.cleaned_data['should_send_own_updates'] + self.profile.settings['enable_desktop_notifications'] = \ + self.cleaned_data['enable_desktop_notifications'] self.profile.default_use_rich_text = \ self.cleaned_data['default_use_rich_text'] self.profile.timezone = self.cleaned_data['timezone'] diff --git a/reviewboard/accounts/models.py b/reviewboard/accounts/models.py index 17ed82cff390ed6aa4efa78a2e2248cfd1f45291..960d67c27bae1f08f63cd6b27f783448c12ee7ac 100644 --- a/reviewboard/accounts/models.py +++ b/reviewboard/accounts/models.py @@ -154,6 +154,8 @@ class Profile(models.Model): timezone = models.CharField(choices=TIMEZONE_CHOICES, default='UTC', max_length=30) + settings = JSONField(null=True, default=dict) + extra_data = JSONField(null=True, default=dict) objects = ProfileManager() @@ -173,6 +175,24 @@ class Profile(models.Model): else: return self.default_use_rich_text + @property + def should_enable_desktop_notifications(self): + """Return whether desktop notifications should be used for this user. + + If the user has chosen whether or not to use desktop notifications + explicitly, then that choice will be respected. Otherwise, we + enable desktop notifications by default. + + Returns: + bool: + If the user has set whether they wish to recieve desktop + notifications, then use their preference. Otherwise, we return + ``True``. + """ + return (not self.settings or + self.settings.get('enable_desktop_notifications', True)) + + def star_review_request(self, review_request): """Mark a review request as starred. diff --git a/reviewboard/static/rb/js/pages/views/reviewablePageView.js b/reviewboard/static/rb/js/pages/views/reviewablePageView.js index 14bcb9488152263aa7b4181a62c8e273f9d2d065..d6895acc95434f9b671abada3373896f3bbfb9c0 100644 --- a/reviewboard/static/rb/js/pages/views/reviewablePageView.js +++ b/reviewboard/static/rb/js/pages/views/reviewablePageView.js @@ -164,6 +164,9 @@ RB.ReviewablePageView = Backbone.View.extend({ this._updatesBubble = null; this._favIconURL = null; this._favIconNotifyURL = null; + this._logoNotificationsURL = null; + + RB.NotificationManager.instance.setup(); }, /* @@ -174,6 +177,7 @@ RB.ReviewablePageView = Backbone.View.extend({ this._favIconURL = $favicon.attr('href'); this._favIconNotifyURL = STATIC_URLS['rb/images/favicon_notify.ico']; + this._logoNotificationsURL = STATIC_URLS['rb/images/logo.png']; this.draftReviewBanner = RB.DraftReviewBannerView.create({ el: $('#review-banner'), @@ -202,44 +206,103 @@ RB.ReviewablePageView = Backbone.View.extend({ }, /* - * Registers for update notifications to the review request from the + * Register for update notifications to the review request from the * server. * * The server will be periodically checked for new updates. When a new - * update arrives, an update bubble will be displayed in the bottom-right - * of the page with the information. + * update arrives, an update bubble will be displayed in the + * bottom-right of the page, and if the user has allowed desktop + * notifications in their account settings, a desktop notification + * will be shown with the update information. */ _registerForUpdates: function() { - this.listenTo(this.reviewRequest, 'updated', function(info) { - this._updateFavIcon(this._favIconNotifyURL); + this.listenTo(this.reviewRequest, 'updated', this._onReviewRequestUpdated); - if (this._updatesBubble) { - this._updatesBubble.remove(); - } + this.reviewRequest.beginCheckForUpdates( + this.options.checkUpdatesType, + this.options.lastActivityTimestamp); + }, - this._updatesBubble = new UpdatesBubbleView({ - updateInfo: info, - reviewRequest: this.reviewRequest - }); + /* + * Catch the review updated event and send the user a visual update. + * + * This function will handle the review updated event and decide whether + * to send a notification depending on browser and user settings. + * + * Args: + * info (Object): + * The last update information for the request. + */ + _onReviewRequestUpdated: function(info) { + if (RB.NotificationManager.instance.shouldNotify()) { + this._showDesktopNotification(info); + } - this.listenTo(this._updatesBubble, 'closed', function() { - this._updateFavIcon(this._favIconURL); - }); + this._showUpdatesBubble(info); + }, - this.listenTo(this._updatesBubble, 'updatePage', function() { - window.location = this.reviewRequest.get('reviewURL'); - }); + /* + * Create the updates bubble showing information about the last update. + * + * Args: + * info (Object): + * The last update information for the request. + */ + _showUpdatesBubble: function(info) { + this._updateFavIcon(this._favIconNotifyURL); + + if (this._updatesBubble) { + this._updatesBubble.remove(); + } - this._updatesBubble.render().$el.appendTo(this.$el); - this._updatesBubble.open(); + this._updatesBubble = new UpdatesBubbleView({ + updateInfo: info, + reviewRequest: this.reviewRequest }); - this.reviewRequest.beginCheckForUpdates( - this.options.checkUpdatesType, - this.options.lastActivityTimestamp); + this.listenTo(this._updatesBubble, 'closed', function() { + this._updateFavIcon(this._favIconURL); + }); + + this.listenTo(this._updatesBubble, 'updatePage', function() { + window.location = this.reviewRequest.get('reviewURL'); + }); + + this._updatesBubble.render().$el.appendTo(this.$el); + this._updatesBubble.open(); }, /* + * Show the user a desktop notification for the last update. + * + * This function will create a notification if the user has not + * disabled desktop notifications and the browser supports HTML5 + * notifications. + * + * Args: + * info (Object): + * The last update information for the request. + */ + _showDesktopNotification: function(info) { + var username = info.user.fullname || info.user.username, + notificationText = gettext('Review request submitted by %s'), + onclick = _.bind(function() { + window.location = this.reviewRequest.get('reviewURL'); + }, this); + + this._updateFavIcon(this._favIconNotifyURL); + + notificationData = { + 'title': interpolate(notificationText, [username]), + 'body': null, + 'iconURL': this._logoNotificationsURL, + 'onclick': onclick + }; + + RB.NotificationManager.instance.notify(notificationData); + }, + + /* * Updates the favicon for the page. * * This is used to change the favicon shown on the page based on whether diff --git a/reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js b/reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js index 8ea7037fc9270f95c9b570db57beaac01fc3b40d..a11d0e93327ba05685d8630211613c24011e0171 100644 --- a/reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js +++ b/reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js @@ -166,13 +166,30 @@ suite('rb/pages/views/ReviewablePageView', function() { expect(bubbleView.trigger).toHaveBeenCalledWith('closed'); }); - it('Update Page', function() { + it('Update Page displays Updates Bubble', function() { spyOn(bubbleView, 'trigger'); $bubble.find('.update-page').click(); expect(bubbleView.trigger).toHaveBeenCalledWith('updatePage'); }); + + it('Update Page calls notify if shouldNotify', function() { + RB.NotificationManager.instance._canNotify = true; + spyOn(RB.NotificationManager.instance, 'notify'); + spyOn(RB.NotificationManager.instance, '_haveNotificationPermissions').andReturn(true); + spyOn(pageView, '_showUpdatesBubble'); + + var info = { + user: { + fullname: 'Hello' + } + }; + + pageView._onReviewRequestUpdated(info); + + expect(RB.NotificationManager.instance.notify).toHaveBeenCalled(); + }); }); }); }); diff --git a/reviewboard/static/rb/js/ui/managers/notificationManagerModel.js b/reviewboard/static/rb/js/ui/managers/notificationManagerModel.js new file mode 100644 index 0000000000000000000000000000000000000000..d380d2c411af2efcef275646f32dd00116fd2953 --- /dev/null +++ b/reviewboard/static/rb/js/ui/managers/notificationManagerModel.js @@ -0,0 +1,123 @@ +/* + * A manager for desktop notifications. + * + * Manages the sending of desktop notifications to the user, including + * checking if certain user conditions are met and deciding which form + * of notification to send depending on the user's browser. + * + * For desktop notifications to be sent to the user, the user must have + * allowed notifications in their browser and account settings. + */ +RB.NotificationManager = Backbone.View.extend({ + NOTIFICATION_LIFETIME_MSECS: 10000, + NOTIFICATION_TYPE: (window.Notification || + window.mozNotification || + window.webkitNotification), + + /* + * Initialize the notification manager. + * + * Sets the initial values used by the notification manager. + * + */ + initialize: function() { + this._notification = null; + }, + + /* + * Set up the notification manager. + * + * This function will request permission to send desktop notifications + * if notifications are allowed in the users preferences, and the + * browser supports notifications. + * + * It must be called before attempting to send notifications. + */ + setup: function() { + this._canNotify = ( + this.NOTIFICATION_TYPE !== undefined && + RB.UserSession.instance.get('enableDesktopNotifications')); + + if (this._canNotify && + !this._haveNotificationPermissions()) { + this.NOTIFICATION_TYPE.requestPermission(); + } + }, + + /* + * Return whether we have permission to send notifications to the user. + * + * Returns: + * boolean: + * ``true`` if the user has enabled notifications in their browser + * Otherwise, ``false`` will be returned. + */ + _haveNotificationPermissions: function() { + return this.NOTIFICATION_TYPE.permission === "granted"; + }, + + /* + * Return whether we should send notifications to the user. + * + * Returns: + * boolean: + * ``true`` if the user has enabled notifications in their user + * settings, the users current browser supports notifications, and + * the user has granted permission for notifications to the + * browser. Otherwise, ``false`` will be returned. + */ + shouldNotify: function() { + return this._canNotify && + this._haveNotificationPermissions(); + }, + + /* + * Send a notification with the options specified in the data parameter. + * + * Args: + * data (Object): + * The last update information for the request. Contains the + * following keys: + * + * * ``title`` (String): The title of the notification. + * + * * ``body`` (String): The body text of the notification. + * + * * ``iconURL`` (String): The URL of the icon to be used + * in the notification Icons are not supported in some + * browsers, and thus will only be show in supported + * browsers. + * + * * ``onclick`` (function): The callback for when a user + * clicks the notification. By defualt this includes + * notification.close, so this does not need to be + * specified by the calling class. + */ + notify: function(data) { + var notification = null; + + if (this._notification) { + this._notification.close(); + } + + this._notification = new this.NOTIFICATION_TYPE( + data.title, { + text: data.body, + icon: data.iconURL + }); + + notification = this._notification; + + this._notification.onclick = function(){ + data.onclick(); + notification.close(); + }; + + _.delay(_.bind(notification.close, notification), + this.NOTIFICATION_LIFETIME_MSECS); + } +}, { + instance: null +}); + +RB.NotificationManager.instance = new RB.NotificationManager(); \ No newline at end of file diff --git a/reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js b/reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js new file mode 100644 index 0000000000000000000000000000000000000000..f50163394c4a1871f27aea0359dd46d28814da62 --- /dev/null +++ b/reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js @@ -0,0 +1,44 @@ +suite('rb/ui/managers/NotificationManagerModel', function() { + beforeEach(function() { + RB.NotificationManager.instance.setup(); + }); + + describe('Notification Manager', function() { + it('Calls external API', function() { + spyOn(RB.NotificationManager.instance, 'NOTIFICATION_TYPE').andReturn({ + 'close': function(){ + return true; + } + }); + + RB.NotificationManager.instance.notify({ + title: 'Test', + body: 'This is a Test' + }); + + expect(RB.NotificationManager.instance.NOTIFICATION_TYPE).toHaveBeenCalled(); + }); + + it('Should notify', function() { + RB.NotificationManager.instance._canNotify = true; + spyOn(RB.NotificationManager.instance, '_haveNotificationPermissions').andReturn(true); + + expect(RB.NotificationManager.instance.shouldNotify()).toBe(true); + }); + + + it('Should not notify due to user permissions', function() { + RB.NotificationManager.instance._canNotify = false; + spyOn(RB.NotificationManager.instance, '_haveNotificationPermissions').andReturn(true); + + expect(RB.NotificationManager.instance.shouldNotify()).toBe(false); + }); + + it('Should not notify due to browser permissions', function() { + RB.NotificationManager.instance._canNotify = true; + spyOn(RB.NotificationManager.instance, '_haveNotificationPermissions').andReturn(false); + + expect(RB.NotificationManager.instance.shouldNotify()).toBe(false); + }); + }); +}); \ No newline at end of file diff --git a/reviewboard/staticbundles.py b/reviewboard/staticbundles.py index 37899c5b53ae29458ea116f23592e74471f185ca..987b12763234ac6d2118d0ee5f5e4015a164d6cc 100644 --- a/reviewboard/staticbundles.py +++ b/reviewboard/staticbundles.py @@ -74,6 +74,7 @@ PIPELINE_JS = dict({ 'rb/js/resources/models/tests/reviewReplyModelTests.js', 'rb/js/resources/models/tests/reviewRequestModelTests.js', 'rb/js/resources/models/tests/validateDiffModelTests.js', + 'rb/js/ui/managers/tests/notificationManagerModelTests.js', 'rb/js/ui/views/tests/dialogViewTests.js', 'rb/js/ui/views/tests/textEditorViewTests.js', 'rb/js/utils/tests/keyBindingUtilsTests.js', @@ -147,6 +148,7 @@ PIPELINE_JS = dict({ 'rb/js/resources/collections/resourceCollection.js', 'rb/js/resources/collections/repositoryBranchesCollection.js', 'rb/js/resources/collections/repositoryCommitsCollection.js', + 'rb/js/ui/managers/notificationManagerModel.js', 'rb/js/ui/views/dialogView.js', 'rb/js/ui/views/textEditorView.js', 'rb/js/ui/views/splitButtonView.js', diff --git a/reviewboard/templates/base.html b/reviewboard/templates/base.html index 42d8525f15c756fcda9151339d01e22740fd3955..452e26f9878a89c625628282f246bcedde3335a9 100644 --- a/reviewboard/templates/base.html +++ b/reviewboard/templates/base.html @@ -15,7 +15,8 @@ MANUAL_URL = '{{RB_MANUAL_URL}}', STATIC_URLS = { 'rb/images/favicon_notify.ico': '{% static "rb/images/favicon_notify.ico" %}', - 'rb/images/resize-grip.png': '{% static "rb/images/resize-grip.png" %}' + 'rb/images/resize-grip.png': '{% static "rb/images/resize-grip.png" %}', + 'rb/images/logo.png': '{% static "rb/images/logo.png" %}' }; {% block jsconsts %}{% endblock %} @@ -113,8 +114,9 @@ authenticated: true, {% if user_profile %} commentsOpenAnIssue: {{user_profile.open_an_issue|yesno:"true,false"}}, -{% endif %} + enableDesktopNotifications: {{user_profile.should_enable_desktop_notifications|yesno:"true,false"}}, defaultUseRichText: {{user_profile.should_use_rich_text|yesno:"true,false"}}, +{% endif %} fullName: "{{request.user|user_displayname|escapejs}}", gravatarURL: "{% filter escapejs %}{% gravatar_url request.user.email %}{% endfilter %}", username: "{{request.user.username|escapejs}}",