diff --git a/internal/cmd/sync.go b/internal/cmd/sync.go index 7e55372..a0dd5bc 100644 --- a/internal/cmd/sync.go +++ b/internal/cmd/sync.go @@ -15,7 +15,9 @@ var ( noop bool force bool notificationDumpPath string - refreshStrategy managerPkg.RefreshStrategy + + refreshStrategy managerPkg.RefreshStrategy + forceStrategy managerPkg.ForceStrategy syncCmd = &cobra.Command{ Use: "sync", @@ -34,8 +36,10 @@ func init() { rootCmd.AddCommand(syncCmd) syncCmd.Flags().BoolVarP(&noop, "noop", "n", false, "Doesn't execute any action") - syncCmd.Flags().BoolVarP(&force, "force", "f", false, "Force the execution of the rules on Done notifications") + + syncCmd.Flags().VarP(&forceStrategy, "force-strategy", "f", fmt.Sprintf("Force strategy: %s", forceStrategy.Allowed())) syncCmd.Flags().VarP(&refreshStrategy, "refresh-strategy", "r", fmt.Sprintf("Refresh strategy: %s", refreshStrategy.Allowed())) + syncCmd.Flags().StringVarP(¬ificationDumpPath, "from-file", "", "", "Path to notification dump in JSON (generate with 'gh api /notifications')") } @@ -52,7 +56,9 @@ func runSync(cmd *cobra.Command, args []string) error { return err } } - manager.WithRefresh(refreshStrategy).WithCaller(caller) + manager.Force = forceStrategy + manager.Refresh = refreshStrategy + manager.WithCaller(caller) if err := manager.Load(); err != nil { slog.Error("Failed to load the notifications", "err", err) diff --git a/internal/gh/enrichments.go b/internal/gh/enrichments.go index ed943b5..112e591 100644 --- a/internal/gh/enrichments.go +++ b/internal/gh/enrichments.go @@ -13,8 +13,8 @@ type Extra struct { HtmlUrl string `json:"html_url"` } -func (c *Client) enrichNotification(n *notifications.Notification) error { - if n.Meta.Done { +func (c *Client) Enrich(n *notifications.Notification) error { + if n == nil { return nil } diff --git a/internal/gh/gh.go b/internal/gh/gh.go index 37abfa9..40fc2b8 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -144,18 +144,6 @@ func (c *Client) paginate() (notifications.Notifications, error) { return list, nil } -func (c *Client) Enrich(ns notifications.Notifications) (notifications.Notifications, error) { - for i, n := range ns { - if err := c.enrichNotification(n); err != nil { - return nil, err - } - - ns[i] = n - } - - return ns, nil -} - func (c *Client) Notifications() (notifications.Notifications, error) { return c.paginate() } diff --git a/internal/gh/gh_test.go b/internal/gh/gh_test.go index ab487cf..cc71e91 100644 --- a/internal/gh/gh_test.go +++ b/internal/gh/gh_test.go @@ -30,15 +30,19 @@ func mockSubjectUrl(id int) string { return "https://subject.url/" + strconv.Itoa(id) } +func mockNotification(id int) *notifications.Notification { + return ¬ifications.Notification{ + Id: strconv.Itoa(id), + Subject: notifications.Subject{ + URL: mockSubjectUrl(id), + }, + } +} + func mockNotifications(ids []int) []*notifications.Notification { n := []*notifications.Notification{} for _, id := range ids { - n = append(n, ¬ifications.Notification{ - Id: strconv.Itoa(id), - Subject: notifications.Subject{ - URL: mockSubjectUrl(id), - }, - }) + n = append(n, mockNotification(id)) } return n } @@ -467,53 +471,36 @@ func TestPaginate(t *testing.T) { func TestNotifications(t *testing.T) { tests := []struct { - name string - calls []mock.Call - notifications []*notifications.Notification - error error + name string + calls mock.Call + notification *notifications.Notification + error error }{ { name: "no notification", }, { - name: "one notification", - calls: []mock.Call{ - {Endpoint: mockSubjectUrl(0)}, - }, - notifications: mockNotifications([]int{0}), + name: "one notification", + calls: mock.Call{Endpoint: mockSubjectUrl(0)}, + notification: mockNotification(0), }, { - name: "multiple notifications", - calls: []mock.Call{ - {Endpoint: mockSubjectUrl(0)}, - {Endpoint: mockSubjectUrl(1)}, - {Endpoint: mockSubjectUrl(2)}, - }, - notifications: mockNotifications([]int{0, 1, 2}), - }, - { - name: "fail to enrich", - calls: []mock.Call{ - {Endpoint: mockSubjectUrl(0)}, - {Error: sampleError}, - }, - notifications: mockNotifications([]int{0, 1}), - error: sampleError, + name: "fail to enrich", + calls: mock.Call{Error: sampleError}, + notification: mockNotification(0), + error: sampleError, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - client := mockClient(test.calls) + client := mockClient([]mock.Call{test.calls}) - notifications, err := client.Enrich(test.notifications) + err := client.Enrich(test.notification) - if test.error == nil { - if !notificationsEqual(notifications, test.notifications) { - t.Errorf("want %#v, got %#v", test.notifications, notifications) - } - } else { + // TODO: make this test check for the author/subject + if test.error != nil { if !errors.Is(err, test.error) { t.Errorf("want %#v, got %#v", test.error, err) } diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 9d8caff..67cd31b 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -19,7 +19,9 @@ type Manager struct { config *config.Data client *gh.Client Actors actors.ActorsMap - refresh RefreshStrategy + + Refresh RefreshStrategy + Force ForceStrategy } func New(config *config.Data) *Manager { @@ -38,12 +40,6 @@ func (m *Manager) WithCaller(caller api.Caller) *Manager { return m } -func (m *Manager) WithRefresh(refresh RefreshStrategy) *Manager { - m.refresh = refresh - - return m -} - func (m *Manager) Load() error { m.Notifications = notifications.Notifications{} @@ -64,12 +60,12 @@ func (m *Manager) Load() error { } func (m *Manager) shouldRefresh(expired bool) bool { - if !expired && m.refresh == ForceRefresh { + if !expired && m.Refresh == ForceRefresh { slog.Info("forcing a refresh") return true } - if expired && m.refresh == PreventRefresh { + if expired && m.Refresh == PreventRefresh { slog.Info("preventing a refresh") return false } @@ -98,7 +94,7 @@ func (m *Manager) refreshNotifications() error { m.Notifications = m.Notifications.Uniq() - m.Notifications, err = m.client.Enrich(m.Notifications) + m.Notifications, err = m.Enrich(m.Notifications) return err } @@ -107,6 +103,22 @@ func (m *Manager) Save() error { return m.cache.Write(m.Notifications.Compact()) } +func (m *Manager) Enrich(ns notifications.Notifications) (notifications.Notifications, error) { + for i, n := range ns { + if n.Meta.Done && !m.Force.Has(ForceEnrich) { + continue + } + + if err := m.client.Enrich(n); err != nil { + return nil, err + } + + ns[i] = n + } + + return ns, nil +} + func (m *Manager) loadCache() (notifications.Notifications, bool, error) { expired, err := m.cache.Expired() if err != nil { @@ -137,7 +149,7 @@ func (m *Manager) Apply(noop, force bool) error { slog.Debug("apply rule", "name", rule.Name, "count", len(selectedIds)) for _, notification := range m.Notifications.FilterFromIds(selectedIds) { - if notification.Meta.Done && !force { + if notification.Meta.Done && !m.Force.Has(ForceApply) { slog.Debug("skipping done notification", "id", notification.Id) continue } diff --git a/internal/manager/refreshStrategy.go b/internal/manager/refreshStrategy.go deleted file mode 100644 index 0b338dc..0000000 --- a/internal/manager/refreshStrategy.go +++ /dev/null @@ -1,47 +0,0 @@ -package manager - -import "fmt" - -// RefreshStrategy is an enum for the refresh strategy. -// It implements https://pkg.go.dev/github.com/spf13/pflag#Value. -type RefreshStrategy int - -const ( - AutoRefresh RefreshStrategy = iota - ForceRefresh - PreventRefresh -) - -func (r RefreshStrategy) String() string { - switch r { - case AutoRefresh: - return "auto" - case ForceRefresh: - return "force" - case PreventRefresh: - return "prevent" - } - return "unknown" -} - -func (r *RefreshStrategy) Allowed() string { - return "auto, force, prevent" -} - -func (r *RefreshStrategy) Set(value string) error { - switch value { - case "auto": - *r = ForceRefresh - case "force": - *r = ForceRefresh - case "prevent": - *r = PreventRefresh - default: - return fmt.Errorf(`must be one of %s`, r.Allowed()) - } - return nil -} - -func (r RefreshStrategy) Type() string { - return "RefreshStrategy" -} diff --git a/internal/manager/strategies.go b/internal/manager/strategies.go new file mode 100644 index 0000000..2f57a22 --- /dev/null +++ b/internal/manager/strategies.go @@ -0,0 +1,113 @@ +package manager + +import ( + "fmt" + "strings" +) + +// RefreshStrategy is an enum for the refresh strategy. +// It implements https://pkg.go.dev/github.com/spf13/pflag#Value. +type RefreshStrategy int + +const ( + // AutoRefresh refreshes the notifications if the cache is expired. + AutoRefresh RefreshStrategy = iota + + // ForceRefresh always refreshes the notifications. + ForceRefresh + + // PreventRefresh never refreshes the notifications. + PreventRefresh +) + +func (r RefreshStrategy) String() string { + switch r { + case AutoRefresh: + return "auto" + case ForceRefresh: + return "force" + case PreventRefresh: + return "prevent" + } + return "unknown" +} + +func (r *RefreshStrategy) Allowed() string { + return "auto, force, prevent" +} + +func (r *RefreshStrategy) Set(value string) error { + switch value { + case "auto": + *r = AutoRefresh + case "force": + *r = ForceRefresh + case "prevent": + *r = PreventRefresh + default: + return fmt.Errorf(`must be one of %s`, r.Allowed()) + } + + return nil +} + +func (r RefreshStrategy) Type() string { + return "RefreshStrategy" +} + +// ForceStrategy is an enum for the force strategy. +// It implements https://pkg.go.dev/github.com/spf13/pflag#Value. +type ForceStrategy int + +const ( + // ForceApply forces the application of the ruleset on all notifications, + // even the ones marked as Done. + ForceApply ForceStrategy = 1 << iota + + // ForceApply forces the enrichment of all notifications, even the ones + // marked as Done. + ForceEnrich +) + +func (r ForceStrategy) Has(s ForceStrategy) bool { + return r&s != 0 +} + +func (r ForceStrategy) String() string { + s := []string{} + + if r.Has(ForceApply) { + s = append(s, "apply") + } + + if r.Has(ForceEnrich) { + s = append(s, "enrich") + } + + return strings.Join(s, ", ") +} + +func (r ForceStrategy) Allowed() string { + return "apply, enrich" +} + +func (r *ForceStrategy) Set(value string) error { + v := strings.Split(value, ",") + + for _, s := range v { + switch s { + case "apply": + *r |= ForceApply + case "enrich": + *r |= ForceEnrich + default: + return fmt.Errorf(`must be one of %s`, r.Allowed()) + } + } + + return nil +} + +func (r ForceStrategy) Type() string { + return "ForceStrategy" +} diff --git a/internal/manager/strategies_test.go b/internal/manager/strategies_test.go new file mode 100644 index 0000000..4df09ca --- /dev/null +++ b/internal/manager/strategies_test.go @@ -0,0 +1,111 @@ +package manager + +import "testing" + +func TestRefreshStrategy(t *testing.T) { + t.Run("String", func(t *testing.T) { + tests := []struct { + strategy RefreshStrategy + want string + }{ + {AutoRefresh, "auto"}, + {ForceRefresh, "force"}, + {PreventRefresh, "prevent"}, + {RefreshStrategy(-1), "unknown"}, + } + + for _, test := range tests { + if got := test.strategy.String(); got != test.want { + t.Errorf("RefreshStrategy.String() = %q, want %q", got, test.want) + } + } + }) + + t.Run("Set", func(t *testing.T) { + tests := []struct { + value string + want RefreshStrategy + error bool + }{ + {"auto", AutoRefresh, false}, + {"force", ForceRefresh, false}, + {"prevent", PreventRefresh, false}, + {"test", 0, true}, + } + + for _, test := range tests { + t.Run(test.value, func(t *testing.T) { + var got RefreshStrategy + error := got.Set(test.value) + + if test.error { + if error == nil { + t.Errorf("expected an error but got none") + } + } else { + if error != nil { + t.Errorf("expected no error but got %#v", error) + } + + if got != test.want { + t.Errorf("RefreshStrategy.Set(%s) = %q, want %q", test.value, got, test.want) + } + } + }) + } + }) +} + +func TestForceStrategy(t *testing.T) { + t.Run("String", func(t *testing.T) { + tests := []struct { + strategy ForceStrategy + want string + }{ + {ForceStrategy(0), ""}, + {ForceApply, "apply"}, + {ForceEnrich, "enrich"}, + {ForceApply | ForceEnrich, "apply, enrich"}, + } + + for _, test := range tests { + if got := test.strategy.String(); got != test.want { + t.Errorf("ForceStrategy.String() = %q, want %q", got, test.want) + } + } + }) + + t.Run("Set", func(t *testing.T) { + tests := []struct { + value string + want ForceStrategy + error bool + }{ + {"apply", ForceApply, false}, + {"enrich", ForceEnrich, false}, + {"apply,enrich", ForceApply | ForceEnrich, false}, + {"test", 0, true}, + } + + for _, test := range tests { + t.Run(test.value, func(t *testing.T) { + var got ForceStrategy + error := got.Set(test.value) + + if test.error { + if error == nil { + t.Errorf("expected an error but got none") + } + } else { + if error != nil { + t.Errorf("expected no error but got %#v", error) + } + + if got != test.want { + t.Errorf("RefreshStrategy.Set(%s) = %q, want %q", test.value, got, test.want) + } + } + }) + } + }) +}