From 769e0a3ea6455cb908cd7167d651024ce5c7eee3 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Thu, 12 Jan 2017 02:27:09 -0200 Subject: [PATCH] Notifications: mark as read/unread and pin (#629) * Use relative URLs * Notifications - Mark as read/unread * Feature of pinning a notification * On view issue, do not mark as read a pinned notification --- cmd/web.go | 5 +- models/issue.go | 2 +- models/notification.go | 57 ++++++++++++-- public/css/index.css | 9 +++ public/less/_user.less | 13 ++++ routers/user/notification.go | 35 ++++++++- templates/base/head.tmpl | 2 +- templates/user/notification/notification.tmpl | 78 +++++++++++++------ 8 files changed, 169 insertions(+), 32 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index e698510aa..a69bcf4b2 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -589,7 +589,10 @@ func runWeb(ctx *cli.Context) error { }) // ***** END: Repository ***** - m.Get("/notifications", reqSignIn, user.Notifications) + m.Group("/notifications", func() { + m.Get("", user.Notifications) + m.Post("/status", user.NotificationStatusPost) + }, reqSignIn) m.Group("/api", func() { apiv1.RegisterRoutes(m) diff --git a/models/issue.go b/models/issue.go index c0377e634..ff7df26c9 100644 --- a/models/issue.go +++ b/models/issue.go @@ -448,7 +448,7 @@ func (issue *Issue) ReadBy(userID int64) error { return err } - if err := setNotificationStatusRead(x, userID, issue.ID); err != nil { + if err := setNotificationStatusReadIfUnread(x, userID, issue.ID); err != nil { return err } diff --git a/models/notification.go b/models/notification.go index e2460e836..6e1a2fbf6 100644 --- a/models/notification.go +++ b/models/notification.go @@ -5,6 +5,7 @@ package models import ( + "fmt" "time" ) @@ -20,6 +21,8 @@ const ( NotificationStatusUnread NotificationStatus = iota + 1 // NotificationStatusRead represents a read notification NotificationStatusRead + // NotificationStatusPinned represents a pinned notification + NotificationStatusPinned ) const ( @@ -182,13 +185,19 @@ func getIssueNotification(e Engine, userID, issueID int64) (*Notification, error } // NotificationsForUser returns notifications for a given user and status -func NotificationsForUser(user *User, status NotificationStatus, page, perPage int) ([]*Notification, error) { - return notificationsForUser(x, user, status, page, perPage) +func NotificationsForUser(user *User, statuses []NotificationStatus, page, perPage int) ([]*Notification, error) { + return notificationsForUser(x, user, statuses, page, perPage) } -func notificationsForUser(e Engine, user *User, status NotificationStatus, page, perPage int) (notifications []*Notification, err error) { +func notificationsForUser(e Engine, user *User, statuses []NotificationStatus, page, perPage int) (notifications []*Notification, err error) { + // FIXME: Xorm does not support aliases types (like NotificationStatus) on In() method + s := make([]uint8, len(statuses)) + for i, status := range statuses { + s[i] = uint8(status) + } + sess := e. Where("user_id = ?", user.ID). - And("status = ?", status). + In("status", s). OrderBy("updated_unix DESC") if page > 0 && perPage > 0 { @@ -241,15 +250,53 @@ func getNotificationCount(e Engine, user *User, status NotificationStatus) (coun return } -func setNotificationStatusRead(e Engine, userID, issueID int64) error { +func setNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error { notification, err := getIssueNotification(e, userID, issueID) // ignore if not exists if err != nil { return nil } + if notification.Status != NotificationStatusUnread { + return nil + } + notification.Status = NotificationStatusRead _, err = e.Id(notification.ID).Update(notification) return err } + +// SetNotificationStatus change the notification status +func SetNotificationStatus(notificationID int64, user *User, status NotificationStatus) error { + notification, err := getNotificationByID(notificationID) + if err != nil { + return err + } + + if notification.UserID != user.ID { + return fmt.Errorf("Can't change notification of another user: %d, %d", notification.UserID, user.ID) + } + + notification.Status = status + + _, err = x.Id(notificationID).Update(notification) + return err +} + +func getNotificationByID(notificationID int64) (*Notification, error) { + notification := new(Notification) + ok, err := x. + Where("id = ?", notificationID). + Get(notification) + + if err != nil { + return nil, err + } + + if !ok { + return nil, fmt.Errorf("Notification %d does not exists", notificationID) + } + + return notification, nil +} diff --git a/public/css/index.css b/public/css/index.css index f5581c3ea..c569209e4 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -2712,6 +2712,12 @@ footer .ui.language .menu { float: left; margin-left: 7px; } +.user.notification .buttons-panel button { + padding: 3px; +} +.user.notification .buttons-panel form { + display: inline-block; +} .user.notification .octicon-issue-opened, .user.notification .octicon-git-pull-request { color: #21ba45; @@ -2722,6 +2728,9 @@ footer .ui.language .menu { .user.notification .octicon-git-merge { color: #a333c8; } +.user.notification .octicon-pin { + color: #2185d0; +} .dashboard { padding-top: 15px; padding-bottom: 80px; diff --git a/public/less/_user.less b/public/less/_user.less index b446351bd..38b73f785 100644 --- a/public/less/_user.less +++ b/public/less/_user.less @@ -85,6 +85,16 @@ margin-left: 7px; } + .buttons-panel { + button { + padding: 3px; + } + + form { + display: inline-block; + } + } + .octicon-issue-opened, .octicon-git-pull-request { color: #21ba45; } @@ -94,5 +104,8 @@ .octicon-git-merge { color: #a333c8; } + .octicon-pin { + color: #2185d0; + } } } diff --git a/routers/user/notification.go b/routers/user/notification.go index 7e556ae2e..4ab93de27 100644 --- a/routers/user/notification.go +++ b/routers/user/notification.go @@ -1,7 +1,9 @@ package user import ( + "errors" "fmt" + "strconv" "strings" "github.com/Unknwon/paginater" @@ -9,6 +11,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/setting" ) const ( @@ -56,7 +59,8 @@ func Notifications(c *context.Context) { status = models.NotificationStatusUnread } - notifications, err := models.NotificationsForUser(c.User, status, page, perPage) + statuses := []models.NotificationStatus{status, models.NotificationStatusPinned} + notifications, err := models.NotificationsForUser(c.User, statuses, page, perPage) if err != nil { c.Handle(500, "ErrNotificationsForUser", err) return @@ -79,3 +83,32 @@ func Notifications(c *context.Context) { c.Data["Page"] = paginater.New(int(total), perPage, page, 5) c.HTML(200, tplNotification) } + +// NotificationStatusPost is a route for changing the status of a notification +func NotificationStatusPost(c *context.Context) { + var ( + notificationID, _ = strconv.ParseInt(c.Req.PostFormValue("notification_id"), 10, 64) + statusStr = c.Req.PostFormValue("status") + status models.NotificationStatus + ) + + switch statusStr { + case "read": + status = models.NotificationStatusRead + case "unread": + status = models.NotificationStatusUnread + case "pinned": + status = models.NotificationStatusPinned + default: + c.Handle(500, "InvalidNotificationStatus", errors.New("Invalid notification status")) + return + } + + if err := models.SetNotificationStatus(notificationID, c.User, status); err != nil { + c.Handle(500, "SetNotificationStatus", err) + return + } + + url := fmt.Sprintf("%s/notifications", setting.AppSubURL) + c.Redirect(url, 303) +} diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl index a0b5220fd..e23043620 100644 --- a/templates/base/head.tmpl +++ b/templates/base/head.tmpl @@ -82,7 +82,7 @@ {{if .IsSigned}}
- + {{.i18n.Tr "notifications"}} diff --git a/templates/user/notification/notification.tmpl b/templates/user/notification/notification.tmpl index ddfcd4f71..3e5c0b710 100644 --- a/templates/user/notification/notification.tmpl +++ b/templates/user/notification/notification.tmpl @@ -5,7 +5,7 @@

{{.i18n.Tr "notification.notifications"}}

- +
{{.i18n.Tr "notification.unread"}} {{if eq .Status 1}} @@ -13,7 +13,7 @@ {{end}}
- +
{{.i18n.Tr "notification.read"}} {{if eq .Status 2}} @@ -30,34 +30,66 @@ {{.i18n.Tr "notification.no_read"}} {{end}} {{else}} -
+
{{range $notification := .Notifications}} {{$issue := $notification.GetIssue}} {{$repo := $notification.GetRepo}} {{$repoOwner := $repo.MustOwner}} -
- - {{if and $issue.IsPull}} - {{if $issue.IsClosed}} - - {{else}} - - {{end}} - {{else}} - {{if $issue.IsClosed}} - - {{else}} - - {{end}} + +
+ {{if ne $notification.Status 3}} +
+ {{$.CsrfTokenHtml}} + + + +
{{end}} + {{if or (eq $notification.Status 1) (eq $notification.Status 3)}} +
+ {{$.CsrfTokenHtml}} + + + +
+ {{else if eq $notification.Status 2}} +
+ {{$.CsrfTokenHtml}} + + + +
+ {{end}} +
-
-
{{$repoOwner.Name}}/{{$repo.Name}}
-
#{{$issue.Index}} - {{$issue.Title}}
-
- -
+ {{if eq $notification.Status 3}} + + {{else if $issue.IsPull}} + {{if $issue.IsClosed}} + + {{else}} + + {{end}} + {{else}} + {{if $issue.IsClosed}} + + {{else}} + + {{end}} + {{end}} + +
+
{{$repoOwner.Name}}/{{$repo.Name}}
+
#{{$issue.Index}} - {{$issue.Title}}
+
+ {{end}}
{{end}}