Skip to content

Commit

Permalink
gh status: show status even if a comment 404s (#7873)
Browse files Browse the repository at this point in the history
  • Loading branch information
rwe committed Sep 5, 2023
1 parent f9df89d commit b91cfd8
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 9 deletions.
31 changes: 22 additions & 9 deletions pkg/cmd/status/status.go
Expand Up @@ -286,7 +286,28 @@ func (s *StatusGetter) LoadNotifications() error {
if !ok {
return nil
}
if actual, err := s.ActualMention(n.Subject.LatestCommentURL); actual != "" && err == nil {
actual, err := s.ActualMention(n.Subject.LatestCommentURL)

if err != nil {
var httpErr api.HTTPError
httpStatusCode := -1

if errors.As(err, &httpErr) {
httpStatusCode = httpErr.StatusCode
}

switch httpStatusCode {
case 403:
s.addAuthError(httpErr.Message, factory.SSOURL())
case 404:
return nil
default:
abortFetching()
return fmt.Errorf("could not fetch comment: %w", err)
}
}

if actual != "" {
// I'm so sorry
split := strings.Split(n.Subject.URL, "/")
fetched <- StatusItem{
Expand All @@ -295,14 +316,6 @@ func (s *StatusGetter) LoadNotifications() error {
preview: actual,
index: n.index,
}
} else if err != nil {
var httpErr api.HTTPError
if errors.As(err, &httpErr) && httpErr.StatusCode == 403 {
s.addAuthError(httpErr.Message, factory.SSOURL())
} else {
abortFetching()
return fmt.Errorf("could not fetch comment: %w", err)
}
}
}
}
Expand Down
57 changes: 57 additions & 0 deletions pkg/cmd/status/status_test.go
Expand Up @@ -295,6 +295,63 @@ func TestStatusRun(t *testing.T) {
warning: You don't have permission to access this resource.
`),
},
{
name: "not found errors",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL("UserCurrent"),
httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`))
reg.Register(
httpmock.REST("GET", "repos/rpd/todo/issues/110"),
httpmock.StringResponse(`{"body":"hello @jillvalentine how are you"}`))
reg.Register(
httpmock.REST("GET", "repos/rpd/todo/issues/4113"),
httpmock.StringResponse(`{"body":"this is a comment"}`))
reg.Register(
httpmock.REST("GET", "repos/cli/cli/issues/1096"),
httpmock.StringResponse(`{"body":"@jillvalentine hi"}`))
reg.Register(
httpmock.REST("GET", "repos/rpd/todo/issues/comments/1065"),
httpmock.StatusStringResponse(404, `{
"message": "Not Found."
}`))
reg.Register(
httpmock.REST("GET", "repos/vilmibm/gh-screensaver/issues/comments/10"),
httpmock.StringResponse(`{"body":"a message for @jillvalentine"}`))
reg.Register(
httpmock.GraphQL("AssignedSearch"),
httpmock.FileResponse("./fixtures/search.json"))
reg.Register(
httpmock.REST("GET", "notifications"),
httpmock.FileResponse("./fixtures/notifications.json"))
reg.Register(
httpmock.REST("GET", "users/jillvalentine/received_events"),
httpmock.FileResponse("./fixtures/events.json"))
},
opts: &StatusOptions{},
wantOut: heredoc.Doc(`
Assigned Issues │ Assigned Pull Requests
vilmibm/testing#157 yolo │ cli/cli#5272 Pin extensions
cli/cli#3223 Repo garden...│ rpd/todo#73 Board up RPD windows
rpd/todo#514 Reducing zo...│ cli/cli#4768 Issue Frecency
vilmibm/testing#74 welp │
adreyer/arkestrator#22 complete mo...│
Review Requests │ Mentions
cli/cli#5272 Pin extensions │ rpd/todo#110 hello @j...
vilmibm/testing#1234 Foobar │ cli/cli#1096 @jillval...
rpd/todo#50 Welcome party...│ vilmibm/gh-screensaver#15 a messag...
cli/cli#4671 This pull req...│
rpd/todo#49 Haircut for Leon│
Repository Activity
rpd/todo#5326 new PR Only write UTF-8 BOM on W...
vilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...
cli/cli#5319 comment on [Codespaces] D... Wondering if we shouldn't...
cli/cli#5300 new issue Terminal bell when a runn...
`),
},
{
name: "notification errors",
httpStubs: func(reg *httpmock.Registry) {
Expand Down

0 comments on commit b91cfd8

Please sign in to comment.