From 1667a7c0da3630ca80f80091fc01c1e563230449 Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Fri, 29 Apr 2022 14:37:33 +0200 Subject: [PATCH] =?UTF-8?q?Chore:=20Modify=20patch=20and=20update=20of=20p?= =?UTF-8?q?reference=20to=20take=20homedashboarduid=20than=20h=E2=80=A6=20?= =?UTF-8?q?(#48281)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * modify patch and update of preference to take homedashboarduid than homedashboardid * to be tested * use getdashboard * update doc --- docs/sources/http_api/preferences.md | 31 ++++++++++++-- pkg/api/dtos/prefs.go | 25 +++++++----- pkg/api/preferences.go | 60 ++++++++++++++++++++++++++-- pkg/api/preferences_test.go | 45 ++++++++++++++++++--- pkg/api/team_test.go | 7 ++++ pkg/services/preference/model.go | 26 ++++++------ public/api-merged.json | 60 +++++++++++++++++----------- public/api-spec.json | 60 +++++++++++++++++----------- 8 files changed, 230 insertions(+), 84 deletions(-) diff --git a/docs/sources/http_api/preferences.md b/docs/sources/http_api/preferences.md index 46a63704e3b..84e48b75527 100644 --- a/docs/sources/http_api/preferences.md +++ b/docs/sources/http_api/preferences.md @@ -35,7 +35,19 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk HTTP/1.1 200 Content-Type: application/json -{"theme":"","homeDashboardId":0,"timezone":""} +{ + "theme": "", + "homeDashboardId": 217, + "homeDashboardUID": "jcIIG-07z", + "timezone": "utc", + "weekStart": "", + "navbar": { + "savedItems": null + }, + "queryHistory": { + "homeTab": "" + } +} ``` ## Update Current User Prefs @@ -52,7 +64,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk { "theme": "", - "homeDashboardId":0, + "homeDashboardUID":"home", "timezone":"utc" } ``` @@ -113,7 +125,18 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk HTTP/1.1 200 Content-Type: application/json -{"theme":"","homeDashboardId":0,"timezone":""} +{ + "theme": "", + "homeDashboardId": 0, + "timezone": "", + "weekStart": "", + "navbar": { + "savedItems": null + }, + "queryHistory": { + "homeTab": "" + } +} ``` ## Update Current Org Prefs @@ -130,7 +153,7 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk { "theme": "", - "homeDashboardId":0, + "homeDashboardUID":"home", "timezone":"utc" } ``` diff --git a/pkg/api/dtos/prefs.go b/pkg/api/dtos/prefs.go index 08ef64b6df5..18dc1f9ca94 100644 --- a/pkg/api/dtos/prefs.go +++ b/pkg/api/dtos/prefs.go @@ -6,12 +6,13 @@ import ( ) type Prefs struct { - Theme string `json:"theme"` - HomeDashboardID int64 `json:"homeDashboardId"` - Timezone string `json:"timezone"` - WeekStart string `json:"weekStart"` - Navbar pref.NavbarPreference `json:"navbar,omitempty"` - QueryHistory pref.QueryHistoryPreference `json:"queryHistory,omitempty"` + Theme string `json:"theme"` + HomeDashboardID int64 `json:"homeDashboardId"` + HomeDashboardUID string `json:"homeDashboardUID,omitempty"` + Timezone string `json:"timezone"` + WeekStart string `json:"weekStart"` + Navbar pref.NavbarPreference `json:"navbar,omitempty"` + QueryHistory pref.QueryHistoryPreference `json:"queryHistory,omitempty"` } // swagger:model @@ -20,7 +21,8 @@ type UpdatePrefsCmd struct { Theme string `json:"theme"` // The numerical :id of a favorited dashboard // Default:0 - HomeDashboardID int64 `json:"homeDashboardId"` + HomeDashboardID int64 `json:"homeDashboardId"` + HomeDashboardUID *string `json:"homeDashboardUID,omitempty"` // Enum: utc,browser Timezone string `json:"timezone"` WeekStart string `json:"weekStart"` @@ -36,8 +38,9 @@ type PatchPrefsCmd struct { // Default:0 HomeDashboardID *int64 `json:"homeDashboardId,omitempty"` // Enum: utc,browser - Timezone *string `json:"timezone,omitempty"` - WeekStart *string `json:"weekStart,omitempty"` - Navbar *pref.NavbarPreference `json:"navbar,omitempty"` - QueryHistory *pref.QueryHistoryPreference `json:"queryHistory,omitempty"` + Timezone *string `json:"timezone,omitempty"` + WeekStart *string `json:"weekStart,omitempty"` + Navbar *pref.NavbarPreference `json:"navbar,omitempty"` + QueryHistory *pref.QueryHistoryPreference `json:"queryHistory,omitempty"` + HomeDashboardUID *string `json:"homeDashboardUID,omitempty"` } diff --git a/pkg/api/preferences.go b/pkg/api/preferences.go index e95003bdff6..1beb04aba0c 100644 --- a/pkg/api/preferences.go +++ b/pkg/api/preferences.go @@ -26,6 +26,20 @@ func (hs *HTTPServer) SetHomeDashboard(c *models.ReqContext) response.Response { cmd.UserID = c.UserId cmd.OrgID = c.OrgId + // the default value of HomeDashboardID is taken from input, when HomeDashboardID is set also, + // UID is used in preference to identify dashboard + dashboardID := cmd.HomeDashboardID + if cmd.HomeDashboardUID != nil { + query := models.GetDashboardQuery{Uid: *cmd.HomeDashboardUID} + err := hs.SQLStore.GetDashboard(c.Req.Context(), &query) + if err != nil { + return response.Error(404, "Dashboard not found", err) + } + dashboardID = query.Result.Id + } + + cmd.HomeDashboardID = dashboardID + if err := hs.preferenceService.Save(c.Req.Context(), &cmd); err != nil { return response.Error(500, "Failed to set home dashboard", err) } @@ -46,11 +60,23 @@ func (hs *HTTPServer) getPreferencesFor(ctx context.Context, orgID, userID, team return response.Error(500, "Failed to get preferences", err) } + var dashboardUID string + + // when homedashboardID is 0, that means it is the default home dashboard, no UID would be returned in the response + if preference.HomeDashboardID != 0 { + query := models.GetDashboardQuery{Id: preference.HomeDashboardID, OrgId: orgID} + err = hs.SQLStore.GetDashboard(ctx, &query) + if err == nil { + dashboardUID = query.Result.Uid + } + } + dto := dtos.Prefs{ - Theme: preference.Theme, - HomeDashboardID: preference.HomeDashboardID, - Timezone: preference.Timezone, - WeekStart: preference.WeekStart, + Theme: preference.Theme, + HomeDashboardID: preference.HomeDashboardID, + HomeDashboardUID: dashboardUID, + Timezone: preference.Timezone, + WeekStart: preference.WeekStart, } if preference.JSONData != nil { @@ -74,6 +100,18 @@ func (hs *HTTPServer) updatePreferencesFor(ctx context.Context, orgID, userID, t if dtoCmd.Theme != lightTheme && dtoCmd.Theme != darkTheme && dtoCmd.Theme != defaultTheme { return response.Error(400, "Invalid theme", nil) } + + dashboardID := dtoCmd.HomeDashboardID + if dtoCmd.HomeDashboardUID != nil { + query := models.GetDashboardQuery{Uid: *dtoCmd.HomeDashboardUID, OrgId: orgID} + err := hs.SQLStore.GetDashboard(ctx, &query) + if err != nil { + return response.Error(404, "Dashboard not found", err) + } + dashboardID = query.Result.Id + } + dtoCmd.HomeDashboardID = dashboardID + saveCmd := pref.SavePreferenceCommand{ UserID: userID, OrgID: orgID, @@ -104,6 +142,19 @@ func (hs *HTTPServer) patchPreferencesFor(ctx context.Context, orgID, userID, te if dtoCmd.Theme != nil && *dtoCmd.Theme != lightTheme && *dtoCmd.Theme != darkTheme && *dtoCmd.Theme != defaultTheme { return response.Error(400, "Invalid theme", nil) } + + // convert dashboard UID to ID in order to store internally if it exists in the query, otherwise take the id from query + dashboardID := dtoCmd.HomeDashboardID + if dtoCmd.HomeDashboardUID != nil { + query := models.GetDashboardQuery{Uid: *dtoCmd.HomeDashboardUID, OrgId: orgID} + err := hs.SQLStore.GetDashboard(ctx, &query) + if err != nil { + return response.Error(404, "Dashboard not found", err) + } + dashboardID = &query.Result.Id + } + dtoCmd.HomeDashboardID = dashboardID + patchCmd := pref.PatchPreferenceCommand{ UserID: userID, OrgID: orgID, @@ -134,6 +185,7 @@ func (hs *HTTPServer) UpdateOrgPreferences(c *models.ReqContext) response.Respon if err := web.Bind(c.Req, &dtoCmd); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) } + return hs.updatePreferencesFor(c.Req.Context(), c.OrgId, 0, 0, &dtoCmd) } diff --git a/pkg/api/preferences_test.go b/pkg/api/preferences_test.go index 4a52fb0fd97..b204bcb83f5 100644 --- a/pkg/api/preferences_test.go +++ b/pkg/api/preferences_test.go @@ -1,13 +1,17 @@ package api import ( + "encoding/json" + "io/ioutil" "net/http" "strings" "testing" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" pref "github.com/grafana/grafana/pkg/services/preference" "github.com/grafana/grafana/pkg/services/preference/preftest" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,15 +22,23 @@ var ( patchOrgPreferencesUrl = "/api/org/preferences/" patchUserPreferencesUrl = "/api/user/preferences/" - testUpdateOrgPreferencesCmd = `{ "theme": "light", "homeDashboardId": 1 }` - testPatchOrgPreferencesCmd = `{"navbar":{"savedItems":[{"id":"snapshots","text":"Snapshots","icon":"camera","url":"/dashboard/snapshots"}]}}` - testPatchOrgPreferencesCmdBad = `this is not json` - testPatchUserPreferencesCmd = `{"navbar":{"savedItems":[{"id":"snapshots","text":"Snapshots","icon":"camera","url":"/dashboard/snapshots"}]}}` - testPatchUserPreferencesCmdBad = `this is not json` + testUpdateOrgPreferencesCmd = `{ "theme": "light", "homeDashboardId": 1 }` + testPatchOrgPreferencesCmd = `{"navbar":{"savedItems":[{"id":"snapshots","text":"Snapshots","icon":"camera","url":"/dashboard/snapshots"}]}}` + testPatchOrgPreferencesCmdBad = `this is not json` + testPatchUserPreferencesCmd = `{"navbar":{"savedItems":[{"id":"snapshots","text":"Snapshots","icon":"camera","url":"/dashboard/snapshots"}]}}` + testPatchUserPreferencesCmdBad = `this is not json` + testUpdateOrgPreferencesWithHomeDashboardUIDCmd = `{ "theme": "light", "homeDashboardUID": "home"}` ) func TestAPIEndpoint_GetCurrentOrgPreferences_LegacyAccessControl(t *testing.T) { sc := setupHTTPServer(t, true, false) + sqlstore := mockstore.NewSQLStoreMock() + sqlstore.ExpectedDashboard = &models.Dashboard{ + Uid: "home", + Id: 1, + } + sc.hs.SQLStore = sqlstore + prefService := preftest.NewPreferenceServiceFake() prefService.ExpectedPreference = &pref.Preference{HomeDashboardID: 1, Theme: "dark"} sc.hs.preferenceService = prefService @@ -44,6 +56,11 @@ func TestAPIEndpoint_GetCurrentOrgPreferences_LegacyAccessControl(t *testing.T) t.Run("Org Admin can get org preferences", func(t *testing.T) { response := callAPI(sc.server, http.MethodGet, getOrgPreferencesURL, nil, t) assert.Equal(t, http.StatusOK, response.Code) + var resp map[string]interface{} + b, err := ioutil.ReadAll(response.Body) + assert.NoError(t, err) + assert.NoError(t, json.Unmarshal(b, &resp)) + assert.Equal(t, "home", resp["homeDashboardUID"]) }) } @@ -51,6 +68,13 @@ func TestAPIEndpoint_GetCurrentOrgPreferences_AccessControl(t *testing.T) { sc := setupHTTPServer(t, true, true) setInitCtxSignedInViewer(sc.initCtx) + sqlstore := mockstore.NewSQLStoreMock() + sqlstore.ExpectedDashboard = &models.Dashboard{ + Uid: "home", + Id: 1, + } + sc.hs.SQLStore = sqlstore + prefService := preftest.NewPreferenceServiceFake() prefService.ExpectedPreference = &pref.Preference{HomeDashboardID: 1, Theme: "dark"} sc.hs.preferenceService = prefService @@ -143,6 +167,17 @@ func TestAPIEndpoint_PatchUserPreferences(t *testing.T) { response := callAPI(sc.server, http.MethodPut, patchUserPreferencesUrl, input, t) assert.Equal(t, http.StatusBadRequest, response.Code) }) + input = strings.NewReader(testUpdateOrgPreferencesWithHomeDashboardUIDCmd) + sqlstore := mockstore.NewSQLStoreMock() + sqlstore.ExpectedDashboard = &models.Dashboard{ + Uid: "home", + Id: 1, + } + sc.hs.SQLStore = sqlstore + t.Run("Returns 200 on success", func(t *testing.T) { + response := callAPI(sc.server, http.MethodPatch, patchUserPreferencesUrl, input, t) + assert.Equal(t, http.StatusOK, response.Code) + }) } func TestAPIEndpoint_PatchOrgPreferences(t *testing.T) { diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go index 90fc4a24e91..97621f59056 100644 --- a/pkg/api/team_test.go +++ b/pkg/api/team_test.go @@ -377,6 +377,13 @@ func TestTeamAPIEndpoint_GetTeamPreferences_RBAC(t *testing.T) { sc.db = sqlstore.InitTestDB(t) _, err := sc.db.CreateTeam("team1", "", 1) + sqlstore := mockstore.NewSQLStoreMock() + sqlstore.ExpectedDashboard = &models.Dashboard{ + Uid: "home", + Id: 1, + } + sc.hs.SQLStore = sqlstore + prefService := preftest.NewPreferenceServiceFake() prefService.ExpectedPreference = &pref.Preference{} sc.hs.preferenceService = prefService diff --git a/pkg/services/preference/model.go b/pkg/services/preference/model.go index ae8b5ca665b..f3f9d2c233c 100644 --- a/pkg/services/preference/model.go +++ b/pkg/services/preference/model.go @@ -42,12 +42,13 @@ type SavePreferenceCommand struct { OrgID int64 TeamID int64 - HomeDashboardID int64 `json:"homeDashboardId,omitempty"` - Timezone string `json:"timezone,omitempty"` - WeekStart string `json:"weekStart,omitempty"` - Theme string `json:"theme,omitempty"` - Navbar *NavbarPreference `json:"navbar,omitempty"` - QueryHistory *QueryHistoryPreference `json:"queryHistory,omitempty"` + HomeDashboardID int64 `json:"homeDashboardId,omitempty"` + HomeDashboardUID *string `json:"homeDashboardUID,omitempty"` + Timezone string `json:"timezone,omitempty"` + WeekStart string `json:"weekStart,omitempty"` + Theme string `json:"theme,omitempty"` + Navbar *NavbarPreference `json:"navbar,omitempty"` + QueryHistory *QueryHistoryPreference `json:"queryHistory,omitempty"` } type PatchPreferenceCommand struct { @@ -55,12 +56,13 @@ type PatchPreferenceCommand struct { OrgID int64 TeamID int64 - HomeDashboardID *int64 `json:"homeDashboardId,omitempty"` - Timezone *string `json:"timezone,omitempty"` - WeekStart *string `json:"weekStart,omitempty"` - Theme *string `json:"theme,omitempty"` - Navbar *NavbarPreference `json:"navbar,omitempty"` - QueryHistory *QueryHistoryPreference `json:"queryHistory,omitempty"` + HomeDashboardID *int64 `json:"homeDashboardId,omitempty"` + HomeDashboardUID *string `json:"homeDashboardUID,omitempty"` + Timezone *string `json:"timezone,omitempty"` + WeekStart *string `json:"weekStart,omitempty"` + Theme *string `json:"theme,omitempty"` + Navbar *NavbarPreference `json:"navbar,omitempty"` + QueryHistory *QueryHistoryPreference `json:"queryHistory,omitempty"` } type NavLink struct { diff --git a/public/api-merged.json b/public/api-merged.json index 63ebf3f33ac..1095292f0d4 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -506,6 +506,14 @@ "summary": "Add a user role assignment.", "operationId": "addUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "x-go-name": "UserID", + "name": "user_id", + "in": "path", + "required": true + }, { "x-go-name": "Body", "name": "body", @@ -514,14 +522,6 @@ "schema": { "$ref": "#/definitions/AddUserRoleCommand" } - }, - { - "type": "integer", - "format": "int64", - "x-go-name": "UserID", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { @@ -547,6 +547,14 @@ "summary": "Remove a user role assignment.", "operationId": "removeUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "x-go-name": "UserID", + "name": "user_id", + "in": "path", + "required": true + }, { "type": "string", "x-go-name": "RoleUID", @@ -560,14 +568,6 @@ "description": "A flag indicating if the assignment is global or not. If set to false, the default org ID of the authenticated user will be used from the request to remove assignment.", "name": "global", "in": "query" - }, - { - "type": "integer", - "format": "int64", - "x-go-name": "UserID", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { @@ -4692,15 +4692,15 @@ "parameters": [ { "type": "string", - "x-go-name": "PermissionID", - "name": "permissionId", + "x-go-name": "DatasourceID", + "name": "datasource_id", "in": "path", "required": true }, { "type": "string", - "x-go-name": "DatasourceID", - "name": "datasource_id", + "x-go-name": "PermissionID", + "name": "permissionId", "in": "path", "required": true } @@ -13591,7 +13591,7 @@ "properties": { "id": { "type": "string", - "x-go-name": "ID" + "x-go-name": "Id" }, "target": { "type": "string", @@ -13606,7 +13606,7 @@ "x-go-name": "Url" } }, - "x-go-package": "github.com/grafana/grafana/pkg/services/preference" + "x-go-package": "github.com/grafana/grafana/pkg/models" }, "NavbarPreference": { "type": "object", @@ -13619,7 +13619,7 @@ "x-go-name": "SavedItems" } }, - "x-go-package": "github.com/grafana/grafana/pkg/models" + "x-go-package": "github.com/grafana/grafana/pkg/services/preference" }, "NewApiKeyResult": { "type": "object", @@ -14106,6 +14106,10 @@ "default": 0, "x-go-name": "HomeDashboardID" }, + "homeDashboardUID": { + "type": "string", + "x-go-name": "HomeDashboardUID" + }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, @@ -14571,6 +14575,10 @@ "format": "int64", "x-go-name": "HomeDashboardID" }, + "homeDashboardUID": { + "type": "string", + "x-go-name": "HomeDashboardUID" + }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, @@ -14686,7 +14694,7 @@ "x-go-name": "HomeTab" } }, - "x-go-package": "github.com/grafana/grafana/pkg/services/preference" + "x-go-package": "github.com/grafana/grafana/pkg/models" }, "Receiver": { "type": "object", @@ -16653,6 +16661,10 @@ "default": 0, "x-go-name": "HomeDashboardID" }, + "homeDashboardUID": { + "type": "string", + "x-go-name": "HomeDashboardUID" + }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, diff --git a/public/api-spec.json b/public/api-spec.json index bf38c563b31..22753102183 100644 --- a/public/api-spec.json +++ b/public/api-spec.json @@ -506,6 +506,14 @@ "summary": "Add a user role assignment.", "operationId": "addUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "x-go-name": "UserID", + "name": "user_id", + "in": "path", + "required": true + }, { "x-go-name": "Body", "name": "body", @@ -514,14 +522,6 @@ "schema": { "$ref": "#/definitions/AddUserRoleCommand" } - }, - { - "type": "integer", - "format": "int64", - "x-go-name": "UserID", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { @@ -547,6 +547,14 @@ "summary": "Remove a user role assignment.", "operationId": "removeUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "x-go-name": "UserID", + "name": "user_id", + "in": "path", + "required": true + }, { "type": "string", "x-go-name": "RoleUID", @@ -560,14 +568,6 @@ "description": "A flag indicating if the assignment is global or not. If set to false, the default org ID of the authenticated user will be used from the request to remove assignment.", "name": "global", "in": "query" - }, - { - "type": "integer", - "format": "int64", - "x-go-name": "UserID", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { @@ -3754,15 +3754,15 @@ "parameters": [ { "type": "string", - "x-go-name": "PermissionID", - "name": "permissionId", + "x-go-name": "DatasourceID", + "name": "datasource_id", "in": "path", "required": true }, { "type": "string", - "x-go-name": "DatasourceID", - "name": "datasource_id", + "x-go-name": "PermissionID", + "name": "permissionId", "in": "path", "required": true } @@ -10707,7 +10707,7 @@ "properties": { "id": { "type": "string", - "x-go-name": "ID" + "x-go-name": "Id" }, "target": { "type": "string", @@ -10722,7 +10722,7 @@ "x-go-name": "Url" } }, - "x-go-package": "github.com/grafana/grafana/pkg/services/preference" + "x-go-package": "github.com/grafana/grafana/pkg/models" }, "NavbarPreference": { "type": "object", @@ -10735,7 +10735,7 @@ "x-go-name": "SavedItems" } }, - "x-go-package": "github.com/grafana/grafana/pkg/models" + "x-go-package": "github.com/grafana/grafana/pkg/services/preference" }, "NewApiKeyResult": { "type": "object", @@ -10964,6 +10964,10 @@ "default": 0, "x-go-name": "HomeDashboardID" }, + "homeDashboardUID": { + "type": "string", + "x-go-name": "HomeDashboardUID" + }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, @@ -11113,6 +11117,10 @@ "format": "int64", "x-go-name": "HomeDashboardID" }, + "homeDashboardUID": { + "type": "string", + "x-go-name": "HomeDashboardUID" + }, "navbar": { "$ref": "#/definitions/NavbarPreference" }, @@ -11171,7 +11179,7 @@ "x-go-name": "HomeTab" } }, - "x-go-package": "github.com/grafana/grafana/pkg/services/preference" + "x-go-package": "github.com/grafana/grafana/pkg/models" }, "RecordingRuleJSON": { "description": "RecordingRuleJSON is the external representation of a recording rule", @@ -12295,6 +12303,10 @@ "default": 0, "x-go-name": "HomeDashboardID" }, + "homeDashboardUID": { + "type": "string", + "x-go-name": "HomeDashboardUID" + }, "navbar": { "$ref": "#/definitions/NavbarPreference" },