diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index bd4f5d79a46..e526283eec7 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -660,25 +660,52 @@ func (hs *HTTPServer) GetDashboardVersions(c *contextmodel.ReqContext) response. Start: c.QueryInt("start"), } - res, err := hs.dashboardVersionService.List(c.Req.Context(), &query) + versions, err := hs.dashboardVersionService.List(c.Req.Context(), &query) if err != nil { return response.Error(404, fmt.Sprintf("No versions found for dashboardId %d", dash.ID), err) } - for _, version := range res { + loginMem := make(map[int64]string, len(versions)) + res := make([]dashver.DashboardVersionMeta, 0, len(versions)) + for _, version := range versions { + msg := version.Message if version.RestoredFrom == version.Version { - version.Message = "Initial save (created by migration)" - continue + msg = "Initial save (created by migration)" } if version.RestoredFrom > 0 { - version.Message = fmt.Sprintf("Restored from version %d", version.RestoredFrom) - continue + msg = fmt.Sprintf("Restored from version %d", version.RestoredFrom) } if version.ParentVersion == 0 { - version.Message = "Initial save" + msg = "Initial save" } + + creator := anonString + if version.CreatedBy > 0 { + login, found := loginMem[version.CreatedBy] + if found { + creator = login + } else { + creator = hs.getUserLogin(c.Req.Context(), version.CreatedBy) + if creator != anonString { + loginMem[version.CreatedBy] = creator + } + } + } + + res = append(res, dashver.DashboardVersionMeta{ + ID: version.ID, + DashboardID: version.DashboardID, + DashboardUID: dash.UID, + Data: version.Data, + ParentVersion: version.ParentVersion, + RestoredFrom: version.RestoredFrom, + Version: version.Version, + Created: version.Created, + Message: msg, + CreatedBy: creator, + }) } return response.JSON(http.StatusOK, res) @@ -1267,7 +1294,7 @@ type GetHomeDashboardResponseBody struct { // swagger:response dashboardVersionsResponse type DashboardVersionsResponse struct { // in: body - Body []*dashver.DashboardVersionDTO `json:"body"` + Body []dashver.DashboardVersionMeta `json:"body"` } // swagger:response dashboardVersionResponse diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 2d2f0d42d22..c6e5731ac00 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -20,6 +21,8 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db/dbtest" + "github.com/grafana/grafana/pkg/infra/localcache" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/plugins" @@ -50,6 +53,7 @@ import ( "github.com/grafana/grafana/pkg/services/tag/tagimpl" "github.com/grafana/grafana/pkg/services/team/teamtest" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) @@ -137,7 +141,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { fakeDash.FolderID = 1 fakeDash.HasACL = false fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() - fakeDashboardVersionService.ExpectedDashboardVersion = &dashver.DashboardVersionDTO{} + fakeDashboardVersionService.ExpectedDashboardVersion = &dashver.DashboardVersionDTO{CreatedBy: 1} teamService := &teamtest.FakeService{} dashboardService := dashboards.NewFakeDashboardService(t) dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(fakeDash, nil) @@ -153,6 +157,9 @@ func TestDashboardAPIEndpoint(t *testing.T) { dashboardVersionService: fakeDashboardVersionService, Kinds: corekind.NewBase(nil), QuotaService: quotatest.New(false, nil), + userService: &usertest.FakeUserService{ + ExpectedUser: &user.User{ID: 1, Login: "test-user"}, + }, } setUp := func() { @@ -222,6 +229,10 @@ func TestDashboardAPIEndpoint(t *testing.T) { hs.callGetDashboardVersion(sc) assert.Equal(t, 200, sc.resp.Code) + var version *dashver.DashboardVersionMeta + err := json.NewDecoder(sc.resp.Body).Decode(&version) + require.NoError(t, err) + assert.NotEqual(t, anonString, version.CreatedBy) }, mockSQLStore) loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/2/versions", @@ -960,6 +971,129 @@ func TestDashboardAPIEndpoint(t *testing.T) { }) } +func TestDashboardVersionsAPIEndpoint(t *testing.T) { + fakeDash := dashboards.NewDashboard("Child dash") + fakeDash.ID = 1 + fakeDash.FolderID = 1 + fakeDash.HasACL = false + + fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake() + dashboardService := dashboards.NewFakeDashboardService(t) + dashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(fakeDash, nil) + + teamService := &teamtest.FakeService{} + + mockSQLStore := dbtest.NewFakeDB() + + cfg := setting.NewCfg() + + getHS := func(userSvc *usertest.FakeUserService) *HTTPServer { + return &HTTPServer{ + Cfg: cfg, + pluginStore: &plugins.FakePluginStore{}, + SQLStore: mockSQLStore, + AccessControl: accesscontrolmock.New(), + Features: featuremgmt.WithFeatures(), + DashboardService: dashboardService, + dashboardVersionService: fakeDashboardVersionService, + Kinds: corekind.NewBase(nil), + QuotaService: quotatest.New(false, nil), + userService: userSvc, + CacheService: localcache.New(5*time.Minute, 10*time.Minute), + log: log.New(), + } + } + + setUp := func() { + viewerRole := org.RoleViewer + editorRole := org.RoleEditor + qResult := []*dashboards.DashboardACLInfoDTO{ + {Role: &viewerRole, Permission: dashboards.PERMISSION_VIEW}, + {Role: &editorRole, Permission: dashboards.PERMISSION_EDIT}, + } + dashboardService.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardACLInfoListQuery")).Return(qResult, nil) + guardian.InitLegacyGuardian(mockSQLStore, dashboardService, teamService) + } + + loggedInUserScenarioWithRole(t, "When user exists and calling GET on", "GET", "/api/dashboards/id/2/versions", + "/api/dashboards/id/:dashboardId/versions", org.RoleEditor, func(sc *scenarioContext) { + setUp() + fakeDashboardVersionService.ExpectedListDashboarVersions = []*dashver.DashboardVersionDTO{ + { + Version: 1, + CreatedBy: 1, + }, + { + Version: 2, + CreatedBy: 1, + }, + } + getHS(&usertest.FakeUserService{ + ExpectedUser: &user.User{ID: 1, Login: "test-user"}, + }).callGetDashboardVersions(sc) + + assert.Equal(t, 200, sc.resp.Code) + var versions []dashver.DashboardVersionMeta + err := json.NewDecoder(sc.resp.Body).Decode(&versions) + require.NoError(t, err) + for _, v := range versions { + assert.Equal(t, "test-user", v.CreatedBy) + } + }, mockSQLStore) + + loggedInUserScenarioWithRole(t, "When user does not exist and calling GET on", "GET", "/api/dashboards/id/2/versions", + "/api/dashboards/id/:dashboardId/versions", org.RoleEditor, func(sc *scenarioContext) { + setUp() + fakeDashboardVersionService.ExpectedListDashboarVersions = []*dashver.DashboardVersionDTO{ + { + Version: 1, + CreatedBy: 1, + }, + { + Version: 2, + CreatedBy: 1, + }, + } + getHS(&usertest.FakeUserService{ + ExpectedError: user.ErrUserNotFound, + }).callGetDashboardVersions(sc) + + assert.Equal(t, 200, sc.resp.Code) + var versions []dashver.DashboardVersionMeta + err := json.NewDecoder(sc.resp.Body).Decode(&versions) + require.NoError(t, err) + for _, v := range versions { + assert.Equal(t, anonString, v.CreatedBy) + } + }, mockSQLStore) + + loggedInUserScenarioWithRole(t, "When failing to get user and calling GET on", "GET", "/api/dashboards/id/2/versions", + "/api/dashboards/id/:dashboardId/versions", org.RoleEditor, func(sc *scenarioContext) { + setUp() + fakeDashboardVersionService.ExpectedListDashboarVersions = []*dashver.DashboardVersionDTO{ + { + Version: 1, + CreatedBy: 1, + }, + { + Version: 2, + CreatedBy: 1, + }, + } + getHS(&usertest.FakeUserService{ + ExpectedError: fmt.Errorf("some error"), + }).callGetDashboardVersions(sc) + + assert.Equal(t, 200, sc.resp.Code) + var versions []dashver.DashboardVersionMeta + err := json.NewDecoder(sc.resp.Body).Decode(&versions) + require.NoError(t, err) + for _, v := range versions { + assert.Equal(t, anonString, v.CreatedBy) + } + }, mockSQLStore) +} + func getDashboardShouldReturn200WithConfig(t *testing.T, sc *scenarioContext, provisioningService provisioning.ProvisioningService, dashboardStore dashboards.Store, dashboardService dashboards.DashboardService, folderStore dashboards.FolderStore) dtos.DashboardFullWithMeta { t.Helper() diff --git a/pkg/services/dashboardversion/model.go b/pkg/services/dashboardversion/model.go index 35cb8facc48..435fc794311 100644 --- a/pkg/services/dashboardversion/model.go +++ b/pkg/services/dashboardversion/model.go @@ -66,7 +66,6 @@ type ListDashboardVersionsQuery struct { Limit int Start int } - type DashboardVersionDTO struct { ID int64 `json:"id"` DashboardID int64 `json:"dashboardId"` diff --git a/public/api-merged.json b/public/api-merged.json index 1c5223710f9..488db48bd5f 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -12628,48 +12628,6 @@ } } }, - "DashboardVersionDTO": { - "type": "object", - "properties": { - "created": { - "type": "string", - "format": "date-time" - }, - "createdBy": { - "type": "integer", - "format": "int64" - }, - "dashboardId": { - "type": "integer", - "format": "int64" - }, - "dashboardUid": { - "type": "string" - }, - "data": { - "$ref": "#/definitions/Json" - }, - "id": { - "type": "integer", - "format": "int64" - }, - "message": { - "type": "string" - }, - "parentVersion": { - "type": "integer", - "format": "int64" - }, - "restoredFrom": { - "type": "integer", - "format": "int64" - }, - "version": { - "type": "integer", - "format": "int64" - } - } - }, "DashboardVersionMeta": { "description": "DashboardVersionMeta extends the DashboardVersionDTO with the names\nassociated with the UserIds, overriding the field with the same name from\nthe DashboardVersionDTO model.", "type": "object", @@ -19656,7 +19614,7 @@ "schema": { "type": "array", "items": { - "$ref": "#/definitions/DashboardVersionDTO" + "$ref": "#/definitions/DashboardVersionMeta" } } }, diff --git a/public/openapi3.json b/public/openapi3.json index 8918292fde4..640444d06fc 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -332,7 +332,7 @@ "application/json": { "schema": { "items": { - "$ref": "#/components/schemas/DashboardVersionDTO" + "$ref": "#/components/schemas/DashboardVersionMeta" }, "type": "array" } @@ -3716,48 +3716,6 @@ }, "type": "object" }, - "DashboardVersionDTO": { - "properties": { - "created": { - "format": "date-time", - "type": "string" - }, - "createdBy": { - "format": "int64", - "type": "integer" - }, - "dashboardId": { - "format": "int64", - "type": "integer" - }, - "dashboardUid": { - "type": "string" - }, - "data": { - "$ref": "#/components/schemas/Json" - }, - "id": { - "format": "int64", - "type": "integer" - }, - "message": { - "type": "string" - }, - "parentVersion": { - "format": "int64", - "type": "integer" - }, - "restoredFrom": { - "format": "int64", - "type": "integer" - }, - "version": { - "format": "int64", - "type": "integer" - } - }, - "type": "object" - }, "DashboardVersionMeta": { "description": "DashboardVersionMeta extends the DashboardVersionDTO with the names\nassociated with the UserIds, overriding the field with the same name from\nthe DashboardVersionDTO model.", "properties": {