From 401b01e1e6e08aa3818ec34f56a59bdf7df7bb58 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 14:57:19 +0100 Subject: [PATCH 01/44] db: add new column uid to the dashboard table. #7883 --- pkg/services/sqlstore/migrations/dashboard_mig.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index 4f1602be931..a17338b73ae 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -150,4 +150,9 @@ func addDashboardMigration(mg *Migrator) { mg.AddMigration("Add column has_acl in dashboard", NewAddColumnMigration(dashboardV2, &Column{ Name: "has_acl", Type: DB_Bool, Nullable: false, Default: "0", })) + + // new uid column + mg.AddMigration("Add column uid in dashboard", NewAddColumnMigration(dashboardV2, &Column{ + Name: "uid", Type: DB_NVarchar, Length: 12, Nullable: true, + })) } From 50aa9ec69c7d8dc836c23f5ce7891c86e0a19764 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 16:24:22 +0100 Subject: [PATCH 02/44] db: add migrations for generating uid for existing dashboards. #7883 --- pkg/services/sqlstore/migrations/dashboard_mig.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index a17338b73ae..e67a2adf73d 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -155,4 +155,9 @@ func addDashboardMigration(mg *Migrator) { mg.AddMigration("Add column uid in dashboard", NewAddColumnMigration(dashboardV2, &Column{ Name: "uid", Type: DB_NVarchar, Length: 12, Nullable: true, })) + + mg.AddMigration("Set uid column values", new(RawSqlMigration). + Sqlite("UPDATE dashboard SET uid=printf('%09d',id) WHERE uid IS NULL;"). + Postgres("UPDATE dashboard SET uid=lpad('' || id,9,'0') WHERE uid IS NULL;"). + Mysql("UPDATE dashboard SET uid=lpad(id,9,'0') WHERE uid IS NULL;")) } From 025a14ec2465d2d028762b06f49239bf2bc56733 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 16:27:52 +0100 Subject: [PATCH 03/44] db: add migrations for creating a unique index for uid. #7883 --- pkg/services/sqlstore/migrations/dashboard_mig.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index e67a2adf73d..1035ef35015 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -160,4 +160,8 @@ func addDashboardMigration(mg *Migrator) { Sqlite("UPDATE dashboard SET uid=printf('%09d',id) WHERE uid IS NULL;"). Postgres("UPDATE dashboard SET uid=lpad('' || id,9,'0') WHERE uid IS NULL;"). Mysql("UPDATE dashboard SET uid=lpad(id,9,'0') WHERE uid IS NULL;")) + + mg.AddMigration("Add index for uid in dashboard", NewAddIndexMigration(dashboardV2, &Index{ + Cols: []string{"uid"}, Type: UniqueIndex, + })) } From 5b35c694dc9b64fe023b8fae3c04d83661a52e66 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 17:20:18 +0100 Subject: [PATCH 04/44] dashboard: generate and include uid in dashboard model. #7883 --- Gopkg.lock | 8 +- Gopkg.toml | 4 + pkg/models/dashboards.go | 15 + pkg/services/sqlstore/dashboard.go | 1 + vendor/github.com/teris-io/shortid/LICENSE | 18 + vendor/github.com/teris-io/shortid/shortid.go | 362 ++++++++++++++++++ 6 files changed, 407 insertions(+), 1 deletion(-) create mode 100644 vendor/github.com/teris-io/shortid/LICENSE create mode 100644 vendor/github.com/teris-io/shortid/shortid.go diff --git a/Gopkg.lock b/Gopkg.lock index 4d5f6ca0a46..8d82b29d622 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -412,6 +412,12 @@ revision = "9e8dc3f972df6c8fcc0375ef492c24d0bb204857" version = "1.6.3" +[[projects]] + branch = "master" + name = "github.com/teris-io/shortid" + packages = ["."] + revision = "771a37caa5cf0c81f585d7b6df4dfc77e0615b5c" + [[projects]] name = "github.com/uber/jaeger-client-go" packages = [ @@ -625,6 +631,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "98e8d8f5fb21fe448aeb3db41c9fed85fe3bf80400e553211cf39a9c05720e01" + inputs-digest = "4de68f1342ba98a637ec8ca7496aeeae2021bf9e4c7c80db7924e14709151a62" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 144dcc1e4af..22c56d29c71 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -193,3 +193,7 @@ ignored = [ non-go = true go-tests = true unused-packages = true + +[[constraint]] + branch = "master" + name = "github.com/teris-io/shortid" diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 091f27ec413..b377e184829 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -7,6 +7,7 @@ import ( "github.com/gosimple/slug" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/teris-io/shortid" ) // Typed errors @@ -39,6 +40,7 @@ var ( // Dashboard model type Dashboard struct { Id int64 + Uid string Slug string OrgId int64 GnetId int64 @@ -61,6 +63,7 @@ type Dashboard struct { // NewDashboard creates a new dashboard func NewDashboard(title string) *Dashboard { dash := &Dashboard{} + dash.Uid = DashboardUid() dash.Data = simplejson.New() dash.Data.Set("title", title) dash.Title = title @@ -107,9 +110,21 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard { dash.GnetId = int64(gnetId) } + if uid, err := dash.Data.Get("uid").String(); err == nil { + dash.Uid = uid + } else { + dash.Uid = DashboardUid() + } + return dash } +func DashboardUid() string { + gen, _ := shortid.New(1, shortid.DefaultABC, 1) + uid, _ := gen.Generate() + return uid +} + // GetDashboardModel turns the command into the savable model func (cmd *SaveDashboardCommand) GetDashboardModel() *Dashboard { dash := NewDashboardFromJson(cmd.Dashboard) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 0b6b60a5e11..b8d3d9a36a5 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -175,6 +175,7 @@ func GetDashboard(query *m.GetDashboardQuery) error { } dashboard.Data.Set("id", dashboard.Id) + dashboard.Data.Set("uid", dashboard.Uid) query.Result = &dashboard return nil } diff --git a/vendor/github.com/teris-io/shortid/LICENSE b/vendor/github.com/teris-io/shortid/LICENSE new file mode 100644 index 00000000000..fc737ef1ce8 --- /dev/null +++ b/vendor/github.com/teris-io/shortid/LICENSE @@ -0,0 +1,18 @@ +MIT License + +Permission is hereby granted, free of charge, to any person obtaining a copy of this +software and associated documentation files (the "Software"), to deal in the Software +without restriction, including without limitation the rights to use, copy, modify, +merge, publish, distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to the following +conditions: + +The above copyright notice and this permission notice shall be included in all copies +or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/vendor/github.com/teris-io/shortid/shortid.go b/vendor/github.com/teris-io/shortid/shortid.go new file mode 100644 index 00000000000..c9461c29168 --- /dev/null +++ b/vendor/github.com/teris-io/shortid/shortid.go @@ -0,0 +1,362 @@ +// Copyright (c) 2016-2017. Oleg Sklyar & teris.io. All rights reserved. +// See the LICENSE file in the project root for licensing information. + +// Original algorithm: +// Copyright (c) 2015 Dylan Greene, contributors: https://github.com/dylang/shortid. +// MIT-license as found in the LICENSE file. + +// Seed computation: based on The Central Randomizer 1.3 +// Copyright (c) 1997 Paul Houle (houle@msc.cornell.edu) + +// Package shortid enables the generation of short, unique, non-sequential and by default URL friendly +// Ids. The package is heavily inspired by the node.js https://github.com/dylang/shortid library. +// +// Id Length +// +// The standard Id length is 9 symbols when generated at a rate of 1 Id per millisecond, +// occasionally it reaches 11 (at the rate of a few thousand Ids per millisecond) and very-very +// rarely it can go beyond that during continuous generation at full throttle on high-performant +// hardware. A test generating 500k Ids at full throttle on conventional hardware generated the +// following Ids at the head and the tail (length > 9 is expected for this test): +// +// -NDveu-9Q +// iNove6iQ9J +// NVDve6-9Q +// VVDvc6i99J +// NVovc6-QQy +// VVoveui9QC +// ... +// tFmGc6iQQs +// KpTvcui99k +// KFTGcuiQ9p +// KFmGeu-Q9O +// tFTvcu-QQt +// tpTveu-99u +// +// Life span +// +// The package guarantees the generation of unique Ids with zero collisions for 34 years +// (1/1/2016-1/1/2050) using the same worker Id within a single (although concurrent) application if +// application restarts take longer than 1 millisecond. The package supports up to 32 works, all +// providing unique sequences. +// +// Implementation details +// +// Although heavily inspired by the node.js shortid library this is +// not a simple Go port. In addition it +// +// - is safe to concurrency; +// - does not require any yearly version/epoch resets; +// - provides stable Id size over a long period at the rate of 1ms; +// - guarantees no collisions (due to guaranteed fixed size of Ids between milliseconds and because +// multiple requests within the same ms lead to longer Ids with the prefix unique to the ms); +// - supports 32 over 16 workers. +// +// The algorithm uses less randomness than the original node.js implementation, which permits to +// extend the life span as well as reduce and guarantee the length. In general terms, each Id +// has the following 3 pieces of information encoded: the millisecond (first 8 symbols), the worker +// Id (9th symbol), running concurrent counter within the same millisecond, only if required, over +// all remaining symbols. The element of randomness per symbol is 1/2 for the worker and the +// millisecond and 0 for the counter. Here 0 means no randomness, i.e. every value is encoded using +// a 64-base alphabet; 1/2 means one of two matching symbols of the supplied alphabet, 1/4 one of +// four matching symbols. The original algorithm of the node.js module uses 1/4 throughout. +// +// All methods accepting the parameters that govern the randomness are exported and can be used +// to directly implement an algorithm with e.g. more randomness, but with longer Ids and shorter +// life spans. +package shortid + +import ( + randc "crypto/rand" + "errors" + "fmt" + "math" + randm "math/rand" + "sync" + "sync/atomic" + "time" + "unsafe" +) + +// Version defined the library version. +const Version = 1.1 + +// DefaultABC is the default URL-friendly alphabet. +const DefaultABC = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_-" + +// Abc represents a shuffled alphabet used to generate the Ids and provides methods to +// encode data. +type Abc struct { + alphabet []rune +} + +// Shortid type represents a short Id generator working with a given alphabet. +type Shortid struct { + abc Abc + worker uint + epoch time.Time // ids can be generated for 34 years since this date + ms uint // ms since epoch for the last id + count uint // request count within the same ms + mx sync.Mutex // locks access to ms and count +} + +var shortid *Shortid + +func init() { + shortid = MustNew(0, DefaultABC, 1) +} + +// GetDefault retrieves the default short Id generator initialised with the default alphabet, +// worker=0 and seed=1. The default can be overwritten using SetDefault. +func GetDefault() *Shortid { + return (*Shortid)(atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&shortid)))) +} + +// SetDefault overwrites the default generator. +func SetDefault(sid *Shortid) { + target := (*unsafe.Pointer)(unsafe.Pointer(&shortid)) + source := unsafe.Pointer(sid) + atomic.SwapPointer(target, source) +} + +// Generate generates an Id using the default generator. +func Generate() (string, error) { + return shortid.Generate() +} + +// MustGenerate acts just like Generate, but panics instead of returning errors. +func MustGenerate() string { + id, err := Generate() + if err == nil { + return id + } + panic(err) +} + +// New constructs an instance of the short Id generator for the given worker number [0,31], alphabet +// (64 unique symbols) and seed value (to shuffle the alphabet). The worker number should be +// different for multiple or distributed processes generating Ids into the same data space. The +// seed, on contrary, should be identical. +func New(worker uint8, alphabet string, seed uint64) (*Shortid, error) { + if worker > 31 { + return nil, errors.New("expected worker in the range [0,31]") + } + abc, err := NewAbc(alphabet, seed) + if err == nil { + sid := &Shortid{ + abc: abc, + worker: uint(worker), + epoch: time.Date(2016, time.January, 1, 0, 0, 0, 0, time.UTC), + ms: 0, + count: 0, + } + return sid, nil + } + return nil, err +} + +// MustNew acts just like New, but panics instead of returning errors. +func MustNew(worker uint8, alphabet string, seed uint64) *Shortid { + sid, err := New(worker, alphabet, seed) + if err == nil { + return sid + } + panic(err) +} + +// Generate generates a new short Id. +func (sid *Shortid) Generate() (string, error) { + return sid.GenerateInternal(nil, sid.epoch) +} + +// MustGenerate acts just like Generate, but panics instead of returning errors. +func (sid *Shortid) MustGenerate() string { + id, err := sid.Generate() + if err == nil { + return id + } + panic(err) +} + +// GenerateInternal should only be used for testing purposes. +func (sid *Shortid) GenerateInternal(tm *time.Time, epoch time.Time) (string, error) { + ms, count := sid.getMsAndCounter(tm, epoch) + idrunes := make([]rune, 9) + if tmp, err := sid.abc.Encode(ms, 8, 5); err == nil { + copy(idrunes, tmp) // first 8 symbols + } else { + return "", err + } + if tmp, err := sid.abc.Encode(sid.worker, 1, 5); err == nil { + idrunes[8] = tmp[0] + } else { + return "", err + } + if count > 0 { + if countrunes, err := sid.abc.Encode(count, 0, 6); err == nil { + // only extend if really need it + idrunes = append(idrunes, countrunes...) + } else { + return "", err + } + } + return string(idrunes), nil +} + +func (sid *Shortid) getMsAndCounter(tm *time.Time, epoch time.Time) (uint, uint) { + sid.mx.Lock() + defer sid.mx.Unlock() + var ms uint + if tm != nil { + ms = uint(tm.Sub(epoch).Nanoseconds() / 1000000) + } else { + ms = uint(time.Now().Sub(epoch).Nanoseconds() / 1000000) + } + if ms == sid.ms { + sid.count++ + } else { + sid.count = 0 + sid.ms = ms + } + return sid.ms, sid.count +} + +// String returns a string representation of the short Id generator. +func (sid *Shortid) String() string { + return fmt.Sprintf("Shortid(worker=%v, epoch=%v, abc=%v)", sid.worker, sid.epoch, sid.abc) +} + +// Abc returns the instance of alphabet used for representing the Ids. +func (sid *Shortid) Abc() Abc { + return sid.abc +} + +// Epoch returns the value of epoch used as the beginning of millisecond counting (normally +// 2016-01-01 00:00:00 local time) +func (sid *Shortid) Epoch() time.Time { + return sid.epoch +} + +// Worker returns the value of worker for this short Id generator. +func (sid *Shortid) Worker() uint { + return sid.worker +} + +// NewAbc constructs a new instance of shuffled alphabet to be used for Id representation. +func NewAbc(alphabet string, seed uint64) (Abc, error) { + runes := []rune(alphabet) + if len(runes) != len(DefaultABC) { + return Abc{}, fmt.Errorf("alphabet must contain %v unique characters", len(DefaultABC)) + } + if nonUnique(runes) { + return Abc{}, errors.New("alphabet must contain unique characters only") + } + abc := Abc{alphabet: nil} + abc.shuffle(alphabet, seed) + return abc, nil +} + +// MustNewAbc acts just like NewAbc, but panics instead of returning errors. +func MustNewAbc(alphabet string, seed uint64) Abc { + res, err := NewAbc(alphabet, seed) + if err == nil { + return res + } + panic(err) +} + +func nonUnique(runes []rune) bool { + found := make(map[rune]struct{}) + for _, r := range runes { + if _, seen := found[r]; !seen { + found[r] = struct{}{} + } + } + return len(found) < len(runes) +} + +func (abc *Abc) shuffle(alphabet string, seed uint64) { + source := []rune(alphabet) + for len(source) > 1 { + seed = (seed*9301 + 49297) % 233280 + i := int(seed * uint64(len(source)) / 233280) + + abc.alphabet = append(abc.alphabet, source[i]) + source = append(source[:i], source[i+1:]...) + } + abc.alphabet = append(abc.alphabet, source[0]) +} + +// Encode encodes a given value into a slice of runes of length nsymbols. In case nsymbols==0, the +// length of the result is automatically computed from data. Even if fewer symbols is required to +// encode the data than nsymbols, all positions are used encoding 0 where required to guarantee +// uniqueness in case further data is added to the sequence. The value of digits [4,6] represents +// represents n in 2^n, which defines how much randomness flows into the algorithm: 4 -- every value +// can be represented by 4 symbols in the alphabet (permitting at most 16 values), 5 -- every value +// can be represented by 2 symbols in the alphabet (permitting at most 32 values), 6 -- every value +// is represented by exactly 1 symbol with no randomness (permitting 64 values). +func (abc *Abc) Encode(val, nsymbols, digits uint) ([]rune, error) { + if digits < 4 || 6 < digits { + return nil, fmt.Errorf("allowed digits range [4,6], found %v", digits) + } + + var computedSize uint = 1 + if val >= 1 { + computedSize = uint(math.Log2(float64(val)))/digits + 1 + } + if nsymbols == 0 { + nsymbols = computedSize + } else if nsymbols < computedSize { + return nil, fmt.Errorf("cannot accommodate data, need %v digits, got %v", computedSize, nsymbols) + } + + mask := 1<>shift) & mask) | random[i] + res[i] = abc.alphabet[index] + } + return res, nil +} + +// MustEncode acts just like Encode, but panics instead of returning errors. +func (abc *Abc) MustEncode(val, size, digits uint) []rune { + res, err := abc.Encode(val, size, digits) + if err == nil { + return res + } + panic(err) +} + +func maskedRandomInts(size, mask int) []int { + ints := make([]int, size) + bytes := make([]byte, size) + if _, err := randc.Read(bytes); err == nil { + for i, b := range bytes { + ints[i] = int(b) & mask + } + } else { + for i := range ints { + ints[i] = randm.Intn(0xff) & mask + } + } + return ints +} + +// String returns a string representation of the Abc instance. +func (abc Abc) String() string { + return fmt.Sprintf("Abc{alphabet='%v')", abc.Alphabet()) +} + +// Alphabet returns the alphabet used as an immutable string. +func (abc Abc) Alphabet() string { + return string(abc.alphabet) +} From fc7bab8bf06940ba4b2919b5eac53129573c850d Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 17:51:10 +0100 Subject: [PATCH 05/44] dashboard: fix failing test. #7883 --- pkg/services/sqlstore/dashboard_version_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/services/sqlstore/dashboard_version_test.go b/pkg/services/sqlstore/dashboard_version_test.go index 6ed37cd6904..cbc33c8ad84 100644 --- a/pkg/services/sqlstore/dashboard_version_test.go +++ b/pkg/services/sqlstore/dashboard_version_test.go @@ -49,6 +49,7 @@ func TestGetDashboardVersion(t *testing.T) { err = GetDashboard(&dashCmd) So(err, ShouldBeNil) + dashCmd.Result.Data.Del("uid") eq := reflect.DeepEqual(dashCmd.Result.Data, query.Result.Data) So(eq, ShouldEqual, true) }) From e229f8aea81063ae3eeeb4fd95a1cb07b2dbf515 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 18:52:19 +0100 Subject: [PATCH 06/44] dashboards: extract short uid generator to util package. #7883 --- pkg/models/dashboards.go | 12 +++--------- pkg/util/shortid_generator.go | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 pkg/util/shortid_generator.go diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index b377e184829..184fe33e134 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -7,7 +7,7 @@ import ( "github.com/gosimple/slug" "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/teris-io/shortid" + "github.com/grafana/grafana/pkg/util" ) // Typed errors @@ -63,7 +63,7 @@ type Dashboard struct { // NewDashboard creates a new dashboard func NewDashboard(title string) *Dashboard { dash := &Dashboard{} - dash.Uid = DashboardUid() + dash.Uid, _ = util.GenerateShortUid() dash.Data = simplejson.New() dash.Data.Set("title", title) dash.Title = title @@ -113,18 +113,12 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard { if uid, err := dash.Data.Get("uid").String(); err == nil { dash.Uid = uid } else { - dash.Uid = DashboardUid() + dash.Uid, _ = util.GenerateShortUid() } return dash } -func DashboardUid() string { - gen, _ := shortid.New(1, shortid.DefaultABC, 1) - uid, _ := gen.Generate() - return uid -} - // GetDashboardModel turns the command into the savable model func (cmd *SaveDashboardCommand) GetDashboardModel() *Dashboard { dash := NewDashboardFromJson(cmd.Dashboard) diff --git a/pkg/util/shortid_generator.go b/pkg/util/shortid_generator.go new file mode 100644 index 00000000000..ab3b9400f4f --- /dev/null +++ b/pkg/util/shortid_generator.go @@ -0,0 +1,24 @@ +package util + +import ( + "github.com/teris-io/shortid" +) + +func init() { + gen, _ := shortid.New(1, shortid.DefaultABC, 1) + shortid.SetDefault(gen) + +} + +// GenerateShortUid generates a short unique identifier. +func GenerateShortUid() (uid string, err error) { + if uid, err = shortid.Generate(); err != nil { + if uid, err = shortid.Generate(); err != nil { + if uid, err = shortid.Generate(); err != nil { + return "", err + } + } + } + + return uid, nil +} From 46e1296700ef08b30e1b70c4c6f4a02cd1220776 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 19:27:53 +0100 Subject: [PATCH 07/44] dashboards: return uid in response to creating/updating a dashboard. #7883 --- pkg/api/dashboard.go | 2 +- pkg/api/dashboard_test.go | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 87c42884e31..bf9f2cadec5 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -226,7 +226,7 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response { } c.TimeRequest(metrics.M_Api_Dashboard_Save) - return Json(200, util.DynMap{"status": "success", "slug": dashboard.Slug, "version": dashboard.Version, "id": dashboard.Id}) + return Json(200, util.DynMap{"status": "success", "slug": dashboard.Slug, "version": dashboard.Version, "id": dashboard.Id, "uid": dashboard.Uid}) } func GetHomeDashboard(c *middleware.Context) Response { diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index e6228878625..5dd55d9ae01 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -135,6 +135,11 @@ func TestDashboardApiEndpoint(t *testing.T) { postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) { CallPostDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) + result := sc.ToJson() + So(result.Get("status").MustString(), ShouldEqual, "success") + So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) + So(result.Get("uid").MustString(), ShouldNotBeNil) + So(result.Get("slug").MustString(), ShouldNotBeNil) }) Convey("When saving a dashboard folder in another folder", func() { @@ -306,6 +311,11 @@ func TestDashboardApiEndpoint(t *testing.T) { postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) { CallPostDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) + result := sc.ToJson() + So(result.Get("status").MustString(), ShouldEqual, "success") + So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) + So(result.Get("uid").MustString(), ShouldNotBeNil) + So(result.Get("slug").MustString(), ShouldNotBeNil) }) }) @@ -378,6 +388,11 @@ func TestDashboardApiEndpoint(t *testing.T) { postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) { CallPostDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) + result := sc.ToJson() + So(result.Get("status").MustString(), ShouldEqual, "success") + So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) + So(result.Get("uid").MustString(), ShouldNotBeNil) + So(result.Get("slug").MustString(), ShouldNotBeNil) }) }) @@ -519,3 +534,10 @@ func postDashboardScenario(desc string, url string, routePattern string, role m. fn(sc) }) } + +func (sc *scenarioContext) ToJson() *simplejson.Json { + var result *simplejson.Json + err := json.NewDecoder(sc.resp.Body).Decode(&result) + So(err, ShouldBeNil) + return result +} From c1cff3849e8ab7cd38565b9e64ec9651aed0dc7f Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 30 Jan 2018 11:15:51 +0100 Subject: [PATCH 08/44] dashboard: change unique index for uid to include org_id Make the max length of uid longer in case we want to change it later #7883 --- pkg/services/sqlstore/migrations/dashboard_mig.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index 1035ef35015..7da7c5a974f 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -151,17 +151,16 @@ func addDashboardMigration(mg *Migrator) { Name: "has_acl", Type: DB_Bool, Nullable: false, Default: "0", })) - // new uid column mg.AddMigration("Add column uid in dashboard", NewAddColumnMigration(dashboardV2, &Column{ - Name: "uid", Type: DB_NVarchar, Length: 12, Nullable: true, + Name: "uid", Type: DB_NVarchar, Length: 40, Nullable: true, })) - mg.AddMigration("Set uid column values", new(RawSqlMigration). + mg.AddMigration("Update uid column values in dashboard", new(RawSqlMigration). Sqlite("UPDATE dashboard SET uid=printf('%09d',id) WHERE uid IS NULL;"). Postgres("UPDATE dashboard SET uid=lpad('' || id,9,'0') WHERE uid IS NULL;"). Mysql("UPDATE dashboard SET uid=lpad(id,9,'0') WHERE uid IS NULL;")) - mg.AddMigration("Add index for uid in dashboard", NewAddIndexMigration(dashboardV2, &Index{ - Cols: []string{"uid"}, Type: UniqueIndex, + mg.AddMigration("Add unique index dashboard_org_id_uid", NewAddIndexMigration(dashboardV2, &Index{ + Cols: []string{"org_id", "uid"}, Type: UniqueIndex, })) } From 13d5db7d19b2faa7e73ce49d819fcbf3a4e5cb38 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 29 Jan 2018 21:23:07 +0100 Subject: [PATCH 09/44] dashboards: add support for retrieving a dashboard by uid Introduces new url in api /dashboards/ for fetching dashboard by unique id Leave the old dashboard by slug url /dashboards/db/ for backward compatibility and for supporting fallback WIP for #7883 --- pkg/api/api.go | 2 + pkg/api/dashboard.go | 17 +- pkg/api/dashboard_test.go | 211 ++++++++++++++++++++++-- pkg/models/dashboards.go | 3 +- pkg/services/sqlstore/dashboard.go | 2 +- pkg/services/sqlstore/dashboard_test.go | 38 ++++- 6 files changed, 249 insertions(+), 24 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index ea082ff4741..2d45868e58d 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -242,6 +242,8 @@ func (hs *HttpServer) registerRoutes() { // Dashboard apiRoute.Group("/dashboards", func(dashboardRoute RouteRegister) { + dashboardRoute.Get("/uid/:uid", wrap(GetDashboard)) + dashboardRoute.Get("/db/:slug", wrap(GetDashboard)) dashboardRoute.Delete("/db/:slug", reqEditorRole, wrap(DeleteDashboard)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index bf9f2cadec5..bfd53513feb 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -44,7 +44,7 @@ func dashboardGuardianResponse(err error) Response { } func GetDashboard(c *middleware.Context) Response { - dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0) + dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0, c.Params(":uid")) if rsp != nil { return rsp } @@ -124,8 +124,15 @@ func getUserLogin(userId int64) string { } } -func getDashboardHelper(orgId int64, slug string, id int64) (*m.Dashboard, Response) { - query := m.GetDashboardQuery{Slug: slug, Id: id, OrgId: orgId} +func getDashboardHelper(orgId int64, slug string, id int64, uid string) (*m.Dashboard, Response) { + var query m.GetDashboardQuery + + if len(uid) > 0 { + query = m.GetDashboardQuery{Uid: uid, Id: id, OrgId: orgId} + } else { + query = m.GetDashboardQuery{Slug: slug, Id: id, OrgId: orgId} + } + if err := bus.Dispatch(&query); err != nil { return nil, ApiError(404, "Dashboard not found", err) } @@ -133,7 +140,7 @@ func getDashboardHelper(orgId int64, slug string, id int64) (*m.Dashboard, Respo } func DeleteDashboard(c *middleware.Context) Response { - dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0) + dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0, "") if rsp != nil { return rsp } @@ -393,7 +400,7 @@ func CalculateDashboardDiff(c *middleware.Context, apiOptions dtos.CalculateDiff // RestoreDashboardVersion restores a dashboard to the given version. func RestoreDashboardVersion(c *middleware.Context, apiCmd dtos.RestoreDashboardVersionCommand) Response { - dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId")) + dash, rsp := getDashboardHelper(c.OrgId, "", c.ParamsInt64(":dashboardId"), "") if rsp != nil { return rsp } diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 5dd55d9ae01..cc718229f96 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -39,8 +39,11 @@ func TestDashboardApiEndpoint(t *testing.T) { fakeDash.FolderId = 1 fakeDash.HasAcl = false + var getDashboardQueries []*m.GetDashboardQuery + bus.AddHandler("test", func(query *m.GetDashboardQuery) error { query.Result = fakeDash + getDashboardQueries = append(getDashboardQueries, query) return nil }) @@ -73,9 +76,13 @@ func TestDashboardApiEndpoint(t *testing.T) { Convey("When user is an Org Viewer", func() { role := m.ROLE_VIEWER - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { dash := GetDashboardShouldReturn200(sc) + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should not be able to edit or save dashboard", func() { So(dash.Meta.CanEdit, ShouldBeFalse) So(dash.Meta.CanSave, ShouldBeFalse) @@ -83,9 +90,27 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + dash := GetDashboardShouldReturn200(sc) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should not be able to edit or save dashboard", func() { + So(dash.Meta.CanEdit, ShouldBeFalse) + So(dash.Meta.CanSave, ShouldBeFalse) + So(dash.Meta.CanAdmin, ShouldBeFalse) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { @@ -107,9 +132,13 @@ func TestDashboardApiEndpoint(t *testing.T) { Convey("When user is an Org Editor", func() { role := m.ROLE_EDITOR - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { dash := GetDashboardShouldReturn200(sc) + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should be able to edit or save dashboard", func() { So(dash.Meta.CanEdit, ShouldBeTrue) So(dash.Meta.CanSave, ShouldBeTrue) @@ -117,9 +146,27 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + dash := GetDashboardShouldReturn200(sc) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should be able to edit or save dashboard", func() { + So(dash.Meta.CanEdit, ShouldBeTrue) + So(dash.Meta.CanSave, ShouldBeTrue) + So(dash.Meta.CanAdmin, ShouldBeFalse) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { @@ -186,8 +233,11 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) + var getDashboardQueries []*m.GetDashboardQuery + bus.AddHandler("test", func(query *m.GetDashboardQuery) error { query.Result = fakeDash + getDashboardQueries = append(getDashboardQueries, query) return nil }) @@ -208,18 +258,39 @@ func TestDashboardApiEndpoint(t *testing.T) { Convey("When user is an Org Viewer and has no permissions for this dashboard", func() { role := m.ROLE_VIEWER - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { sc.handlerFunc = GetDashboard sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should be denied access", func() { So(sc.resp.Code, ShouldEqual, 403) }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + sc.handlerFunc = GetDashboard + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should be denied access", func() { + So(sc.resp.Code, ShouldEqual, 403) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { @@ -241,18 +312,39 @@ func TestDashboardApiEndpoint(t *testing.T) { Convey("When user is an Org Editor and has no permissions for this dashboard", func() { role := m.ROLE_EDITOR - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { sc.handlerFunc = GetDashboard sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should be denied access", func() { So(sc.resp.Code, ShouldEqual, 403) }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + sc.handlerFunc = GetDashboard + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should be denied access", func() { + So(sc.resp.Code, ShouldEqual, 403) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { @@ -283,9 +375,13 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { dash := GetDashboardShouldReturn200(sc) + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should be able to get dashboard with edit rights", func() { So(dash.Meta.CanEdit, ShouldBeTrue) So(dash.Meta.CanSave, ShouldBeTrue) @@ -293,9 +389,27 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + dash := GetDashboardShouldReturn200(sc) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should be able to get dashboard with edit rights", func() { + So(dash.Meta.CanEdit, ShouldBeTrue) + So(dash.Meta.CanSave, ShouldBeTrue) + So(dash.Meta.CanAdmin, ShouldBeFalse) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { @@ -332,9 +446,13 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { dash := GetDashboardShouldReturn200(sc) + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should be able to get dashboard with edit rights but can save should be false", func() { So(dash.Meta.CanEdit, ShouldBeTrue) So(dash.Meta.CanSave, ShouldBeFalse) @@ -342,9 +460,27 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + dash := GetDashboardShouldReturn200(sc) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should be able to get dashboard with edit rights but can save should be false", func() { + So(dash.Meta.CanEdit, ShouldBeTrue) + So(dash.Meta.CanSave, ShouldBeFalse) + So(dash.Meta.CanAdmin, ShouldBeFalse) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) }) @@ -360,9 +496,13 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { dash := GetDashboardShouldReturn200(sc) + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should be able to get dashboard with edit rights", func() { So(dash.Meta.CanEdit, ShouldBeTrue) So(dash.Meta.CanSave, ShouldBeTrue) @@ -370,9 +510,27 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + dash := GetDashboardShouldReturn200(sc) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should be able to get dashboard with edit rights", func() { + So(dash.Meta.CanEdit, ShouldBeTrue) + So(dash.Meta.CanSave, ShouldBeTrue) + So(dash.Meta.CanAdmin, ShouldBeTrue) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { @@ -408,18 +566,39 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { dash := GetDashboardShouldReturn200(sc) + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) + Convey("Should not be able to edit or save dashboard", func() { So(dash.Meta.CanEdit, ShouldBeFalse) So(dash.Meta.CanSave, ShouldBeFalse) }) }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/2", "/api/dashboards/:id", role, func(sc *scenarioContext) { + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + dash := GetDashboardShouldReturn200(sc) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + + Convey("Should not be able to edit or save dashboard", func() { + So(dash.Meta.CanEdit, ShouldBeFalse) + So(dash.Meta.CanSave, ShouldBeFalse) + }) + }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by slug", func() { + So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") + }) }) loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 184fe33e134..a858666a44e 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -186,8 +186,9 @@ type DeleteDashboardCommand struct { // type GetDashboardQuery struct { - Slug string // required if no Id is specified + Slug string // required if no Id or Uid is specified Id int64 // optional if slug is set + Uid string // optional if slug is set OrgId int64 Result *Dashboard diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index b8d3d9a36a5..497afa8f73f 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -165,7 +165,7 @@ func setHasAcl(sess *DBSession, dash *m.Dashboard) error { } func GetDashboard(query *m.GetDashboardQuery) error { - dashboard := m.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id} + dashboard := m.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id, Uid: query.Uid} has, err := x.Get(&dashboard) if err != nil { diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index a552bd0546a..18e283883a2 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -30,15 +30,33 @@ func TestDashboardDataAccess(t *testing.T) { So(savedDash.Id, ShouldNotEqual, 0) So(savedDash.IsFolder, ShouldBeFalse) So(savedDash.FolderId, ShouldBeGreaterThan, 0) + So(len(savedDash.Uid), ShouldBeGreaterThan, 0) So(savedFolder.Title, ShouldEqual, "1 test dash folder") So(savedFolder.Slug, ShouldEqual, "1-test-dash-folder") So(savedFolder.Id, ShouldNotEqual, 0) So(savedFolder.IsFolder, ShouldBeTrue) So(savedFolder.FolderId, ShouldEqual, 0) + So(len(savedFolder.Uid), ShouldBeGreaterThan, 0) }) - Convey("Should be able to get dashboard", func() { + Convey("Should be able to get dashboard by id", func() { + query := m.GetDashboardQuery{ + Id: savedDash.Id, + OrgId: 1, + } + + err := GetDashboard(&query) + So(err, ShouldBeNil) + + So(query.Result.Title, ShouldEqual, "test dash 23") + So(query.Result.Slug, ShouldEqual, "test-dash-23") + So(query.Result.Id, ShouldEqual, savedDash.Id) + So(query.Result.Uid, ShouldEqual, savedDash.Uid) + So(query.Result.IsFolder, ShouldBeFalse) + }) + + Convey("Should be able to get dashboard by slug", func() { query := m.GetDashboardQuery{ Slug: "test-dash-23", OrgId: 1, @@ -49,6 +67,24 @@ func TestDashboardDataAccess(t *testing.T) { So(query.Result.Title, ShouldEqual, "test dash 23") So(query.Result.Slug, ShouldEqual, "test-dash-23") + So(query.Result.Id, ShouldEqual, savedDash.Id) + So(query.Result.Uid, ShouldEqual, savedDash.Uid) + So(query.Result.IsFolder, ShouldBeFalse) + }) + + Convey("Should be able to get dashboard by uid", func() { + query := m.GetDashboardQuery{ + Uid: savedDash.Uid, + OrgId: 1, + } + + err := GetDashboard(&query) + So(err, ShouldBeNil) + + So(query.Result.Title, ShouldEqual, "test dash 23") + So(query.Result.Slug, ShouldEqual, "test-dash-23") + So(query.Result.Id, ShouldEqual, savedDash.Id) + So(query.Result.Uid, ShouldEqual, savedDash.Uid) So(query.Result.IsFolder, ShouldBeFalse) }) From 7ee691dc480ab6f47689e5d7f7f69504a9313be7 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 30 Jan 2018 13:11:48 +0100 Subject: [PATCH 10/44] dashboards: api for retrieving uid by slug. #7883 --- pkg/api/api.go | 1 + pkg/api/dashboard.go | 15 +++++++ pkg/api/dashboard_test.go | 88 +++++++++++++++++++++++++++++++++++++++ pkg/models/dashboards.go | 6 +++ 4 files changed, 110 insertions(+) diff --git a/pkg/api/api.go b/pkg/api/api.go index 2d45868e58d..1c3036eccee 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -245,6 +245,7 @@ func (hs *HttpServer) registerRoutes() { dashboardRoute.Get("/uid/:uid", wrap(GetDashboard)) dashboardRoute.Get("/db/:slug", wrap(GetDashboard)) + dashboardRoute.Get("/db/:slug/uid", wrap(GetDashboardUidBySlug)) dashboardRoute.Delete("/db/:slug", reqEditorRole, wrap(DeleteDashboard)) dashboardRoute.Post("/calculate-diff", bind(dtos.CalculateDiffOptions{}), wrap(CalculateDashboardDiff)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index bfd53513feb..0c30fd54ee5 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -113,6 +113,21 @@ func GetDashboard(c *middleware.Context) Response { return Json(200, dto) } +func GetDashboardUidBySlug(c *middleware.Context) Response { + dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0, "") + if rsp != nil { + return rsp + } + + guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser) + if canView, err := guardian.CanView(); err != nil || !canView { + fmt.Printf("%v", err) + return dashboardGuardianResponse(err) + } + + return Json(200, util.DynMap{"uid": dash.Uid}) +} + func getUserLogin(userId int64) string { query := m.GetUserByIdQuery{Id: userId} err := bus.Dispatch(&query) diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index cc718229f96..4255764d892 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -47,6 +47,11 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) + bus.AddHandler("test", func(query *m.GetDashboardUidBySlugQuery) error { + query.Result = fakeDash.Uid + return nil + }) + viewerRole := m.ROLE_VIEWER editorRole := m.ROLE_EDITOR @@ -104,6 +109,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + uid := GetDashboardUidBySlugShouldReturn200(sc) + + Convey("Should return uid", func() { + So(uid, ShouldEqual, fakeDash.Uid) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -160,6 +173,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + uid := GetDashboardUidBySlugShouldReturn200(sc) + + Convey("Should return uid", func() { + So(uid, ShouldEqual, fakeDash.Uid) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -241,6 +262,11 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) + bus.AddHandler("test", func(query *m.GetDashboardUidBySlugQuery) error { + query.Result = fakeDash.Uid + return nil + }) + bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error { query.Result = []*m.Team{} return nil @@ -284,6 +310,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + CallGetDashboardUidBySlug(sc) + + Convey("Should be denied access", func() { + So(sc.resp.Code, ShouldEqual, 403) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -338,6 +372,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + CallGetDashboardUidBySlug(sc) + + Convey("Should be denied access", func() { + So(sc.resp.Code, ShouldEqual, 403) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -403,6 +445,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + uid := GetDashboardUidBySlugShouldReturn200(sc) + + Convey("Should return uid", func() { + So(uid, ShouldEqual, fakeDash.Uid) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -474,6 +524,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + uid := GetDashboardUidBySlugShouldReturn200(sc) + + Convey("Should return uid", func() { + So(uid, ShouldEqual, fakeDash.Uid) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -524,6 +582,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + uid := GetDashboardUidBySlugShouldReturn200(sc) + + Convey("Should return uid", func() { + So(uid, ShouldEqual, fakeDash.Uid) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -592,6 +658,14 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { + uid := GetDashboardUidBySlugShouldReturn200(sc) + + Convey("Should return uid", func() { + So(uid, ShouldEqual, fakeDash.Uid) + }) + }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -632,6 +706,20 @@ func GetDashboardShouldReturn200(sc *scenarioContext) dtos.DashboardFullWithMeta return dash } +func GetDashboardUidBySlugShouldReturn200(sc *scenarioContext) string { + CallGetDashboardUidBySlug(sc) + + So(sc.resp.Code, ShouldEqual, 200) + + result := sc.ToJson() + return result.Get("uid").MustString() +} + +func CallGetDashboardUidBySlug(sc *scenarioContext) { + sc.handlerFunc = GetDashboardUidBySlug + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() +} + func CallGetDashboardVersion(sc *scenarioContext) { bus.AddHandler("test", func(query *m.GetDashboardVersionQuery) error { query.Result = &m.DashboardVersion{} diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index a858666a44e..0dfbd3b924f 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -219,3 +219,9 @@ type GetDashboardSlugByIdQuery struct { Id int64 Result string } + +type GetDashboardUidBySlugQuery struct { + OrgId int64 + Slug string + Result string +} From 9fb7b887db0d571b421ad34f9b021ec1c5f840bd Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 30 Jan 2018 15:24:14 +0100 Subject: [PATCH 11/44] dashboards: add url property to dashboard meta and search api responses #7883 --- pkg/api/dashboard.go | 6 ++++++ pkg/api/dtos/dashboard.go | 1 + pkg/models/dashboards.go | 12 ++++++++++++ pkg/services/search/models.go | 1 + pkg/services/sqlstore/dashboard.go | 9 +++++++++ pkg/services/sqlstore/dashboard_test.go | 3 +++ pkg/services/sqlstore/search_builder.go | 1 + 7 files changed, 33 insertions(+) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 0c30fd54ee5..efb9a790f2d 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -101,6 +101,12 @@ func GetDashboard(c *middleware.Context) Response { meta.FolderTitle = query.Result.Title } + if dash.IsFolder { + meta.Url = m.GetFolderUrl(dash.Uid, dash.Slug) + } else { + meta.Url = m.GetDashboardUrl(dash.Uid, dash.Slug) + } + // make sure db version is in sync with json model version dash.Data.Set("version", dash.Version) diff --git a/pkg/api/dtos/dashboard.go b/pkg/api/dtos/dashboard.go index 0be0537527b..604bb8d12ed 100644 --- a/pkg/api/dtos/dashboard.go +++ b/pkg/api/dtos/dashboard.go @@ -16,6 +16,7 @@ type DashboardMeta struct { CanAdmin bool `json:"canAdmin"` CanStar bool `json:"canStar"` Slug string `json:"slug"` + Url string `json:"url"` Expires time.Time `json:"expires"` Created time.Time `json:"created"` Updated time.Time `json:"updated"` diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 0dfbd3b924f..616ebfeb187 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -2,11 +2,13 @@ package models import ( "errors" + "fmt" "strings" "time" "github.com/gosimple/slug" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -156,6 +158,16 @@ func SlugifyTitle(title string) string { return slug.Make(strings.ToLower(title)) } +// GetDashboardUrl return the html url for a dashboard +func GetDashboardUrl(uid string, slug string) string { + return fmt.Sprintf("%s/d/%s/%s", setting.AppSubUrl, uid, slug) +} + +// GetFolderUrl return the html url for a folder +func GetFolderUrl(folderUid string, slug string) string { + return fmt.Sprintf("%s/f/%v/%s", setting.AppSubUrl, folderUid, slug) +} + // // COMMANDS // diff --git a/pkg/services/search/models.go b/pkg/services/search/models.go index cf510ed8462..6214a854db7 100644 --- a/pkg/services/search/models.go +++ b/pkg/services/search/models.go @@ -15,6 +15,7 @@ type Hit struct { Id int64 `json:"id"` Title string `json:"title"` Uri string `json:"uri"` + Url string `json:"url"` Slug string `json:"slug"` Type HitType `json:"type"` Tags []string `json:"tags"` diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 497afa8f73f..84cfd1999c7 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -182,6 +182,7 @@ func GetDashboard(query *m.GetDashboardQuery) error { type DashboardSearchProjection struct { Id int64 + Uid string Title string Slug string Term string @@ -257,10 +258,17 @@ func makeQueryResult(query *search.FindPersistedDashboardsQuery, res []Dashboard for _, item := range res { hit, exists := hits[item.Id] if !exists { + var url string + if item.IsFolder { + url = m.GetFolderUrl(item.Uid, item.Slug) + } else { + url = m.GetDashboardUrl(item.Uid, item.Slug) + } hit = &search.Hit{ Id: item.Id, Title: item.Title, Uri: "db/" + item.Slug, + Url: url, Slug: item.Slug, Type: getHitType(item), FolderId: item.FolderId, @@ -268,6 +276,7 @@ func makeQueryResult(query *search.FindPersistedDashboardsQuery, res []Dashboard FolderSlug: item.FolderSlug, Tags: []string{}, } + query.Result = append(query.Result, hit) hits[item.Id] = hit } diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 18e283883a2..19e82614718 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "fmt" "testing" "github.com/go-xorm/xorm" @@ -145,6 +146,7 @@ func TestDashboardDataAccess(t *testing.T) { So(len(query.Result), ShouldEqual, 1) hit := query.Result[0] So(hit.Type, ShouldEqual, search.DashHitFolder) + So(hit.Url, ShouldEqual, fmt.Sprintf("/f/%s/%s", savedFolder.Uid, savedFolder.Slug)) }) Convey("Should be able to search for a dashboard folder's children", func() { @@ -160,6 +162,7 @@ func TestDashboardDataAccess(t *testing.T) { So(len(query.Result), ShouldEqual, 2) hit := query.Result[0] So(hit.Id, ShouldEqual, savedDash.Id) + So(hit.Url, ShouldEqual, fmt.Sprintf("/d/%s/%s", savedDash.Uid, savedDash.Slug)) }) Convey("Should be able to search for dashboard by dashboard ids", func() { diff --git a/pkg/services/sqlstore/search_builder.go b/pkg/services/sqlstore/search_builder.go index ddf192da5ff..480e7fa99e6 100644 --- a/pkg/services/sqlstore/search_builder.go +++ b/pkg/services/sqlstore/search_builder.go @@ -101,6 +101,7 @@ func (sb *SearchBuilder) buildSelect() { sb.sql.WriteString( `SELECT dashboard.id, + dashboard.uid, dashboard.title, dashboard.slug, dashboard_tag.term, From 4829ea0e9f994f3befa566383142688039147ed2 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 30 Jan 2018 23:07:21 +0100 Subject: [PATCH 12/44] util: remove retry logic in shortid_generator Use shortid.MustGenerate() instead of shortid.Generate(). Instead of returning errors it will panic. --- pkg/models/dashboards.go | 4 ++-- pkg/util/shortid_generator.go | 13 ++----------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 616ebfeb187..e9b3768578e 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -65,7 +65,7 @@ type Dashboard struct { // NewDashboard creates a new dashboard func NewDashboard(title string) *Dashboard { dash := &Dashboard{} - dash.Uid, _ = util.GenerateShortUid() + dash.Uid = util.GenerateShortUid() dash.Data = simplejson.New() dash.Data.Set("title", title) dash.Title = title @@ -115,7 +115,7 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard { if uid, err := dash.Data.Get("uid").String(); err == nil { dash.Uid = uid } else { - dash.Uid, _ = util.GenerateShortUid() + dash.Uid = util.GenerateShortUid() } return dash diff --git a/pkg/util/shortid_generator.go b/pkg/util/shortid_generator.go index ab3b9400f4f..067f7c756ba 100644 --- a/pkg/util/shortid_generator.go +++ b/pkg/util/shortid_generator.go @@ -7,18 +7,9 @@ import ( func init() { gen, _ := shortid.New(1, shortid.DefaultABC, 1) shortid.SetDefault(gen) - } // GenerateShortUid generates a short unique identifier. -func GenerateShortUid() (uid string, err error) { - if uid, err = shortid.Generate(); err != nil { - if uid, err = shortid.Generate(); err != nil { - if uid, err = shortid.Generate(); err != nil { - return "", err - } - } - } - - return uid, nil +func GenerateShortUid() string { + return shortid.MustGenerate() } From fd59241e35bc6584bfcef2a1c9dfc41f19337b38 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 30 Jan 2018 23:18:08 +0100 Subject: [PATCH 13/44] dashboards: revert adding api for retrieving uid by slug Since we're already have possibility to get a dashboard by slug it makes little sense to have a separate endpoint in api for retrieving uid by slug. #7883 --- pkg/api/api.go | 1 - pkg/api/dashboard.go | 15 ------- pkg/api/dashboard_test.go | 88 --------------------------------------- pkg/models/dashboards.go | 6 --- 4 files changed, 110 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 1c3036eccee..2d45868e58d 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -245,7 +245,6 @@ func (hs *HttpServer) registerRoutes() { dashboardRoute.Get("/uid/:uid", wrap(GetDashboard)) dashboardRoute.Get("/db/:slug", wrap(GetDashboard)) - dashboardRoute.Get("/db/:slug/uid", wrap(GetDashboardUidBySlug)) dashboardRoute.Delete("/db/:slug", reqEditorRole, wrap(DeleteDashboard)) dashboardRoute.Post("/calculate-diff", bind(dtos.CalculateDiffOptions{}), wrap(CalculateDashboardDiff)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index efb9a790f2d..a1c9950c457 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -119,21 +119,6 @@ func GetDashboard(c *middleware.Context) Response { return Json(200, dto) } -func GetDashboardUidBySlug(c *middleware.Context) Response { - dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0, "") - if rsp != nil { - return rsp - } - - guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser) - if canView, err := guardian.CanView(); err != nil || !canView { - fmt.Printf("%v", err) - return dashboardGuardianResponse(err) - } - - return Json(200, util.DynMap{"uid": dash.Uid}) -} - func getUserLogin(userId int64) string { query := m.GetUserByIdQuery{Id: userId} err := bus.Dispatch(&query) diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 4255764d892..cc718229f96 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -47,11 +47,6 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - bus.AddHandler("test", func(query *m.GetDashboardUidBySlugQuery) error { - query.Result = fakeDash.Uid - return nil - }) - viewerRole := m.ROLE_VIEWER editorRole := m.ROLE_EDITOR @@ -109,14 +104,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - uid := GetDashboardUidBySlugShouldReturn200(sc) - - Convey("Should return uid", func() { - So(uid, ShouldEqual, fakeDash.Uid) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -173,14 +160,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - uid := GetDashboardUidBySlugShouldReturn200(sc) - - Convey("Should return uid", func() { - So(uid, ShouldEqual, fakeDash.Uid) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -262,11 +241,6 @@ func TestDashboardApiEndpoint(t *testing.T) { return nil }) - bus.AddHandler("test", func(query *m.GetDashboardUidBySlugQuery) error { - query.Result = fakeDash.Uid - return nil - }) - bus.AddHandler("test", func(query *m.GetTeamsByUserQuery) error { query.Result = []*m.Team{} return nil @@ -310,14 +284,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - CallGetDashboardUidBySlug(sc) - - Convey("Should be denied access", func() { - So(sc.resp.Code, ShouldEqual, 403) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -372,14 +338,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - CallGetDashboardUidBySlug(sc) - - Convey("Should be denied access", func() { - So(sc.resp.Code, ShouldEqual, 403) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -445,14 +403,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - uid := GetDashboardUidBySlugShouldReturn200(sc) - - Convey("Should return uid", func() { - So(uid, ShouldEqual, fakeDash.Uid) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -524,14 +474,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - uid := GetDashboardUidBySlugShouldReturn200(sc) - - Convey("Should return uid", func() { - So(uid, ShouldEqual, fakeDash.Uid) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -582,14 +524,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - uid := GetDashboardUidBySlugShouldReturn200(sc) - - Convey("Should return uid", func() { - So(uid, ShouldEqual, fakeDash.Uid) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -658,14 +592,6 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) - loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/db/child-dash/uid", "/api/dashboards/db/:slug/uid", role, func(sc *scenarioContext) { - uid := GetDashboardUidBySlugShouldReturn200(sc) - - Convey("Should return uid", func() { - So(uid, ShouldEqual, fakeDash.Uid) - }) - }) - loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/child-dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { CallDeleteDashboard(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -706,20 +632,6 @@ func GetDashboardShouldReturn200(sc *scenarioContext) dtos.DashboardFullWithMeta return dash } -func GetDashboardUidBySlugShouldReturn200(sc *scenarioContext) string { - CallGetDashboardUidBySlug(sc) - - So(sc.resp.Code, ShouldEqual, 200) - - result := sc.ToJson() - return result.Get("uid").MustString() -} - -func CallGetDashboardUidBySlug(sc *scenarioContext) { - sc.handlerFunc = GetDashboardUidBySlug - sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() -} - func CallGetDashboardVersion(sc *scenarioContext) { bus.AddHandler("test", func(query *m.GetDashboardVersionQuery) error { query.Result = &m.DashboardVersion{} diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index e9b3768578e..84ede04f226 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -231,9 +231,3 @@ type GetDashboardSlugByIdQuery struct { Id int64 Result string } - -type GetDashboardUidBySlugQuery struct { - OrgId int64 - Slug string - Result string -} From 6ab526881a87d0e6d4b7a9454ffba744c09413dd Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 00:06:31 +0100 Subject: [PATCH 14/44] dashboards: ensure that uid is returned from getSaveModelClone Without this the uid will not be sent to the backend when saving a dashboard. #7883 --- public/app/features/dashboard/dashboard_model.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/public/app/features/dashboard/dashboard_model.ts b/public/app/features/dashboard/dashboard_model.ts index dccd910a85e..a416e610132 100644 --- a/public/app/features/dashboard/dashboard_model.ts +++ b/public/app/features/dashboard/dashboard_model.ts @@ -12,6 +12,7 @@ import { DashboardMigrator } from './dashboard_migration'; export class DashboardModel { id: any; + uid: any; title: any; autoUpdate: any; description: any; @@ -56,6 +57,7 @@ export class DashboardModel { this.events = new Emitter(); this.id = data.id || null; + this.uid = data.uid || null; this.revision = data.revision; this.title = data.title || 'No Title'; this.autoUpdate = data.autoUpdate; From 369597f7b2b8093bf5509a8d85012aef9cc1772f Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 30 Jan 2018 23:37:54 +0100 Subject: [PATCH 15/44] dashboards: return url in response to save dashboard. #7883 --- pkg/api/dashboard.go | 16 +++++++++++++++- pkg/api/dashboard_test.go | 36 +++++++++++++++--------------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index a1c9950c457..7799ad868f9 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -238,8 +238,22 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response { return ApiError(500, "Invalid alert data. Cannot save dashboard", err) } + var url string + if dash.IsFolder { + url = m.GetFolderUrl(dashboard.Uid, dashboard.Slug) + } else { + url = m.GetDashboardUrl(dashboard.Uid, dashboard.Slug) + } + c.TimeRequest(metrics.M_Api_Dashboard_Save) - return Json(200, util.DynMap{"status": "success", "slug": dashboard.Slug, "version": dashboard.Version, "id": dashboard.Id, "uid": dashboard.Uid}) + return Json(200, util.DynMap{ + "status": "success", + "slug": dashboard.Slug, + "version": dashboard.Version, + "id": dashboard.Id, + "uid": dashboard.Uid, + "url": url, + }) } func GetHomeDashboard(c *middleware.Context) Response { diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index cc718229f96..0bd586f6067 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -180,13 +180,7 @@ func TestDashboardApiEndpoint(t *testing.T) { }) postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) { - CallPostDashboard(sc) - So(sc.resp.Code, ShouldEqual, 200) - result := sc.ToJson() - So(result.Get("status").MustString(), ShouldEqual, "success") - So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) - So(result.Get("uid").MustString(), ShouldNotBeNil) - So(result.Get("slug").MustString(), ShouldNotBeNil) + CallPostDashboardShouldReturnSuccess(sc) }) Convey("When saving a dashboard folder in another folder", func() { @@ -423,13 +417,7 @@ func TestDashboardApiEndpoint(t *testing.T) { }) postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) { - CallPostDashboard(sc) - So(sc.resp.Code, ShouldEqual, 200) - result := sc.ToJson() - So(result.Get("status").MustString(), ShouldEqual, "success") - So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) - So(result.Get("uid").MustString(), ShouldNotBeNil) - So(result.Get("slug").MustString(), ShouldNotBeNil) + CallPostDashboardShouldReturnSuccess(sc) }) }) @@ -544,13 +532,7 @@ func TestDashboardApiEndpoint(t *testing.T) { }) postDashboardScenario("When calling POST on", "/api/dashboards", "/api/dashboards", role, cmd, func(sc *scenarioContext) { - CallPostDashboard(sc) - So(sc.resp.Code, ShouldEqual, 200) - result := sc.ToJson() - So(result.Get("status").MustString(), ShouldEqual, "success") - So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) - So(result.Get("uid").MustString(), ShouldNotBeNil) - So(result.Get("slug").MustString(), ShouldNotBeNil) + CallPostDashboardShouldReturnSuccess(sc) }) }) @@ -678,6 +660,18 @@ func CallPostDashboard(sc *scenarioContext) { sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() } +func CallPostDashboardShouldReturnSuccess(sc *scenarioContext) { + CallPostDashboard(sc) + + So(sc.resp.Code, ShouldEqual, 200) + result := sc.ToJson() + So(result.Get("status").MustString(), ShouldEqual, "success") + So(result.Get("id").MustInt64(), ShouldBeGreaterThan, 0) + So(result.Get("uid").MustString(), ShouldNotBeNil) + So(result.Get("slug").MustString(), ShouldNotBeNil) + So(result.Get("url").MustString(), ShouldNotBeNil) +} + func postDashboardScenario(desc string, url string, routePattern string, role m.RoleType, cmd m.SaveDashboardCommand, fn scenarioFunc) { Convey(desc+" "+url, func() { defer bus.ClearBusHandlers() From aefcff26a61f026958ee3ffe58f13b2b07ea43cd Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 00:26:26 +0100 Subject: [PATCH 16/44] dashboards: add new default frontend route for loading a dashboard New default route /d// where dashboard are loaded by unique identifier. If old route /dashboard/db/ are used, try to lookup dashboard by slug and redirect to new default route. #7883 --- public/app/core/services/backend_srv.ts | 4 ++++ .../app/features/dashboard/dashboard_loader_srv.ts | 6 +++--- public/app/routes/dashboard_loaders.ts | 14 ++++++++++++-- public/app/routes/routes.ts | 6 ++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 3204db0ce5d..9ade4264bb3 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -225,6 +225,10 @@ export class BackendSrv { return this.get('/api/dashboards/' + type + '/' + slug); } + getDashboardByUid(uid: string) { + return this.get(`/api/dashboards/uid/${uid}`); + } + saveDashboard(dash, options) { options = options || {}; diff --git a/public/app/features/dashboard/dashboard_loader_srv.ts b/public/app/features/dashboard/dashboard_loader_srv.ts index e4def4385e1..9e458c4e4bb 100644 --- a/public/app/features/dashboard/dashboard_loader_srv.ts +++ b/public/app/features/dashboard/dashboard_loader_srv.ts @@ -35,18 +35,18 @@ export class DashboardLoaderSrv { }; } - loadDashboard(type, slug) { + loadDashboard(type, slug, uid) { var promise; if (type === 'script') { promise = this._loadScriptedDashboard(slug); } else if (type === 'snapshot') { - promise = this.backendSrv.get('/api/snapshots/' + this.$routeParams.slug).catch(() => { + promise = this.backendSrv.get('/api/snapshots/' + slug).catch(() => { return this._dashboardLoadFailed('Snapshot not found', true); }); } else { promise = this.backendSrv - .getDashboard(this.$routeParams.type, this.$routeParams.slug) + .getDashboardByUid(uid) .then(result => { if (result.meta.isFolder) { this.$rootScope.appEvent('alert-error', ['Dashboard not found']); diff --git a/public/app/routes/dashboard_loaders.ts b/public/app/routes/dashboard_loaders.ts index f4f0b36ac72..e3e6f4c18bc 100644 --- a/public/app/routes/dashboard_loaders.ts +++ b/public/app/routes/dashboard_loaders.ts @@ -5,7 +5,7 @@ export class LoadDashboardCtrl { constructor($scope, $routeParams, dashboardLoaderSrv, backendSrv, $location) { $scope.appEvent('dashboard-fetch-start'); - if (!$routeParams.slug) { + if (!$routeParams.uid && !$routeParams.slug) { backendSrv.get('/api/dashboards/home').then(function(homeDash) { if (homeDash.redirectUri) { $location.path('dashboard/' + homeDash.redirectUri); @@ -18,7 +18,17 @@ export class LoadDashboardCtrl { return; } - dashboardLoaderSrv.loadDashboard($routeParams.type, $routeParams.slug).then(function(result) { + // if no uid, redirect to new route based on slug + if (!$routeParams.uid) { + backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { + if (res) { + $location.path(res.meta.url); + } + }); + return; + } + + dashboardLoaderSrv.loadDashboard($routeParams.type, $routeParams.slug, $routeParams.uid).then(function(result) { if ($routeParams.keepRows) { result.meta.keepRows = true; } diff --git a/public/app/routes/routes.ts b/public/app/routes/routes.ts index ec65e4ec25a..081fb88af6e 100644 --- a/public/app/routes/routes.ts +++ b/public/app/routes/routes.ts @@ -14,6 +14,12 @@ export function setupAngularRoutes($routeProvider, $locationProvider) { reloadOnSearch: false, pageClass: 'page-dashboard', }) + .when('/d/:uid/:slug', { + templateUrl: 'public/app/partials/dashboard.html', + controller: 'LoadDashboardCtrl', + reloadOnSearch: false, + pageClass: 'page-dashboard', + }) .when('/dashboard/:type/:slug', { templateUrl: 'public/app/partials/dashboard.html', controller: 'LoadDashboardCtrl', From 8009307c16732e39eb83e482cc720f3897d29b9b Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 00:32:51 +0100 Subject: [PATCH 17/44] dashboards: when saving dashboard redirect if url changes Redirect if new dashboard created or existing url changes. #7883 --- public/app/features/dashboard/dashboard_srv.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/public/app/features/dashboard/dashboard_srv.ts b/public/app/features/dashboard/dashboard_srv.ts index 708ba047d2b..07fc14f2f2e 100644 --- a/public/app/features/dashboard/dashboard_srv.ts +++ b/public/app/features/dashboard/dashboard_srv.ts @@ -73,9 +73,8 @@ export class DashboardSrv { postSave(clone, data) { this.dash.version = data.version; - var dashboardUrl = '/dashboard/db/' + data.slug; - if (dashboardUrl !== this.$location.path()) { - this.$location.url(dashboardUrl); + if (data.url !== this.$location.path()) { + this.$location.url(data.url); } this.$rootScope.appEvent('dashboard-saved', this.dash); From f2014223b490911b699a723ba3e8f1b58e9affa4 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 00:42:17 +0100 Subject: [PATCH 18/44] dashboards: use new *url* prop from dashboard search for linking to dashboards #7883 --- public/app/core/services/search_srv.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/public/app/core/services/search_srv.ts b/public/app/core/services/search_srv.ts index a909b4af09f..a0989e74aed 100644 --- a/public/app/core/services/search_srv.ts +++ b/public/app/core/services/search_srv.ts @@ -41,10 +41,7 @@ export class SearchSrv { .map(orderId => { return _.find(result, { id: orderId }); }) - .filter(hit => hit && !hit.isStarred) - .map(hit => { - return this.transformToViewModel(hit); - }); + .filter(hit => hit && !hit.isStarred); }); } @@ -81,17 +78,12 @@ export class SearchSrv { score: -2, expanded: this.starredIsOpen, toggle: this.toggleStarred.bind(this), - items: result.map(this.transformToViewModel), + items: result, }; } }); } - private transformToViewModel(hit) { - hit.url = 'dashboard/db/' + hit.slug; - return hit; - } - search(options) { let sections: any = {}; let promises = []; @@ -181,7 +173,7 @@ export class SearchSrv { } section.expanded = true; - section.items.push(this.transformToViewModel(hit)); + section.items.push(hit); } } @@ -198,7 +190,7 @@ export class SearchSrv { }; return this.backendSrv.search(query).then(results => { - section.items = _.map(results, this.transformToViewModel); + section.items = results; return Promise.resolve(section); }); } From 7734df1d1859d7aed6a8c04d0a3131eaa1cd7e86 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 00:48:36 +0100 Subject: [PATCH 19/44] dashboards: fix links to recently viewed and starred dashboards Use new property url from dashboard search for linking to dashboards #7883 --- public/app/plugins/panel/dashlist/module.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/plugins/panel/dashlist/module.html b/public/app/plugins/panel/dashlist/module.html index 7f71811ac08..8fa3e7ef71f 100644 --- a/public/app/plugins/panel/dashlist/module.html +++ b/public/app/plugins/panel/dashlist/module.html @@ -4,7 +4,7 @@ {{group.header}}
- + {{dash.title}} From 6d2a555866b354d66ca270f6bd6ee2b7e0d7673d Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 30 Jan 2018 17:06:23 +0100 Subject: [PATCH 20/44] Drops unique index orgid_slug from dashboards. --- pkg/services/sqlstore/migrations/dashboard_mig.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/services/sqlstore/migrations/dashboard_mig.go b/pkg/services/sqlstore/migrations/dashboard_mig.go index 7da7c5a974f..edca733e174 100644 --- a/pkg/services/sqlstore/migrations/dashboard_mig.go +++ b/pkg/services/sqlstore/migrations/dashboard_mig.go @@ -163,4 +163,8 @@ func addDashboardMigration(mg *Migrator) { mg.AddMigration("Add unique index dashboard_org_id_uid", NewAddIndexMigration(dashboardV2, &Index{ Cols: []string{"org_id", "uid"}, Type: UniqueIndex, })) + + mg.AddMigration("Remove unique index org_id_slug", NewDropIndexMigration(dashboardV2, &Index{ + Cols: []string{"org_id", "slug"}, Type: UniqueIndex, + })) } From bb3183f6cdc8e92c98d02a079cb1a8f1af776e75 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 30 Jan 2018 18:57:33 +0100 Subject: [PATCH 21/44] removes uniqnes check on slug when saving dashboards --- pkg/api/dashboard.go | 2 +- pkg/models/dashboards.go | 3 ++- pkg/services/sqlstore/dashboard.go | 19 ++++++++++--------- pkg/services/sqlstore/dashboard_test.go | 16 ++++++++++++++++ .../sqlstore/dashboard_version_test.go | 5 ++--- tests/test-app/dashboards/connections.json | 1 + .../dashboards/connections_result.json | 1 + 7 files changed, 33 insertions(+), 14 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index a1c9950c457..77d3d8882f5 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -214,7 +214,7 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response { } if err != nil { - if err == m.ErrDashboardWithSameNameExists { + if err == m.ErrDashboardWithSameUIDExists { return Json(412, util.DynMap{"status": "name-exists", "message": err.Error()}) } if err == m.ErrDashboardVersionMismatch { diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 84ede04f226..cf7f6355537 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -16,7 +16,7 @@ import ( var ( ErrDashboardNotFound = errors.New("Dashboard not found") ErrDashboardSnapshotNotFound = errors.New("Dashboard snapshot not found") - ErrDashboardWithSameNameExists = errors.New("A dashboard with the same name already exists") + ErrDashboardWithSameUIDExists = errors.New("A dashboard with the same uid already exists") ErrDashboardVersionMismatch = errors.New("The dashboard has been changed by someone else") ErrDashboardTitleEmpty = errors.New("Dashboard title cannot be empty") ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") @@ -116,6 +116,7 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard { dash.Uid = uid } else { dash.Uid = util.GenerateShortUid() + dash.Data.Set("uid", dash.Uid) } return dash diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 84cfd1999c7..376e4e3c382 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -25,7 +25,7 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { dash := cmd.GetDashboardModel() // try get existing dashboard - var existing, sameTitle m.Dashboard + var existing m.Dashboard if dash.Id > 0 { dashWithIdExists, err := sess.Where("id=? AND org_id=?", dash.Id, dash.OrgId).Get(&existing) @@ -51,19 +51,20 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { } } - sameTitleExists, err := sess.Where("org_id=? AND slug=?", dash.OrgId, dash.Slug).Get(&sameTitle) + var sameUid m.Dashboard + sameUidExists, err := sess.Where("org_id=? AND uid=?", dash.OrgId, dash.Uid).Get(&sameUid) if err != nil { return err } - if sameTitleExists { - // another dashboard with same name - if dash.Id != sameTitle.Id { + if sameUidExists { + // another dashboard with same uid + if dash.Id != sameUid.Id { if cmd.Overwrite { - dash.Id = sameTitle.Id - dash.Version = sameTitle.Version + dash.Id = sameUid.Id + dash.Version = sameUid.Version } else { - return m.ErrDashboardWithSameNameExists + return m.ErrDashboardWithSameUIDExists } } } @@ -89,7 +90,7 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { dash.Updated = cmd.UpdatedAt } - affectedRows, err = sess.MustCols("folder_id", "has_acl").Id(dash.Id).Update(dash) + affectedRows, err = sess.MustCols("folder_id", "has_acl").ID(dash.Id).Update(dash) } if err != nil { diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 19e82614718..e8fb5332d32 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -207,6 +207,22 @@ func TestDashboardDataAccess(t *testing.T) { } err := SaveDashboard(&cmd) + So(err, ShouldBeNil) + }) + + Convey("Should not be able to save dashboard with same uid", func() { + cmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": nil, + "title": "test dash 23", + "uid": "dsfalkjngailuedt", + }), + } + + err := SaveDashboard(&cmd) + So(err, ShouldBeNil) + err = SaveDashboard(&cmd) So(err, ShouldNotBeNil) }) diff --git a/pkg/services/sqlstore/dashboard_version_test.go b/pkg/services/sqlstore/dashboard_version_test.go index cbc33c8ad84..e20ac897b3d 100644 --- a/pkg/services/sqlstore/dashboard_version_test.go +++ b/pkg/services/sqlstore/dashboard_version_test.go @@ -12,7 +12,7 @@ import ( ) func updateTestDashboard(dashboard *m.Dashboard, data map[string]interface{}) { - data["title"] = dashboard.Title + data["uid"] = dashboard.Uid saveCmd := m.SaveDashboardCommand{ OrgId: dashboard.OrgId, @@ -44,12 +44,11 @@ func TestGetDashboardVersion(t *testing.T) { dashCmd := m.GetDashboardQuery{ OrgId: savedDash.OrgId, - Slug: savedDash.Slug, + Uid: savedDash.Uid, } err = GetDashboard(&dashCmd) So(err, ShouldBeNil) - dashCmd.Result.Data.Del("uid") eq := reflect.DeepEqual(dashCmd.Result.Data, query.Result.Data) So(eq, ShouldEqual, true) }) diff --git a/tests/test-app/dashboards/connections.json b/tests/test-app/dashboards/connections.json index cc189d2d113..042dc4baec5 100644 --- a/tests/test-app/dashboards/connections.json +++ b/tests/test-app/dashboards/connections.json @@ -7,6 +7,7 @@ } ], + "uid": "1MHHlVjzz", "title": "Nginx Connections", "revision": 25, "schemaVersion": 11, diff --git a/tests/test-app/dashboards/connections_result.json b/tests/test-app/dashboards/connections_result.json index 4bf0570ac1e..2d2bc8de238 100644 --- a/tests/test-app/dashboards/connections_result.json +++ b/tests/test-app/dashboards/connections_result.json @@ -16,5 +16,6 @@ ], "schemaVersion": 11, "title": "Nginx Connections", + "uid": "1MHHlVjzz", "version": 0 } From 3efb3d8efa11e05d8e629f1043f657ecd89d0572 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 11:01:01 +0100 Subject: [PATCH 22/44] dashboards: add new default frontend route for rendering a dashboard panel New default route /d-solo// where dashboard panel are rendered by unique identifier and panel identifier. If old route /dashboard-solo/db/ are used, try to lookup dashboard by slug and redirect to new default route. Change url logic for sharing a panel so that the new default route for rendering a dashboard panel are used. #7883 --- public/app/features/dashboard/shareModalCtrl.ts | 7 ++----- .../dashboard/specs/share_modal_ctrl_specs.ts | 4 ++-- public/app/features/panel/solo_panel_ctrl.ts | 14 ++++++++++++-- public/app/routes/routes.ts | 6 ++++++ 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/public/app/features/dashboard/shareModalCtrl.ts b/public/app/features/dashboard/shareModalCtrl.ts index 508658cd53b..2bde5c2d198 100644 --- a/public/app/features/dashboard/shareModalCtrl.ts +++ b/public/app/features/dashboard/shareModalCtrl.ts @@ -73,17 +73,14 @@ export class ShareModalCtrl { $scope.shareUrl = linkSrv.addParamsToUrl(baseUrl, params); - var soloUrl = baseUrl.replace(config.appSubUrl + '/dashboard/', config.appSubUrl + '/dashboard-solo/'); + var soloUrl = baseUrl.replace(config.appSubUrl + '/d/', config.appSubUrl + '/d-solo/'); delete params.fullscreen; delete params.edit; soloUrl = linkSrv.addParamsToUrl(soloUrl, params); $scope.iframeHtml = ''; - $scope.imageUrl = soloUrl.replace( - config.appSubUrl + '/dashboard-solo/', - config.appSubUrl + '/render/dashboard-solo/' - ); + $scope.imageUrl = soloUrl.replace(config.appSubUrl + '/d-solo/', config.appSubUrl + '/render/d-solo/'); $scope.imageUrl += '&width=1000'; $scope.imageUrl += '&height=500'; $scope.imageUrl += '&tz=UTC' + encodeURIComponent(moment().format('Z')); diff --git a/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts b/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts index 8bf8f53ed46..e65d9c98f82 100644 --- a/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts +++ b/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts @@ -43,12 +43,12 @@ describe('ShareModalCtrl', function() { }); it('should generate render url', function() { - ctx.$location.$$absUrl = 'http://dashboards.grafana.com/dashboard/db/my-dash'; + ctx.$location.$$absUrl = 'http://dashboards.grafana.com/d/abcdefghi/my-dash'; ctx.scope.panel = { id: 22 }; ctx.scope.init(); - var base = 'http://dashboards.grafana.com/render/dashboard-solo/db/my-dash'; + var base = 'http://dashboards.grafana.com/render/d-solo/abcdefghi/my-dash'; var params = '?from=1000&to=2000&orgId=1&panelId=22&width=1000&height=500&tz=UTC'; expect(ctx.scope.imageUrl).to.contain(base + params); }); diff --git a/public/app/features/panel/solo_panel_ctrl.ts b/public/app/features/panel/solo_panel_ctrl.ts index f141f89eb80..d8642bea4a0 100644 --- a/public/app/features/panel/solo_panel_ctrl.ts +++ b/public/app/features/panel/solo_panel_ctrl.ts @@ -2,7 +2,7 @@ import angular from 'angular'; export class SoloPanelCtrl { /** @ngInject */ - constructor($scope, $routeParams, $location, dashboardLoaderSrv, contextSrv) { + constructor($scope, $routeParams, $location, dashboardLoaderSrv, contextSrv, backendSrv) { var panelId; $scope.init = function() { @@ -13,7 +13,17 @@ export class SoloPanelCtrl { $scope.onAppEvent('dashboard-initialized', $scope.initPanelScope); - dashboardLoaderSrv.loadDashboard($routeParams.type, $routeParams.slug).then(function(result) { + // if no uid, redirect to new route based on slug + if (!$routeParams.uid) { + backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { + if (res) { + $location.path(res.meta.url.replace('/d/', '/d-solo/')); + } + }); + return; + } + + dashboardLoaderSrv.loadDashboard($routeParams.type, $routeParams.slug, $routeParams.uid).then(function(result) { result.meta.soloMode = true; $scope.initDashboard(result, $scope); }); diff --git a/public/app/routes/routes.ts b/public/app/routes/routes.ts index 081fb88af6e..865f4ce1682 100644 --- a/public/app/routes/routes.ts +++ b/public/app/routes/routes.ts @@ -26,6 +26,12 @@ export function setupAngularRoutes($routeProvider, $locationProvider) { reloadOnSearch: false, pageClass: 'page-dashboard', }) + .when('/d-solo/:uid/:slug', { + templateUrl: 'public/app/features/panel/partials/soloPanel.html', + controller: 'SoloPanelCtrl', + reloadOnSearch: false, + pageClass: 'page-dashboard', + }) .when('/dashboard-solo/:type/:slug', { templateUrl: 'public/app/features/panel/partials/soloPanel.html', controller: 'SoloPanelCtrl', From a99331cdb94c12d43d7d385a35945c8163294e61 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 14:05:24 +0100 Subject: [PATCH 23/44] dashboards: redirect from old url used to load dashboard to new url If legacy backend routes (/dashboard/db/ and /dashboard-solo/db/) are requested we try to redirect to new routes with a 301 Moved Permanently #7883 --- pkg/api/api.go | 8 +++- pkg/middleware/dashboard_redirect.go | 46 +++++++++++++++++++ pkg/middleware/dashboard_redirect_test.go | 54 +++++++++++++++++++++++ pkg/middleware/middleware_test.go | 14 ++++++ 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 pkg/middleware/dashboard_redirect.go create mode 100644 pkg/middleware/dashboard_redirect_test.go diff --git a/pkg/api/api.go b/pkg/api/api.go index 2d45868e58d..18e18bf5d54 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -15,6 +15,8 @@ func (hs *HttpServer) registerRoutes() { reqGrafanaAdmin := middleware.Auth(&middleware.AuthOptions{ReqSignedIn: true, ReqGrafanaAdmin: true}) reqEditorRole := middleware.RoleAuth(m.ROLE_EDITOR, m.ROLE_ADMIN) reqOrgAdmin := middleware.RoleAuth(m.ROLE_ADMIN) + redirectFromLegacyDashboardUrl := middleware.RedirectFromLegacyDashboardUrl() + redirectFromLegacyDashboardSoloUrl := middleware.RedirectFromLegacyDashboardSoloUrl() quota := middleware.Quota bind := binding.Bind @@ -63,9 +65,11 @@ func (hs *HttpServer) registerRoutes() { r.Get("/plugins/:id/edit", reqSignedIn, Index) r.Get("/plugins/:id/page/:page", reqSignedIn, Index) - r.Get("/dashboard/*", reqSignedIn, Index) + r.Get("/d/:uid/:slug", reqSignedIn, Index) + r.Get("/dashboard/db/:slug", reqSignedIn, redirectFromLegacyDashboardUrl, Index) r.Get("/dashboard-solo/snapshot/*", Index) - r.Get("/dashboard-solo/*", reqSignedIn, Index) + r.Get("/d-solo/:uid/:slug", reqSignedIn, Index) + r.Get("/dashboard-solo/db/:slug", reqSignedIn, redirectFromLegacyDashboardSoloUrl, Index) r.Get("/import/dashboard", reqSignedIn, Index) r.Get("/dashboards/", reqSignedIn, Index) r.Get("/dashboards/*", reqSignedIn, Index) diff --git a/pkg/middleware/dashboard_redirect.go b/pkg/middleware/dashboard_redirect.go new file mode 100644 index 00000000000..1ca4ef741c6 --- /dev/null +++ b/pkg/middleware/dashboard_redirect.go @@ -0,0 +1,46 @@ +package middleware + +import ( + "strings" + + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" + "gopkg.in/macaron.v1" +) + +func getDashboardUrlBySlug(orgId int64, slug string) (string, error) { + query := m.GetDashboardQuery{Slug: slug, OrgId: orgId} + + if err := bus.Dispatch(&query); err != nil { + return "", m.ErrDashboardNotFound + } + + return m.GetDashboardUrl(query.Result.Uid, query.Result.Slug), nil +} + +func RedirectFromLegacyDashboardUrl() macaron.Handler { + return func(c *Context) { + slug := c.Params("slug") + + if slug != "" { + if url, err := getDashboardUrlBySlug(c.OrgId, slug); err == nil { + c.Redirect(url, 301) + return + } + } + } +} + +func RedirectFromLegacyDashboardSoloUrl() macaron.Handler { + return func(c *Context) { + slug := c.Params("slug") + + if slug != "" { + if url, err := getDashboardUrlBySlug(c.OrgId, slug); err == nil { + url = strings.Replace(url, "/d/", "/d-solo/", 1) + c.Redirect(url, 301) + return + } + } + } +} diff --git a/pkg/middleware/dashboard_redirect_test.go b/pkg/middleware/dashboard_redirect_test.go new file mode 100644 index 00000000000..bff4ee2253c --- /dev/null +++ b/pkg/middleware/dashboard_redirect_test.go @@ -0,0 +1,54 @@ +package middleware + +import ( + "strings" + "testing" + + "github.com/grafana/grafana/pkg/bus" + m "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" +) + +func TestMiddlewareDashboardRedirect(t *testing.T) { + Convey("Given the dashboard redirect middleware", t, func() { + bus.ClearBusHandlers() + redirectFromLegacyDashboardUrl := RedirectFromLegacyDashboardUrl() + redirectFromLegacyDashboardSoloUrl := RedirectFromLegacyDashboardSoloUrl() + + fakeDash := m.NewDashboard("Child dash") + fakeDash.Id = 1 + fakeDash.FolderId = 1 + fakeDash.HasAcl = false + + bus.AddHandler("test", func(query *m.GetDashboardQuery) error { + query.Result = fakeDash + return nil + }) + + middlewareScenario("GET dashboard by legacy url", func(sc *scenarioContext) { + sc.m.Get("/dashboard/db/:slug", redirectFromLegacyDashboardUrl, sc.defaultHandler) + + sc.fakeReqWithParams("GET", "/dashboard/db/dash", map[string]string{}).exec() + + Convey("Should redirect to new dashboard url with a 301 Moved Permanently", func() { + So(sc.resp.Code, ShouldEqual, 301) + redirectUrl, _ := sc.resp.Result().Location() + So(redirectUrl.Path, ShouldEqual, m.GetDashboardUrl(fakeDash.Uid, fakeDash.Slug)) + }) + }) + + middlewareScenario("GET dashboard solo by legacy url", func(sc *scenarioContext) { + sc.m.Get("/dashboard-solo/db/:slug", redirectFromLegacyDashboardSoloUrl, sc.defaultHandler) + + sc.fakeReqWithParams("GET", "/dashboard-solo/db/dash", map[string]string{}).exec() + + Convey("Should redirect to new dashboard url with a 301 Moved Permanently", func() { + So(sc.resp.Code, ShouldEqual, 301) + redirectUrl, _ := sc.resp.Result().Location() + expectedUrl := m.GetDashboardUrl(fakeDash.Uid, fakeDash.Slug) + expectedUrl = strings.Replace(expectedUrl, "/d/", "/d-solo/", 1) + So(redirectUrl.Path, ShouldEqual, expectedUrl) + }) + }) + }) +} diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 0d9e0e5b973..ffd8e8a0af0 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -399,6 +399,20 @@ func (sc *scenarioContext) fakeReq(method, url string) *scenarioContext { return sc } +func (sc *scenarioContext) fakeReqWithParams(method, url string, queryParams map[string]string) *scenarioContext { + sc.resp = httptest.NewRecorder() + req, err := http.NewRequest(method, url, nil) + q := req.URL.Query() + for k, v := range queryParams { + q.Add(k, v) + } + req.URL.RawQuery = q.Encode() + So(err, ShouldBeNil) + sc.req = req + + return sc +} + func (sc *scenarioContext) handler(fn handlerFunc) *scenarioContext { sc.handlerFunc = fn return sc From 57edf890338e93f70f8c2fe3176d7702d7c90f1c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 14:07:49 +0100 Subject: [PATCH 24/44] dashboards: make scripted dashboards work using the old legacy urls Scripted dashboards are still requested from /dashboard/script/scripted.js #7883 --- pkg/api/api.go | 2 ++ public/app/features/dashboard/shareModalCtrl.ts | 9 +++++++-- .../dashboard/specs/share_modal_ctrl_specs.ts | 11 +++++++++++ public/app/features/panel/solo_panel_ctrl.ts | 2 +- public/app/routes/dashboard_loaders.ts | 2 +- 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 18e18bf5d54..6d2207971e7 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -67,9 +67,11 @@ func (hs *HttpServer) registerRoutes() { r.Get("/d/:uid/:slug", reqSignedIn, Index) r.Get("/dashboard/db/:slug", reqSignedIn, redirectFromLegacyDashboardUrl, Index) + r.Get("/dashboard/script/*", reqSignedIn, Index) r.Get("/dashboard-solo/snapshot/*", Index) r.Get("/d-solo/:uid/:slug", reqSignedIn, Index) r.Get("/dashboard-solo/db/:slug", reqSignedIn, redirectFromLegacyDashboardSoloUrl, Index) + r.Get("/dashboard-solo/script/*", reqSignedIn, Index) r.Get("/import/dashboard", reqSignedIn, Index) r.Get("/dashboards/", reqSignedIn, Index) r.Get("/dashboards/*", reqSignedIn, Index) diff --git a/public/app/features/dashboard/shareModalCtrl.ts b/public/app/features/dashboard/shareModalCtrl.ts index 2bde5c2d198..8e30eaef91e 100644 --- a/public/app/features/dashboard/shareModalCtrl.ts +++ b/public/app/features/dashboard/shareModalCtrl.ts @@ -73,14 +73,19 @@ export class ShareModalCtrl { $scope.shareUrl = linkSrv.addParamsToUrl(baseUrl, params); - var soloUrl = baseUrl.replace(config.appSubUrl + '/d/', config.appSubUrl + '/d-solo/'); + var soloUrl = baseUrl.replace(config.appSubUrl + '/dashboard/', config.appSubUrl + '/dashboard-solo/'); + soloUrl = soloUrl.replace(config.appSubUrl + '/d/', config.appSubUrl + '/d-solo/'); delete params.fullscreen; delete params.edit; soloUrl = linkSrv.addParamsToUrl(soloUrl, params); $scope.iframeHtml = ''; - $scope.imageUrl = soloUrl.replace(config.appSubUrl + '/d-solo/', config.appSubUrl + '/render/d-solo/'); + $scope.imageUrl = soloUrl.replace( + config.appSubUrl + '/dashboard-solo/', + config.appSubUrl + '/render/dashboard-solo/' + ); + $scope.imageUrl = $scope.imageUrl.replace(config.appSubUrl + '/d-solo/', config.appSubUrl + '/render/d-solo/'); $scope.imageUrl += '&width=1000'; $scope.imageUrl += '&height=500'; $scope.imageUrl += '&tz=UTC' + encodeURIComponent(moment().format('Z')); diff --git a/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts b/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts index e65d9c98f82..fc70a54a41c 100644 --- a/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts +++ b/public/app/features/dashboard/specs/share_modal_ctrl_specs.ts @@ -53,6 +53,17 @@ describe('ShareModalCtrl', function() { expect(ctx.scope.imageUrl).to.contain(base + params); }); + it('should generate render url for scripted dashboard', function() { + ctx.$location.$$absUrl = 'http://dashboards.grafana.com/dashboard/script/my-dash.js'; + + ctx.scope.panel = { id: 22 }; + + ctx.scope.init(); + var base = 'http://dashboards.grafana.com/render/dashboard-solo/script/my-dash.js'; + var params = '?from=1000&to=2000&orgId=1&panelId=22&width=1000&height=500&tz=UTC'; + expect(ctx.scope.imageUrl).to.contain(base + params); + }); + it('should remove panel id when no panel in scope', function() { ctx.$location.path('/test'); ctx.scope.options.forCurrent = true; diff --git a/public/app/features/panel/solo_panel_ctrl.ts b/public/app/features/panel/solo_panel_ctrl.ts index d8642bea4a0..575c9e4c3b9 100644 --- a/public/app/features/panel/solo_panel_ctrl.ts +++ b/public/app/features/panel/solo_panel_ctrl.ts @@ -14,7 +14,7 @@ export class SoloPanelCtrl { $scope.onAppEvent('dashboard-initialized', $scope.initPanelScope); // if no uid, redirect to new route based on slug - if (!$routeParams.uid) { + if (!($routeParams.type === 'script' || $routeParams.type === 'snapshot') && !$routeParams.uid) { backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { if (res) { $location.path(res.meta.url.replace('/d/', '/d-solo/')); diff --git a/public/app/routes/dashboard_loaders.ts b/public/app/routes/dashboard_loaders.ts index e3e6f4c18bc..971c83f0414 100644 --- a/public/app/routes/dashboard_loaders.ts +++ b/public/app/routes/dashboard_loaders.ts @@ -19,7 +19,7 @@ export class LoadDashboardCtrl { } // if no uid, redirect to new route based on slug - if (!$routeParams.uid) { + if (!($routeParams.type === 'script' || $routeParams.type === 'snapshot') && !$routeParams.uid) { backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { if (res) { $location.path(res.meta.url); From 7e9605259407d8db483c5482c96c3d65b07e8e38 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 31 Jan 2018 13:47:28 +0100 Subject: [PATCH 25/44] ensure dashboard title is unique in folder --- pkg/models/dashboards.go | 17 +- pkg/services/sqlstore/dashboard.go | 24 ++ .../sqlstore/dashboard_folder_test.go | 219 +++++++++++++ pkg/services/sqlstore/dashboard_test.go | 287 +++++------------- 4 files changed, 336 insertions(+), 211 deletions(-) create mode 100644 pkg/services/sqlstore/dashboard_folder_test.go diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index cf7f6355537..c4361eefd7d 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -14,14 +14,15 @@ import ( // Typed errors var ( - ErrDashboardNotFound = errors.New("Dashboard not found") - ErrDashboardSnapshotNotFound = errors.New("Dashboard snapshot not found") - ErrDashboardWithSameUIDExists = errors.New("A dashboard with the same uid already exists") - ErrDashboardVersionMismatch = errors.New("The dashboard has been changed by someone else") - ErrDashboardTitleEmpty = errors.New("Dashboard title cannot be empty") - ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") - ErrDashboardContainsInvalidAlertData = errors.New("Invalid alert data. Cannot save dashboard") - ErrDashboardFailedToUpdateAlertData = errors.New("Failed to save alert data") + ErrDashboardNotFound = errors.New("Dashboard not found") + ErrDashboardSnapshotNotFound = errors.New("Dashboard snapshot not found") + ErrDashboardWithSameUIDExists = errors.New("A dashboard with the same uid already exists") + ErrDashboardWithSameNameInFolderExists = errors.New("A dashboard with the same name in the folder already exists") + ErrDashboardVersionMismatch = errors.New("The dashboard has been changed by someone else") + ErrDashboardTitleEmpty = errors.New("Dashboard title cannot be empty") + ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") + ErrDashboardContainsInvalidAlertData = errors.New("Invalid alert data. Cannot save dashboard") + ErrDashboardFailedToUpdateAlertData = errors.New("Failed to save alert data") ) type UpdatePluginDashboardError struct { diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 376e4e3c382..c378d70af10 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -20,6 +20,8 @@ func init() { bus.AddHandler("sql", GetDashboardsByPluginId) } + + func SaveDashboard(cmd *m.SaveDashboardCommand) error { return inTransaction(func(sess *DBSession) error { dash := cmd.GetDashboardModel() @@ -69,6 +71,11 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { } } + err = guaranteeDashboardNameIsUniqueInFolder(sess, dash) + if err != nil { + return err + } + err = setHasAcl(sess, dash) if err != nil { return err @@ -140,6 +147,23 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { }) } +func guaranteeDashboardNameIsUniqueInFolder(sess *DBSession, dash *m.Dashboard) error { + var sameNameInFolder m.Dashboard + sameNameInFolderExist, err := sess.Where("org_id=? AND title=? AND folder_id = ? AND uid <> ?", + dash.OrgId, dash.Title, dash.FolderId, dash.Uid). + Get(&sameNameInFolder) + + if err != nil { + return err + } + + if sameNameInFolderExist { + return m.ErrDashboardWithSameNameInFolderExists + } + + return nil +} + func setHasAcl(sess *DBSession, dash *m.Dashboard) error { // check if parent has acl if dash.FolderId > 0 { diff --git a/pkg/services/sqlstore/dashboard_folder_test.go b/pkg/services/sqlstore/dashboard_folder_test.go new file mode 100644 index 00000000000..6f880e7bb4c --- /dev/null +++ b/pkg/services/sqlstore/dashboard_folder_test.go @@ -0,0 +1,219 @@ +package sqlstore + +import ( + "testing" + + "github.com/go-xorm/xorm" + . "github.com/smartystreets/goconvey/convey" + + m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/search" +) + +func TestDashboardFolderDataAccess(t *testing.T) { + var x *xorm.Engine + + Convey("Testing DB", t, func() { + x = InitTestDB(t) + + Convey("Given one dashboard folder with two dashboards and one dashboard in the root folder", func() { + folder := insertTestDashboard("1 test dash folder", 1, 0, true, "prod", "webapp") + dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod", "webapp") + childDash := insertTestDashboard("test dash 23", 1, folder.Id, false, "prod", "webapp") + insertTestDashboard("test dash 45", 1, folder.Id, false, "prod") + + currentUser := createUser("viewer", "Viewer", false) + + Convey("and no acls are set", func() { + Convey("should return all dashboards", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("and acl is set for dashboard folder", func() { + var otherUser int64 = 999 + updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) + + Convey("should not return folder", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 1) + So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) + }) + + Convey("when the user is given permission", func() { + updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT) + + Convey("should be able to access folder", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("when the user is an admin", func() { + Convey("should be able to access folder", func() { + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{ + UserId: currentUser.Id, + OrgId: 1, + OrgRole: m.ROLE_ADMIN, + }, + OrgId: 1, + DashboardIds: []int64{folder.Id, dashInRoot.Id}, + } + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + }) + + Convey("and acl is set for dashboard child and folder has all permissions removed", func() { + var otherUser int64 = 999 + aclId := updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) + removeAcl(aclId) + updateTestDashboardWithAcl(childDash.Id, otherUser, m.PERMISSION_EDIT) + + Convey("should not return folder or child", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 1) + So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) + }) + + Convey("when the user is given permission to child", func() { + updateTestDashboardWithAcl(childDash.Id, currentUser.Id, m.PERMISSION_EDIT) + + Convey("should be able to search for child dashboard but not folder", func() { + query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 2) + So(query.Result[0].Id, ShouldEqual, childDash.Id) + So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("when the user is an admin", func() { + Convey("should be able to search for child dash and folder", func() { + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{ + UserId: currentUser.Id, + OrgId: 1, + OrgRole: m.ROLE_ADMIN, + }, + OrgId: 1, + DashboardIds: []int64{folder.Id, dashInRoot.Id, childDash.Id}, + } + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 3) + So(query.Result[0].Id, ShouldEqual, folder.Id) + So(query.Result[1].Id, ShouldEqual, childDash.Id) + So(query.Result[2].Id, ShouldEqual, dashInRoot.Id) + }) + }) + }) + }) + + Convey("Given two dashboard folders with one dashboard each and one dashboard in the root folder", func() { + folder1 := insertTestDashboard("1 test dash folder", 1, 0, true, "prod") + folder2 := insertTestDashboard("2 test dash folder", 1, 0, true, "prod") + dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod") + childDash1 := insertTestDashboard("child dash 1", 1, folder1.Id, false, "prod") + childDash2 := insertTestDashboard("child dash 2", 1, folder2.Id, false, "prod") + + currentUser := createUser("viewer", "Viewer", false) + var rootFolderId int64 = 0 + + Convey("and one folder is expanded, the other collapsed", func() { + Convey("should return dashboards in root and expanded folder", func() { + query := &search.FindPersistedDashboardsQuery{FolderIds: []int64{rootFolderId, folder1.Id}, SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1} + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 4) + So(query.Result[0].Id, ShouldEqual, folder1.Id) + So(query.Result[1].Id, ShouldEqual, folder2.Id) + So(query.Result[2].Id, ShouldEqual, childDash1.Id) + So(query.Result[3].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("and acl is set for one dashboard folder", func() { + var otherUser int64 = 999 + updateTestDashboardWithAcl(folder1.Id, otherUser, m.PERMISSION_EDIT) + + Convey("and a dashboard is moved from folder without acl to the folder with an acl", func() { + movedDash := moveDashboard(1, childDash2.Data, folder1.Id) + So(movedDash.HasAcl, ShouldBeTrue) + + Convey("should not return folder with acl or its children", func() { + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, + OrgId: 1, + DashboardIds: []int64{folder1.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, + } + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 1) + So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("and a dashboard is moved from folder with acl to the folder without an acl", func() { + movedDash := moveDashboard(1, childDash1.Data, folder2.Id) + So(movedDash.HasAcl, ShouldBeFalse) + + Convey("should return folder without acl and its children", func() { + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, + OrgId: 1, + DashboardIds: []int64{folder2.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, + } + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 4) + So(query.Result[0].Id, ShouldEqual, folder2.Id) + So(query.Result[1].Id, ShouldEqual, childDash1.Id) + So(query.Result[2].Id, ShouldEqual, childDash2.Id) + So(query.Result[3].Id, ShouldEqual, dashInRoot.Id) + }) + }) + + Convey("and a dashboard with an acl is moved to the folder without an acl", func() { + updateTestDashboardWithAcl(childDash1.Id, otherUser, m.PERMISSION_EDIT) + movedDash := moveDashboard(1, childDash1.Data, folder2.Id) + So(movedDash.HasAcl, ShouldBeTrue) + + Convey("should return folder without acl but not the dashboard with acl", func() { + query := &search.FindPersistedDashboardsQuery{ + SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, + OrgId: 1, + DashboardIds: []int64{folder2.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, + } + err := SearchDashboards(query) + So(err, ShouldBeNil) + So(len(query.Result), ShouldEqual, 3) + So(query.Result[0].Id, ShouldEqual, folder2.Id) + So(query.Result[1].Id, ShouldEqual, childDash2.Id) + So(query.Result[2].Id, ShouldEqual, dashInRoot.Id) + }) + }) + }) + }) + + }) +} diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index e8fb5332d32..6e77b44f8c1 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -196,18 +196,64 @@ func TestDashboardDataAccess(t *testing.T) { }) }) - Convey("Should not be able to save dashboard with same name", func() { - cmd := m.SaveDashboardCommand{ + Convey("Should be able to save dashboards with same name in different folders", func() { + firstSaveCmd := m.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ "id": nil, - "title": "test dash 23", + "title": "test dash folder and title", "tags": []interface{}{}, + "uid": "randomHash", }), + FolderId: 3, } - err := SaveDashboard(&cmd) + err := SaveDashboard(&firstSaveCmd) So(err, ShouldBeNil) + + secondSaveCmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": nil, + "title": "test dash folder and title", + "tags": []interface{}{}, + "uid": "moreRandomHash", + }), + FolderId: 1, + } + + err = SaveDashboard(&secondSaveCmd) + So(err, ShouldBeNil) + }) + + Convey("Should not be able to save dashboard with same name in the same folder", func() { + firstSaveCmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": nil, + "title": "test dash folder and title", + "tags": []interface{}{}, + "uid": "randomHash", + }), + FolderId: 3, + } + + err := SaveDashboard(&firstSaveCmd) + So(err, ShouldBeNil) + + secondSaveCmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": nil, + "title": "test dash folder and title", + "tags": []interface{}{}, + "uid": "moreRandomHash", + }), + FolderId: 3, + } + + err = SaveDashboard(&secondSaveCmd) + So(err, ShouldEqual, m.ErrDashboardWithSameNameInFolderExists) }) Convey("Should not be able to save dashboard with same uid", func() { @@ -226,6 +272,40 @@ func TestDashboardDataAccess(t *testing.T) { So(err, ShouldNotBeNil) }) + Convey("Should be able to update dashboard with the same title and folder id", func() { + cmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + //"id": 1, + "uid": "randomHash", + "title": "folderId", + "style": "light", + "tags": []interface{}{}, + }), + FolderId: 2, + } + + err := SaveDashboard(&cmd) + So(err, ShouldBeNil) + So(cmd.Result.FolderId, ShouldEqual, 2) + + cmd = m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "id": cmd.Result.Id, + "uid": "randomHash", + "title": "folderId", + "style": "dark", + "version": cmd.Result.Version, + "tags": []interface{}{}, + }), + FolderId: 2, + } + + err = SaveDashboard(&cmd) + So(err, ShouldBeNil) + }) + Convey("Should be able to update dashboard and remove folderId", func() { cmd := m.SaveDashboardCommand{ OrgId: 1, @@ -315,205 +395,6 @@ func TestDashboardDataAccess(t *testing.T) { }) }) - Convey("Given one dashboard folder with two dashboards and one dashboard in the root folder", func() { - folder := insertTestDashboard("1 test dash folder", 1, 0, true, "prod", "webapp") - dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod", "webapp") - childDash := insertTestDashboard("test dash 23", 1, folder.Id, false, "prod", "webapp") - insertTestDashboard("test dash 45", 1, folder.Id, false, "prod") - - currentUser := createUser("viewer", "Viewer", false) - - Convey("and no acls are set", func() { - Convey("should return all dashboards", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 2) - So(query.Result[0].Id, ShouldEqual, folder.Id) - So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) - }) - }) - - Convey("and acl is set for dashboard folder", func() { - var otherUser int64 = 999 - updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) - - Convey("should not return folder", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 1) - So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) - }) - - Convey("when the user is given permission", func() { - updateTestDashboardWithAcl(folder.Id, currentUser.Id, m.PERMISSION_EDIT) - - Convey("should be able to access folder", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, dashInRoot.Id}} - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 2) - So(query.Result[0].Id, ShouldEqual, folder.Id) - So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) - }) - }) - - Convey("when the user is an admin", func() { - Convey("should be able to access folder", func() { - query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{ - UserId: currentUser.Id, - OrgId: 1, - OrgRole: m.ROLE_ADMIN, - }, - OrgId: 1, - DashboardIds: []int64{folder.Id, dashInRoot.Id}, - } - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 2) - So(query.Result[0].Id, ShouldEqual, folder.Id) - So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) - }) - }) - }) - - Convey("and acl is set for dashboard child and folder has all permissions removed", func() { - var otherUser int64 = 999 - aclId := updateTestDashboardWithAcl(folder.Id, otherUser, m.PERMISSION_EDIT) - removeAcl(aclId) - updateTestDashboardWithAcl(childDash.Id, otherUser, m.PERMISSION_EDIT) - - Convey("should not return folder or child", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 1) - So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) - }) - - Convey("when the user is given permission to child", func() { - updateTestDashboardWithAcl(childDash.Id, currentUser.Id, m.PERMISSION_EDIT) - - Convey("should be able to search for child dashboard but not folder", func() { - query := &search.FindPersistedDashboardsQuery{SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1, DashboardIds: []int64{folder.Id, childDash.Id, dashInRoot.Id}} - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 2) - So(query.Result[0].Id, ShouldEqual, childDash.Id) - So(query.Result[1].Id, ShouldEqual, dashInRoot.Id) - }) - }) - - Convey("when the user is an admin", func() { - Convey("should be able to search for child dash and folder", func() { - query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{ - UserId: currentUser.Id, - OrgId: 1, - OrgRole: m.ROLE_ADMIN, - }, - OrgId: 1, - DashboardIds: []int64{folder.Id, dashInRoot.Id, childDash.Id}, - } - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 3) - So(query.Result[0].Id, ShouldEqual, folder.Id) - So(query.Result[1].Id, ShouldEqual, childDash.Id) - So(query.Result[2].Id, ShouldEqual, dashInRoot.Id) - }) - }) - }) - }) - - Convey("Given two dashboard folders with one dashboard each and one dashboard in the root folder", func() { - folder1 := insertTestDashboard("1 test dash folder", 1, 0, true, "prod") - folder2 := insertTestDashboard("2 test dash folder", 1, 0, true, "prod") - dashInRoot := insertTestDashboard("test dash 67", 1, 0, false, "prod") - childDash1 := insertTestDashboard("child dash 1", 1, folder1.Id, false, "prod") - childDash2 := insertTestDashboard("child dash 2", 1, folder2.Id, false, "prod") - - currentUser := createUser("viewer", "Viewer", false) - var rootFolderId int64 = 0 - - Convey("and one folder is expanded, the other collapsed", func() { - Convey("should return dashboards in root and expanded folder", func() { - query := &search.FindPersistedDashboardsQuery{FolderIds: []int64{rootFolderId, folder1.Id}, SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, OrgId: 1} - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 4) - So(query.Result[0].Id, ShouldEqual, folder1.Id) - So(query.Result[1].Id, ShouldEqual, folder2.Id) - So(query.Result[2].Id, ShouldEqual, childDash1.Id) - So(query.Result[3].Id, ShouldEqual, dashInRoot.Id) - }) - }) - - Convey("and acl is set for one dashboard folder", func() { - var otherUser int64 = 999 - updateTestDashboardWithAcl(folder1.Id, otherUser, m.PERMISSION_EDIT) - - Convey("and a dashboard is moved from folder without acl to the folder with an acl", func() { - movedDash := moveDashboard(1, childDash2.Data, folder1.Id) - So(movedDash.HasAcl, ShouldBeTrue) - - Convey("should not return folder with acl or its children", func() { - query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, - OrgId: 1, - DashboardIds: []int64{folder1.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, - } - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 1) - So(query.Result[0].Id, ShouldEqual, dashInRoot.Id) - }) - }) - - Convey("and a dashboard is moved from folder with acl to the folder without an acl", func() { - movedDash := moveDashboard(1, childDash1.Data, folder2.Id) - So(movedDash.HasAcl, ShouldBeFalse) - - Convey("should return folder without acl and its children", func() { - query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, - OrgId: 1, - DashboardIds: []int64{folder2.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, - } - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 4) - So(query.Result[0].Id, ShouldEqual, folder2.Id) - So(query.Result[1].Id, ShouldEqual, childDash1.Id) - So(query.Result[2].Id, ShouldEqual, childDash2.Id) - So(query.Result[3].Id, ShouldEqual, dashInRoot.Id) - }) - }) - - Convey("and a dashboard with an acl is moved to the folder without an acl", func() { - updateTestDashboardWithAcl(childDash1.Id, otherUser, m.PERMISSION_EDIT) - movedDash := moveDashboard(1, childDash1.Data, folder2.Id) - So(movedDash.HasAcl, ShouldBeTrue) - - Convey("should return folder without acl but not the dashboard with acl", func() { - query := &search.FindPersistedDashboardsQuery{ - SignedInUser: &m.SignedInUser{UserId: currentUser.Id, OrgId: 1}, - OrgId: 1, - DashboardIds: []int64{folder2.Id, childDash1.Id, childDash2.Id, dashInRoot.Id}, - } - err := SearchDashboards(query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 3) - So(query.Result[0].Id, ShouldEqual, folder2.Id) - So(query.Result[1].Id, ShouldEqual, childDash2.Id) - So(query.Result[2].Id, ShouldEqual, dashInRoot.Id) - }) - }) - }) - }) - Convey("Given a plugin with imported dashboards", func() { pluginId := "test-app" From 3da2ab61e0ccf63adb6c602af9e9abfed2cee18b Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 31 Jan 2018 14:36:14 +0100 Subject: [PATCH 26/44] Verifies requirement of id in dashboards. --- pkg/services/sqlstore/dashboard_test.go | 34 ++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 6e77b44f8c1..007ac4a01d5 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -276,7 +276,6 @@ func TestDashboardDataAccess(t *testing.T) { cmd := m.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ - //"id": 1, "uid": "randomHash", "title": "folderId", "style": "light", @@ -306,6 +305,39 @@ func TestDashboardDataAccess(t *testing.T) { So(err, ShouldBeNil) }) + Convey("Should not be able to update using just uid", func() { + cmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDash.Uid, + "title": "folderId", + "version": savedDash.Version, + "tags": []interface{}{}, + }), + FolderId: savedDash.FolderId, + } + + err := SaveDashboard(&cmd) + So(err, ShouldEqual, m.ErrDashboardWithSameUIDExists) + }) + + Convey("Should be able to update using just uid with overwrite", func() { + cmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "uid": savedDash.Uid, + "title": "folderId", + "version": savedDash.Version, + "tags": []interface{}{}, + }), + FolderId: savedDash.FolderId, + Overwrite: true, + } + + err := SaveDashboard(&cmd) + So(err, ShouldBeNil) + }) + Convey("Should be able to update dashboard and remove folderId", func() { cmd := m.SaveDashboardCommand{ OrgId: 1, From 16a1642831a521e7a5a57a3fe298f742c7e873da Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 31 Jan 2018 15:15:15 +0100 Subject: [PATCH 27/44] gofmt... --- pkg/services/sqlstore/dashboard.go | 2 -- pkg/services/sqlstore/dashboard_test.go | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index c378d70af10..4c04fe1251a 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -20,8 +20,6 @@ func init() { bus.AddHandler("sql", GetDashboardsByPluginId) } - - func SaveDashboard(cmd *m.SaveDashboardCommand) error { return inTransaction(func(sess *DBSession) error { dash := cmd.GetDashboardModel() diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 007ac4a01d5..87010849a77 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -276,10 +276,10 @@ func TestDashboardDataAccess(t *testing.T) { cmd := m.SaveDashboardCommand{ OrgId: 1, Dashboard: simplejson.NewFromAny(map[string]interface{}{ - "uid": "randomHash", - "title": "folderId", - "style": "light", - "tags": []interface{}{}, + "uid": "randomHash", + "title": "folderId", + "style": "light", + "tags": []interface{}{}, }), FolderId: 2, } @@ -330,7 +330,7 @@ func TestDashboardDataAccess(t *testing.T) { "version": savedDash.Version, "tags": []interface{}{}, }), - FolderId: savedDash.FolderId, + FolderId: savedDash.FolderId, Overwrite: true, } From 75cf9ae27de2887b8e0149814463e0b9053e0a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 31 Jan 2018 17:26:35 +0100 Subject: [PATCH 28/44] fix: use replace when redirecting to new url --- public/app/routes/dashboard_loaders.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/routes/dashboard_loaders.ts b/public/app/routes/dashboard_loaders.ts index 971c83f0414..4ad77512ad1 100644 --- a/public/app/routes/dashboard_loaders.ts +++ b/public/app/routes/dashboard_loaders.ts @@ -22,7 +22,7 @@ export class LoadDashboardCtrl { if (!($routeParams.type === 'script' || $routeParams.type === 'snapshot') && !$routeParams.uid) { backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { if (res) { - $location.path(res.meta.url); + $location.path(res.meta.url).replace(); } }); return; From 58cfb236250e0d5b3bac72c1d261abcfb5572435 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 31 Jan 2018 17:27:28 +0100 Subject: [PATCH 29/44] retry uid generation --- pkg/middleware/dashboard_redirect_test.go | 2 + pkg/models/dashboards.go | 6 +-- pkg/services/sqlstore/dashboard.go | 62 ++++++++++++++++------- pkg/services/sqlstore/dashboard_test.go | 28 +++++++++- 4 files changed, 74 insertions(+), 24 deletions(-) diff --git a/pkg/middleware/dashboard_redirect_test.go b/pkg/middleware/dashboard_redirect_test.go index bff4ee2253c..21fc12e5e84 100644 --- a/pkg/middleware/dashboard_redirect_test.go +++ b/pkg/middleware/dashboard_redirect_test.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/bus" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/util" . "github.com/smartystreets/goconvey/convey" ) @@ -19,6 +20,7 @@ func TestMiddlewareDashboardRedirect(t *testing.T) { fakeDash.Id = 1 fakeDash.FolderId = 1 fakeDash.HasAcl = false + fakeDash.Uid = util.GenerateShortUid() bus.AddHandler("test", func(query *m.GetDashboardQuery) error { query.Result = fakeDash diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index c4361eefd7d..e06bc0d1f0d 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -9,7 +9,6 @@ import ( "github.com/gosimple/slug" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/util" ) // Typed errors @@ -23,6 +22,7 @@ var ( ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") ErrDashboardContainsInvalidAlertData = errors.New("Invalid alert data. Cannot save dashboard") ErrDashboardFailedToUpdateAlertData = errors.New("Failed to save alert data") + ErrDashboardFailedGenerateUniqueUid = errors.New("Failed to generate unique dashboard uid.") ) type UpdatePluginDashboardError struct { @@ -66,7 +66,6 @@ type Dashboard struct { // NewDashboard creates a new dashboard func NewDashboard(title string) *Dashboard { dash := &Dashboard{} - dash.Uid = util.GenerateShortUid() dash.Data = simplejson.New() dash.Data.Set("title", title) dash.Title = title @@ -115,9 +114,6 @@ func NewDashboardFromJson(data *simplejson.Json) *Dashboard { if uid, err := dash.Data.Get("uid").String(); err == nil { dash.Uid = uid - } else { - dash.Uid = util.GenerateShortUid() - dash.Data.Set("uid", dash.Uid) } return dash diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 4c04fe1251a..f93e7f402ae 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/metrics" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/search" + "github.com/grafana/grafana/pkg/util" ) func init() { @@ -20,6 +21,8 @@ func init() { bus.AddHandler("sql", GetDashboardsByPluginId) } +var generateNewUid func() string = util.GenerateShortUid + func SaveDashboard(cmd *m.SaveDashboardCommand) error { return inTransaction(func(sess *DBSession) error { dash := cmd.GetDashboardModel() @@ -27,7 +30,7 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { // try get existing dashboard var existing m.Dashboard - if dash.Id > 0 { + if dash.Id != 0 { dashWithIdExists, err := sess.Where("id=? AND org_id=?", dash.Id, dash.OrgId).Get(&existing) if err != nil { return err @@ -49,27 +52,36 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { if existing.PluginId != "" && cmd.Overwrite == false { return m.UpdatePluginDashboardError{PluginId: existing.PluginId} } - } + } else if dash.Uid != "" { + var sameUid m.Dashboard + sameUidExists, err := sess.Where("org_id=? AND uid=?", dash.OrgId, dash.Uid).Get(&sameUid) + if err != nil { + return err + } - var sameUid m.Dashboard - sameUidExists, err := sess.Where("org_id=? AND uid=?", dash.OrgId, dash.Uid).Get(&sameUid) - if err != nil { - return err - } - - if sameUidExists { - // another dashboard with same uid - if dash.Id != sameUid.Id { - if cmd.Overwrite { - dash.Id = sameUid.Id - dash.Version = sameUid.Version - } else { - return m.ErrDashboardWithSameUIDExists + if sameUidExists { + // another dashboard with same uid + if dash.Id != sameUid.Id { + if cmd.Overwrite { + dash.Id = sameUid.Id + dash.Version = sameUid.Version + } else { + return m.ErrDashboardWithSameUIDExists + } } } } - err = guaranteeDashboardNameIsUniqueInFolder(sess, dash) + if dash.Uid == "" { + uid, err := generateNewDashboardUid(sess, dash.OrgId) + if err != nil { + return err + } + dash.Uid = uid + dash.Data.Set("uid", uid) + } + + err := guaranteeDashboardNameIsUniqueInFolder(sess, dash) if err != nil { return err } @@ -144,6 +156,22 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { return err }) } +func generateNewDashboardUid(sess *DBSession, orgId int64) (string, error) { + for i := 0; i < 3; i++ { + uid := generateNewUid() + + exists, err := sess.Where("org_id=? AND uid=?", orgId, uid).Get(&m.Dashboard{}) + if err != nil { + return "", err + } + + if !exists { + return uid, nil + } + } + + return "", m.ErrDashboardFailedGenerateUniqueUid +} func guaranteeDashboardNameIsUniqueInFolder(sess *DBSession, dash *m.Dashboard) error { var sameNameInFolder m.Dashboard diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 87010849a77..6e240b9585e 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -5,12 +5,12 @@ import ( "testing" "github.com/go-xorm/xorm" - . "github.com/smartystreets/goconvey/convey" - "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/search" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util" + . "github.com/smartystreets/goconvey/convey" ) func TestDashboardDataAccess(t *testing.T) { @@ -338,6 +338,30 @@ func TestDashboardDataAccess(t *testing.T) { So(err, ShouldBeNil) }) + Convey("Should retry generation of uid once if it fails.", func() { + timesCalled := 0 + generateNewUid = func() string { + timesCalled += 1 + if timesCalled <= 2 { + return savedDash.Uid + } else { + return util.GenerateShortUid() + } + } + cmd := m.SaveDashboardCommand{ + OrgId: 1, + Dashboard: simplejson.NewFromAny(map[string]interface{}{ + "title": "new dash 12334", + "tags": []interface{}{}, + }), + } + + err := SaveDashboard(&cmd) + So(err, ShouldBeNil) + + generateNewUid = util.GenerateShortUid + }) + Convey("Should be able to update dashboard and remove folderId", func() { cmd := m.SaveDashboardCommand{ OrgId: 1, From 95d063621e29562b20d92589bb375363dd8c332d Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 16:46:31 +0100 Subject: [PATCH 30/44] dashboards: new route for deleting dashboards by uid --- pkg/api/api.go | 1 + pkg/api/dashboard.go | 20 ++++++++++ pkg/api/dashboard_test.go | 81 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) diff --git a/pkg/api/api.go b/pkg/api/api.go index 6d2207971e7..d10b459cd6a 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -249,6 +249,7 @@ func (hs *HttpServer) registerRoutes() { // Dashboard apiRoute.Group("/dashboards", func(dashboardRoute RouteRegister) { dashboardRoute.Get("/uid/:uid", wrap(GetDashboard)) + dashboardRoute.Delete("/uid/:uid", wrap(DeleteDashboardByUid)) dashboardRoute.Get("/db/:slug", wrap(GetDashboard)) dashboardRoute.Delete("/db/:slug", reqEditorRole, wrap(DeleteDashboard)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 7799ad868f9..ae8d819b981 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -165,6 +165,26 @@ func DeleteDashboard(c *middleware.Context) Response { return Json(200, resp) } +func DeleteDashboardByUid(c *middleware.Context) Response { + dash, rsp := getDashboardHelper(c.OrgId, "", 0, c.Params(":uid")) + if rsp != nil { + return rsp + } + + guardian := guardian.NewDashboardGuardian(dash.Id, c.OrgId, c.SignedInUser) + if canSave, err := guardian.CanSave(); err != nil || !canSave { + return dashboardGuardianResponse(err) + } + + cmd := m.DeleteDashboardCommand{OrgId: c.OrgId, Id: dash.Id} + if err := bus.Dispatch(&cmd); err != nil { + return ApiError(500, "Failed to delete dashboard", err) + } + + var resp = map[string]interface{}{"title": dash.Title} + return Json(200, resp) +} + func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response { cmd.OrgId = c.OrgId cmd.UserId = c.UserId diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 0bd586f6067..3d8f49b5f23 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -113,6 +113,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -169,6 +178,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 200) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -287,6 +305,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -341,6 +368,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -406,6 +442,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 200) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -470,6 +515,15 @@ func TestDashboardApiEndpoint(t *testing.T) { So(getDashboardQueries[0].Slug, ShouldEqual, "child-dash") }) }) + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) }) Convey("When user is an Org Viewer but has an admin permission", func() { @@ -521,6 +575,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 200) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 200) @@ -583,6 +646,15 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/uid/abcdefghi", "/api/dashboards/uid/:uid", role, func(sc *scenarioContext) { + CallDeleteDashboardByUid(sc) + So(sc.resp.Code, ShouldEqual, 403) + + Convey("Should lookup dashboard by uid", func() { + So(getDashboardQueries[0].Uid, ShouldEqual, "abcdefghi") + }) + }) + loggedInUserScenarioWithRole("When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { CallGetDashboardVersion(sc) So(sc.resp.Code, ShouldEqual, 403) @@ -643,6 +715,15 @@ func CallDeleteDashboard(sc *scenarioContext) { sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() } +func CallDeleteDashboardByUid(sc *scenarioContext) { + bus.AddHandler("test", func(cmd *m.DeleteDashboardCommand) error { + return nil + }) + + sc.handlerFunc = DeleteDashboardByUid + sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() +} + func CallPostDashboard(sc *scenarioContext) { bus.AddHandler("test", func(cmd *alerting.ValidateDashboardAlertsCommand) error { return nil From b23560ed5a33e76eda5292cff7b231a9ca3d98b3 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 16:51:06 +0100 Subject: [PATCH 31/44] dashboards: add validation to delete dashboard by slug Validates that there are only one folder/dashboard having that slug, otherwise returns 412 Precondition Failed --- pkg/api/dashboard.go | 10 +++++++ pkg/api/dashboard_test.go | 43 ++++++++++++++++++++++++++++++ pkg/models/dashboards.go | 10 ++++++- pkg/services/sqlstore/dashboard.go | 11 ++++++++ 4 files changed, 73 insertions(+), 1 deletion(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index ae8d819b981..7b3977df0d8 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -146,6 +146,16 @@ func getDashboardHelper(orgId int64, slug string, id int64, uid string) (*m.Dash } func DeleteDashboard(c *middleware.Context) Response { + query := m.GetDashboardsBySlugQuery{OrgId: c.OrgId, Slug: c.Params(":slug")} + + if err := bus.Dispatch(&query); err != nil { + return ApiError(500, "Failed to retrieve dashboards by slug", err) + } + + if len(query.Result) > 1 { + return Json(412, util.DynMap{"status": "multiple-slugs-exists", "message": m.ErrDashboardsWithSameSlugExists.Error()}) + } + dash, rsp := getDashboardHelper(c.OrgId, c.Params(":slug"), 0, "") if rsp != nil { return rsp diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 3d8f49b5f23..5ff9efc84e6 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -39,6 +39,12 @@ func TestDashboardApiEndpoint(t *testing.T) { fakeDash.FolderId = 1 fakeDash.HasAcl = false + bus.AddHandler("test", func(query *m.GetDashboardsBySlugQuery) error { + dashboards := []*m.Dashboard{fakeDash} + query.Result = dashboards + return nil + }) + var getDashboardQueries []*m.GetDashboardQuery bus.AddHandler("test", func(query *m.GetDashboardQuery) error { @@ -232,6 +238,12 @@ func TestDashboardApiEndpoint(t *testing.T) { fakeDash.HasAcl = true setting.ViewersCanEdit = false + bus.AddHandler("test", func(query *m.GetDashboardsBySlugQuery) error { + dashboards := []*m.Dashboard{fakeDash} + query.Result = dashboards + return nil + }) + aclMockResp := []*m.DashboardAclInfoDTO{ { DashboardId: 1, @@ -671,6 +683,37 @@ func TestDashboardApiEndpoint(t *testing.T) { }) }) }) + + Convey("Given two dashboards with the same title in different folders", t, func() { + dashOne := m.NewDashboard("dash") + dashOne.Id = 2 + dashOne.FolderId = 1 + dashOne.HasAcl = false + + dashTwo := m.NewDashboard("dash") + dashTwo.Id = 4 + dashTwo.FolderId = 3 + dashTwo.HasAcl = false + + bus.AddHandler("test", func(query *m.GetDashboardsBySlugQuery) error { + dashboards := []*m.Dashboard{dashOne, dashTwo} + query.Result = dashboards + return nil + }) + + role := m.ROLE_EDITOR + + loggedInUserScenarioWithRole("When calling DELETE on", "DELETE", "/api/dashboards/db/dash", "/api/dashboards/db/:slug", role, func(sc *scenarioContext) { + CallDeleteDashboard(sc) + + Convey("Should result in 412 Precondition failed", func() { + So(sc.resp.Code, ShouldEqual, 412) + result := sc.ToJson() + So(result.Get("status").MustString(), ShouldEqual, "multiple-slugs-exists") + So(result.Get("message").MustString(), ShouldEqual, m.ErrDashboardsWithSameSlugExists.Error()) + }) + }) + }) } func GetDashboardShouldReturn200(sc *scenarioContext) dtos.DashboardFullWithMeta { diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 84ede04f226..dc296d2b3bd 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -22,6 +22,7 @@ var ( ErrDashboardFolderCannotHaveParent = errors.New("A Dashboard Folder cannot be added to another folder") ErrDashboardContainsInvalidAlertData = errors.New("Invalid alert data. Cannot save dashboard") ErrDashboardFailedToUpdateAlertData = errors.New("Failed to save alert data") + ErrDashboardsWithSameSlugExists = errors.New("Multiple dashboards with the same slug exists") ) type UpdatePluginDashboardError struct { @@ -165,7 +166,7 @@ func GetDashboardUrl(uid string, slug string) string { // GetFolderUrl return the html url for a folder func GetFolderUrl(folderUid string, slug string) string { - return fmt.Sprintf("%s/f/%v/%s", setting.AppSubUrl, folderUid, slug) + return fmt.Sprintf("%s/dashboards/f/%s/%s", setting.AppSubUrl, folderUid, slug) } // @@ -231,3 +232,10 @@ type GetDashboardSlugByIdQuery struct { Id int64 Result string } + +type GetDashboardsBySlugQuery struct { + OrgId int64 + Slug string + + Result []*Dashboard +} diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 84cfd1999c7..43e9d03e649 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -386,3 +386,14 @@ func GetDashboardSlugById(query *m.GetDashboardSlugByIdQuery) error { query.Result = slug.Slug return nil } + +func GetDashboardsBySlug(query *m.GetDashboardsBySlugQuery) error { + var dashboards = make([]*m.Dashboard, 0) + + if err := x.Where("org_id=? AND slug=?", query.OrgId, query.Slug).Find(&dashboards); err != nil { + return err + } + + query.Result = dashboards + return nil +} From 92a0171a9bf2ca42ebf7c3c2201a5199ee1e8060 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 17:15:00 +0100 Subject: [PATCH 32/44] folders: change the front end route for browsing folders Change the front end route for folders to /dashboards/f//. Use new route for deleting dashboard/folder by uid. Retrieve dashboard/folder by uid when moving or deleting dashboards/folders. --- pkg/services/search/models.go | 1 + pkg/services/sqlstore/dashboard.go | 1 + pkg/services/sqlstore/dashboard_test.go | 2 +- .../manage_dashboards/manage_dashboards.ts | 6 ++--- public/app/core/services/backend_srv.ts | 22 +++++++++---------- public/app/core/services/search_srv.ts | 6 +++-- .../app/core/specs/manage_dashboards.jest.ts | 16 +++++++------- .../features/dashboard/create_folder_ctrl.ts | 4 +--- .../dashboard/folder_dashboards_ctrl.ts | 9 ++++---- .../features/dashboard/folder_page_loader.ts | 13 +++++------ .../dashboard/folder_permissions_ctrl.ts | 7 +++--- .../dashboard/folder_settings_ctrl.ts | 16 +++++++------- .../features/dashboard/settings/settings.ts | 2 +- public/app/routes/routes.ts | 6 ++--- 14 files changed, 56 insertions(+), 55 deletions(-) diff --git a/pkg/services/search/models.go b/pkg/services/search/models.go index 6214a854db7..c543befc454 100644 --- a/pkg/services/search/models.go +++ b/pkg/services/search/models.go @@ -13,6 +13,7 @@ const ( type Hit struct { Id int64 `json:"id"` + Uid string `json:"uid"` Title string `json:"title"` Uri string `json:"uri"` Url string `json:"url"` diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 43e9d03e649..a23f07cd895 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -266,6 +266,7 @@ func makeQueryResult(query *search.FindPersistedDashboardsQuery, res []Dashboard } hit = &search.Hit{ Id: item.Id, + Uid: item.Uid, Title: item.Title, Uri: "db/" + item.Slug, Url: url, diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 19e82614718..55b27b1e56a 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -146,7 +146,7 @@ func TestDashboardDataAccess(t *testing.T) { So(len(query.Result), ShouldEqual, 1) hit := query.Result[0] So(hit.Type, ShouldEqual, search.DashHitFolder) - So(hit.Url, ShouldEqual, fmt.Sprintf("/f/%s/%s", savedFolder.Uid, savedFolder.Slug)) + So(hit.Url, ShouldEqual, fmt.Sprintf("/dashboards/f/%s/%s", savedFolder.Uid, savedFolder.Slug)) }) Convey("Should be able to search for a dashboard folder's children", func() { diff --git a/public/app/core/components/manage_dashboards/manage_dashboards.ts b/public/app/core/components/manage_dashboards/manage_dashboards.ts index 011e020f588..af87c86a671 100644 --- a/public/app/core/components/manage_dashboards/manage_dashboards.ts +++ b/public/app/core/components/manage_dashboards/manage_dashboards.ts @@ -91,10 +91,10 @@ export class ManageDashboardsCtrl { for (const section of this.sections) { if (section.checked && section.id !== 0) { - selectedDashboards.folders.push(section.slug); + selectedDashboards.folders.push(section.uid); } else { const selected = _.filter(section.items, { checked: true }); - selectedDashboards.dashboards.push(..._.map(selected, 'slug')); + selectedDashboards.dashboards.push(..._.map(selected, 'uid')); } } @@ -185,7 +185,7 @@ export class ManageDashboardsCtrl { for (const section of this.sections) { const selected = _.filter(section.items, { checked: true }); - selectedDashboards.push(..._.map(selected, 'slug')); + selectedDashboards.push(..._.map(selected, 'uid')); } return selectedDashboards; diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 9ade4264bb3..b6791267d4f 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -257,11 +257,11 @@ export class BackendSrv { }); } - deleteDashboard(slug) { + deleteDashboard(uid) { let deferred = this.$q.defer(); - this.getDashboard('db', slug).then(fullDash => { - this.delete(`/api/dashboards/db/${slug}`) + this.getDashboardByUid(uid).then(fullDash => { + this.delete(`/api/dashboards/uid/${uid}`) .then(() => { deferred.resolve(fullDash); }) @@ -273,21 +273,21 @@ export class BackendSrv { return deferred.promise; } - deleteDashboards(dashboardSlugs) { + deleteDashboards(dashboardUids) { const tasks = []; - for (let slug of dashboardSlugs) { - tasks.push(this.createTask(this.deleteDashboard.bind(this), true, slug)); + for (let uid of dashboardUids) { + tasks.push(this.createTask(this.deleteDashboard.bind(this), true, uid)); } return this.executeInOrder(tasks, []); } - moveDashboards(dashboardSlugs, toFolder) { + moveDashboards(dashboardUids, toFolder) { const tasks = []; - for (let slug of dashboardSlugs) { - tasks.push(this.createTask(this.moveDashboard.bind(this), true, slug, toFolder)); + for (let uid of dashboardUids) { + tasks.push(this.createTask(this.moveDashboard.bind(this), true, uid, toFolder)); } return this.executeInOrder(tasks, []).then(result => { @@ -299,10 +299,10 @@ export class BackendSrv { }); } - private moveDashboard(slug, toFolder) { + private moveDashboard(uid, toFolder) { let deferred = this.$q.defer(); - this.getDashboard('db', slug).then(fullDash => { + this.getDashboardByUid(uid).then(fullDash => { const model = new DashboardModel(fullDash.dashboard, fullDash.meta); if ((!fullDash.meta.folderId && toFolder.id === 0) || fullDash.meta.folderId === toFolder.id) { diff --git a/public/app/core/services/search_srv.ts b/public/app/core/services/search_srv.ts index a0989e74aed..9092b30fd4b 100644 --- a/public/app/core/services/search_srv.ts +++ b/public/app/core/services/search_srv.ts @@ -128,11 +128,12 @@ export class SearchSrv { if (hit.type === 'dash-folder') { sections[hit.id] = { id: hit.id, + uid: hit.uid, title: hit.title, expanded: false, items: [], toggle: this.toggleFolder.bind(this), - url: `dashboards/folder/${hit.id}/${hit.slug}`, + url: hit.url, slug: hit.slug, icon: 'fa fa-folder', score: _.keys(sections).length, @@ -150,8 +151,9 @@ export class SearchSrv { if (hit.folderId) { section = { id: hit.folderId, + uid: hit.uid, title: hit.folderTitle, - url: `dashboards/folder/${hit.folderId}/${hit.folderSlug}`, + url: hit.url, slug: hit.slug, items: [], icon: 'fa fa-folder-open', diff --git a/public/app/core/specs/manage_dashboards.jest.ts b/public/app/core/specs/manage_dashboards.jest.ts index bd257c32f9b..ab67271ea8b 100644 --- a/public/app/core/specs/manage_dashboards.jest.ts +++ b/public/app/core/specs/manage_dashboards.jest.ts @@ -483,22 +483,22 @@ describe('ManageDashboards', () => { ctrl.sections = [ { id: 1, + uid: 'folder', title: 'folder', - items: [{ id: 2, checked: true, slug: 'folder-dash' }], + items: [{ id: 2, checked: true, uid: 'folder-dash' }], checked: true, - slug: 'folder', }, { id: 3, title: 'folder-2', - items: [{ id: 3, checked: true, slug: 'folder-2-dash' }], + items: [{ id: 3, checked: true, uid: 'folder-2-dash' }], checked: false, - slug: 'folder-2', + uid: 'folder-2', }, { id: 0, title: 'Root', - items: [{ id: 3, checked: true, slug: 'root-dash' }], + items: [{ id: 3, checked: true, uid: 'root-dash' }], checked: true, }, ]; @@ -535,14 +535,14 @@ describe('ManageDashboards', () => { { id: 1, title: 'folder', - items: [{ id: 2, checked: true, slug: 'dash' }], + items: [{ id: 2, checked: true, uid: 'dash' }], checked: false, - slug: 'folder', + uid: 'folder', }, { id: 0, title: 'Root', - items: [{ id: 3, checked: true, slug: 'dash-2' }], + items: [{ id: 3, checked: true, uid: 'dash-2' }], checked: false, }, ]; diff --git a/public/app/features/dashboard/create_folder_ctrl.ts b/public/app/features/dashboard/create_folder_ctrl.ts index 414054232c2..f3d9167278b 100644 --- a/public/app/features/dashboard/create_folder_ctrl.ts +++ b/public/app/features/dashboard/create_folder_ctrl.ts @@ -19,9 +19,7 @@ export class CreateFolderCtrl { return this.backendSrv.createDashboardFolder(this.title).then(result => { appEvents.emit('alert-success', ['Folder Created', 'OK']); - - var folderUrl = `dashboards/folder/${result.dashboard.id}/${result.meta.slug}`; - this.$location.url(folderUrl); + this.$location.url(result.meta.url); }); } diff --git a/public/app/features/dashboard/folder_dashboards_ctrl.ts b/public/app/features/dashboard/folder_dashboards_ctrl.ts index 9748ca889d1..5eb79c82f7e 100644 --- a/public/app/features/dashboard/folder_dashboards_ctrl.ts +++ b/public/app/features/dashboard/folder_dashboards_ctrl.ts @@ -3,15 +3,16 @@ import { FolderPageLoader } from './folder_page_loader'; export class FolderDashboardsCtrl { navModel: any; folderId: number; + uid: string; /** @ngInject */ constructor(private backendSrv, navModelSrv, private $routeParams) { - if (this.$routeParams.folderId && this.$routeParams.slug) { - this.folderId = $routeParams.folderId; + if (this.$routeParams.uid) { + this.uid = $routeParams.uid; - const loader = new FolderPageLoader(this.backendSrv, this.$routeParams); + const loader = new FolderPageLoader(this.backendSrv); - loader.load(this, this.folderId, 'manage-folder-dashboards'); + loader.load(this, this.uid, 'manage-folder-dashboards'); } } } diff --git a/public/app/features/dashboard/folder_page_loader.ts b/public/app/features/dashboard/folder_page_loader.ts index c2ec057ab8d..178b8f903dd 100755 --- a/public/app/features/dashboard/folder_page_loader.ts +++ b/public/app/features/dashboard/folder_page_loader.ts @@ -1,9 +1,9 @@ import _ from 'lodash'; export class FolderPageLoader { - constructor(private backendSrv, private $routeParams) {} + constructor(private backendSrv) {} - load(ctrl, folderId, activeChildId) { + load(ctrl, uid, activeChildId) { ctrl.navModel = { main: { icon: 'fa fa-folder-open', @@ -38,12 +38,13 @@ export class FolderPageLoader { }, }; - return this.backendSrv.getDashboard('db', this.$routeParams.slug).then(result => { + return this.backendSrv.getDashboardByUid(uid).then(result => { + ctrl.folderId = result.dashboard.id; const folderTitle = result.dashboard.title; ctrl.navModel.main.text = ''; ctrl.navModel.main.breadcrumbs = [{ title: 'Dashboards', url: 'dashboards' }, { title: folderTitle }]; - const folderUrl = this.createFolderUrl(folderId, result.meta.type, result.meta.slug); + const folderUrl = result.meta.url; const dashTab = _.find(ctrl.navModel.main.children, { id: 'manage-folder-dashboards', @@ -63,8 +64,4 @@ export class FolderPageLoader { return result; }); } - - createFolderUrl(folderId: number, type: string, slug: string) { - return `dashboards/folder/${folderId}/${slug}`; - } } diff --git a/public/app/features/dashboard/folder_permissions_ctrl.ts b/public/app/features/dashboard/folder_permissions_ctrl.ts index fc275046442..7a5e4c6af7d 100644 --- a/public/app/features/dashboard/folder_permissions_ctrl.ts +++ b/public/app/features/dashboard/folder_permissions_ctrl.ts @@ -3,13 +3,14 @@ import { FolderPageLoader } from './folder_page_loader'; export class FolderPermissionsCtrl { navModel: any; folderId: number; + uid: string; /** @ngInject */ constructor(private backendSrv, navModelSrv, private $routeParams) { - if (this.$routeParams.folderId && this.$routeParams.slug) { - this.folderId = $routeParams.folderId; + if (this.$routeParams.uid) { + this.uid = $routeParams.uid; - new FolderPageLoader(this.backendSrv, this.$routeParams).load(this, this.folderId, 'manage-folder-permissions'); + new FolderPageLoader(this.backendSrv).load(this, this.uid, 'manage-folder-permissions'); } } } diff --git a/public/app/features/dashboard/folder_settings_ctrl.ts b/public/app/features/dashboard/folder_settings_ctrl.ts index 88abc5f9874..4f05ef29a42 100644 --- a/public/app/features/dashboard/folder_settings_ctrl.ts +++ b/public/app/features/dashboard/folder_settings_ctrl.ts @@ -5,6 +5,7 @@ export class FolderSettingsCtrl { folderPageLoader: FolderPageLoader; navModel: any; folderId: number; + uid: string; canSave = false; dashboard: any; meta: any; @@ -13,11 +14,11 @@ export class FolderSettingsCtrl { /** @ngInject */ constructor(private backendSrv, navModelSrv, private $routeParams, private $location) { - if (this.$routeParams.folderId && this.$routeParams.slug) { - this.folderId = $routeParams.folderId; + if (this.$routeParams.uid) { + this.uid = $routeParams.uid; - this.folderPageLoader = new FolderPageLoader(this.backendSrv, this.$routeParams); - this.folderPageLoader.load(this, this.folderId, 'manage-folder-settings').then(result => { + this.folderPageLoader = new FolderPageLoader(this.backendSrv); + this.folderPageLoader.load(this, this.uid, 'manage-folder-settings').then(result => { this.dashboard = result.dashboard; this.meta = result.meta; this.canSave = result.meta.canSave; @@ -38,9 +39,8 @@ export class FolderSettingsCtrl { return this.backendSrv .saveDashboard(this.dashboard, { overwrite: false }) .then(result => { - var folderUrl = this.folderPageLoader.createFolderUrl(this.folderId, this.meta.type, result.slug); - if (folderUrl !== this.$location.path()) { - this.$location.url(folderUrl + '/settings'); + if (result.url !== this.$location.path()) { + this.$location.url(result.url + '/settings'); } appEvents.emit('dashboard-saved'); @@ -65,7 +65,7 @@ export class FolderSettingsCtrl { icon: 'fa-trash', yesText: 'Delete', onConfirm: () => { - return this.backendSrv.deleteDashboard(this.meta.slug).then(() => { + return this.backendSrv.deleteDashboard(this.dashboard.uid).then(() => { appEvents.emit('alert-success', ['Folder Deleted', `${this.dashboard.title} has been deleted`]); this.$location.url('dashboards'); }); diff --git a/public/app/features/dashboard/settings/settings.ts b/public/app/features/dashboard/settings/settings.ts index 4f678713ffc..b6d7c0b7c50 100755 --- a/public/app/features/dashboard/settings/settings.ts +++ b/public/app/features/dashboard/settings/settings.ts @@ -174,7 +174,7 @@ export class SettingsCtrl { } deleteDashboardConfirmed() { - this.backendSrv.deleteDashboard(this.dashboard.meta.slug).then(() => { + this.backendSrv.deleteDashboard(this.dashboard.uid).then(() => { appEvents.emit('alert-success', ['Dashboard Deleted', this.dashboard.title + ' has been deleted']); this.$location.url('/'); }); diff --git a/public/app/routes/routes.ts b/public/app/routes/routes.ts index 865f4ce1682..e183c341f5e 100644 --- a/public/app/routes/routes.ts +++ b/public/app/routes/routes.ts @@ -74,17 +74,17 @@ export function setupAngularRoutes($routeProvider, $locationProvider) { controller: 'CreateFolderCtrl', controllerAs: 'ctrl', }) - .when('/dashboards/folder/:folderId/:slug/permissions', { + .when('/dashboards/f/:uid/:slug/permissions', { templateUrl: 'public/app/features/dashboard/partials/folder_permissions.html', controller: 'FolderPermissionsCtrl', controllerAs: 'ctrl', }) - .when('/dashboards/folder/:folderId/:slug/settings', { + .when('/dashboards/f/:uid/:slug/settings', { templateUrl: 'public/app/features/dashboard/partials/folder_settings.html', controller: 'FolderSettingsCtrl', controllerAs: 'ctrl', }) - .when('/dashboards/folder/:folderId/:slug', { + .when('/dashboards/f/:uid/:slug', { templateUrl: 'public/app/features/dashboard/partials/folder_dashboards.html', controller: 'FolderDashboardsCtrl', controllerAs: 'ctrl', From 035b724725728c870e3898a1236993ee221b4d20 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 17:25:14 +0100 Subject: [PATCH 33/44] dashboards: remove slug property in dashboard search responses Removes slug property in dashboard search responses since this property isn't needed anymore and it haven't been released to any stable release. --- pkg/services/search/models.go | 1 - pkg/services/sqlstore/dashboard.go | 1 - public/app/core/services/search_srv.ts | 2 -- 3 files changed, 4 deletions(-) diff --git a/pkg/services/search/models.go b/pkg/services/search/models.go index c543befc454..f5d87b1f5c8 100644 --- a/pkg/services/search/models.go +++ b/pkg/services/search/models.go @@ -17,7 +17,6 @@ type Hit struct { Title string `json:"title"` Uri string `json:"uri"` Url string `json:"url"` - Slug string `json:"slug"` Type HitType `json:"type"` Tags []string `json:"tags"` IsStarred bool `json:"isStarred"` diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index a23f07cd895..c59b9574b74 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -270,7 +270,6 @@ func makeQueryResult(query *search.FindPersistedDashboardsQuery, res []Dashboard Title: item.Title, Uri: "db/" + item.Slug, Url: url, - Slug: item.Slug, Type: getHitType(item), FolderId: item.FolderId, FolderTitle: item.FolderTitle, diff --git a/public/app/core/services/search_srv.ts b/public/app/core/services/search_srv.ts index 9092b30fd4b..cb9862f8cce 100644 --- a/public/app/core/services/search_srv.ts +++ b/public/app/core/services/search_srv.ts @@ -134,7 +134,6 @@ export class SearchSrv { items: [], toggle: this.toggleFolder.bind(this), url: hit.url, - slug: hit.slug, icon: 'fa fa-folder', score: _.keys(sections).length, }; @@ -154,7 +153,6 @@ export class SearchSrv { uid: hit.uid, title: hit.folderTitle, url: hit.url, - slug: hit.slug, items: [], icon: 'fa fa-folder-open', toggle: this.toggleFolder.bind(this), From 8fc648ac4373dde512bb4943906a7cf1154cf59a Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 18:10:28 +0100 Subject: [PATCH 34/44] dashboards: fix updating folder so that correct url is returned --- public/app/core/services/backend_srv.ts | 11 +++++++++++ public/app/features/dashboard/folder_settings_ctrl.ts | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index b6791267d4f..6d44967ef39 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -257,6 +257,17 @@ export class BackendSrv { }); } + updateDashboardFolder(dash, options) { + options = options || {}; + + return this.post('/api/dashboards/db/', { + dashboard: dash, + isFolder: true, + overwrite: options.overwrite === true, + message: options.message || '', + }); + } + deleteDashboard(uid) { let deferred = this.$q.defer(); diff --git a/public/app/features/dashboard/folder_settings_ctrl.ts b/public/app/features/dashboard/folder_settings_ctrl.ts index 4f05ef29a42..ddc35166cad 100644 --- a/public/app/features/dashboard/folder_settings_ctrl.ts +++ b/public/app/features/dashboard/folder_settings_ctrl.ts @@ -37,7 +37,7 @@ export class FolderSettingsCtrl { this.dashboard.title = this.title.trim(); return this.backendSrv - .saveDashboard(this.dashboard, { overwrite: false }) + .updateDashboardFolder(this.dashboard, { overwrite: false }) .then(result => { if (result.url !== this.$location.path()) { this.$location.url(result.url + '/settings'); @@ -84,7 +84,7 @@ export class FolderSettingsCtrl { yesText: 'Save & Overwrite', icon: 'fa-warning', onConfirm: () => { - this.backendSrv.saveDashboard(this.dashboard, { overwrite: true }); + this.backendSrv.updateDashboardFolder(this.dashboard, { overwrite: true }); }, }); } From c0c3f17d841570799d41c4811a6ba17607211c8e Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 18:16:45 +0100 Subject: [PATCH 35/44] dashboards: when restoring a dashboard to an older version, set current uid --- pkg/api/dashboard.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 7b3977df0d8..f8cdc1324f8 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -473,6 +473,7 @@ func RestoreDashboardVersion(c *middleware.Context, apiCmd dtos.RestoreDashboard saveCmd.UserId = c.UserId saveCmd.Dashboard = version.Data saveCmd.Dashboard.Set("version", dash.Version) + saveCmd.Dashboard.Set("uid", dash.Uid) saveCmd.Message = fmt.Sprintf("Restored from version %d", version.Version) return PostDashboard(c, saveCmd) From 40c83e3e2224bf24311e4fd688bfa50fc4c6e97c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 18:24:47 +0100 Subject: [PATCH 36/44] dashboards: update dashboard/folder url if browser url is not the same as from backend --- .../app/features/dashboard/folder_dashboards_ctrl.ts | 8 ++++++-- .../features/dashboard/folder_permissions_ctrl.ts | 8 ++++++-- .../app/features/dashboard/folder_settings_ctrl.ts | 12 ++++++++---- public/app/routes/dashboard_loaders.ts | 4 ++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/public/app/features/dashboard/folder_dashboards_ctrl.ts b/public/app/features/dashboard/folder_dashboards_ctrl.ts index 5eb79c82f7e..dd0bf7772d8 100644 --- a/public/app/features/dashboard/folder_dashboards_ctrl.ts +++ b/public/app/features/dashboard/folder_dashboards_ctrl.ts @@ -6,13 +6,17 @@ export class FolderDashboardsCtrl { uid: string; /** @ngInject */ - constructor(private backendSrv, navModelSrv, private $routeParams) { + constructor(private backendSrv, navModelSrv, private $routeParams, $location) { if (this.$routeParams.uid) { this.uid = $routeParams.uid; const loader = new FolderPageLoader(this.backendSrv); - loader.load(this, this.uid, 'manage-folder-dashboards'); + loader.load(this, this.uid, 'manage-folder-dashboards').then(folder => { + if ($location.path() !== folder.meta.url) { + $location.path(folder.meta.url).replace(); + } + }); } } } diff --git a/public/app/features/dashboard/folder_permissions_ctrl.ts b/public/app/features/dashboard/folder_permissions_ctrl.ts index 7a5e4c6af7d..fa1d71ef371 100644 --- a/public/app/features/dashboard/folder_permissions_ctrl.ts +++ b/public/app/features/dashboard/folder_permissions_ctrl.ts @@ -6,11 +6,15 @@ export class FolderPermissionsCtrl { uid: string; /** @ngInject */ - constructor(private backendSrv, navModelSrv, private $routeParams) { + constructor(private backendSrv, navModelSrv, private $routeParams, $location) { if (this.$routeParams.uid) { this.uid = $routeParams.uid; - new FolderPageLoader(this.backendSrv).load(this, this.uid, 'manage-folder-permissions'); + new FolderPageLoader(this.backendSrv).load(this, this.uid, 'manage-folder-permissions').then(folder => { + if ($location.path() !== folder.meta.url) { + $location.path(`${folder.meta.url}/permissions`).replace(); + } + }); } } } diff --git a/public/app/features/dashboard/folder_settings_ctrl.ts b/public/app/features/dashboard/folder_settings_ctrl.ts index ddc35166cad..004ba2efa9f 100644 --- a/public/app/features/dashboard/folder_settings_ctrl.ts +++ b/public/app/features/dashboard/folder_settings_ctrl.ts @@ -18,10 +18,14 @@ export class FolderSettingsCtrl { this.uid = $routeParams.uid; this.folderPageLoader = new FolderPageLoader(this.backendSrv); - this.folderPageLoader.load(this, this.uid, 'manage-folder-settings').then(result => { - this.dashboard = result.dashboard; - this.meta = result.meta; - this.canSave = result.meta.canSave; + this.folderPageLoader.load(this, this.uid, 'manage-folder-settings').then(folder => { + if ($location.path() !== folder.meta.url) { + $location.path(`${folder.meta.url}/settings`).replace(); + } + + this.dashboard = folder.dashboard; + this.meta = folder.meta; + this.canSave = folder.meta.canSave; this.title = this.dashboard.title; }); } diff --git a/public/app/routes/dashboard_loaders.ts b/public/app/routes/dashboard_loaders.ts index 4ad77512ad1..80e74819069 100644 --- a/public/app/routes/dashboard_loaders.ts +++ b/public/app/routes/dashboard_loaders.ts @@ -29,6 +29,10 @@ export class LoadDashboardCtrl { } dashboardLoaderSrv.loadDashboard($routeParams.type, $routeParams.slug, $routeParams.uid).then(function(result) { + if ($location.path() !== result.meta.url) { + $location.path(result.meta.url).replace(); + } + if ($routeParams.keepRows) { result.meta.keepRows = true; } From 90933b06216ab6c766de776cb1791ddcb4dc9f54 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 31 Jan 2018 23:14:48 +0100 Subject: [PATCH 37/44] dashboard: refactor logic for retrieving url for folder/dashboard --- pkg/api/dashboard.go | 16 +++------------- pkg/models/dashboards.go | 14 ++++++++++++++ pkg/services/sqlstore/dashboard.go | 8 +------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 7799ad868f9..fe7ea430f73 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -90,6 +90,7 @@ func GetDashboard(c *middleware.Context) Response { IsFolder: dash.IsFolder, FolderId: dash.FolderId, FolderTitle: "Root", + Url: dash.GetUrl(), } // lookup folder title @@ -101,12 +102,6 @@ func GetDashboard(c *middleware.Context) Response { meta.FolderTitle = query.Result.Title } - if dash.IsFolder { - meta.Url = m.GetFolderUrl(dash.Uid, dash.Slug) - } else { - meta.Url = m.GetDashboardUrl(dash.Uid, dash.Slug) - } - // make sure db version is in sync with json model version dash.Data.Set("version", dash.Version) @@ -238,12 +233,7 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response { return ApiError(500, "Invalid alert data. Cannot save dashboard", err) } - var url string - if dash.IsFolder { - url = m.GetFolderUrl(dashboard.Uid, dashboard.Slug) - } else { - url = m.GetDashboardUrl(dashboard.Uid, dashboard.Slug) - } + dashboard.IsFolder = dash.IsFolder c.TimeRequest(metrics.M_Api_Dashboard_Save) return Json(200, util.DynMap{ @@ -252,7 +242,7 @@ func PostDashboard(c *middleware.Context, cmd m.SaveDashboardCommand) Response { "version": dashboard.Version, "id": dashboard.Id, "uid": dashboard.Uid, - "url": url, + "url": dashboard.GetUrl(), }) } diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 84ede04f226..1b0c3f8987d 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -158,6 +158,20 @@ func SlugifyTitle(title string) string { return slug.Make(strings.ToLower(title)) } +// GetUrl return the html url for a folder if it's folder, otherwise for a dashboard +func (dash *Dashboard) GetUrl() string { + return GetDashboardFolderUrl(dash.IsFolder, dash.Uid, dash.Slug) +} + +// GetDashboardFolderUrl return the html url for a folder if it's folder, otherwise for a dashboard +func GetDashboardFolderUrl(isFolder bool, uid string, slug string) string { + if isFolder { + return GetFolderUrl(uid, slug) + } + + return GetDashboardUrl(uid, slug) +} + // GetDashboardUrl return the html url for a dashboard func GetDashboardUrl(uid string, slug string) string { return fmt.Sprintf("%s/d/%s/%s", setting.AppSubUrl, uid, slug) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 84cfd1999c7..b12c1d70b9e 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -258,17 +258,11 @@ func makeQueryResult(query *search.FindPersistedDashboardsQuery, res []Dashboard for _, item := range res { hit, exists := hits[item.Id] if !exists { - var url string - if item.IsFolder { - url = m.GetFolderUrl(item.Uid, item.Slug) - } else { - url = m.GetDashboardUrl(item.Uid, item.Slug) - } hit = &search.Hit{ Id: item.Id, Title: item.Title, Uri: "db/" + item.Slug, - Url: url, + Url: m.GetDashboardFolderUrl(item.IsFolder, item.Uid, item.Slug), Slug: item.Slug, Type: getHitType(item), FolderId: item.FolderId, From ee9c40818803ee97322f676cdb7a9ead99d28ea8 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 1 Feb 2018 13:21:24 +0100 Subject: [PATCH 38/44] folders: changes needed due to merge --- .../ManageDashboards/FolderPermissions.tsx | 2 +- .../ManageDashboards/FolderSettings.jest.tsx | 4 ++-- .../ManageDashboards/FolderSettings.tsx | 6 ++++-- .../manage_dashboards/manage_dashboards.ts | 12 ++++++------ public/app/core/services/backend_srv.ts | 2 +- .../dashboard/partials/folder_dashboards.html | 2 +- public/app/stores/FolderStore/FolderStore.ts | 17 +++++++++-------- public/app/stores/NavStore/NavStore.jest.ts | 10 +++++----- public/app/stores/NavStore/NavStore.ts | 12 +++--------- public/test/mocks/common.ts | 1 + 10 files changed, 33 insertions(+), 35 deletions(-) diff --git a/public/app/containers/ManageDashboards/FolderPermissions.tsx b/public/app/containers/ManageDashboards/FolderPermissions.tsx index 3214382732a..e6e788b7585 100644 --- a/public/app/containers/ManageDashboards/FolderPermissions.tsx +++ b/public/app/containers/ManageDashboards/FolderPermissions.tsx @@ -16,7 +16,7 @@ export class FolderPermissions extends Component { loadStore() { const { nav, folder, view } = this.props; - return folder.load(view.routeParams.get('slug') as string).then(res => { + return folder.load(view.routeParams.get('uid') as string).then(res => { return nav.initFolderNav(toJS(folder.folder), 'manage-folder-permissions'); }); } diff --git a/public/app/containers/ManageDashboards/FolderSettings.jest.tsx b/public/app/containers/ManageDashboards/FolderSettings.jest.tsx index 3355b657f03..bf7b35ed05d 100644 --- a/public/app/containers/ManageDashboards/FolderSettings.jest.tsx +++ b/public/app/containers/ManageDashboards/FolderSettings.jest.tsx @@ -9,14 +9,14 @@ describe('FolderSettings', () => { let page; beforeAll(() => { - backendSrv.getDashboard.mockReturnValue( + backendSrv.getDashboardByUid.mockReturnValue( Promise.resolve({ dashboard: { id: 1, title: 'Folder Name', }, meta: { - slug: 'folder-name', + url: '/dashboards/f/uid/folder-name', canSave: true, }, }) diff --git a/public/app/containers/ManageDashboards/FolderSettings.tsx b/public/app/containers/ManageDashboards/FolderSettings.tsx index ef3377622df..a6349764a14 100644 --- a/public/app/containers/ManageDashboards/FolderSettings.tsx +++ b/public/app/containers/ManageDashboards/FolderSettings.tsx @@ -20,10 +20,12 @@ export class FolderSettings extends React.Component { loadStore() { const { nav, folder, view } = this.props; - return folder.load(view.routeParams.get('slug') as string).then(res => { + return folder.load(view.routeParams.get('uid') as string).then(res => { this.formSnapshot = getSnapshot(folder); this.dashboard = res.dashboard; + view.updatePathAndQuery(`${res.meta.url}/settings`, {}, {}); + return nav.initFolderNav(toJS(folder.folder), 'manage-folder-settings'); }); } @@ -51,7 +53,7 @@ export class FolderSettings extends React.Component { folder .saveFolder(this.dashboard, { overwrite: false }) .then(newUrl => { - view.updatePathAndQuery(newUrl, '', ''); + view.updatePathAndQuery(newUrl, {}, {}); appEvents.emit('dashboard-saved'); appEvents.emit('alert-success', ['Folder saved']); diff --git a/public/app/core/components/manage_dashboards/manage_dashboards.ts b/public/app/core/components/manage_dashboards/manage_dashboards.ts index 9eb14a69a6e..d0f905f5f16 100644 --- a/public/app/core/components/manage_dashboards/manage_dashboards.ts +++ b/public/app/core/components/manage_dashboards/manage_dashboards.ts @@ -34,7 +34,7 @@ export class ManageDashboardsCtrl { // used when managing dashboards for a specific folder folderId?: number; - folderSlug?: string; + folderUid?: string; // if user can add new folders and/or add new dashboards canSave: boolean; @@ -74,11 +74,11 @@ export class ManageDashboardsCtrl { return this.initDashboardList(result); }) .then(() => { - if (!this.folderSlug) { + if (!this.folderUid) { return; } - return this.backendSrv.getDashboard('db', this.folderSlug).then(dash => { + return this.backendSrv.getDashboardByUid(this.folderUid).then(dash => { this.canSave = dash.meta.canSave; }); }); @@ -179,8 +179,8 @@ export class ManageDashboardsCtrl { }); } - private deleteFoldersAndDashboards(slugs) { - this.backendSrv.deleteDashboards(slugs).then(result => { + private deleteFoldersAndDashboards(uids) { + this.backendSrv.deleteDashboards(uids).then(result => { const folders = _.filter(result, dash => dash.meta.isFolder); const folderCount = folders.length; const dashboards = _.filter(result, dash => !dash.meta.isFolder); @@ -334,7 +334,7 @@ export function manageDashboardsDirective() { controllerAs: 'ctrl', scope: { folderId: '=', - folderSlug: '=', + folderUid: '=', }, }; } diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 6d44967ef39..0a8b305ea53 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -257,7 +257,7 @@ export class BackendSrv { }); } - updateDashboardFolder(dash, options) { + saveFolder(dash, options) { options = options || {}; return this.post('/api/dashboards/db/', { diff --git a/public/app/features/dashboard/partials/folder_dashboards.html b/public/app/features/dashboard/partials/folder_dashboards.html index a03c53233f5..1082bee402e 100644 --- a/public/app/features/dashboard/partials/folder_dashboards.html +++ b/public/app/features/dashboard/partials/folder_dashboards.html @@ -1,5 +1,5 @@
- +
diff --git a/public/app/stores/FolderStore/FolderStore.ts b/public/app/stores/FolderStore/FolderStore.ts index d0216e32ad7..6f14e7221f8 100644 --- a/public/app/stores/FolderStore/FolderStore.ts +++ b/public/app/stores/FolderStore/FolderStore.ts @@ -2,8 +2,8 @@ import { types, getEnv, flow } from 'mobx-state-tree'; export const Folder = types.model('Folder', { id: types.identifier(types.number), - slug: types.string, title: types.string, + url: types.string, canSave: types.boolean, hasChanged: types.boolean, }); @@ -13,13 +13,13 @@ export const FolderStore = types folder: types.maybe(Folder), }) .actions(self => ({ - load: flow(function* load(slug: string) { + load: flow(function* load(uid: string) { const backendSrv = getEnv(self).backendSrv; - const res = yield backendSrv.getDashboard('db', slug); + const res = yield backendSrv.getDashboardByUid(uid); self.folder = Folder.create({ id: res.dashboard.id, title: res.dashboard.title, - slug: res.meta.slug, + url: res.meta.url, canSave: res.meta.canSave, hasChanged: false, }); @@ -35,14 +35,15 @@ export const FolderStore = types const backendSrv = getEnv(self).backendSrv; dashboard.title = self.folder.title.trim(); - const res = yield backendSrv.saveDashboard(dashboard, options); - self.folder.slug = res.slug; - return `dashboards/folder/${self.folder.id}/${res.slug}/settings`; + const res = yield backendSrv.saveFolder(dashboard, options); + self.folder.url = res.url; + + return `${self.folder.url}/settings`; }), deleteFolder: flow(function* deleteFolder() { const backendSrv = getEnv(self).backendSrv; - return backendSrv.deleteDashboard(self.folder.slug); + return backendSrv.deleteDashboard(self.folder.url); }), })); diff --git a/public/app/stores/NavStore/NavStore.jest.ts b/public/app/stores/NavStore/NavStore.jest.ts index b1ad820910d..43d4496c858 100644 --- a/public/app/stores/NavStore/NavStore.jest.ts +++ b/public/app/stores/NavStore/NavStore.jest.ts @@ -3,12 +3,12 @@ import { NavStore } from './NavStore'; describe('NavStore', () => { const folderId = 1; const folderTitle = 'Folder Name'; - const folderSlug = 'folder-name'; + const folderUrl = '/dashboards/f/uid/folder-name'; const canAdmin = true; const folder = { id: folderId, - slug: folderSlug, + url: folderUrl, title: folderTitle, canAdmin: canAdmin, }; @@ -33,9 +33,9 @@ describe('NavStore', () => { it('Should set correct urls for each tab', () => { expect(store.main.children.length).toBe(3); - expect(store.main.children[0].url).toBe(`dashboards/folder/${folderId}/${folderSlug}`); - expect(store.main.children[1].url).toBe(`dashboards/folder/${folderId}/${folderSlug}/permissions`); - expect(store.main.children[2].url).toBe(`dashboards/folder/${folderId}/${folderSlug}/settings`); + expect(store.main.children[0].url).toBe(folderUrl); + expect(store.main.children[1].url).toBe(`${folderUrl}/permissions`); + expect(store.main.children[2].url).toBe(`${folderUrl}/settings`); }); it('Should set active tab', () => { diff --git a/public/app/stores/NavStore/NavStore.ts b/public/app/stores/NavStore/NavStore.ts index d04c5bf0259..86348c00487 100644 --- a/public/app/stores/NavStore/NavStore.ts +++ b/public/app/stores/NavStore/NavStore.ts @@ -41,8 +41,6 @@ export const NavStore = types }, initFolderNav(folder: any, activeChildId: string) { - const folderUrl = createFolderUrl(folder.id, folder.slug); - let main = { icon: 'fa fa-folder-open', id: 'manage-folder', @@ -56,21 +54,21 @@ export const NavStore = types icon: 'fa fa-fw fa-th-large', id: 'manage-folder-dashboards', text: 'Dashboards', - url: folderUrl, + url: folder.url, }, { active: activeChildId === 'manage-folder-permissions', icon: 'fa fa-fw fa-lock', id: 'manage-folder-permissions', text: 'Permissions', - url: folderUrl + '/permissions', + url: `${folder.url}/permissions`, }, { active: activeChildId === 'manage-folder-settings', icon: 'fa fa-fw fa-cog', id: 'manage-folder-settings', text: 'Settings', - url: folderUrl + '/settings', + url: `${folder.url}/settings`, }, ], }; @@ -118,7 +116,3 @@ export const NavStore = types self.main = NavItem.create(main); }, })); - -function createFolderUrl(folderId: number, slug: string) { - return `dashboards/folder/${folderId}/${slug}`; -} diff --git a/public/test/mocks/common.ts b/public/test/mocks/common.ts index da63381ddf4..3f80227435d 100644 --- a/public/test/mocks/common.ts +++ b/public/test/mocks/common.ts @@ -1,6 +1,7 @@ export const backendSrv = { get: jest.fn(), getDashboard: jest.fn(), + getDashboardByUid: jest.fn(), post: jest.fn(), }; From cdf2fd64b7a281a0677b8d3cec7cac7ba4841b4e Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 1 Feb 2018 13:28:04 +0100 Subject: [PATCH 39/44] route params from angular to view store should be updated on routeChangeSuccess --- public/app/core/services/bridge_srv.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/public/app/core/services/bridge_srv.ts b/public/app/core/services/bridge_srv.ts index f0166d89e9d..ed4abb9b894 100644 --- a/public/app/core/services/bridge_srv.ts +++ b/public/app/core/services/bridge_srv.ts @@ -34,10 +34,7 @@ export class BridgeSrv { }); this.$rootScope.$on('$routeChangeSuccess', (evt, data) => { - let angularUrl = this.$location.url(); - if (store.view.currentUrl !== angularUrl) { - store.view.updatePathAndQuery(this.$location.path(), this.$location.search(), this.$route.current.params); - } + store.view.updatePathAndQuery(this.$location.path(), this.$location.search(), this.$route.current.params); }); reaction( @@ -45,7 +42,9 @@ export class BridgeSrv { currentUrl => { let angularUrl = this.$location.url(); if (angularUrl !== currentUrl) { - this.$location.url(currentUrl); + this.$timeout(() => { + this.$location.url(currentUrl); + }); console.log('store updating angular $location.url', currentUrl); } } From 992fd37010061e752b97799eddb2fbe26434c9e3 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 1 Feb 2018 13:32:00 +0100 Subject: [PATCH 40/44] alert: use new url format --- pkg/models/dashboards.go | 17 +++++++++- pkg/services/alerting/eval_context.go | 47 ++++++++++++++------------- pkg/services/alerting/notifier.go | 4 +-- pkg/services/sqlstore/dashboard.go | 21 +++++++++++- 4 files changed, 63 insertions(+), 26 deletions(-) diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 51ba62419b2..5675f494411 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -171,11 +171,16 @@ func GetDashboardFolderUrl(isFolder bool, uid string, slug string) string { return GetDashboardUrl(uid, slug) } -// GetDashboardUrl return the html url for a dashboard +// Return the html url for a dashboard func GetDashboardUrl(uid string, slug string) string { return fmt.Sprintf("%s/d/%s/%s", setting.AppSubUrl, uid, slug) } +// Return the full url for a dashboard +func GetFullDashboardUrl(uid string, slug string) string { + return fmt.Sprintf("%s%s", setting.AppUrl, GetDashboardUrl(uid, slug)) +} + // GetFolderUrl return the html url for a folder func GetFolderUrl(folderUid string, slug string) string { return fmt.Sprintf("%s/dashboards/f/%s/%s", setting.AppSubUrl, folderUid, slug) @@ -277,3 +282,13 @@ type DashboardPermissionForUser struct { Permission PermissionType `json:"permission"` PermissionName string `json:"permissionName"` } + +type DashboardRef struct { + Uid string + Slug string +} + +type GetDashboardUIDByIdQuery struct { + Id int64 + Result *DashboardRef +} diff --git a/pkg/services/alerting/eval_context.go b/pkg/services/alerting/eval_context.go index f5663deb8ca..60c5530d486 100644 --- a/pkg/services/alerting/eval_context.go +++ b/pkg/services/alerting/eval_context.go @@ -12,17 +12,19 @@ import ( ) type EvalContext struct { - Firing bool - IsTestRun bool - EvalMatches []*EvalMatch - Logs []*ResultLogEntry - Error error - ConditionEvals string - StartTime time.Time - EndTime time.Time - Rule *Rule - log log.Logger - dashboardSlug string + Firing bool + IsTestRun bool + EvalMatches []*EvalMatch + Logs []*ResultLogEntry + Error error + ConditionEvals string + StartTime time.Time + EndTime time.Time + Rule *Rule + log log.Logger + + dashboardRef *m.DashboardRef + ImagePublicUrl string ImageOnDiskPath string NoDataFound bool @@ -83,29 +85,30 @@ func (c *EvalContext) GetNotificationTitle() string { return "[" + c.GetStateModel().Text + "] " + c.Rule.Name } -func (c *EvalContext) GetDashboardSlug() (string, error) { - if c.dashboardSlug != "" { - return c.dashboardSlug, nil +func (c *EvalContext) GetDashboardUID() (*m.DashboardRef, error) { + if c.dashboardRef != nil { + return c.dashboardRef, nil } - slugQuery := &m.GetDashboardSlugByIdQuery{Id: c.Rule.DashboardId} - if err := bus.Dispatch(slugQuery); err != nil { - return "", err + uidQuery := &m.GetDashboardUIDByIdQuery{Id: c.Rule.DashboardId} + if err := bus.Dispatch(uidQuery); err != nil { + return nil, err } - c.dashboardSlug = slugQuery.Result - return c.dashboardSlug, nil + c.dashboardRef = uidQuery.Result + return c.dashboardRef, nil } +const urlFormat = "%s?fullscreen=true&edit=true&tab=alert&panelId=%d&orgId=%d" + func (c *EvalContext) GetRuleUrl() (string, error) { if c.IsTestRun { return setting.AppUrl, nil } - if slug, err := c.GetDashboardSlug(); err != nil { + if ref, err := c.GetDashboardUID(); err != nil { return "", err } else { - ruleUrl := fmt.Sprintf("%sdashboard/db/%s?fullscreen&edit&tab=alert&panelId=%d&orgId=%d", setting.AppUrl, slug, c.Rule.PanelId, c.Rule.OrgId) - return ruleUrl, nil + return fmt.Sprintf(urlFormat, m.GetFullDashboardUrl(ref.Uid, ref.Slug), c.Rule.PanelId, c.Rule.OrgId), nil } } diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 47c9e0c590e..af9ba52a52a 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -87,10 +87,10 @@ func (n *notificationService) uploadImage(context *EvalContext) (err error) { IsAlertContext: true, } - if slug, err := context.GetDashboardSlug(); err != nil { + if ref, err := context.GetDashboardUID(); err != nil { return err } else { - renderOpts.Path = fmt.Sprintf("dashboard-solo/db/%s?&panelId=%d", slug, context.Rule.PanelId) + renderOpts.Path = fmt.Sprintf("d-solo/%s/%s?panelId=%d", ref.Uid, ref.Slug, context.Rule.PanelId) } if imagePath, err := renderer.RenderToPng(renderOpts); err != nil { diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index a4e63a50633..cf9624b2f22 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -19,6 +19,7 @@ func init() { bus.AddHandler("sql", SearchDashboards) bus.AddHandler("sql", GetDashboardTags) bus.AddHandler("sql", GetDashboardSlugById) + bus.AddHandler("sql", GetDashboardUIDById) bus.AddHandler("sql", GetDashboardsByPluginId) bus.AddHandler("sql", GetFoldersForSignedInUser) bus.AddHandler("sql", GetDashboardPermissionsForUser) @@ -159,6 +160,7 @@ func SaveDashboard(cmd *m.SaveDashboardCommand) error { return err }) } + func generateNewDashboardUid(sess *DBSession, orgId int64) (string, error) { for i := 0; i < 3; i++ { uid := generateNewUid() @@ -539,7 +541,7 @@ func GetDashboardSlugById(query *m.GetDashboardSlugByIdQuery) error { var rawSql = `SELECT slug from dashboard WHERE Id=?` var slug = DashboardSlugDTO{} - exists, err := x.Sql(rawSql, query.Id).Get(&slug) + exists, err := x.SQL(rawSql, query.Id).Get(&slug) if err != nil { return err @@ -561,3 +563,20 @@ func GetDashboardsBySlug(query *m.GetDashboardsBySlugQuery) error { query.Result = dashboards return nil } + +func GetDashboardUIDById(query *m.GetDashboardUIDByIdQuery) error { + var rawSql = `SELECT uid, slug from dashboard WHERE Id=?` + + us := &m.DashboardRef{} + + exists, err := x.SQL(rawSql, query.Id).Get(us) + + if err != nil { + return err + } else if exists == false { + return m.ErrDashboardNotFound + } + + query.Result = us + return nil +} From 74ca6f6dbfea03f5e12e01f2b82b2453a4b89a8e Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 1 Feb 2018 13:16:56 +0100 Subject: [PATCH 41/44] changes dashboard url in alertlist --- pkg/api/alerting.go | 3 ++- public/app/containers/AlertRuleList/AlertRuleList.jest.tsx | 2 +- public/app/containers/AlertRuleList/AlertRuleList.tsx | 2 +- .../AlertRuleList/__snapshots__/AlertRuleList.jest.tsx.snap | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index 42ea091ef10..dee1b7451f6 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -105,7 +105,8 @@ func transformToDTOs(alerts []*models.Alert, c *middleware.Context) ([]*dtos.Ale for _, alert := range alertDTOs { for _, dash := range dashboardsQuery.Result { if alert.DashboardId == dash.Id { - alert.DashbboardUri = "db/" + dash.Slug + alert.DashbboardUri = models.GetDashboardUrl(dash.Uid, dash.Slug) + break } } } diff --git a/public/app/containers/AlertRuleList/AlertRuleList.jest.tsx b/public/app/containers/AlertRuleList/AlertRuleList.jest.tsx index f5aa07b454a..eaeba48f0a6 100644 --- a/public/app/containers/AlertRuleList/AlertRuleList.jest.tsx +++ b/public/app/containers/AlertRuleList/AlertRuleList.jest.tsx @@ -23,7 +23,7 @@ describe('AlertRuleList', () => { .format(), evalData: {}, executionError: '', - dashboardUri: 'db/mygool', + dashboardUri: 'd/ufkcofof/my-goal', canEdit: true, }, ]) diff --git a/public/app/containers/AlertRuleList/AlertRuleList.tsx b/public/app/containers/AlertRuleList/AlertRuleList.tsx index d2712706154..5ce1efd9ee3 100644 --- a/public/app/containers/AlertRuleList/AlertRuleList.tsx +++ b/public/app/containers/AlertRuleList/AlertRuleList.tsx @@ -137,7 +137,7 @@ export class AlertRuleItem extends React.Component { 'fa-pause': !rule.isPaused, }); - let ruleUrl = `dashboard/${rule.dashboardUri}?panelId=${rule.panelId}&fullscreen&edit&tab=alert`; + let ruleUrl = `${rule.dashboardUri}?panelId=${rule.panelId}&fullscreen=true&edit=true&tab=alert`; return (
  • diff --git a/public/app/containers/AlertRuleList/__snapshots__/AlertRuleList.jest.tsx.snap b/public/app/containers/AlertRuleList/__snapshots__/AlertRuleList.jest.tsx.snap index da5fa5f12c4..0914f050a0f 100644 --- a/public/app/containers/AlertRuleList/__snapshots__/AlertRuleList.jest.tsx.snap +++ b/public/app/containers/AlertRuleList/__snapshots__/AlertRuleList.jest.tsx.snap @@ -21,7 +21,7 @@ exports[`AlertRuleList should render 1 rule 1`] = ` className="alert-rule-item__name" > Date: Thu, 1 Feb 2018 13:49:34 +0100 Subject: [PATCH 42/44] make it easier for dashboards to generate ur; --- pkg/api/alerting.go | 2 +- pkg/models/dashboards.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index dee1b7451f6..1fc3d893681 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -105,7 +105,7 @@ func transformToDTOs(alerts []*models.Alert, c *middleware.Context) ([]*dtos.Ale for _, alert := range alertDTOs { for _, dash := range dashboardsQuery.Result { if alert.DashboardId == dash.Id { - alert.DashbboardUri = models.GetDashboardUrl(dash.Uid, dash.Slug) + alert.DashbboardUri = dash.GenerateUrl() break } } diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index 5675f494411..089a98c4f00 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -162,6 +162,11 @@ func (dash *Dashboard) GetUrl() string { return GetDashboardFolderUrl(dash.IsFolder, dash.Uid, dash.Slug) } +// Return the html url for a dashboard +func (dash *Dashboard) GenerateUrl() string { + return GetDashboardUrl(dash.Uid, dash.Slug) +} + // GetDashboardFolderUrl return the html url for a folder if it's folder, otherwise for a dashboard func GetDashboardFolderUrl(isFolder bool, uid string, slug string) string { if isFolder { From 90207bcb7dc23d2fe01481cb4d6ba4531291dc88 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 1 Feb 2018 14:13:42 +0100 Subject: [PATCH 43/44] register handler for get dashboards by slug --- pkg/services/sqlstore/dashboard.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index cf9624b2f22..fd07ccd5b2c 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -23,6 +23,7 @@ func init() { bus.AddHandler("sql", GetDashboardsByPluginId) bus.AddHandler("sql", GetFoldersForSignedInUser) bus.AddHandler("sql", GetDashboardPermissionsForUser) + bus.AddHandler("sql", GetDashboardsBySlug) } var generateNewUid func() string = util.GenerateShortUid @@ -554,7 +555,7 @@ func GetDashboardSlugById(query *m.GetDashboardSlugByIdQuery) error { } func GetDashboardsBySlug(query *m.GetDashboardsBySlugQuery) error { - var dashboards = make([]*m.Dashboard, 0) + var dashboards []*m.Dashboard if err := x.Where("org_id=? AND slug=?", query.OrgId, query.Slug).Find(&dashboards); err != nil { return err From 3db328f516d61caaa3ea7f6a2338395c4624f3de Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 1 Feb 2018 16:30:48 +0100 Subject: [PATCH 44/44] fix for dashboard/folder url's when having a sub path in root_url config --- .../ManageDashboards/FolderPermissions.tsx | 1 + public/app/core/services/bridge_srv.ts | 23 +++++-------------- public/app/core/specs/bridge_srv.jest.ts | 22 ------------------ public/app/core/specs/location_util.jest.ts | 16 +++++++++++++ public/app/core/utils/location_util.ts | 14 +++++++++++ .../dashboard/folder_dashboards_ctrl.ts | 7 ++++-- public/app/features/panel/solo_panel_ctrl.ts | 4 +++- public/app/routes/dashboard_loaders.ts | 12 ++++++---- 8 files changed, 53 insertions(+), 46 deletions(-) delete mode 100644 public/app/core/specs/bridge_srv.jest.ts create mode 100644 public/app/core/specs/location_util.jest.ts create mode 100644 public/app/core/utils/location_util.ts diff --git a/public/app/containers/ManageDashboards/FolderPermissions.tsx b/public/app/containers/ManageDashboards/FolderPermissions.tsx index e6e788b7585..07700923b54 100644 --- a/public/app/containers/ManageDashboards/FolderPermissions.tsx +++ b/public/app/containers/ManageDashboards/FolderPermissions.tsx @@ -17,6 +17,7 @@ export class FolderPermissions extends Component { loadStore() { const { nav, folder, view } = this.props; return folder.load(view.routeParams.get('uid') as string).then(res => { + view.updatePathAndQuery(`${res.meta.url}/permissions`, {}, {}); return nav.initFolderNav(toJS(folder.folder), 'manage-folder-permissions'); }); } diff --git a/public/app/core/services/bridge_srv.ts b/public/app/core/services/bridge_srv.ts index ed4abb9b894..4a5649a6c52 100644 --- a/public/app/core/services/bridge_srv.ts +++ b/public/app/core/services/bridge_srv.ts @@ -1,30 +1,18 @@ import coreModule from 'app/core/core_module'; -import config from 'app/core/config'; import appEvents from 'app/core/app_events'; import { store } from 'app/stores/store'; import { reaction } from 'mobx'; +import locationUtil from 'app/core/utils/location_util'; // Services that handles angular -> mobx store sync & other react <-> angular sync export class BridgeSrv { - private appSubUrl; private fullPageReloadRoutes; /** @ngInject */ constructor(private $location, private $timeout, private $window, private $rootScope, private $route) { - this.appSubUrl = config.appSubUrl; this.fullPageReloadRoutes = ['/logout']; } - // Angular's $location does not like and absolute urls - stripBaseFromUrl(url = '') { - const appSubUrl = this.appSubUrl; - const stripExtraChars = appSubUrl.endsWith('/') ? 1 : 0; - const urlWithoutBase = - url.length > 0 && url.indexOf(appSubUrl) === 0 ? url.slice(appSubUrl.length - stripExtraChars) : url; - - return urlWithoutBase; - } - init() { this.$rootScope.$on('$routeUpdate', (evt, data) => { let angularUrl = this.$location.url(); @@ -41,17 +29,18 @@ export class BridgeSrv { () => store.view.currentUrl, currentUrl => { let angularUrl = this.$location.url(); - if (angularUrl !== currentUrl) { + const url = locationUtil.stripBaseFromUrl(currentUrl); + if (angularUrl !== url) { this.$timeout(() => { - this.$location.url(currentUrl); + this.$location.url(url); }); - console.log('store updating angular $location.url', currentUrl); + console.log('store updating angular $location.url', url); } } ); appEvents.on('location-change', payload => { - const urlWithoutBase = this.stripBaseFromUrl(payload.href); + const urlWithoutBase = locationUtil.stripBaseFromUrl(payload.href); if (this.fullPageReloadRoutes.indexOf(urlWithoutBase) > -1) { this.$window.location.href = payload.href; return; diff --git a/public/app/core/specs/bridge_srv.jest.ts b/public/app/core/specs/bridge_srv.jest.ts deleted file mode 100644 index 2e32f9bd600..00000000000 --- a/public/app/core/specs/bridge_srv.jest.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { BridgeSrv } from 'app/core/services/bridge_srv'; - -jest.mock('app/core/config', () => { - return { - appSubUrl: '/subUrl', - }; -}); - -describe('BridgeSrv', () => { - let searchSrv; - - beforeEach(() => { - searchSrv = new BridgeSrv(null, null, null, null, null); - }); - - describe('With /subUrl as appSubUrl', () => { - it('/subUrl should be stripped', () => { - const urlWithoutMaster = searchSrv.stripBaseFromUrl('/subUrl/grafana/'); - expect(urlWithoutMaster).toBe('/grafana/'); - }); - }); -}); diff --git a/public/app/core/specs/location_util.jest.ts b/public/app/core/specs/location_util.jest.ts new file mode 100644 index 00000000000..c109ab7315a --- /dev/null +++ b/public/app/core/specs/location_util.jest.ts @@ -0,0 +1,16 @@ +import locationUtil from 'app/core/utils/location_util'; + +jest.mock('app/core/config', () => { + return { + appSubUrl: '/subUrl', + }; +}); + +describe('locationUtil', () => { + describe('With /subUrl as appSubUrl', () => { + it('/subUrl should be stripped', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/grafana/'); + expect(urlWithoutMaster).toBe('/grafana/'); + }); + }); +}); diff --git a/public/app/core/utils/location_util.ts b/public/app/core/utils/location_util.ts new file mode 100644 index 00000000000..f8d6aa4ee5f --- /dev/null +++ b/public/app/core/utils/location_util.ts @@ -0,0 +1,14 @@ +import config from 'app/core/config'; + +const _stripBaseFromUrl = url => { + const appSubUrl = config.appSubUrl; + const stripExtraChars = appSubUrl.endsWith('/') ? 1 : 0; + const urlWithoutBase = + url.length > 0 && url.indexOf(appSubUrl) === 0 ? url.slice(appSubUrl.length - stripExtraChars) : url; + + return urlWithoutBase; +}; + +export default { + stripBaseFromUrl: _stripBaseFromUrl, +}; diff --git a/public/app/features/dashboard/folder_dashboards_ctrl.ts b/public/app/features/dashboard/folder_dashboards_ctrl.ts index dd0bf7772d8..8ee942445ae 100644 --- a/public/app/features/dashboard/folder_dashboards_ctrl.ts +++ b/public/app/features/dashboard/folder_dashboards_ctrl.ts @@ -1,4 +1,5 @@ import { FolderPageLoader } from './folder_page_loader'; +import locationUtil from 'app/core/utils/location_util'; export class FolderDashboardsCtrl { navModel: any; @@ -13,8 +14,10 @@ export class FolderDashboardsCtrl { const loader = new FolderPageLoader(this.backendSrv); loader.load(this, this.uid, 'manage-folder-dashboards').then(folder => { - if ($location.path() !== folder.meta.url) { - $location.path(folder.meta.url).replace(); + const url = locationUtil.stripBaseFromUrl(folder.meta.url); + + if (url !== $location.path()) { + $location.path(url).replace(); } }); } diff --git a/public/app/features/panel/solo_panel_ctrl.ts b/public/app/features/panel/solo_panel_ctrl.ts index 575c9e4c3b9..6e1c8a4db65 100644 --- a/public/app/features/panel/solo_panel_ctrl.ts +++ b/public/app/features/panel/solo_panel_ctrl.ts @@ -1,4 +1,5 @@ import angular from 'angular'; +import locationUtil from 'app/core/utils/location_util'; export class SoloPanelCtrl { /** @ngInject */ @@ -17,7 +18,8 @@ export class SoloPanelCtrl { if (!($routeParams.type === 'script' || $routeParams.type === 'snapshot') && !$routeParams.uid) { backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { if (res) { - $location.path(res.meta.url.replace('/d/', '/d-solo/')); + const url = locationUtil.stripBaseFromUrl(res.meta.url.replace('/d/', '/d-solo/')); + $location.path(url).replace(); } }); return; diff --git a/public/app/routes/dashboard_loaders.ts b/public/app/routes/dashboard_loaders.ts index 80e74819069..e17ca6fb054 100644 --- a/public/app/routes/dashboard_loaders.ts +++ b/public/app/routes/dashboard_loaders.ts @@ -1,8 +1,9 @@ import coreModule from 'app/core/core_module'; +import locationUtil from 'app/core/utils/location_util'; export class LoadDashboardCtrl { /** @ngInject */ - constructor($scope, $routeParams, dashboardLoaderSrv, backendSrv, $location) { + constructor($scope, $routeParams, dashboardLoaderSrv, backendSrv, $location, $browser) { $scope.appEvent('dashboard-fetch-start'); if (!$routeParams.uid && !$routeParams.slug) { @@ -22,15 +23,18 @@ export class LoadDashboardCtrl { if (!($routeParams.type === 'script' || $routeParams.type === 'snapshot') && !$routeParams.uid) { backendSrv.get(`/api/dashboards/db/${$routeParams.slug}`).then(res => { if (res) { - $location.path(res.meta.url).replace(); + const url = locationUtil.stripBaseFromUrl(res.meta.url); + $location.path(url).replace(); } }); return; } dashboardLoaderSrv.loadDashboard($routeParams.type, $routeParams.slug, $routeParams.uid).then(function(result) { - if ($location.path() !== result.meta.url) { - $location.path(result.meta.url).replace(); + const url = locationUtil.stripBaseFromUrl(result.meta.url); + + if (url !== $location.path()) { + $location.path(url).replace(); } if ($routeParams.keepRows) {