Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(explore): Fill dashboard name when adding new chart from dashboard view #20129

Merged
merged 4 commits into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions superset-frontend/src/addSlice/AddSliceContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import React, { ReactNode } from 'react';
import rison from 'rison';
import { styled, t, SupersetClient, JsonResponse } from '@superset-ui/core';
import { getUrlParam } from 'src/utils/urlUtils';
import { URL_PARAMS } from 'src/constants';
import { isNullish } from 'src/utils/common';
import Button from 'src/components/Button';
import { Select, Steps } from 'src/components';
import { FormLabel } from 'src/components/Form';
Expand Down Expand Up @@ -195,10 +198,12 @@ export default class AddSliceContainer extends React.PureComponent<
}

exploreUrl() {
const dashboardId = getUrlParam(URL_PARAMS.dashboardId);
const formData = encodeURIComponent(
JSON.stringify({
viz_type: this.state.visType,
datasource: this.state.datasource?.value,
...(!isNullish(dashboardId) && { dashboardId }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a cool function, why nullish?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!dashboardId would exclude dashboardId === 0, which is a valid id 😄

}),
);
return `/superset/explore/?form_data=${formData}`;
Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export const URL_PARAMS = {
name: 'dataset_id',
type: 'string',
},
dashboardId: {
name: 'dashboard_id',
type: 'string',
},
force: {
name: 'force',
type: 'boolean',
Expand Down
14 changes: 12 additions & 2 deletions superset-frontend/src/dashboard/components/DashboardGrid.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const propTypes = {
resizeComponent: PropTypes.func.isRequired,
setDirectPathToChild: PropTypes.func.isRequired,
width: PropTypes.number.isRequired,
dashboardId: PropTypes.number,
};

const defaultProps = {};
Expand Down Expand Up @@ -143,6 +144,7 @@ class DashboardGrid extends React.PureComponent {
editMode,
canEdit,
setEditMode,
dashboardId,
} = this.props;
const columnPlusGutterWidth =
(width + GRID_GUTTER_SIZE) / GRID_COLUMN_COUNT;
Expand All @@ -167,7 +169,11 @@ class DashboardGrid extends React.PureComponent {
</>
}
buttonAction={() => {
window.open('/chart/add', '_blank', 'noopener noreferrer');
window.open(
`/chart/add?dashboard_id=${dashboardId}`,
'_blank',
'noopener noreferrer',
);
}}
image="chart.svg"
/>
Expand All @@ -186,7 +192,11 @@ class DashboardGrid extends React.PureComponent {
</>
}
buttonAction={() => {
window.open('/chart/add', '_blank', 'noopener noreferrer');
window.open(
`/chart/add?dashboard_id=${dashboardId}`,
'_blank',
'noopener noreferrer',
);
}}
image="chart.svg"
/>
Expand Down
7 changes: 6 additions & 1 deletion superset-frontend/src/dashboard/components/SliceAdder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const propTypes = {
editMode: PropTypes.bool,
height: PropTypes.number,
filterboxMigrationState: FILTER_BOX_MIGRATION_STATES,
dashboardId: PropTypes.number,
};

const defaultProps = {
Expand Down Expand Up @@ -276,7 +277,11 @@ class SliceAdder extends React.Component {
buttonStyle="link"
buttonSize="xsmall"
onClick={() =>
window.open('/chart/add', '_blank', 'noopener noreferrer')
window.open(
`/chart/add?dashboard_id=${this.props.dashboardId}`,
'_blank',
'noopener noreferrer',
)
}
>
<Icons.PlusSmall />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ class Tab extends React.PureComponent {
isComponentVisible,
canEdit,
setEditMode,
dashboardId,
} = this.props;

const shouldDisplayEmptyState = tabComponent.children.length === 0;
Expand Down Expand Up @@ -183,7 +184,7 @@ class Tab extends React.PureComponent {
<span>
{t('You can')}{' '}
<a
href="/chart/add"
href={`/chart/add?dashboard_id=${dashboardId}`}
rel="noopener noreferrer"
target="_blank"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,5 +294,5 @@ test('Render tab content with no children, editMode: true, canEdit: true', () =>
).toBeVisible();
expect(
screen.getByRole('link', { name: 'create a new chart' }),
).toHaveAttribute('href', '/chart/add');
).toHaveAttribute('href', '/chart/add?dashboard_id=23');
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function mapStateToProps({ dashboardState, dashboardInfo }) {
return {
editMode: dashboardState.editMode,
canEdit: dashboardInfo.dash_edit_perm,
dashboardId: dashboardInfo.id,
};
}

Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/dashboard/containers/SliceAdder.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ function mapStateToProps(
return {
height: ownProps.height,
userId: dashboardInfo.userId,
dashboardId: dashboardInfo.id,
selectedSliceIds: dashboardState.sliceIds,
slices: sliceEntities.slices,
isLoading: sliceEntities.isLoading,
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/dashboard/stylesheets/dashboard.less
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
/* header has mysterious extra margin */
header.top {
margin-bottom: 2px;
z-index: 10;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main menu items on the right were not clickable because in edit mode the right panel was covering them (they were visible but unclickable because of invisible part of that panel covering them)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test that by opening dashboard in edit mode and then try to click the "+" icon in top right corner

}

body {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,17 @@ function mapStateToProps(state) {
const chartKey = Object.keys(charts)[0];
const chart = charts[chartKey];

let dashboardId = Number(explore.form_data?.dashboardId);
if (Number.isNaN(dashboardId)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?? operator doesn't exclude NaNs. I also don't want to use || operator in order not to exclude dashboards with id 0

dashboardId = undefined;
}

return {
isDatasourceMetaLoading: explore.isDatasourceMetaLoading,
datasource: explore.datasource,
datasource_type: explore.datasource.type,
datasourceId: explore.datasource_id,
dashboardId: explore.form_data ? explore.form_data.dashboardId : undefined,
dashboardId,
controls: explore.controls,
can_overwrite: !!explore.can_overwrite,
can_add: !!explore.can_add,
Expand Down
3 changes: 1 addition & 2 deletions superset-frontend/src/explore/components/SaveModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
checked={this.state.action === 'saveas'}
onChange={() => this.changeAction('saveas')}
>
{' '}
{t('Save as ...')} &nbsp;
{t('Save as...')}
</Radio>
</FormItem>
<hr />
Expand Down
14 changes: 10 additions & 4 deletions superset-frontend/src/views/components/MenuRight.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,21 @@
*/
import React, { Fragment, useState, useEffect } from 'react';
import rison from 'rison';
import { MainNav as Menu } from 'src/components/Menu';
import { useSelector } from 'react-redux';
import { Link } from 'react-router-dom';
import {
t,
styled,
css,
SupersetTheme,
SupersetClient,
} from '@superset-ui/core';
import { MainNav as Menu } from 'src/components/Menu';
import { Tooltip } from 'src/components/Tooltip';
import { Link } from 'react-router-dom';
import Icons from 'src/components/Icons';
import findPermission, { isUserAdmin } from 'src/dashboard/util/findPermission';
import { useSelector } from 'react-redux';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import { RootState } from 'src/dashboard/types';
import LanguagePicker from './LanguagePicker';
import DatabaseModal from '../CRUD/data/database/DatabaseModal';
import { uploadUserPerms } from '../CRUD/utils';
Expand Down Expand Up @@ -89,6 +90,9 @@ const RightMenu = ({
const user = useSelector<any, UserWithPermissionsAndRoles>(
state => state.user,
);
const dashboardId = useSelector<RootState, number | undefined>(
state => state.dashboardInfo?.id,
);

const { roles } = user;
const {
Expand Down Expand Up @@ -162,7 +166,9 @@ const RightMenu = ({
},
{
label: t('Chart'),
url: '/chart/add',
url: Number.isInteger(dashboardId)
? `/chart/add?dashboard_id=${dashboardId}`
: '/chart/add',
icon: 'fa-fw fa-bar-chart',
perm: 'can_write',
view: 'Chart',
Expand Down