From 7ec6cddd27f36f50f06669b208674feb0dc19535 Mon Sep 17 00:00:00 2001
From: Sasha Varlamov <sasha@sashavarlamov.com>
Date: Thu, 7 Dec 2017 12:52:57 +0700
Subject: [PATCH] Add 'mark all read' option to notifications (#3097)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Add 'mark all read' option to notifications
Signed-off-by: Sasha Varlamov <sasha@sashavarlamov.com>
* Fix exported comment
Signed-off-by: Sasha Varlamov <sasha@sashavarlamov.com>
* Format method comments
Signed-off-by: Sasha Varlamov <sasha@sashavarlamov.com>
* Fix exported comment
Signed-off-by: Sasha Varlamov <sasha@sashavarlamov.com>
Format method comments
Signed-off-by: Sasha Varlamov <sasha@sashavarlamov.com>
Tests for reactions (#3083)
* Unit tests for reactions
* Fix import order
Signed-off-by: Lauris Bukšis-Haberkorns <lauris@nix.lv>
Fix reaction possition when there is attachments (#3099)
Refactor notifications swap function
* Accept change to drop beforeupdate call
* Update purge notifications error message for consistency
* Drop unnecessary check for mark all as read button
* Remove debugging comment
---
models/fixtures/notification.yml | 22 +++++++++++++++
models/notification.go | 10 +++++++
models/notification_test.go | 28 ++++++++++++++++---
options/locale/locale_en-US.ini | 1 +
routers/routes/routes.go | 1 +
routers/user/notification.go | 12 ++++++++
templates/user/notification/notification.tmpl | 8 ++++++
7 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/models/fixtures/notification.yml b/models/fixtures/notification.yml
index d90936709e..fe5c47287d 100644
--- a/models/fixtures/notification.yml
+++ b/models/fixtures/notification.yml
@@ -19,3 +19,25 @@
issue_id: 2
created_unix: 946684800
updated_unix: 946684800
+
+-
+ id: 3
+ user_id: 2
+ repo_id: 1
+ status: 3 # pinned
+ source: 1 # issue
+ updated_by: 1
+ issue_id: 2
+ created_unix: 946684800
+ updated_unix: 946684800
+
+-
+ id: 4
+ user_id: 2
+ repo_id: 1
+ status: 1 # unread
+ source: 1 # issue
+ updated_by: 1
+ issue_id: 2
+ created_unix: 946684800
+ updated_unix: 946684800
\ No newline at end of file
diff --git a/models/notification.go b/models/notification.go
index 46da059852..dcd624e514 100644
--- a/models/notification.go
+++ b/models/notification.go
@@ -311,3 +311,13 @@ func getNotificationByID(notificationID int64) (*Notification, error) {
return notification, nil
}
+
+// UpdateNotificationStatuses updates the statuses of all of a user's notifications that are of the currentStatus type to the desiredStatus
+func UpdateNotificationStatuses(user *User, currentStatus NotificationStatus, desiredStatus NotificationStatus) error {
+ n := &Notification{Status: desiredStatus, UpdatedBy: user.ID}
+ _, err := x.
+ Where("user_id = ? AND status = ?", user.ID, currentStatus).
+ Cols("status", "updated_by", "updated_unix").
+ Update(n)
+ return err
+}
diff --git a/models/notification_test.go b/models/notification_test.go
index 07d9ee764c..01c960c929 100644
--- a/models/notification_test.go
+++ b/models/notification_test.go
@@ -31,9 +31,11 @@ func TestNotificationsForUser(t *testing.T) {
statuses := []NotificationStatus{NotificationStatusRead, NotificationStatusUnread}
notfs, err := NotificationsForUser(user, statuses, 1, 10)
assert.NoError(t, err)
- if assert.Len(t, notfs, 1) {
+ if assert.Len(t, notfs, 2) {
assert.EqualValues(t, 2, notfs[0].ID)
assert.EqualValues(t, user.ID, notfs[0].UserID)
+ assert.EqualValues(t, 4, notfs[1].ID)
+ assert.EqualValues(t, user.ID, notfs[1].UserID)
}
}
@@ -57,12 +59,12 @@ func TestNotification_GetIssue(t *testing.T) {
func TestGetNotificationCount(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
- user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
- cnt, err := GetNotificationCount(user, NotificationStatusUnread)
+ user := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
+ cnt, err := GetNotificationCount(user, NotificationStatusRead)
assert.NoError(t, err)
assert.EqualValues(t, 0, cnt)
- cnt, err = GetNotificationCount(user, NotificationStatusRead)
+ cnt, err = GetNotificationCount(user, NotificationStatusUnread)
assert.NoError(t, err)
assert.EqualValues(t, 1, cnt)
}
@@ -79,3 +81,21 @@ func TestSetNotificationStatus(t *testing.T) {
assert.Error(t, SetNotificationStatus(1, user, NotificationStatusRead))
assert.Error(t, SetNotificationStatus(NonexistentID, user, NotificationStatusRead))
}
+
+func TestUpdateNotificationStatuses(t *testing.T) {
+ assert.NoError(t, PrepareTestDatabase())
+ user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
+ notfUnread := AssertExistsAndLoadBean(t,
+ &Notification{UserID: user.ID, Status: NotificationStatusUnread}).(*Notification)
+ notfRead := AssertExistsAndLoadBean(t,
+ &Notification{UserID: user.ID, Status: NotificationStatusRead}).(*Notification)
+ notfPinned := AssertExistsAndLoadBean(t,
+ &Notification{UserID: user.ID, Status: NotificationStatusPinned}).(*Notification)
+ assert.NoError(t, UpdateNotificationStatuses(user, NotificationStatusUnread, NotificationStatusRead))
+ AssertExistsAndLoadBean(t,
+ &Notification{ID: notfUnread.ID, Status: NotificationStatusRead})
+ AssertExistsAndLoadBean(t,
+ &Notification{ID: notfRead.ID, Status: NotificationStatusRead})
+ AssertExistsAndLoadBean(t,
+ &Notification{ID: notfPinned.ID, Status: NotificationStatusPinned})
+}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index c0d286f1fb..1944ac8d4a 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1547,6 +1547,7 @@ no_read = You do not have any read notifications.
pin = Pin notification
mark_as_read = Mark as read
mark_as_unread = Mark as unread
+mark_all_as_read = Mark all as read
[gpg]
error.extract_sign = Failed to extract signature
diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index f4fb135629..cdbbd22801 100644
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -706,6 +706,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Group("/notifications", func() {
m.Get("", user.Notifications)
m.Post("/status", user.NotificationStatusPost)
+ m.Post("/purge", user.NotificationPurgePost)
}, reqSignIn)
m.Group("/api", func() {
diff --git a/routers/user/notification.go b/routers/user/notification.go
index 77a4a59dc8..c7f23afe69 100644
--- a/routers/user/notification.go
+++ b/routers/user/notification.go
@@ -112,3 +112,15 @@ func NotificationStatusPost(c *context.Context) {
url := fmt.Sprintf("%s/notifications", setting.AppSubURL)
c.Redirect(url, 303)
}
+
+// NotificationPurgePost is a route for 'purging' the list of notifications - marking all unread as read
+func NotificationPurgePost(c *context.Context) {
+ err := models.UpdateNotificationStatuses(c.User, models.NotificationStatusUnread, models.NotificationStatusRead)
+ if err != nil {
+ c.Handle(500, "ErrUpdateNotificationStatuses", err)
+ return
+ }
+
+ url := fmt.Sprintf("%s/notifications", setting.AppSubURL)
+ c.Redirect(url, 303)
+}
diff --git a/templates/user/notification/notification.tmpl b/templates/user/notification/notification.tmpl
index da6bd07d2a..3c30bbe305 100644
--- a/templates/user/notification/notification.tmpl
+++ b/templates/user/notification/notification.tmpl
@@ -14,6 +14,14 @@
<a href="{{AppSubUrl}}/notifications?q=read" class="{{if eq .Status 2}}active{{end}} item">
{{.i18n.Tr "notification.read"}}
</a>
+ {{if and (eq .Status 1) (.NotificationUnreadCount)}}
+ <form action="{{AppSubUrl}}/notifications/purge" method="POST" style="margin-left: auto;">
+ {{$.CsrfTokenHtml}}
+ <button class="ui mini button primary" title='{{$.i18n.Tr "notification.mark_all_as_read"}}'>
+ <i class="octicon octicon-checklist"></i>
+ </button>
+ </form>
+ {{end}}
</div>
<div class="ui bottom attached active tab segment">
{{if eq (len .Notifications) 0}}