Skip to content

Commit 0c25135

Browse files
committed
Propagate activity lookup failures
Return request failures from activity and user lookups instead of printing and continuing. The command now emits one diagnostic and stops before rendering incomplete activity data.
1 parent db2239b commit 0c25135

4 files changed

Lines changed: 105 additions & 9 deletions

File tree

cmd/activity/list.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package activity
22

33
import (
4+
stderrors "errors"
5+
46
"github.com/spf13/cobra"
57

68
openerrors "github.com/opf/openproject-cli/components/errors"
@@ -29,7 +31,9 @@ func listActivities(_ *cobra.Command, _ []string) error {
2931
func listWorkPackageActivities(wpId string) error {
3032
acts, err := work_packages.Activities(wpId)
3133
if err != nil {
32-
printer.ErrorText(err.Error())
34+
if !stderrors.Is(err, openerrors.ErrHandled) {
35+
printer.Error(err)
36+
}
3337
return openerrors.ErrHandled
3438
}
3539

@@ -40,7 +44,13 @@ func listWorkPackageActivities(wpId string) error {
4044
}
4145
}
4246

43-
userList := users.ByIds(userIds)
47+
userList, err := users.ByIds(userIds)
48+
if err != nil {
49+
if !stderrors.Is(err, openerrors.ErrHandled) {
50+
printer.Error(err)
51+
}
52+
return openerrors.ErrHandled
53+
}
4454
printer.Activities(acts, userList)
4555
return nil
4656
}

cmd/activity/list_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package activity
2+
3+
import (
4+
"errors"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/url"
8+
"strings"
9+
"testing"
10+
11+
openerrors "github.com/opf/openproject-cli/components/errors"
12+
"github.com/opf/openproject-cli/components/printer"
13+
"github.com/opf/openproject-cli/components/requests"
14+
)
15+
16+
func TestListWorkPackageActivitiesReportsFetchErrorsOnce(t *testing.T) {
17+
server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, _ *http.Request) {
18+
http.Error(response, "activity failure", http.StatusInternalServerError)
19+
}))
20+
t.Cleanup(server.Close)
21+
22+
testingPrinter := initActivityTest(t, server.URL)
23+
err := listWorkPackageActivities("42")
24+
25+
if !errors.Is(err, openerrors.ErrHandled) {
26+
t.Fatalf("listWorkPackageActivities error = %v, want ErrHandled", err)
27+
}
28+
if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 {
29+
t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult)
30+
}
31+
if testingPrinter.Result != "" {
32+
t.Errorf("stdout = %q, want empty", testingPrinter.Result)
33+
}
34+
}
35+
36+
func TestListWorkPackageActivitiesReportsUserFetchErrorsOnce(t *testing.T) {
37+
server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
38+
response.Header().Set("Content-Type", "application/json")
39+
if request.URL.Path == "/api/v3/work_packages/42/activities" {
40+
_, _ = response.Write([]byte(`{
41+
"_embedded": {
42+
"elements": [{
43+
"id": 1,
44+
"comment": {"raw": ""},
45+
"details": [],
46+
"_links": {"user": {"href": "/api/v3/users/7"}}
47+
}]
48+
}
49+
}`))
50+
return
51+
}
52+
http.Error(response, "user failure", http.StatusInternalServerError)
53+
}))
54+
t.Cleanup(server.Close)
55+
56+
testingPrinter := initActivityTest(t, server.URL)
57+
err := listWorkPackageActivities("42")
58+
59+
if !errors.Is(err, openerrors.ErrHandled) {
60+
t.Fatalf("listWorkPackageActivities error = %v, want ErrHandled", err)
61+
}
62+
if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 {
63+
t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult)
64+
}
65+
if testingPrinter.Result != "" {
66+
t.Errorf("stdout = %q, want empty", testingPrinter.Result)
67+
}
68+
}
69+
70+
func initActivityTest(t *testing.T, serverURL string) *printer.TestingPrinter {
71+
t.Helper()
72+
73+
host, err := url.Parse(serverURL)
74+
if err != nil {
75+
t.Fatal(err)
76+
}
77+
requests.Init(host, "", false)
78+
79+
testingPrinter := &printer.TestingPrinter{}
80+
printer.Init(testingPrinter)
81+
if err := printer.InitRenderer("json"); err != nil {
82+
t.Fatal(err)
83+
}
84+
t.Cleanup(func() {
85+
_ = printer.InitRenderer("text")
86+
})
87+
return testingPrinter
88+
}

components/resources/users/functions.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,15 @@ import (
44
"github.com/opf/openproject-cli/components/common"
55
"github.com/opf/openproject-cli/components/parser"
66
"github.com/opf/openproject-cli/components/paths"
7-
"github.com/opf/openproject-cli/components/printer"
87
"github.com/opf/openproject-cli/components/requests"
98
"github.com/opf/openproject-cli/components/resources"
109
"github.com/opf/openproject-cli/dtos"
1110
"github.com/opf/openproject-cli/models"
1211
)
1312

14-
func ByIds(ids []uint64) []*models.User {
13+
func ByIds(ids []uint64) ([]*models.User, error) {
1514
if len(ids) == 0 {
16-
return []*models.User{}
15+
return []*models.User{}, nil
1716
}
1817
var filters []requests.Filter
1918
filters = append(filters, IdFilter(ids))
@@ -24,11 +23,11 @@ func ByIds(ids []uint64) []*models.User {
2423

2524
response, err := requests.Get(requestUrl, &query)
2625
if err != nil {
27-
printer.Error(err)
26+
return nil, err
2827
}
2928

3029
userCollection := parser.Parse[dtos.UserCollectionDto](response)
31-
return userCollection.Convert()
30+
return userCollection.Convert(), nil
3231
}
3332

3433
func Me() (*models.User, error) {

components/resources/work_packages/activities.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package work_packages
33
import (
44
"github.com/opf/openproject-cli/components/parser"
55
"github.com/opf/openproject-cli/components/paths"
6-
"github.com/opf/openproject-cli/components/printer"
76
"github.com/opf/openproject-cli/components/requests"
87
"github.com/opf/openproject-cli/dtos"
98
"github.com/opf/openproject-cli/models"
@@ -12,7 +11,7 @@ import (
1211
func Activities(id string) (activites []*models.Activity, err error) {
1312
response, err := requests.Get(paths.WorkPackageActivities(id), nil)
1413
if err != nil {
15-
printer.Error(err)
14+
return nil, err
1615
}
1716

1817
activitiesDto := parser.Parse[dtos.ActivityCollectionDto](response)

0 commit comments

Comments
 (0)