Skip to content

Commit

Permalink
Merge branch 'master' into villebro/generic-series-limit
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro committed Sep 16, 2021
2 parents 89819a0 + 092ef5b commit 40764d8
Show file tree
Hide file tree
Showing 56 changed files with 872 additions and 675 deletions.
2 changes: 1 addition & 1 deletion .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / stateme
[BASIC]

# Good variable names which should always be accepted, separated by a comma
good-names=_,df,ex,f,i,id,j,k,l,o,pk,Run,ts,v,x
good-names=_,df,ex,f,i,id,j,k,l,o,pk,Run,ts,v,x,y

# Bad variable names which should always be refused, separated by a comma
bad-names=fd,foo,bar,baz,toto,tutu,tata
Expand Down
3 changes: 0 additions & 3 deletions RELEASING/changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
# 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.
#
# pylint: disable=no-value-for-parameter

import csv as lib_csv
import os
import re
Expand Down
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ assists people when migrating to a new version.
### Breaking Changes

- [16660](https://github.com/apache/incubator-superset/pull/16660): The `columns` Jinja parameter has been renamed `table_columns` to make the `columns` query object parameter available in the Jinja context.
- [16711](https://github.com/apache/incubator-superset/pull/16711): The `url_param` Jinja function will now by default escape the result. For instance, the value `O'Brien` will now be changed to `O''Brien`. To disable this behavior, call `url_param` with `escape_result` set to `False`: `url_param("my_key", "my default", escape_result=False)`.

### Potential Downtime
### Deprecations
Expand Down
36 changes: 36 additions & 0 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ test('searches for label or value', async () => {
expect(options[0]).toHaveTextContent(option.label);
});

test('ignores case when searching', async () => {
render(<Select {...defaultProps} />);
await type('george');
expect(await findSelectOption('George')).toBeInTheDocument();
});

test('ignores special keys when searching', async () => {
render(<Select {...defaultProps} />);
await type('{shift}');
expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
});

test('searches for custom fields', async () => {
render(<Select {...defaultProps} optionFilterProps={['label', 'gender']} />);
await type('Liam');
Expand Down Expand Up @@ -593,6 +605,30 @@ test('async - does not fire a new request for the same search input', async () =
expect(loadOptions).toHaveBeenCalledTimes(1);
});

test('async - does not fire a new request if all values have been fetched', async () => {
const mock = jest.fn(loadOptions);
const search = 'George';
const pageSize = OPTIONS.length;
render(<Select {...defaultProps} options={mock} pageSize={pageSize} />);
await open();
expect(mock).toHaveBeenCalledTimes(1);
await type(search);
expect(await findSelectOption(search)).toBeInTheDocument();
expect(mock).toHaveBeenCalledTimes(1);
});

test('async - fires a new request if all values have not been fetched', async () => {
const mock = jest.fn(loadOptions);
const search = 'George';
const pageSize = OPTIONS.length / 2;
render(<Select {...defaultProps} options={mock} pageSize={pageSize} />);
await open();
expect(mock).toHaveBeenCalledTimes(1);
await type(search);
expect(await findSelectOption(search)).toBeInTheDocument();
expect(mock).toHaveBeenCalledTimes(2);
});

/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
Expand Down
47 changes: 36 additions & 11 deletions superset-frontend/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import React, {
ReactElement,
ReactNode,
RefObject,
KeyboardEvent,
UIEvent,
useEffect,
useMemo,
Expand Down Expand Up @@ -209,6 +210,7 @@ const Select = ({
const [page, setPage] = useState(0);
const [totalCount, setTotalCount] = useState(0);
const [loadingEnabled, setLoadingEnabled] = useState(!lazyLoading);
const [allValuesLoaded, setAllValuesLoaded] = useState(false);
const fetchedQueries = useRef(new Map<string, number>());
const mappedMode = isSingleMode
? undefined
Expand Down Expand Up @@ -332,20 +334,35 @@ const Select = ({
});

const handleData = (data: OptionsType) => {
let mergedData: OptionsType = [];
if (data && Array.isArray(data) && data.length) {
const dataValues = new Set();
data.forEach(option =>
dataValues.add(String(option.value).toLocaleLowerCase()),
);

// merges with existing and creates unique options
setSelectOptions(prevOptions => [
...prevOptions,
...data.filter(
newOpt =>
!prevOptions.find(prevOpt => prevOpt.value === newOpt.value),
),
]);
setSelectOptions(prevOptions => {
mergedData = [
...prevOptions.filter(
previousOption =>
!dataValues.has(String(previousOption.value).toLocaleLowerCase()),
),
...data,
];
return mergedData;
});
}
return mergedData;
};

const handlePaginatedFetch = useMemo(
() => (value: string, page: number, pageSize: number) => {
if (allValuesLoaded) {
setIsLoading(false);
setIsTyping(false);
return;
}
const key = `${value};${page};${pageSize}`;
const cachedCount = fetchedQueries.current.get(key);
if (cachedCount) {
Expand All @@ -358,17 +375,24 @@ const Select = ({
const fetchOptions = options as OptionsPagePromise;
fetchOptions(value, page, pageSize)
.then(({ data, totalCount }: OptionsTypePage) => {
handleData(data);
const mergedData = handleData(data);
fetchedQueries.current.set(key, totalCount);
setTotalCount(totalCount);
if (
!fetchOnlyOnSearch &&
value === '' &&
mergedData.length >= totalCount
) {
setAllValuesLoaded(true);
}
})
.catch(onError)
.finally(() => {
setIsLoading(false);
setIsTyping(false);
});
},
[options],
[allValuesLoaded, fetchOnlyOnSearch, options],
);

const handleOnSearch = useMemo(
Expand Down Expand Up @@ -459,8 +483,8 @@ const Select = ({
return error ? <Error error={error} /> : originNode;
};

const onInputKeyDown = () => {
if (isAsync && !isTyping) {
const onInputKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
if (event.key.length === 1 && isAsync && !isTyping) {
setIsTyping(true);
}
};
Expand All @@ -487,6 +511,7 @@ const Select = ({
setSelectOptions(
options && Array.isArray(options) ? options : EMPTY_OPTIONS,
);
setAllValuesLoaded(false);
}, [options]);

useEffect(() => {
Expand Down
4 changes: 4 additions & 0 deletions superset/charts/commands/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import json
from typing import Any, Dict

from flask import g
from sqlalchemy.orm import Session

from superset.models.slice import Slice
Expand All @@ -39,4 +40,7 @@ def import_chart(
if chart.id is None:
session.flush()

if hasattr(g, "user") and g.user:
chart.owners.append(g.user)

return chart
6 changes: 2 additions & 4 deletions superset/commands/importers/v1/examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# pylint: disable=protected-access

from typing import Any, Dict, List, Set, Tuple

from marshmallow import Schema
Expand Down Expand Up @@ -78,16 +76,16 @@ def run(self) -> None:

@classmethod
def _get_uuids(cls) -> Set[str]:
# pylint: disable=protected-access
return (
ImportDatabasesCommand._get_uuids()
| ImportDatasetsCommand._get_uuids()
| ImportChartsCommand._get_uuids()
| ImportDashboardsCommand._get_uuids()
)

# pylint: disable=too-many-locals, arguments-differ
@staticmethod
def _import(
def _import( # pylint: disable=arguments-differ,too-many-locals
session: Session,
configs: Dict[str, Any],
overwrite: bool = False,
Expand Down
18 changes: 13 additions & 5 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from flask import Blueprint
from flask_appbuilder.security.manager import AUTH_DB
from pandas.io.parsers import STR_NA_VALUES
from werkzeug.local import LocalProxy

from superset.jinja_context import BaseTemplateProcessor
from superset.stats_logger import DummyStatsLogger
Expand Down Expand Up @@ -178,9 +179,9 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
# Note: the default impl leverages SqlAlchemyUtils' EncryptedType, which defaults
# to AES-128 under the covers using the app's SECRET_KEY as key material.
#
# pylint: disable=C0103
SQLALCHEMY_ENCRYPTED_FIELD_TYPE_ADAPTER = SQLAlchemyUtilsAdapter

SQLALCHEMY_ENCRYPTED_FIELD_TYPE_ADAPTER = ( # pylint: disable=invalid-name
SQLAlchemyUtilsAdapter
)
# The limit of queries fetched for query search
QUERY_SEARCH_LIMIT = 1000

Expand Down Expand Up @@ -841,7 +842,7 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
CSV_TO_HIVE_UPLOAD_DIRECTORY = "EXTERNAL_HIVE_TABLES/"
# Function that creates upload directory dynamically based on the
# database used, user and schema provided.
def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC(
def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name
database: "Database",
user: "models.User", # pylint: disable=unused-argument
schema: Optional[str],
Expand Down Expand Up @@ -986,7 +987,14 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC(
# def SQL_QUERY_MUTATOR(sql, user_name, security_manager, database):
# dttm = datetime.now().isoformat()
# return f"-- [SQL LAB] {username} {dttm}\n{sql}"
SQL_QUERY_MUTATOR = None
def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
sql: str,
user_name: Optional[str],
security_manager: LocalProxy,
database: "Database",
) -> str:
return sql


# Enable / disable scheduled email reports
#
Expand Down
13 changes: 8 additions & 5 deletions superset/connectors/druid/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# pylint: disable=too-many-ancestors
import json
import logging
from datetime import datetime
Expand Down Expand Up @@ -62,7 +61,9 @@ def ensure_enabled(self) -> None:
raise NotFound()


class DruidColumnInlineView(CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView):
class DruidColumnInlineView( # pylint: disable=too-many-ancestors
CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView,
):
datamodel = SQLAInterface(models.DruidColumn)
include_route_methods = RouteMethod.RELATED_VIEW_SET

Expand Down Expand Up @@ -149,7 +150,9 @@ def post_add(self, item: "DruidColumnInlineView") -> None:
self.post_update(item)


class DruidMetricInlineView(CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView):
class DruidMetricInlineView( # pylint: disable=too-many-ancestors
CompactCRUDMixin, EnsureEnabledMixin, SupersetModelView,
):
datamodel = SQLAInterface(models.DruidMetric)
include_route_methods = RouteMethod.RELATED_VIEW_SET

Expand Down Expand Up @@ -202,7 +205,7 @@ class DruidMetricInlineView(CompactCRUDMixin, EnsureEnabledMixin, SupersetModelV
edit_form_extra_fields = add_form_extra_fields


class DruidClusterModelView(
class DruidClusterModelView( # pylint: disable=too-many-ancestors
EnsureEnabledMixin, SupersetModelView, DeleteMixin, YamlExportMixin,
):
datamodel = SQLAInterface(models.DruidCluster)
Expand Down Expand Up @@ -266,7 +269,7 @@ def _delete(self, pk: int) -> None:
DeleteMixin._delete(self, pk)


class DruidDatasourceModelView(
class DruidDatasourceModelView( # pylint: disable=too-many-ancestors
EnsureEnabledMixin, DatasourceModelView, DeleteMixin, YamlExportMixin,
):
datamodel = SQLAInterface(models.DruidDatasource)
Expand Down
4 changes: 4 additions & 0 deletions superset/dashboards/commands/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import logging
from typing import Any, Dict, Set

from flask import g
from sqlalchemy.orm import Session

from superset.models.dashboard import Dashboard
Expand Down Expand Up @@ -155,4 +156,7 @@ def import_dashboard(
if dashboard.id is None:
session.flush()

if hasattr(g, "user") and g.user:
dashboard.owners.append(g.user)

return dashboard
12 changes: 6 additions & 6 deletions superset/dashboards/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# pylint: disable=too-few-public-methods
from typing import Any, Optional

from flask_appbuilder.security.sqla.models import Role
Expand All @@ -31,7 +29,7 @@
from superset.views.base_api import BaseFavoriteFilter


class DashboardTitleOrSlugFilter(BaseFilter):
class DashboardTitleOrSlugFilter(BaseFilter): # pylint: disable=too-few-public-methods
name = _("Title or Slug")
arg_name = "title_or_slug"

Expand All @@ -47,7 +45,9 @@ def apply(self, query: Query, value: Any) -> Query:
)


class DashboardFavoriteFilter(BaseFavoriteFilter):
class DashboardFavoriteFilter( # pylint: disable=too-few-public-methods
BaseFavoriteFilter
):
"""
Custom filter for the GET list that filters all dashboards that a user has favored
"""
Expand All @@ -57,7 +57,7 @@ class DashboardFavoriteFilter(BaseFavoriteFilter):
model = Dashboard


class DashboardAccessFilter(BaseFilter):
class DashboardAccessFilter(BaseFilter): # pylint: disable=too-few-public-methods
"""
List dashboards with the following criteria:
1. Those which the user owns
Expand Down Expand Up @@ -140,7 +140,7 @@ def apply(self, query: Query, value: Any) -> Query:
return query


class FilterRelatedRoles(BaseFilter):
class FilterRelatedRoles(BaseFilter): # pylint: disable=too-few-public-methods
"""
A filter to allow searching for related roles of a resource.
Expand Down
Loading

0 comments on commit 40764d8

Please sign in to comment.