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!: pass datasource_type and datasource_id to form_data #19981

Merged
merged 5 commits into from
Jun 2, 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
10 changes: 10 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,18 @@ export const URL_PARAMS = {
name: 'slice_id',
type: 'string',
},
datasourceId: {
name: 'datasource_id',
type: 'string',
},
datasetId: {
name: 'dataset_id',
type: 'string',
},
datasourceType: {
name: 'datasource_type',
type: 'string',
},
dashboardId: {
name: 'dashboard_id',
type: 'string',
Expand All @@ -88,6 +96,8 @@ export const URL_PARAMS = {
export const RESERVED_CHART_URL_PARAMS: string[] = [
URL_PARAMS.formDataKey.name,
URL_PARAMS.sliceId.name,
URL_PARAMS.datasourceId.name,
URL_PARAMS.datasourceType.name,
Copy link
Member Author

Choose a reason for hiding this comment

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

@michael-s-molina this could be an old URL, too? I'll datasetId back in here..

URL_PARAMS.datasetId.name,
];
export const RESERVED_DASHBOARD_URL_PARAMS: string[] = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ export default class Chart extends React.Component {
: undefined;
const key = await postFormData(
this.props.datasource.id,
this.props.datasource.type,
this.props.formData,
this.props.slice.slice_id,
nextTabId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ test('generates a new form_data param when none is available', async () => {
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('dataset_id'),
expect.stringMatching('datasource_id'),
);
replaceState.mockRestore();
});
Expand All @@ -109,7 +109,7 @@ test('generates a different form_data param when one is provided and is mounting
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('dataset_id'),
expect.stringMatching('datasource_id'),
);
replaceState.mockRestore();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,24 @@ const ExplorePanelContainer = styled.div`
`;

const updateHistory = debounce(
async (formData, datasetId, isReplace, standalone, force, title, tabId) => {
async (
formData,
datasourceId,
datasourceType,
isReplace,
standalone,
force,
title,
tabId,
) => {
const payload = { ...formData };
const chartId = formData.slice_id;
const additionalParam = {};
if (chartId) {
additionalParam[URL_PARAMS.sliceId.name] = chartId;
} else {
additionalParam[URL_PARAMS.datasetId.name] = datasetId;
additionalParam[URL_PARAMS.datasourceId.name] = datasourceId;
additionalParam[URL_PARAMS.datasourceType.name] = datasourceType;
}

const urlParams = payload?.url_params || {};
Expand All @@ -173,11 +183,24 @@ const updateHistory = debounce(
let key;
let stateModifier;
if (isReplace) {
key = await postFormData(datasetId, formData, chartId, tabId);
key = await postFormData(
datasourceId,
datasourceType,
formData,
chartId,
tabId,
);
stateModifier = 'replaceState';
} else {
key = getUrlParam(URL_PARAMS.formDataKey);
await putFormData(datasetId, key, formData, chartId, tabId);
await putFormData(
datasourceId,
datasourceType,
key,
formData,
chartId,
tabId,
);
stateModifier = 'pushState';
}
const url = mountExploreUrl(
Expand Down Expand Up @@ -229,11 +252,12 @@ function ExploreViewContainer(props) {
dashboardId: props.dashboardId,
}
: props.form_data;
const datasetId = props.datasource.id;
const { id: datasourceId, type: datasourceType } = props.datasource;

updateHistory(
formData,
datasetId,
datasourceId,
datasourceType,
isReplace,
props.standalone,
props.force,
Expand All @@ -245,6 +269,7 @@ function ExploreViewContainer(props) {
props.dashboardId,
props.form_data,
props.datasource.id,
props.datasource.type,
props.standalone,
props.force,
tabId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ class DatasourceControl extends React.PureComponent {
const isMissingDatasource = datasource.id == null;
let isMissingParams = false;
if (isMissingDatasource) {
const datasetId = getUrlParam(URL_PARAMS.datasetId);
const datasourceId = getUrlParam(URL_PARAMS.datasourceId);
const sliceId = getUrlParam(URL_PARAMS.sliceId);
if (!datasetId && !sliceId) {
if (!datasourceId && !sliceId) {
isMissingParams = true;
}
}
Expand Down
29 changes: 22 additions & 7 deletions superset-frontend/src/explore/exploreUtils/formData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import { omit } from 'lodash';
import { SupersetClient, JsonObject } from '@superset-ui/core';

type Payload = {
dataset_id: number;
datasource_id: number;
datasource_type: string;
form_data: string;
chart_id?: number;
};
Expand All @@ -42,12 +43,14 @@ const assembleEndpoint = (key?: string, tabId?: string) => {
};

const assemblePayload = (
datasetId: number,
datasourceId: number,
datasourceType: string,
formData: JsonObject,
chartId?: number,
) => {
const payload: Payload = {
dataset_id: datasetId,
datasource_id: datasourceId,
datasource_type: datasourceType,
form_data: JSON.stringify(sanitizeFormData(formData)),
};
if (chartId) {
Expand All @@ -57,24 +60,36 @@ const assemblePayload = (
};

export const postFormData = (
datasetId: number,
datasourceId: number,
datasourceType: string,
formData: JsonObject,
chartId?: number,
tabId?: string,
): Promise<string> =>
SupersetClient.post({
endpoint: assembleEndpoint(undefined, tabId),
jsonPayload: assemblePayload(datasetId, formData, chartId),
jsonPayload: assemblePayload(
datasourceId,
datasourceType,
formData,
chartId,
),
}).then(r => r.json.key);

export const putFormData = (
datasetId: number,
datasourceId: number,
datasourceType: string,
key: string,
formData: JsonObject,
chartId?: number,
tabId?: string,
): Promise<string> =>
SupersetClient.put({
endpoint: assembleEndpoint(key, tabId),
jsonPayload: assemblePayload(datasetId, formData, chartId),
jsonPayload: assemblePayload(
datasourceId,
datasourceType,
formData,
chartId,
),
}).then(r => r.json.message);
3 changes: 2 additions & 1 deletion superset/cachekeys/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
datasource_type_description,
datasource_uid_description,
)
from superset.utils.core import DatasourceType


class Datasource(Schema):
Expand All @@ -36,7 +37,7 @@ class Datasource(Schema):
)
datasource_type = fields.String(
description=datasource_type_description,
validate=validate.OneOf(choices=("druid", "table", "view")),
validate=validate.OneOf(choices=[ds.value for ds in DatasourceType]),
required=True,
)

Expand Down
7 changes: 4 additions & 3 deletions superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from superset.utils import pandas_postprocessing, schema as utils
from superset.utils.core import (
AnnotationType,
DatasourceType,
FilterOperator,
PostProcessingBoxplotWhiskerType,
PostProcessingContributionOrientation,
Expand Down Expand Up @@ -198,7 +199,7 @@ class ChartPostSchema(Schema):
datasource_id = fields.Integer(description=datasource_id_description, required=True)
datasource_type = fields.String(
description=datasource_type_description,
validate=validate.OneOf(choices=("druid", "table", "view")),
validate=validate.OneOf(choices=[ds.value for ds in DatasourceType]),
required=True,
)
datasource_name = fields.String(
Expand Down Expand Up @@ -244,7 +245,7 @@ class ChartPutSchema(Schema):
)
datasource_type = fields.String(
description=datasource_type_description,
validate=validate.OneOf(choices=("druid", "table", "view")),
validate=validate.OneOf(choices=[ds.value for ds in DatasourceType]),
allow_none=True,
)
dashboards = fields.List(fields.Integer(description=dashboards_description))
Expand Down Expand Up @@ -983,7 +984,7 @@ class ChartDataDatasourceSchema(Schema):
)
type = fields.String(
description="Datasource type",
validate=validate.OneOf(choices=("druid", "table")),
validate=validate.OneOf(choices=[ds.value for ds in DatasourceType]),
)


Expand Down
18 changes: 17 additions & 1 deletion superset/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,24 @@ def __init__(self) -> None:
super().__init__([_("Some roles do not exist")], field_name="roles")


class DatasourceTypeInvalidError(ValidationError):
status = 422

def __init__(self) -> None:
super().__init__(
[_("Datasource type is invalid")], field_name="datasource_type"
)


class DatasourceNotFoundValidationError(ValidationError):
status = 404

def __init__(self) -> None:
super().__init__([_("Dataset does not exist")], field_name="datasource_id")
super().__init__([_("Datasource does not exist")], field_name="datasource_id")


class QueryNotFoundValidationError(ValidationError):
status = 404

def __init__(self) -> None:
super().__init__([_("Query does not exist")], field_name="datasource_id")
6 changes: 3 additions & 3 deletions superset/dao/datasource/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@
class DatasourceDAO(BaseDAO):

sources: Dict[DatasourceType, Type[Datasource]] = {
DatasourceType.SQLATABLE: SqlaTable,
DatasourceType.TABLE: SqlaTable,
DatasourceType.QUERY: Query,
DatasourceType.SAVEDQUERY: SavedQuery,
DatasourceType.DATASET: Dataset,
DatasourceType.TABLE: Table,
DatasourceType.SLTABLE: Table,
}

@classmethod
Expand All @@ -66,7 +66,7 @@ def get_datasource(

@classmethod
def get_all_sqlatables_datasources(cls, session: Session) -> List[Datasource]:
source_class = DatasourceDAO.sources[DatasourceType.SQLATABLE]
source_class = DatasourceDAO.sources[DatasourceType.TABLE]
qry = session.query(source_class)
qry = source_class.default_query(qry)
return qry.all()
Expand Down
4 changes: 3 additions & 1 deletion superset/databases/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import TabState
from superset.utils.core import DatasourceType

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -75,7 +76,8 @@ def get_related_objects(cls, database_id: int) -> Dict[str, Any]:
charts = (
db.session.query(Slice)
.filter(
Slice.datasource_id.in_(dataset_ids), Slice.datasource_type == "table"
Slice.datasource_id.in_(dataset_ids),
Slice.datasource_type == DatasourceType.TABLE,
)
.all()
)
Expand Down
4 changes: 3 additions & 1 deletion superset/datasets/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from superset.models.core import Database
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.utils.core import DatasourceType
from superset.views.base import DatasourceFilter

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -56,7 +57,8 @@ def get_related_objects(database_id: int) -> Dict[str, Any]:
charts = (
db.session.query(Slice)
.filter(
Slice.datasource_id == database_id, Slice.datasource_type == "table"
Slice.datasource_id == database_id,
Slice.datasource_type == DatasourceType.TABLE,
)
.all()
)
Expand Down
8 changes: 6 additions & 2 deletions superset/examples/birth_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from superset.models.core import Database
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.utils.core import DatasourceType

from ..utils.database import get_example_database
from .helpers import (
Expand Down Expand Up @@ -205,13 +206,16 @@ def create_slices(tbl: SqlaTable, admin_owner: bool) -> Tuple[List[Slice], List[
if admin_owner:
slice_props = dict(
datasource_id=tbl.id,
datasource_type="table",
datasource_type=DatasourceType.TABLE,
owners=[admin],
created_by=admin,
)
else:
slice_props = dict(
datasource_id=tbl.id, datasource_type="table", owners=[], created_by=admin
datasource_id=tbl.id,
datasource_type=DatasourceType.TABLE,
owners=[],
created_by=admin,
)

print("Creating some slices")
Expand Down
3 changes: 2 additions & 1 deletion superset/examples/country_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from superset import db
from superset.connectors.sqla.models import SqlMetric
from superset.models.slice import Slice
from superset.utils.core import DatasourceType

from .helpers import (
get_example_data,
Expand Down Expand Up @@ -112,7 +113,7 @@ def load_country_map_data(only_metadata: bool = False, force: bool = False) -> N
slc = Slice(
slice_name="Birth in France by department in 2016",
viz_type="country_map",
datasource_type="table",
datasource_type=DatasourceType.TABLE,
datasource_id=tbl.id,
params=get_slice_json(slice_data),
)
Expand Down
Loading