From 7c2f16d04fbea354dd0b569b45822087954e1783 Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Tue, 16 Jun 2020 21:13:35 +0800 Subject: [PATCH 1/3] Add type filter to System Notices view After #10745 was merged, the number of System Notices increased obviously. Add the type filter is useful to help users to quickly check out the useful message they want. Signed-off-by: a1012112796 <1012112796@qq.com> --- models/admin.go | 49 +++++++++++++++++++++++++-------- models/admin_test.go | 17 ++++++++++-- models/fixtures/notice.yml | 2 +- modules/cron/tasks.go | 6 ++-- options/locale/locale_en-US.ini | 5 +++- routers/admin/notice.go | 16 +++++++++-- templates/admin/notice.tmpl | 20 ++++++++++++++ 7 files changed, 93 insertions(+), 22 deletions(-) diff --git a/models/admin.go b/models/admin.go index 01f0ba34865c..dbf82ac118eb 100644 --- a/models/admin.go +++ b/models/admin.go @@ -20,8 +20,10 @@ type NoticeType int const ( //NoticeRepository type NoticeRepository NoticeType = iota + 1 - // NoticeTask type - NoticeTask + // NoticeTaskSuccess type + NoticeTaskSuccess + // NoticeTaskFail type + NoticeTaskFail ) // Notice represents a system notice for admin. @@ -59,6 +61,17 @@ func CreateRepositoryNotice(desc string, args ...interface{}) error { return createNotice(x, NoticeRepository, desc, args...) } +// CreateTaskNotice creates new system notice with type NoticeTaskSuccess or NoticeTaskFail. +func CreateTaskNotice(isSuccess bool, desc string, args ...interface{}) (err error) { + if isSuccess { + err = createNotice(x, NoticeTaskSuccess, desc, args...) + } else { + err = createNotice(x, NoticeTaskFail, desc, args...) + } + + return +} + // RemoveAllWithNotice removes all directories in given path and // creates a system notice when error occurs. func RemoveAllWithNotice(title, path string) { @@ -76,18 +89,32 @@ func removeAllWithNotice(e Engine, title, path string) { } // CountNotices returns number of notices. -func CountNotices() int64 { - count, _ := x.Count(new(Notice)) - return count +func CountNotices(typ int) (count int64, err error) { + if typ >= 1 { + count, err = x.Where("type = ?", typ).Count(new(Notice)) + } else { + count, err = x.Count(new(Notice)) + } + return } // Notices returns notices in given page. -func Notices(page, pageSize int) ([]*Notice, error) { - notices := make([]*Notice, 0, pageSize) - return notices, x. - Limit(pageSize, (page-1)*pageSize). - Desc("id"). - Find(¬ices) +func Notices(typ, page, pageSize int) (notices []*Notice, err error) { + notices = make([]*Notice, 0, pageSize) + + if typ >= 1 { + err = x.Where("type = ?", typ). + Limit(pageSize, (page-1)*pageSize). + Desc("id"). + Find(¬ices) + } else { + err = x. + Limit(pageSize, (page-1)*pageSize). + Desc("id"). + Find(¬ices) + } + + return notices, err } // DeleteNotice deletes a system notice by given ID. diff --git a/models/admin_test.go b/models/admin_test.go index b4383cb2d313..70568cd4112f 100644 --- a/models/admin_test.go +++ b/models/admin_test.go @@ -46,24 +46,35 @@ func TestCreateRepositoryNotice(t *testing.T) { func TestCountNotices(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - assert.Equal(t, int64(3), CountNotices()) + num, err := CountNotices(0) + assert.NoError(t, err) + assert.Equal(t, int64(3), num) + num, err = CountNotices(2) + assert.NoError(t, err) + assert.Equal(t, int64(1), num) } func TestNotices(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - notices, err := Notices(1, 2) + notices, err := Notices(0, 1, 2) assert.NoError(t, err) if assert.Len(t, notices, 2) { assert.Equal(t, int64(3), notices[0].ID) assert.Equal(t, int64(2), notices[1].ID) } - notices, err = Notices(2, 2) + notices, err = Notices(0, 2, 2) assert.NoError(t, err) if assert.Len(t, notices, 1) { assert.Equal(t, int64(1), notices[0].ID) } + + notices, err = Notices(2, 1, 2) + assert.NoError(t, err) + if assert.Len(t, notices, 1) { + assert.Equal(t, int64(3), notices[0].ID) + } } func TestDeleteNotice(t *testing.T) { diff --git a/models/fixtures/notice.yml b/models/fixtures/notice.yml index af08f07bfa13..b327f3beb4b4 100644 --- a/models/fixtures/notice.yml +++ b/models/fixtures/notice.yml @@ -10,5 +10,5 @@ - id: 3 - type: 1 # NoticeRepository + type: 2 # NoticeTaskSuccess description: description3 diff --git a/modules/cron/tasks.go b/modules/cron/tasks.go index a97326bd0f39..d817fdda6920 100644 --- a/modules/cron/tasks.go +++ b/modules/cron/tasks.go @@ -88,17 +88,17 @@ func (t *Task) RunWithUser(doer *models.User, config Config) { if err := t.fun(ctx, doer, config); err != nil { if models.IsErrCancelled(err) { message := err.(models.ErrCancelled).Message - if err := models.CreateNotice(models.NoticeTask, config.FormatMessage(t.Name, "aborted", doer, message)); err != nil { + if err := models.CreateTaskNotice(false, config.FormatMessage(t.Name, "aborted", doer, message)); err != nil { log.Error("CreateNotice: %v", err) } return } - if err := models.CreateNotice(models.NoticeTask, config.FormatMessage(t.Name, "error", doer, err)); err != nil { + if err := models.CreateTaskNotice(false, config.FormatMessage(t.Name, "error", doer, err)); err != nil { log.Error("CreateNotice: %v", err) } return } - if err := models.CreateNotice(models.NoticeTask, config.FormatMessage(t.Name, "finished", doer)); err != nil { + if err := models.CreateTaskNotice(true, config.FormatMessage(t.Name, "finished", doer)); err != nil { log.Error("CreateNotice: %v", err) } }) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 662ea49acaeb..fe4de785b3b7 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2288,7 +2288,10 @@ notices.delete_selected = Delete Selected notices.delete_all = Delete All Notices notices.type = Type notices.type_1 = Repository -notices.type_2 = Task +notices.type_2 = Success Task +notices.type_3 = Fail Task +notices.type_all = All Type +notices.type_filter = Type Filter notices.desc = Description notices.op = Op. notices.delete_success = The system notices have been deleted. diff --git a/routers/admin/notice.go b/routers/admin/notice.go index ad2bad21630c..c16763e6a3c4 100644 --- a/routers/admin/notice.go +++ b/routers/admin/notice.go @@ -25,13 +25,19 @@ func Notices(ctx *context.Context) { ctx.Data["PageIsAdmin"] = true ctx.Data["PageIsAdminNotices"] = true - total := models.CountNotices() page := ctx.QueryInt("page") if page <= 1 { page = 1 } - notices, err := models.Notices(page, setting.UI.Admin.NoticePagingNum) + typ := ctx.QueryInt("type") + total, err := models.CountNotices(typ) + if err != nil { + ctx.ServerError("CountNotices", err) + return + } + + notices, err := models.Notices(typ, page, setting.UI.Admin.NoticePagingNum) if err != nil { ctx.ServerError("Notices", err) return @@ -40,7 +46,11 @@ func Notices(ctx *context.Context) { ctx.Data["Total"] = total - ctx.Data["Page"] = context.NewPagination(int(total), setting.UI.Admin.NoticePagingNum, page, 5) + p := context.NewPagination(int(total), setting.UI.Admin.NoticePagingNum, page, 5) + ctx.Data["Page"] = p + + ctx.Data["NoticesType"] = typ + p.AddParam(ctx, "type", "NoticesType") ctx.HTML(200, tplNotices) } diff --git a/templates/admin/notice.tmpl b/templates/admin/notice.tmpl index 5311ad8f79cd..aaf5cd4b0c4f 100644 --- a/templates/admin/notice.tmpl +++ b/templates/admin/notice.tmpl @@ -5,6 +5,26 @@ {{template "base/alert" .}}

{{.i18n.Tr "admin.notices.system_notice_list"}} ({{.i18n.Tr "admin.total" .Total}}) +
+ + {{.i18n.Tr "admin.notices.type_filter"}} : + {{if eq .NoticesType 1}} + {{.i18n.Tr "admin.notices.type_1"}} + {{else if eq .NoticesType 2}} + {{.i18n.Tr "admin.notices.type_2"}} + {{else if eq .NoticesType 3}} + {{.i18n.Tr "admin.notices.type_3"}} + {{else}} + {{.i18n.Tr "admin.notices.type_all"}} + {{end}} + +
+ {{.i18n.Tr "admin.notices.type_1"}} + {{.i18n.Tr "admin.notices.type_2"}} + {{.i18n.Tr "admin.notices.type_3"}} + {{.i18n.Tr "admin.notices.type_all"}} +
+

From 1c18e12d6172459415820a43f41e0648741553d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=99=BA=E8=B6=85?= <1012112796@qq.com> Date: Thu, 18 Jun 2020 10:31:40 +0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Lauris BH --- models/admin.go | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/models/admin.go b/models/admin.go index dbf82ac118eb..f6769e8eefa5 100644 --- a/models/admin.go +++ b/models/admin.go @@ -62,14 +62,11 @@ func CreateRepositoryNotice(desc string, args ...interface{}) error { } // CreateTaskNotice creates new system notice with type NoticeTaskSuccess or NoticeTaskFail. -func CreateTaskNotice(isSuccess bool, desc string, args ...interface{}) (err error) { +func CreateTaskNotice(isSuccess bool, desc string, args ...interface{}) error { if isSuccess { - err = createNotice(x, NoticeTaskSuccess, desc, args...) - } else { - err = createNotice(x, NoticeTaskFail, desc, args...) + return createNotice(x, NoticeTaskSuccess, desc, args...) } - - return + return createNotice(x, NoticeTaskFail, desc, args...) } // RemoveAllWithNotice removes all directories in given path and @@ -89,32 +86,26 @@ func removeAllWithNotice(e Engine, title, path string) { } // CountNotices returns number of notices. -func CountNotices(typ int) (count int64, err error) { +func CountNotices(typ int) (int64, error) { if typ >= 1 { - count, err = x.Where("type = ?", typ).Count(new(Notice)) - } else { - count, err = x.Count(new(Notice)) + return x.Where("type = ?", typ).Count(new(Notice)) } - return + return x.Count(new(Notice)) } // Notices returns notices in given page. -func Notices(typ, page, pageSize int) (notices []*Notice, err error) { - notices = make([]*Notice, 0, pageSize) - +func Notices(typ, page, pageSize int) ([]*Notice, error) { + notices := make([]*Notice, 0, pageSize) if typ >= 1 { - err = x.Where("type = ?", typ). + return notices, x.Where("type = ?", typ). Limit(pageSize, (page-1)*pageSize). Desc("id"). Find(¬ices) - } else { - err = x. + } + return notices, x. Limit(pageSize, (page-1)*pageSize). Desc("id"). Find(¬ices) - } - - return notices, err } // DeleteNotice deletes a system notice by given ID. From d8506060a53acfad6bb5d3be392899dc83a791de Mon Sep 17 00:00:00 2001 From: a1012112796 <1012112796@qq.com> Date: Thu, 18 Jun 2020 10:48:19 +0800 Subject: [PATCH 3/3] move button to right --- models/admin.go | 6 +++--- templates/admin/notice.tmpl | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/models/admin.go b/models/admin.go index f6769e8eefa5..5eacf36b47b0 100644 --- a/models/admin.go +++ b/models/admin.go @@ -103,9 +103,9 @@ func Notices(typ, page, pageSize int) ([]*Notice, error) { Find(¬ices) } return notices, x. - Limit(pageSize, (page-1)*pageSize). - Desc("id"). - Find(¬ices) + Limit(pageSize, (page-1)*pageSize). + Desc("id"). + Find(¬ices) } // DeleteNotice deletes a system notice by given ID. diff --git a/templates/admin/notice.tmpl b/templates/admin/notice.tmpl index aaf5cd4b0c4f..01edbd85159c 100644 --- a/templates/admin/notice.tmpl +++ b/templates/admin/notice.tmpl @@ -5,7 +5,7 @@ {{template "base/alert" .}}

{{.i18n.Tr "admin.notices.system_notice_list"}} ({{.i18n.Tr "admin.total" .Total}}) -
+
{{.i18n.Tr "admin.notices.type_filter"}} : {{if eq .NoticesType 1}}