Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
467a128
on dashboard api calls - take the id from the beginning of the slug, …
Aug 15, 2019
1e818f6
add dashboard id when showing links to dashboards
Aug 15, 2019
90ef7cc
change path to include new name when renaming dashboards
Aug 15, 2019
8392126
Merge branch 'master' into id-and-slug-for-dashboards
Aug 17, 2019
aba6e3b
move slug generation to backend
Aug 18, 2019
0ae13b0
redirect to new name after changing (this time with a proper promise)
Aug 18, 2019
0783351
oh right, we already have a slug function
Aug 18, 2019
1aedc44
add spec that makes sure that renamed dashboards are redirected to the
Aug 18, 2019
b7a7a55
use id-slug in all Cypress specs
Aug 19, 2019
a851402
move dashboards from /dashboard/:slug to /dashboards/:id-:name_as_slug
Aug 19, 2019
75b71d7
Merge branch 'master' into id-and-slug-for-dashboards
gabrieldutra Jun 26, 2020
6ea5b97
Update dashboard url as its name changes
gabrieldutra Jun 26, 2020
8e5f491
Update separator to be "/"
gabrieldutra Jun 26, 2020
3f47fb8
Update missing dashboard urls
gabrieldutra Jun 26, 2020
2649292
Update api not to depend on int id
gabrieldutra Jun 26, 2020
59a9143
Use '-' instead of '/' as separator and update Dashboard.get calls
gabrieldutra Jun 30, 2020
169200e
slug -> name_as_slug
gabrieldutra Jun 30, 2020
1c9bbbe
Keep slug urls on cypress
gabrieldutra Jun 30, 2020
2cf590b
Update route path
gabrieldutra Jul 1, 2020
49482d9
Use legacy attr for GET
gabrieldutra Jul 2, 2020
52041a3
Use getter for urlForDashboard
gabrieldutra Jul 2, 2020
2ccc4c1
Update dashboard url when loaded by slug
gabrieldutra Jul 2, 2020
08c14de
Update Dashboard routes to use id instead of slug
gabrieldutra Jul 2, 2020
d7728d1
Update Dashboard handler tests
gabrieldutra Jul 2, 2020
d166069
Update Cypress tests
gabrieldutra Jul 3, 2020
210a755
Merge branch 'master' into id-and-slug-for-dashboards
gabrieldutra Jul 3, 2020
7a8cb0e
Fix create new dashboard spec
gabrieldutra Jul 3, 2020
0957129
Merge branch 'master' into id-and-slug-for-dashboards
gabrieldutra Jul 6, 2020
eb82583
Use axios { params }
gabrieldutra Jul 6, 2020
fd26a9f
Drop Ternary operator
gabrieldutra Jul 13, 2020
6e68b91
Send updated slug directly in 'slug' attr
gabrieldutra Jul 13, 2020
2de74b3
Merge branch 'master' into id-and-slug-for-dashboards
gabrieldutra Jul 13, 2020
cd867a7
Update multiple urls Dashboard test name
gabrieldutra Jul 13, 2020
212d4b4
Update route names
gabrieldutra Jul 13, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions client/app/components/dashboards/CreateDashboardDialog.jsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { trim } from "lodash";
import React, { useState } from "react";
import { axios } from "@/services/axios";
import Modal from "antd/lib/modal";
import Input from "antd/lib/input";
import DynamicComponent from "@/components/DynamicComponent";
import { wrap as wrapDialog, DialogPropType } from "@/components/DialogWrapper";
import navigateTo from "@/components/ApplicationArea/navigateTo";
import recordEvent from "@/services/recordEvent";
import { policy } from "@/services/policy";
import { Dashboard } from "@/services/dashboard";

function CreateDashboardDialog({ dialog }) {
const [name, setName] = useState("");
Expand All @@ -25,9 +25,9 @@ function CreateDashboardDialog({ dialog }) {
if (name !== "") {
setSaveInProgress(true);

axios.post("api/dashboards", { name }).then(data => {
Dashboard.save({ name }).then(data => {
dialog.close();
navigateTo(`dashboard/${data.slug}?edit`);
navigateTo(`${data.url}?edit`);
});
recordEvent("create", "dashboard");
}
Expand Down
4 changes: 2 additions & 2 deletions client/app/components/queries/AddToDashboardDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function AddToDashboardDialog({ dialog, visualization }) {

function addWidgetToDashboard() {
// Load dashboard with all widgets
Dashboard.get({ slug: selectedDashboard.slug })
Dashboard.get(selectedDashboard)
.then(dashboard => {
dashboard.addWidget(visualization);
return dashboard;
Expand All @@ -51,7 +51,7 @@ function AddToDashboardDialog({ dialog, visualization }) {
notification.success(
"Widget added to dashboard",
<React.Fragment>
<a href={`dashboard/${dashboard.slug}`} onClick={() => notification.close(key)}>
<a href={`${dashboard.url}`} onClick={() => notification.close(key)}>
{dashboard.name}
</a>
<QueryTagsControl isDraft={dashboard.is_draft} tags={dashboard.tags} />
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/dashboards/DashboardList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class DashboardList extends React.Component {
Columns.custom.sortable(
(text, item) => (
<React.Fragment>
<a className="table-main-title" href={"dashboard/" + item.slug} data-test={item.slug}>
<a className="table-main-title" href={item.url} data-test={`DashboardId${item.id}`}>
{item.name}
</a>
<DashboardTagsControl
Expand Down
31 changes: 25 additions & 6 deletions client/app/pages/dashboards/DashboardPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { Dashboard } from "@/services/dashboard";
import recordEvent from "@/services/recordEvent";
import resizeObserver from "@/services/resizeObserver";
import routes from "@/services/routes";
import location from "@/services/location";
import url from "@/services/url";
import useImmutableCallback from "@/lib/hooks/useImmutableCallback";

import useDashboard from "./hooks/useDashboard";
Expand Down Expand Up @@ -109,7 +111,7 @@ function DashboardComponent(props) {
}, [pageContainer, editingLayout]);

return (
<div className="container" ref={setPageContainer}>
<div className="container" ref={setPageContainer} data-test={`DashboardId${dashboard.id}Container`}>
<DashboardHeader dashboardOptions={dashboardOptions} />
{!isEmpty(globalParameters) && (
<div className="dashboard-parameters m-b-10 p-15 bg-white tiled" data-test="DashboardParameters">
Expand Down Expand Up @@ -145,35 +147,52 @@ DashboardComponent.propTypes = {
dashboard: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
};

function DashboardPage({ dashboardSlug, onError }) {
function DashboardPage({ dashboardSlug, dashboardId, onError }) {
const [dashboard, setDashboard] = useState(null);
const handleError = useImmutableCallback(onError);

useEffect(() => {
Dashboard.get({ slug: dashboardSlug })
Dashboard.get({ id: dashboardId, slug: dashboardSlug })
.then(dashboardData => {
recordEvent("view", "dashboard", dashboardData.id);
setDashboard(dashboardData);

// if loaded by slug, update location url to use the id
if (!dashboardId) {
location.setPath(url.parse(dashboardData.url).pathname, true);
}
})
.catch(handleError);
}, [dashboardSlug, handleError]);
}, [dashboardId, dashboardSlug, handleError]);

return <div className="dashboard-page">{dashboard && <DashboardComponent dashboard={dashboard} />}</div>;
}

DashboardPage.propTypes = {
dashboardSlug: PropTypes.string.isRequired,
dashboardSlug: PropTypes.string,
dashboardId: PropTypes.string,
onError: PropTypes.func,
};

DashboardPage.defaultProps = {
dashboardSlug: null,
dashboardId: null,
onError: PropTypes.func,
};

// route kept for backward compatibility
routes.register(
"Dashboards.ViewOrEdit",
"Dashboards.LegacyViewOrEdit",
routeWithUserSession({
path: "/dashboard/:dashboardSlug",
render: pageProps => <DashboardPage {...pageProps} />,
})
);

routes.register(
"Dashboards.ViewOrEdit",
routeWithUserSession({
path: "/dashboards/:dashboardId([^-]+)(-.*)?",
render: pageProps => <DashboardPage {...pageProps} />,
})
);
13 changes: 8 additions & 5 deletions client/app/pages/dashboards/hooks/useDashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useState, useEffect, useMemo, useCallback, useRef } from "react";
import { isEmpty, includes, compact, map, has, pick, keys, extend, every, get } from "lodash";
import notification from "@/services/notification";
import location from "@/services/location";
import url from "@/services/url";
import { Dashboard, collectDashboardFilters } from "@/services/dashboard";
import { currentUser } from "@/services/auth";
import recordEvent from "@/services/recordEvent";
Expand Down Expand Up @@ -63,15 +64,17 @@ function useDashboard(dashboardData) {
const updateDashboard = useCallback(
(data, includeVersion = true) => {
setDashboard(currentDashboard => extend({}, currentDashboard, data));
// for some reason the request uses the id as slug
data = { ...data, slug: dashboard.id };
data = { ...data, id: dashboard.id };
if (includeVersion) {
data = { ...data, version: dashboard.version };
}
return Dashboard.save(data)
.then(updatedDashboard =>
setDashboard(currentDashboard => extend({}, currentDashboard, pick(updatedDashboard, keys(data))))
)
.then(updatedDashboard => {
setDashboard(currentDashboard => extend({}, currentDashboard, pick(updatedDashboard, keys(data))));
if (has(data, "name")) {
location.setPath(url.parse(updatedDashboard.url).pathname, true);
}
})
.catch(error => {
const status = get(error, "response.status");
if (status === 403) {
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/home/Home.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function DashboardAndQueryFavoritesList() {
<FavoriteList
title="Favorite Dashboards"
resource={Dashboard}
itemUrl={dashboard => `dashboard/${dashboard.slug}`}
itemUrl={dashboard => dashboard.url}
emptyState={
<p>
<span className="btn-favourite m-r-5">
Expand Down
23 changes: 18 additions & 5 deletions client/app/services/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { currentUser } from "@/services/auth";
import location from "@/services/location";
import { cloneParameter } from "@/services/parameters";

export const urlForDashboard = ({ id, slug }) => `dashboards/${id}-${slug}`;

export function collectDashboardFilters(dashboard, queryResults, urlParams) {
const filters = {};
_.each(queryResults, queryResult => {
Expand Down Expand Up @@ -123,6 +125,11 @@ function calculateNewWidgetPosition(existingWidgets, newWidget) {

export function Dashboard(dashboard) {
_.extend(this, dashboard);
Object.defineProperty(this, "url", {
get: function() {
return urlForDashboard(this);
},
});
}

function prepareDashboardWidgets(widgets) {
Expand All @@ -147,17 +154,23 @@ function transformResponse(data) {
return data;
}

const saveOrCreateUrl = data => (data.slug ? `api/dashboards/${data.slug}` : "api/dashboards");
const saveOrCreateUrl = data => (data.id ? `api/dashboards/${data.id}` : "api/dashboards");
const DashboardService = {
get: ({ slug }) => axios.get(`api/dashboards/${slug}`).then(transformResponse),
get: ({ id, slug }) => {
const params = {};
if (!id) {
params.legacy = null;
}
return axios.get(`api/dashboards/${id || slug}`, { params }).then(transformResponse);
},
getByToken: ({ token }) => axios.get(`api/dashboards/public/${token}`).then(transformResponse),
save: data => axios.post(saveOrCreateUrl(data), data).then(transformResponse),
delete: ({ slug }) => axios.delete(`api/dashboards/${slug}`).then(transformResponse),
delete: ({ id }) => axios.delete(`api/dashboards/${id}`).then(transformResponse),
query: params => axios.get("api/dashboards", { params }).then(transformResponse),
recent: params => axios.get("api/dashboards/recent", { params }).then(transformResponse),
favorites: params => axios.get("api/dashboards/favorites", { params }).then(transformResponse),
favorite: ({ slug }) => axios.post(`api/dashboards/${slug}/favorite`),
unfavorite: ({ slug }) => axios.delete(`api/dashboards/${slug}/favorite`),
favorite: ({ id }) => axios.post(`api/dashboards/${id}/favorite`),
unfavorite: ({ id }) => axios.delete(`api/dashboards/${id}/favorite`),
};

_.extend(Dashboard, DashboardService);
Expand Down
37 changes: 26 additions & 11 deletions client/cypress/integration/dashboard/dashboard_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ describe("Dashboard", () => {
});

cy.wait("@NewDashboard").then(xhr => {
const slug = Cypress._.get(xhr, "response.body.slug");
assert.isDefined(slug, "Dashboard api call returns slug");
const id = Cypress._.get(xhr, "response.body.id");
assert.isDefined(id, "Dashboard api call returns id");

cy.visit("/dashboards");
cy.getByTestId("DashboardLayoutContent").within(() => {
cy.getByTestId(slug).should("exist");
cy.getByTestId(`DashboardId${id}`).should("exist");
});
});
});

it("archives dashboard", () => {
createDashboard("Foo Bar").then(({ slug }) => {
cy.visit(`/dashboard/${slug}`);
createDashboard("Foo Bar").then(({ id }) => {
cy.visit(`/dashboards/${id}`);

cy.getByTestId("DashboardMoreButton").click();

Expand All @@ -52,7 +52,22 @@ describe("Dashboard", () => {

cy.visit("/dashboards");
cy.getByTestId("DashboardLayoutContent").within(() => {
cy.getByTestId(slug).should("not.exist");
cy.getByTestId(`DashboardId${id}`).should("not.exist");
});
});
});

it("is accessible through multiple urls", () => {
cy.server();
cy.route("GET", "api/dashboards/*").as("LoadDashboard");
createDashboard("Dashboard multiple urls").then(({ id, slug }) => {
[`/dashboards/${id}`, `/dashboards/${id}-anything-here`, `/dashboard/${slug}`].forEach(url => {
cy.visit(url);
cy.wait("@LoadDashboard");
cy.getByTestId(`DashboardId${id}Container`).should("exist");

// assert it always use the "/dashboards/{id}" path
cy.location("pathname").should("contain", `/dashboards/${id}`);
});
});
});
Expand All @@ -61,9 +76,9 @@ describe("Dashboard", () => {
before(function() {
cy.login();
createDashboard("Foo Bar")
.then(({ slug, id }) => {
this.dashboardUrl = `/dashboard/${slug}`;
this.dashboardEditUrl = `/dashboard/${slug}?edit`;
.then(({ id }) => {
this.dashboardUrl = `/dashboards/${id}`;
this.dashboardEditUrl = `/dashboards/${id}?edit`;
return addTextbox(id, "Hello World!").then(getWidgetTestId);
})
.then(elTestId => {
Expand Down Expand Up @@ -117,8 +132,8 @@ describe("Dashboard", () => {
context("viewport width is at 767px", () => {
before(function() {
cy.login();
createDashboard("Foo Bar").then(({ slug }) => {
this.dashboardUrl = `/dashboard/${slug}`;
createDashboard("Foo Bar").then(({ id }) => {
this.dashboardUrl = `/dashboards/${id}`;
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { expectTagsToContain, typeInTagsSelectAndSave } from "../../support/tags
describe("Dashboard Tags", () => {
beforeEach(function() {
cy.login();
createDashboard("Foo Bar").then(({ slug }) => cy.visit(`/dashboard/${slug}`));
createDashboard("Foo Bar").then(({ id }) => cy.visit(`/dashboards/${id}`));
});

it("is possible to add and edit tags", () => {
Expand Down
2 changes: 1 addition & 1 deletion client/cypress/integration/dashboard/filters_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe("Dashboard Filters", () => {
.as("widget1TestId")
.then(() => createQueryAndAddWidget(dashboard.id, queryData, { position: { col: 4 } }))
.as("widget2TestId")
.then(() => cy.visit(`/dashboard/${dashboard.slug}`));
.then(() => cy.visit(`/dashboards/${dashboard.id}`));
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ describe("Grid compliant widgets", () => {
cy.login();
cy.viewport(1215 + menuWidth, 800);
createDashboard("Foo Bar")
.then(({ slug, id }) => {
this.dashboardUrl = `/dashboard/${slug}`;
.then(({ id }) => {
this.dashboardUrl = `/dashboards/${id}`;
return addTextbox(id, "Hello World!").then(getWidgetTestId);
})
.then(elTestId => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ describe("Parameter Mapping", () => {
beforeEach(function() {
cy.login();
createDashboard("Foo Bar")
.then(({ slug, id }) => {
.then(({ id }) => {
this.dashboardId = id;
this.dashboardUrl = `/dashboard/${slug}`;
this.dashboardUrl = `/dashboards/${id}`;
})
.then(() => {
const queryData = {
Expand Down
4 changes: 2 additions & 2 deletions client/cypress/integration/dashboard/sharing_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { editDashboard, shareDashboard, createQueryAndAddWidget } from "../../su
describe("Dashboard Sharing", () => {
beforeEach(function() {
cy.login();
createDashboard("Foo Bar").then(({ slug, id }) => {
createDashboard("Foo Bar").then(({ id }) => {
this.dashboardId = id;
this.dashboardUrl = `/dashboard/${slug}`;
this.dashboardUrl = `/dashboards/${id}`;
});
});

Expand Down
4 changes: 2 additions & 2 deletions client/cypress/integration/dashboard/textbox_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { getWidgetTestId, editDashboard } from "../../support/dashboard";
describe("Textbox", () => {
beforeEach(function() {
cy.login();
createDashboard("Foo Bar").then(({ slug, id }) => {
createDashboard("Foo Bar").then(({ id }) => {
this.dashboardId = id;
this.dashboardUrl = `/dashboard/${slug}`;
this.dashboardUrl = `/dashboards/${id}`;
});
});

Expand Down
4 changes: 2 additions & 2 deletions client/cypress/integration/dashboard/widget_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { createQueryAndAddWidget, editDashboard, resizeBy } from "../../support/
describe("Widget", () => {
beforeEach(function() {
cy.login();
createDashboard("Foo Bar").then(({ slug, id }) => {
createDashboard("Foo Bar").then(({ id }) => {
this.dashboardId = id;
this.dashboardUrl = `/dashboard/${slug}`;
this.dashboardUrl = `/dashboards/${id}`;
});
});

Expand Down
2 changes: 1 addition & 1 deletion client/cypress/integration/visualizations/pivot_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe("Pivot", () => {

createDashboard("Pivot Visualization")
.then(dashboard => {
this.dashboardUrl = `/dashboard/${dashboard.slug}`;
this.dashboardUrl = `/dashboards/${dashboard.id}`;
return cy.all(
pivotTables.map(pivot => () =>
createVisualization(this.queryId, "PIVOT", pivot.name, pivot.options).then(visualization =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe("Sankey and Sunburst", () => {

it("takes a snapshot with Sunburst (1 - 5 stages)", function() {
createDashboard("Sunburst Visualization").then(dashboard => {
this.dashboardUrl = `/dashboard/${dashboard.slug}`;
this.dashboardUrl = `/dashboards/${dashboard.id}`;
return cy
.all(
STAGES_WIDGETS.map(sunburst => () =>
Expand All @@ -123,7 +123,7 @@ describe("Sankey and Sunburst", () => {

it("takes a snapshot with Sankey (1 - 5 stages)", function() {
createDashboard("Sankey Visualization").then(dashboard => {
this.dashboardUrl = `/dashboard/${dashboard.slug}`;
this.dashboardUrl = `/dashboards/${dashboard.id}`;
return cy
.all(
STAGES_WIDGETS.map(sankey => () =>
Expand Down
Loading