add subcomands for notifications #283

Closed
khmarbaise wants to merge 7 commits from khmarbaise/tea:issue-243-add_subcomands_for_notifications into main
6 changed files with 228 additions and 15 deletions
Showing only changes of commit 74047bee0d - Show all commits

View File

@ -5,7 +5,9 @@
package cmd
import (
"code.gitea.io/tea/cmd/flags"
"log"
"code.gitea.io/tea/cmd/notifications"
"code.gitea.io/tea/modules/context"
"code.gitea.io/tea/modules/print"
@ -18,27 +20,21 @@ var CmdNotifications = cli.Command{
Name: "notifications",
Aliases: []string{"notification", "n"},
Usage: "Show notifications",
Description: "Show notifications, by default based of the current repo and unread one",
Description: "Show notifications, by default based of the current repo",
Action: runNotifications,
Subcommands: []*cli.Command{
&notifications.CmdNotificationsList,
&notifications.CmdNotificationsPinned,
&notifications.CmdNotificationsRead,
&notifications.CmdNotificationsUnread,
},
Flags: append([]cli.Flag{
&cli.BoolFlag{
Name: "all",
Aliases: []string{"a"},
Usage: "show all notifications of related gitea instance",
},
&cli.BoolFlag{
Name: "read",
Aliases: []string{"rd"},
Usage: "show read notifications instead unread",
},
&cli.BoolFlag{
Name: "pinned",
Aliases: []string{"pd"},
Usage: "show pinned notifications instead unread",
},
&flags.PaginationPageFlag,
&flags.PaginationLimitFlag,
}, flags.AllDefaultFlags...),
}),
}
func runNotifications(cmd *cli.Context) error {
@ -80,3 +76,8 @@ func runNotifications(cmd *cli.Context) error {
print.NotificationsList(news, ctx.Output, ctx.Bool("all"))
return nil
}
func runNotificationsDetails(ctx *cli.Context) error {
log.Fatal("Not yet implemented.")
return nil
}

40
cmd/notifications/list.go Normal file
View File

@ -0,0 +1,40 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package notifications
import (
"code.gitea.io/sdk/gitea"
"code.gitea.io/tea/cmd/flags"
"code.gitea.io/tea/modules/task"
"github.com/urfave/cli/v2"
)
// CmdNotificationsList represents a sub command of notifications to list notifications
var CmdNotificationsList = cli.Command{
Name: "ls",
Aliases: []string{"list"},
Usage: "List notifications....",
Description: `List notification....`,
noerw marked this conversation as resolved Outdated
Outdated
Review
- 	Usage:       "List notifications....",
+ 	Usage:       "List notifications",
```diff - Usage: "List notifications....", + Usage: "List notifications", ```
Action: RunNotificationsList,
Flags: append([]cli.Flag{
&cli.BoolFlag{
noerw marked this conversation as resolved Outdated
Outdated
Review

Please move these common flags into cmd/flags/flags.go as NotificationFlags

Please move these common flags into `cmd/flags/flags.go` as `NotificationFlags`

Good idea not even realized that. Thanks.

Good idea not even realized that. Thanks.
Name: "all",
Aliases: []string{"a"},
Usage: "show all notifications of related gitea instance",
noerw marked this conversation as resolved Outdated
Outdated
Review
- 			Usage:   "show all notifications of related gitea instance",
+ 			Usage:   "Show notifications across all your repos instead of the current repo only",
```diff - Usage: "show all notifications of related gitea instance", + Usage: "Show notifications across all your repos instead of the current repo only", ```
},
khmarbaise marked this conversation as resolved Outdated
Outdated
Review

@khmarbaise can you move task.ListNotifications back to cmd/notification module

I dont see a reason to add cli & flag dependency into task module

@khmarbaise can you move task.ListNotifications back to cmd/notification module I dont see a reason to add **cli** & **flag** dependency into task module

So making a file notifications_list.go which contains that. I will do and change it accordingly.

So making a file `notifications_list.go` which contains that. I will do and change it accordingly.

Done as suggested.

Done as suggested.
&cli.StringFlag{
Name: "state",
Usage: "Filter by milestone state (all|open|closed)",
noerw marked this conversation as resolved Outdated
Outdated
Review
  • Are valid states really called all/open/closed? all/read/unread?
  • Doesn't this flag duplicate functionality from the subcommands read/unread?
  • s/milestone/notification
- Are valid states really called all/open/closed? all/read/unread? - Doesn't this flag duplicate functionality from the subcommands read/unread? - s/milestone/notification

Yes of course.

Yes of course.
DefaultText: "open",
},
&flags.PaginationPageFlag,
&flags.PaginationLimitFlag,
}, flags.AllDefaultFlags...),
}
// RunNotificationsList list notifications
func RunNotificationsList(ctx *cli.Context) error {
return task.ListNotifications(ctx, []gitea.NotifyStatus{})
}

View File

@ -0,0 +1,41 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package notifications
import (
"code.gitea.io/sdk/gitea"
"code.gitea.io/tea/cmd/flags"
"code.gitea.io/tea/modules/task"
"github.com/urfave/cli/v2"
)
// CmdNotificationsPinned represents a sub command of notifications to list pinned notifications
var CmdNotificationsPinned = cli.Command{
Name: "pinned",
Aliases: []string{"pin"},
noerw marked this conversation as resolved Outdated
Outdated
Review

"pin" makes it sound like a verb, not a read list-command.
I'd drop this alias

"pin" makes it sound like a verb, not a read list-command. I'd drop this alias

What about with an alias pd instead?

What about with an alias `pd` instead?
Usage: "show pinned notifications",
Description: `show pinned notifications`,
Action: RunNotificationsPinned,
Flags: append([]cli.Flag{
&cli.BoolFlag{
Name: "all",
Aliases: []string{"a"},
Usage: "show all notifications of related gitea instance",
},
&cli.StringFlag{
Name: "state",
Usage: "Filter by milestone state (all|open|closed)",
DefaultText: "open",
},
&flags.PaginationPageFlag,
&flags.PaginationLimitFlag,
}, flags.AllDefaultFlags...),
}
// RunNotificationsPinned will show notifications with status pinned.
func RunNotificationsPinned(ctx *cli.Context) error {
var statuses = []gitea.NotifyStatus{gitea.NotifyStatusPinned}
return task.ListNotifications(ctx, statuses)
}

41
cmd/notifications/read.go Normal file
View File

@ -0,0 +1,41 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package notifications
import (
"code.gitea.io/sdk/gitea"
"code.gitea.io/tea/cmd/flags"
"code.gitea.io/tea/modules/task"
"github.com/urfave/cli/v2"
)
// CmdNotificationsRead represents a sub command of notifications to list read notifications
var CmdNotificationsRead = cli.Command{
Name: "read",
Aliases: []string{},
Usage: "show read notifications instead",
Description: `show read notifications instead`,
Action: RunNotificationsRead,
Flags: append([]cli.Flag{
&cli.BoolFlag{
Name: "all",
Aliases: []string{"a"},
Usage: "show all notifications of related gitea instance",
},
&cli.StringFlag{
Name: "state",
Usage: "Filter by milestone state (all|open|closed)",
DefaultText: "open",
},
&flags.PaginationPageFlag,
&flags.PaginationLimitFlag,
}, flags.AllDefaultFlags...),
}
// RunNotificationsList list notifications
func RunNotificationsRead(ctx *cli.Context) error {
var statuses = []gitea.NotifyStatus{gitea.NotifyStatusRead}
return task.ListNotifications(ctx, statuses)
}

View File

@ -0,0 +1,41 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package notifications
import (
"code.gitea.io/sdk/gitea"
"code.gitea.io/tea/cmd/flags"
"code.gitea.io/tea/modules/task"
"github.com/urfave/cli/v2"
)
// CmdNotificationsUnread represents a sub command of notifications to list unread notifications.
var CmdNotificationsUnread = cli.Command{
Name: "unread",
Aliases: []string{},
Usage: "show unread notifications",
Description: `show unread notifications`,
Action: RunNotificationsUnread,
Flags: append([]cli.Flag{
&cli.BoolFlag{
Name: "all",
Aliases: []string{"a"},
Usage: "show all notifications of related gitea instance",
},
&cli.StringFlag{
Name: "state",
Usage: "Filter by milestone state (all|open|closed)",
DefaultText: "open",
},
&flags.PaginationPageFlag,
&flags.PaginationLimitFlag,
}, flags.AllDefaultFlags...),
}
// RunNotificationsList list notifications
func RunNotificationsUnread(ctx *cli.Context) error {
var statuses = []gitea.NotifyStatus{gitea.NotifyStatusUnread}
return task.ListNotifications(ctx, statuses)
}

View File

@ -0,0 +1,49 @@
// Copyright 2020 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
noerw marked this conversation as resolved Outdated

Is this the way to go with common code from cmd/... ? Or should that put into cmd/notifications/ ?

Is this the way to go with common code from cmd/... ? Or should that put into cmd/notifications/ ?
package task
import (
"log"
"code.gitea.io/tea/cmd/flags"
"code.gitea.io/tea/modules/config"
"code.gitea.io/tea/modules/print"
"code.gitea.io/sdk/gitea"
"github.com/urfave/cli/v2"
)
func ListNotifications(ctx *cli.Context, status []gitea.NotifyStatus) error {
//TODO: What is the purpose of the following?
listOpts := flags.GetListOptions(ctx)
if listOpts.Page == 0 {
listOpts.Page = 1
}
6543 marked this conversation as resolved Outdated
Outdated
Review

@6453 ?

@6453 ?

If you take a look into flags.GetListOptions(ctx)

func GetListOptions(ctx *cli.Context) gitea.ListOptions {
	page := ctx.Int("page")
	limit := ctx.Int("limit")
	if limit != 0 && page == 0 {
		page = 1
	}
	return gitea.ListOptions{
		Page:     page,
		PageSize: limit,
	}
}
If you take a look into `flags.GetListOptions(ctx)` ``` func GetListOptions(ctx *cli.Context) gitea.ListOptions { page := ctx.Int("page") limit := ctx.Int("limit") if limit != 0 && page == 0 { page = 1 } return gitea.ListOptions{ Page: page, PageSize: limit, } } ```

After I've played a little bit with it which means removed the lines from notification_list.go I've not seen a difference. May I have missed a thing... Can someone enlighten me ?

After I've played a little bit with it which means removed the lines from notification_list.go I've not seen a difference. May I have missed a thing... Can someone enlighten me ?
Outdated
Review

I don't understand why, but these lines are needed still, without I get no results for tea notif read

I don't understand why, but these lines are needed still, without I get no results for `tea notif read`
Outdated
Review

in short: it enforces paggination

in short: it enforces paggination
var news []*gitea.NotificationThread
var err error
var allRelated = ctx.Bool("all")
if allRelated {
login := config.InitCommandLoginOnly(flags.GlobalLoginValue)
news, _, err = login.Client().ListNotifications(gitea.ListNotificationOptions{
ListOptions: listOpts,
Status: status,
})
} else {
login, owner, repo := config.InitCommand(flags.GlobalRepoValue, flags.GlobalLoginValue, flags.GlobalRemoteValue)
news, _, err = login.Client().ListRepoNotifications(owner, repo, gitea.ListNotificationOptions{
ListOptions: listOpts,
Status: status,
})
}
if err != nil {
log.Fatal(err)
}
print.NotificationsList(news, flags.GlobalOutputValue, allRelated)
return nil
}