From b91cfd8484e26a5190998f274bf1f56f1d61804a Mon Sep 17 00:00:00 2001 From: Robert Estelle Date: Mon, 4 Sep 2023 19:34:44 -0700 Subject: [PATCH] `gh status`: show status even if a comment 404s (#7873) --- pkg/cmd/status/status.go | 31 +++++++++++++------ pkg/cmd/status/status_test.go | 57 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 7e39d63d098..4e6bd37818b 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -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{ @@ -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) - } } } } diff --git a/pkg/cmd/status/status_test.go b/pkg/cmd/status/status_test.go index c592c90a0f4..d9fdd54bcf5 100644 --- a/pkg/cmd/status/status_test.go +++ b/pkg/cmd/status/status_test.go @@ -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) {