QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-04-07 10:31:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: ensure that users viewing chart does not automatically save edit data (#16077)

Browse Source 
* add last_change_at migration

* add last_saved_by db migration

* finish rest of api migration

* run precommit

* fix name

* run precommitt

* remove unused mods

* merge migrations

* Update superset/migrations/versions/6d20ba9ecb33_add_last_saved_at_to_slice_model.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/migrations/versions/6d20ba9ecb33_add_last_saved_at_to_slice_model.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/migrations/versions/f6196627326f_update_chart_permissions.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* fix test

* precommit

* remove print

* fix test

* change test

* test commit

* test 2

* test 3

* third time the charm

* fix put req

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
This commit is contained in:
Phillip Kelley-Dotson
2021-08-10 09:29:49 -07:00
committed by GitHub
parent 63ace7b288
commit f0e3b68cc2
10 changed files with 150 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
superset-frontend/src/explore/components/ExploreChartPanel.jsx
View File

@@ -147,6 +147,7 @@ const ExploreChartPanel = props => {
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
query_context: JSON.stringify(queryContext),
query_context_generation: true,
}),
});
}

23
superset-frontend/src/views/CRUD/chart/ChartList.tsx
View File

@@ -25,6 +25,7 @@ import {
import React, { useMemo, useState } from 'react';
import rison from 'rison';
import { uniqBy } from 'lodash';
import moment from 'moment';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import {
createErrorHandler,
@@ -270,23 +271,33 @@ function ChartList(props: ChartListProps) {
Cell: ({
row: {
original: {
changed_by_name: changedByName,
last_saved_by: lastSavedBy,
changed_by_url: changedByUrl,
},
},
}: any) => <a href={changedByUrl}>{changedByName}</a>,
}: any) => (
<a href={changedByUrl}>
{lastSavedBy?.first_name
? `${lastSavedBy?.first_name} ${lastSavedBy?.last_name}`
: null}
</a>
),
Header: t('Modified by'),
accessor: 'changed_by.first_name',
accessor: 'last_saved_by',
size: 'xl',
},
{
Cell: ({
row: {
original: { changed_on_delta_humanized: changedOn },
original: { last_saved_at: lastSavedAt },
},
}: any) => <span className="no-wrap">{changedOn}</span>,
}: any) => (
<span className="no-wrap">
{lastSavedAt ? moment.utc(lastSavedAt).fromNow() : null}
</span>
),
Header: t('Last modified'),
accessor: 'changed_on_delta_humanized',
accessor: 'last_saved_at',
size: 'xl',
},
{

12
superset/charts/api.py
View File

@@ -152,6 +152,10 @@ class ChartRestApi(BaseSupersetModelRestApi):
"description_markeddown",
"edit_url",
"id",
"last_saved_at",
"last_saved_by.id",
"last_saved_by.first_name",
"last_saved_by.last_name",
"owners.first_name",
"owners.id",
"owners.last_name",
@@ -170,12 +174,20 @@ class ChartRestApi(BaseSupersetModelRestApi):
"changed_on_delta_humanized",
"datasource_id",
"datasource_name",
"last_saved_at",
"last_saved_by.id",
"last_saved_by.first_name",
"last_saved_by.last_name",
"slice_name",
"viz_type",
]
search_columns = [
"created_by",
"changed_by",
"last_saved_at",
"last_saved_by.id",
"last_saved_by.first_name",
"last_saved_by.last_name",
"datasource_id",
"datasource_name",
"datasource_type",

3
superset/charts/commands/create.py
View File

@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import logging
from datetime import datetime
from typing import Any, Dict, List, Optional
from flask_appbuilder.models.sqla import Model
@@ -43,6 +44,8 @@ class CreateChartCommand(BaseCommand):
def run(self) -> Model:
self.validate()
try:
self._properties["last_saved_at"] = datetime.now()
self._properties["last_saved_by"] = self._actor
chart = ChartDAO.create(self._properties)
except DAOCreateFailedError as ex:
logger.exception(ex.exception)

4
superset/charts/commands/update.py
View File

@@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import logging
from datetime import datetime
from typing import Any, Dict, List, Optional
from flask_appbuilder.models.sqla import Model
@@ -51,6 +52,9 @@ class UpdateChartCommand(BaseCommand):
def run(self) -> Model:
self.validate()
try:
if self._properties.get("query_context_generation") is None:
self._properties["last_saved_at"] = datetime.now()
self._properties["last_saved_by"] = self._actor
chart = ChartDAO.update(self._model, self._properties)
except DAOUpdateFailedError as ex:
logger.exception(ex.exception)

11
superset/charts/schemas.py
View File

@@ -82,6 +82,11 @@ query_context_description = (
"in order to generate the data the visualization, and in what "
"format the data should be returned."
)
query_context_generation_description = (
"The query context generation represents whether the query_context"
"is user generated or not so that it does not update user modfied"
"state."
)
cache_timeout_description = (
"Duration (in seconds) of the caching timeout "
"for this chart. Note this defaults to the datasource/table"
@@ -176,6 +181,9 @@ class ChartPostSchema(Schema):
allow_none=True,
validate=utils.validate_json,
)
query_context_generation = fields.Boolean(
description=query_context_generation_description, allow_none=True
)
cache_timeout = fields.Integer(
description=cache_timeout_description, allow_none=True
)
@@ -211,6 +219,9 @@ class ChartPutSchema(Schema):
query_context = fields.String(
description=query_context_description, allow_none=True
)
query_context_generation = fields.Boolean(
description=query_context_generation_description, allow_none=True
)
cache_timeout = fields.Integer(
description=cache_timeout_description, allow_none=True
)

66
superset/migrations/versions/6d20ba9ecb33_add_last_saved_at_to_slice_model.py Normal file
View File

@@ -0,0 +1,66 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""add_last_saved_at_to_slice_model
Revision ID: 6d20ba9ecb33
Revises: ('ae1ed299413b', 'f6196627326f')
Create Date: 2021-08-02 21:14:58.200438
"""
# revision identifiers, used by Alembic.
revision = "6d20ba9ecb33"
down_revision = ("ae1ed299413b", "f6196627326f")
import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql
def upgrade():
with op.batch_alter_table("slices") as batch_op:
batch_op.add_column(sa.Column("last_saved_at", sa.DateTime(), nullable=True))
batch_op.add_column(sa.Column("last_saved_by_fk", sa.Integer(), nullable=True))
batch_op.create_foreign_key(
"slices_last_saved_by_fk", "ab_user", ["last_saved_by_fk"], ["id"]
)
# now do data migration, copy values from changed_on and changed_by
slices_table = sa.Table(
"slices",
sa.MetaData(),
sa.Column("changed_on", sa.DateTime(), nullable=True),
sa.Column("changed_by_fk", sa.Integer(), nullable=True),
sa.Column("last_saved_at", sa.DateTime(), nullable=True),
sa.Column("last_saved_by_fk", sa.Integer(), nullable=True),
)
conn = op.get_bind()
conn.execute(
slices_table.update().values(
last_saved_at=slices_table.c.changed_on,
last_saved_by_fk=slices_table.c.changed_by_fk,
)
)
# ### end Alembic commands ###
def downgrade():
with op.batch_alter_table("slices") as batch_op:
batch_op.drop_constraint("slices_last_saved_by_fk", type_="foreignkey")
batch_op.drop_column("last_saved_by_fk")
batch_op.drop_column("last_saved_at")
# ### end Alembic commands ###

9
superset/models/slice.py
View File

@@ -23,7 +23,7 @@ import sqlalchemy as sqla
from flask_appbuilder import Model
from flask_appbuilder.models.decorators import renders
from markupsafe import escape, Markup
from sqlalchemy import Column, ForeignKey, Integer, String, Table, Text
from sqlalchemy import Column, DateTime, ForeignKey, Integer, String, Table, Text
from sqlalchemy.engine.base import Connection
from sqlalchemy.orm import relationship
from sqlalchemy.orm.mapper import Mapper
@@ -71,6 +71,13 @@ class Slice( # pylint: disable=too-many-instance-attributes,too-many-public-met
cache_timeout = Column(Integer)
perm = Column(String(1000))
schema_perm = Column(String(1000))
# the last time a user has saved the chart, changed_on is referencing
# when the database row was last written
last_saved_at = Column(DateTime, nullable=True)
last_saved_by_fk = Column(Integer, ForeignKey("ab_user.id"), nullable=True,)
last_saved_by = relationship(
security_manager.user_model, foreign_keys=[last_saved_by_fk]
)
owners = relationship(security_manager.user_model, secondary=slice_user)
table = relationship(
"SqlaTable",

4
superset/views/core.py
View File

@@ -604,7 +604,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
)
form_data = get_form_data()[0]
try:
datasource_id, datasource_type = get_datasource_info(
datasource_id, datasource_type, form_data
@@ -719,7 +718,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
user_id = g.user.get_id() if g.user else None
form_data, slc = get_form_data(use_slice_data=True)
query_context = request.form.get("query_context")
# Flash the SIP-15 message if the slice is owned by the current user and has not
# been updated, i.e., is not using the [start, end) interval.
if (
@@ -948,6 +946,8 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
slc.viz_type = form_data["viz_type"]
slc.datasource_type = datasource_type
slc.datasource_id = datasource_id
slc.last_saved_by = g.user
slc.last_saved_at = datetime.now()
slc.slice_name = slice_name
slc.query_context = query_context

26
tests/integration_tests/charts/commands_tests.py
View File

@@ -17,15 +17,18 @@
# pylint: disable=no-self-use, invalid-name
import json
from datetime import datetime
from unittest.mock import patch
import pytest
import yaml
from flask import g
from superset import db, security_manager
from superset.charts.commands.exceptions import ChartNotFoundError
from superset.charts.commands.export import ExportChartsCommand
from superset.charts.commands.importers.v1 import ImportChartsCommand
from superset.charts.commands.update import UpdateChartCommand
from superset.commands.exceptions import CommandInvalidError
from superset.commands.importers.exceptions import IncorrectVersionError
from superset.connectors.sqla.models import SqlaTable
@@ -275,3 +278,26 @@ class TestImportChartsCommand(SupersetTestCase):
"database_name": ["Missing data for required field."],
}
}
class TestChartsUpdateCommand(SupersetTestCase):
@patch("superset.views.base.g")
@patch("superset.security.manager.g")
@pytest.mark.usefixtures("load_energy_table_with_slice")
def test_update_v1_response(self, mock_sm_g, mock_g):
""""Test that a chart command updates properties"""
pk = db.session.query(Slice).all()[0].id
actor = security_manager.find_user(username="admin")
mock_g.user = mock_sm_g.user = actor
model_id = pk
json_obj = {
"description": "test for update",
"cache_timeout": None,
"owners": [1],
}
command = UpdateChartCommand(actor, model_id, json_obj)
last_saved_before = db.session.query(Slice).get(pk).last_saved_at
command.run()
chart = db.session.query(Slice).get(pk)
assert chart.last_saved_at != last_saved_before
assert chart.last_saved_by == actor