Skip to content

Commit

Permalink
feat(notification): add LastCommentor field (#168)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nobe4
nobe4 authored Sep 14, 2024
1 parent 0467283 commit ff20a75
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 43 deletions.
6 changes: 0 additions & 6 deletions internal/api/mock/mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"io"
"log/slog"
"net/http"

"github.com/nobe4/gh-not/internal/api"
)

type Mock struct {
Expand All @@ -25,10 +23,6 @@ func (e *MockError) Error() string {
return fmt.Sprintf("mock error for call [%s %s]: %s", e.verb, e.endpoint, e.message)
}

func New(c []Call) api.Requestor {
return &Mock{Calls: c}
}

func (m *Mock) Done() error {
if m.index < len(m.Calls) {
return &MockError{"", "", fmt.Sprintf("%d calls remaining", len(m.Calls)-m.index)}
Expand Down
66 changes: 56 additions & 10 deletions internal/gh/enrichments.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/nobe4/gh-not/internal/notifications"
)

type Extra struct {
type ThreadExtra struct {
User notifications.User `json:"user"`
State string `json:"state"`
HtmlUrl string `json:"html_url"`
Expand All @@ -20,27 +20,73 @@ func (c *Client) Enrich(n *notifications.Notification) error {
return nil
}

slog.Debug("enriching", "id", n.Id, "url", n.Subject.URL)
resp, err := c.API.Request(http.MethodGet, n.Subject.URL, nil)
threadExtra, err := c.getThreadExtra(n)
if err != nil {
return err
}
n.Author = threadExtra.User
n.Subject.State = threadExtra.State
n.Subject.HtmlUrl = threadExtra.HtmlUrl

LastCommentor, err := c.getLastCommentor(n)
if err != nil {
return err
}
n.LatestCommentor = LastCommentor

return nil
}

func (c *Client) getThreadExtra(n *notifications.Notification) (ThreadExtra, error) {
if n.Subject.URL == "" {
return ThreadExtra{}, nil
}

slog.Debug("getting the thread extra", "id", n.Id, "url", n.Subject.URL)
resp, err := c.API.Request(http.MethodGet, n.Subject.URL, nil)
if err != nil {
return ThreadExtra{}, err
}
defer resp.Body.Close()

body, err := io.ReadAll(resp.Body)
if err != nil {
return err
return ThreadExtra{}, err
}

extra := Extra{}
extra := ThreadExtra{}
err = json.Unmarshal(body, &extra)
if err != nil {
return err
return ThreadExtra{}, err
}

n.Author = extra.User
n.Subject.State = extra.State
n.Subject.HtmlUrl = extra.HtmlUrl
return extra, nil
}

return nil
func (c *Client) getLastCommentor(n *notifications.Notification) (notifications.User, error) {
if n.Subject.LatestCommentUrl == "" {
return notifications.User{}, nil
}

slog.Debug("getting the last commentor", "id", n.Id, "url", n.Subject.LatestCommentUrl)
resp, err := c.API.Request(http.MethodGet, n.Subject.LatestCommentUrl, nil)
if err != nil {
return notifications.User{}, err
}
defer resp.Body.Close()

body, err := io.ReadAll(resp.Body)
if err != nil {
return notifications.User{}, err
}

comment := struct {
User notifications.User `json:"user"`
}{}
err = json.Unmarshal(body, &comment)
if err != nil {
return notifications.User{}, err
}

return comment.User, nil
}
77 changes: 55 additions & 22 deletions internal/gh/gh_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ func mockSubjectUrl(id int) string {
return "https://subject.url/" + strconv.Itoa(id)
}

func mockLatestCommentUrl(id int) string {
return "https://latest.comment.url/" + strconv.Itoa(id)
}

func mockNotification(id int) *notifications.Notification {
return &notifications.Notification{
Id: strconv.Itoa(id),
Subject: notifications.Subject{
URL: mockSubjectUrl(id),
URL: mockSubjectUrl(id),
LatestCommentUrl: mockLatestCommentUrl(id),
},
}
}
Expand Down Expand Up @@ -87,13 +92,14 @@ func notificationsEqual(a, b []*notifications.Notification) bool {
return true
}

func mockClient(c []mock.Call) *Client {
func mockClient(c []mock.Call) (*Client, *mock.Mock) {
mock := &mock.Mock{Calls: c}
return &Client{
API: mock.New(c),
API: mock,
endpoint: endpoint,
maxRetry: 100,
maxPage: 100,
}
}, mock
}

func TestIsRetryable(t *testing.T) {
Expand Down Expand Up @@ -247,7 +253,7 @@ func TestNextPageLink(t *testing.T) {
func TestRequest(t *testing.T) {
t.Run("errors", func(t *testing.T) {
expectedError := errors.New("error")
client := mockClient([]mock.Call{{Error: expectedError}})
client, api := mockClient([]mock.Call{{Error: expectedError}})

_, _, err := client.request(verb, endpoint, nil)
if err == nil {
Expand All @@ -257,14 +263,18 @@ func TestRequest(t *testing.T) {
if err != expectedError {
t.Errorf("expected %#v, got %#v", expectedError, err)
}

if err := api.Done(); err != nil {
t.Fatal(err)
}
})

t.Run("calls parse", func(t *testing.T) {
response := &http.Response{
Body: io.NopCloser(strings.NewReader(`[{"id":"0"}]`)),
Header: http.Header{"Link": []string{`<https://next.page>; rel="next"`}},
}
client := mockClient([]mock.Call{{Response: response}})
client, api := mockClient([]mock.Call{{Response: response}})

notifications, next, err := client.request(verb, endpoint, nil)
if err != nil {
Expand All @@ -282,6 +292,10 @@ func TestRequest(t *testing.T) {
if notifications[0].Id != "0" {
t.Errorf("expected notification id 0, got %s", notifications[0].Id)
}

if err := api.Done(); err != nil {
t.Fatal(err)
}
})
}

Expand Down Expand Up @@ -338,7 +352,7 @@ func TestRetry(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
client := mockClient(test.calls)
client, api := mockClient(test.calls)
client.maxRetry = test.maxRetry

notifications, _, err := client.retry(verb, endpoint, nil)
Expand All @@ -350,6 +364,9 @@ func TestRetry(t *testing.T) {
if !notificationsEqual(notifications, test.notifications) {
t.Errorf("want %#v, got %#v", test.notifications, notifications)
}
if err := api.Done(); err != nil {
t.Fatal(err)
}
})
}
}
Expand Down Expand Up @@ -389,7 +406,6 @@ func TestPaginate(t *testing.T) {
calls: []mock.Call{
{Error: retriableError},
{Error: retriableError},
{Error: retriableError},
},
error: retryError,
},
Expand All @@ -416,7 +432,6 @@ func TestPaginate(t *testing.T) {
maxPage: 1,
calls: []mock.Call{
{Response: mockNotificationsResponse(t, []int{0}, true)},
{Error: sampleError},
},
notifications: mockNotifications([]int{0}),
},
Expand Down Expand Up @@ -444,15 +459,14 @@ func TestPaginate(t *testing.T) {
calls: []mock.Call{
{Response: mockNotificationsResponse(t, []int{0}, true)},
{Response: mockNotificationsResponse(t, []int{1}, true)},
{Response: mockNotificationsResponse(t, []int{2}, true)},
},
notifications: mockNotifications([]int{0, 1}),
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
client := mockClient(test.calls)
client, api := mockClient(test.calls)
client.maxRetry = test.maxRetry
client.maxPage = test.maxPage

Expand All @@ -465,14 +479,17 @@ func TestPaginate(t *testing.T) {
if !notificationsEqual(notifications, test.notifications) {
t.Errorf("want %#v, got %#v", test.notifications, notifications)
}
if err := api.Done(); err != nil {
t.Fatal(err)
}
})
}
}

func TestEnrich(t *testing.T) {
tests := []struct {
name string
calls mock.Call
calls []mock.Call
notification *notifications.Notification
assertError func(*testing.T, error)
}{
Expand All @@ -482,17 +499,25 @@ func TestEnrich(t *testing.T) {
},
{
name: "one notification",
calls: mock.Call{
Endpoint: mockSubjectUrl(0),
Response: &http.Response{
Body: io.NopCloser(strings.NewReader(`{"author":{"login":"author"},"subject":{"title":"subject"}}`)),
calls: []mock.Call{
{
Endpoint: mockSubjectUrl(0),
Response: &http.Response{
Body: io.NopCloser(strings.NewReader(`{"author":{"login":"author"},"subject":{"title":"subject"}}`)),
},
},
{
Endpoint: mockLatestCommentUrl(0),
Response: &http.Response{
Body: io.NopCloser(strings.NewReader(`{"author":{"login":"author"}}`)),
},
},
},
notification: mockNotification(0),
},
{
name: "fail to enrich",
calls: mock.Call{Error: sampleError},
calls: []mock.Call{{Error: sampleError}},
notification: mockNotification(0),
assertError: func(t *testing.T, err error) {
if err != sampleError {
Expand All @@ -502,10 +527,12 @@ func TestEnrich(t *testing.T) {
},
{
name: "fail to unmarshal",
calls: mock.Call{
Endpoint: mockSubjectUrl(0),
Response: &http.Response{
Body: io.NopCloser(strings.NewReader(`not json`)),
calls: []mock.Call{
{
Endpoint: mockSubjectUrl(0),
Response: &http.Response{
Body: io.NopCloser(strings.NewReader(`not json`)),
},
},
},
notification: mockNotification(0),
Expand All @@ -524,13 +551,19 @@ func TestEnrich(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
client := mockClient([]mock.Call{test.calls})
client, api := mockClient(test.calls)

err := client.Enrich(test.notification)

// TODO: make this test check for the author/subject
if test.assertError != nil {
test.assertError(t, err)
} else if err != nil {
t.Errorf("expected no error but got %#v", err)
}

if err := api.Done(); err != nil {
t.Fatal(err)
}
})
}
Expand Down
12 changes: 8 additions & 4 deletions internal/notifications/notifications.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ type Notification struct {
Subject Subject `json:"subject"`

// Enriched API fields
Author User `json:"author"`
Author User `json:"author"`
LatestCommentor User `json:"latest_commentor"`

// gh-not specific fields
// Those fields are not part of the GitHub API and will persist between
Expand All @@ -53,9 +54,10 @@ type Meta struct {

type Subject struct {
// Standard API fields
Title string `json:"title"`
URL string `json:"url"`
Type string `json:"type"`
Title string `json:"title"`
URL string `json:"url"`
Type string `json:"type"`
LatestCommentUrl string `json:"latest_comment_url"`

// Enriched API fields
State string `json:"state"`
Expand Down Expand Up @@ -110,6 +112,8 @@ func (n Notification) Equal(other *Notification) bool {
n.Subject.HtmlUrl == other.Subject.HtmlUrl &&
n.Author.Login == other.Author.Login &&
n.Author.Type == other.Author.Type &&
n.LatestCommentor.Login == other.LatestCommentor.Login &&
n.LatestCommentor.Type == other.LatestCommentor.Type &&
n.Meta.Hidden == other.Meta.Hidden &&
n.Meta.Done == other.Meta.Done &&
n.Meta.RemoteExists == other.Meta.RemoteExists
Expand Down
6 changes: 5 additions & 1 deletion internal/notifications/view.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ func (n Notifications) Render() error {
printer.AddField(n.Repository.FullName)
printer.AddField(n.Author.Login)
printer.AddField(n.Subject.Title)
printer.AddField(text.RelativeTimeAgo(time.Now(), n.UpdatedAt))
relativeTime := text.RelativeTimeAgo(time.Now(), n.UpdatedAt)
if n.LatestCommentor.Login != "" {
relativeTime += " by " + n.LatestCommentor.Login
}
printer.AddField(relativeTime)
printer.EndRow()
}

Expand Down

0 comments on commit ff20a75

Please sign in to comment.