Skip to content

Commit

Permalink
fix: Fix Uniqueness check before update for Sqllab Overwrites (apache…
Browse files Browse the repository at this point in the history
…#16859)

* add condition to make sure columns are available before throwing

* fix

* remove database_id from history changed

* refactor update call to datasets

* cleanup

* oops

* prettier
  • Loading branch information
hughhhh committed Sep 29, 2021
1 parent 39efd28 commit 7708da8
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 83 deletions.
35 changes: 30 additions & 5 deletions superset-frontend/src/SqlLab/components/ResultSet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@ import { RadioChangeEvent } from 'antd/lib/radio';
import Button from 'src/components/Button';
import shortid from 'shortid';
import rison from 'rison';
import { styled, t, makeApi } from '@superset-ui/core';
import {
styled,
t,
makeApi,
SupersetClient,
JsonResponse,
} from '@superset-ui/core';
import { debounce } from 'lodash';
import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
import { put as updateDatset } from 'src/api/dataset';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
import Loading from '../../components/Loading';
import ExploreCtasResultsButton from './ExploreCtasResultsButton';
Expand Down Expand Up @@ -133,6 +138,27 @@ const ResultSetErrorMessage = styled.div`
padding-top: ${({ theme }) => 4 * theme.gridUnit}px;
`;

const updateDataset = async (
datasetId: number,
sql: string,
columns: Array<Record<string, any>>,
overrideColumns: boolean,
) => {
const endpoint = `api/v1/dataset/${datasetId}?override_columns=${overrideColumns}`;
const headers = { 'Content-Type': 'application/json' };
const body = JSON.stringify({
sql,
columns,
});

const data: JsonResponse = await SupersetClient.put({
endpoint,
headers,
body,
});
return data.json.result;
};

export default class ResultSet extends React.PureComponent<
ResultSetProps,
ResultSetState
Expand Down Expand Up @@ -236,12 +262,11 @@ export default class ResultSet extends React.PureComponent<
};

handleOverwriteDataset = async () => {
const { sql, results, dbId } = this.props.query;
const { sql, results } = this.props.query;
const { datasetToOverwrite } = this.state;

await updateDatset(
await updateDataset(
datasetToOverwrite.datasetId,
dbId,
sql,
results.selected_columns.map(d => ({ column_name: d.name })),
true,
Expand Down
63 changes: 0 additions & 63 deletions superset-frontend/src/api/dataset.ts

This file was deleted.

4 changes: 1 addition & 3 deletions superset/datasets/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ def run(self) -> Model:
if self._model:
try:
dataset = DatasetDAO.update(
model=self._model,
properties=self._properties,
override_columns=self.override_columns,
model=self._model, properties=self._properties,
)
return dataset
except DAOUpdateFailedError as ex:
Expand Down
15 changes: 3 additions & 12 deletions superset/datasets/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,31 +144,22 @@ def validate_metrics_uniqueness(dataset_id: int, metrics_names: List[str]) -> bo

@classmethod
def update( # pylint: disable=arguments-differ
cls,
model: SqlaTable,
properties: Dict[str, Any],
commit: bool = True,
override_columns: bool = False,
cls, model: SqlaTable, properties: Dict[str, Any], commit: bool = True
) -> Optional[SqlaTable]:
"""
Updates a Dataset model on the metadata DB
"""

if "columns" in properties:
properties["columns"] = cls.update_columns(
model, properties.get("columns", []), commit=commit
)

if "metrics" in properties:
properties["metrics"] = cls.update_metrics(
model, properties.get("metrics", []), commit=commit
)

if override_columns:
# remove columns initially for full refresh
original_properties = properties["columns"]
properties["columns"] = []
super().update(model, properties, commit=commit)
properties["columns"] = original_properties

return super().update(model, properties, commit=False)

@classmethod
Expand Down

0 comments on commit 7708da8

Please sign in to comment.