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.

Sign up for GitHub

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

[WIP] Feature/buisness type database changes #15154

Closed
wants to merge 5 commits into from
Closed

[WIP] Feature/buisness type database changes #15154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c90ea76
[cherry-pick] Adding business type to models
cccs-RyanS Jun 14, 2021
f33d620
Running linter
cccs-RyanS Jun 14, 2021
a2a802e
Merge with master
cccs-RyanS Jul 6, 2021
306c77f
adding deature flage ENABLE_BUSINESS_TYPES
cccs-RyanS Jul 6, 2021
c0eef48
Adding feature flag
cccs-RyanS Jul 14, 2021
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
135 changes: 113 additions & 22 deletions superset-frontend/src/datasource/DatasourceEditor.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,30 @@ function ColumnCollectionTable({
return (
<CollectionTable
collection={columns}
tableColumns={['column_name', 'type', 'is_dttm', 'filterable', 'groupby']}
sortColumns={['column_name', 'type', 'is_dttm', 'filterable', 'groupby']}
tableColumns={
isFeatureEnabled(FeatureFlag.ENABLE_BUSINESS_TYPES)
? [
'column_name',
'business_type',
'type',
'is_dttm',
'filterable',
'groupby',
]
: ['column_name', 'type', 'is_dttm', 'filterable', 'groupby']
}
sortColumns={
isFeatureEnabled(FeatureFlag.ENABLE_BUSINESS_TYPES)
? [
'column_name',
'business_type',
'type',
'is_dttm',
'filterable',
'groupby',
]
: ['column_name', 'type', 'is_dttm', 'filterable', 'groupby']
}
allowDeletes
allowAddItem={allowAddItem}
itemGenerator={itemGenerator}
Expand Down Expand Up @@ -184,12 +206,26 @@ function ColumnCollectionTable({
{allowEditDataType && (
<Field
fieldKey="type"
label={t('Data type')}
label={t('Data type :)')}
control={
<SelectControl choices={DATA_TYPES} name="type" freeForm />
}
/>
)}
{isFeatureEnabled(FeatureFlag.ENABLE_BUSINESS_TYPES) ? (
<Field
fieldKey="business_type"
label={t('Business type')}
control={
<TextControl
controlId="business_type"
placeholder={t('Business type')}
/>
}
/>
) : (
<></>
)}
<Field
fieldKey="python_date_format"
label={t('Datetime format')}
Expand Down Expand Up @@ -225,26 +261,81 @@ function ColumnCollectionTable({
</Fieldset>
</FormContainer>
}
columnLabels={{
column_name: t('Column'),
type: t('Data type'),
groupby: t('Is dimension'),
is_dttm: t('Is temporal'),
filterable: t('Is filterable'),
}}
columnLabels={
isFeatureEnabled(FeatureFlag.ENABLE_BUSINESS_TYPES)
? {
column_name: t('Column'),
business_type: t('Business type'),
type: t('Data type'),
groupby: t('Is dimension'),
is_dttm: t('Is temporal'),
filterable: t('Is filterable'),
}
: {
column_name: t('Column'),
type: t('Data type'),
groupby: t('Is dimension'),
is_dttm: t('Is temporal'),
filterable: t('Is filterable'),
}
}
onChange={onChange}
itemRenderers={{
column_name: (v, onItemChange) =>
editableColumnName ? (
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
) : (
v
),
type: d => (d ? <Label>{d}</Label> : null),
is_dttm: checkboxGenerator,
filterable: checkboxGenerator,
groupby: checkboxGenerator,
}}
itemRenderers={
isFeatureEnabled(FeatureFlag.ENABLE_BUSINESS_TYPES)
? {
column_name: (v, onItemChange) =>
editableColumnName ? (
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
) : (
v
),
type: d => (d ? <Label>{d}</Label> : null),
business_type: d => <Label onChange={onChange}>{d}</Label>,
is_dttm: checkboxGenerator,
filterable: checkboxGenerator,
groupby: checkboxGenerator,
}
: {
column_name: (v, onItemChange) =>
editableColumnName ? (
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
) : (
v
),
type: d => (d ? <Label>{d}</Label> : null),
is_dttm: checkboxGenerator,
filterable: checkboxGenerator,
groupby: checkboxGenerator,
}
}
bitemRenders={
isFeatureEnabled(FeatureFlag.ENABLE_BUSINESS_TYPES)
? {
column_name: (v, onItemChange) =>
editableColumnName ? (
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
) : (
v
),
type: d => (d ? <Label>{d}</Label> : null),
business_type: d => <Label onChange={onChange}>{d}</Label>,
is_dttm: checkboxGenerator,
filterable: checkboxGenerator,
groupby: checkboxGenerator,
}
: {
column_name: (v, onItemChange) =>
editableColumnName ? (
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
) : (
v
),
type: d => (d ? <Label>{d}</Label> : null),
is_dttm: checkboxGenerator,
filterable: checkboxGenerator,
groupby: checkboxGenerator,
}
}
/>
);
}
Expand Down
1 change: 1 addition & 0 deletions superset/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ def _try_json_readsha( # pylint: disable=unused-argument
"OMNIBAR": False,
"DASHBOARD_RBAC": False,
"ENABLE_EXPLORE_DRAG_AND_DROP": False,
"ENABLE_BUSINESS_TYPES": False,
# Enabling ALERTS_ATTACH_REPORTS, the system sends email and slack message
# with screenshot and link
# Disables ALERTS_ATTACH_REPORTS, the system DOES NOT generate screenshot
Expand Down
2 changes: 2 additions & 0 deletions superset/connectors/base/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ class BaseColumn(AuditMixinNullable, ImportExportMixin):
verbose_name = Column(String(1024))
is_active = Column(Boolean, default=True)
type = Column(String(32))
business_type = Column(String(255))
Copy link
Member

Choose a reason for hiding this comment

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

Hi, @cccs-RyanS... do you want to give more context about this PR before putting work into it? I'm worried that this will need a DB migration, and this columns seems very specific.

Copy link
Contributor Author

@cccs-RyanS cccs-RyanS Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hey so my org had discussed this with Ville Brofeldt and I was waiting on some feedback (which is why its still a draft with very little context, my apologies). The idea was to have business types on top of the db types to have added flexibility. Let me speak to my coworker and I can get you a bunch of extra context and I will add more context to the PR. Thanks for taking a look!

groupby = Column(Boolean, default=True)
filterable = Column(Boolean, default=True)
description = Column(Text)
Expand Down Expand Up @@ -608,6 +609,7 @@ def data(self) -> Dict[str, Any]:
"groupby",
"is_dttm",
"type",
"business_type",
)
return {s: getattr(self, s) for s in attrs if hasattr(self, s)}

Expand Down
1 change: 1 addition & 0 deletions superset/connectors/druid/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ class DruidColumn(Model, BaseColumn):
"description",
"dimension_spec_json",
"verbose_name",
"business_type",
]
update_from_object_fields = export_fields
export_parent = "datasource"
Expand Down
2 changes: 2 additions & 0 deletions superset/connectors/sqla/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ class TableColumn(Model, BaseColumn):
"is_dttm",
"is_active",
"type",
"business_type",
"groupby",
"filterable",
"expression",
Expand Down Expand Up @@ -375,6 +376,7 @@ def data(self) -> Dict[str, Any]:
"is_dttm",
"type",
"type_generic",
"business_type",
"python_date_format",
)
return {s: getattr(self, s) for s in attrs if hasattr(self, s)}
Expand Down
3 changes: 3 additions & 0 deletions superset/connectors/sqla/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class TableColumnInlineView( # pylint: disable=too-many-ancestors
"verbose_name",
"description",
"type",
"business_type",
"groupby",
"filterable",
"table",
Expand All @@ -86,6 +87,7 @@ class TableColumnInlineView( # pylint: disable=too-many-ancestors
"column_name",
"verbose_name",
"type",
"business_type",
"groupby",
"filterable",
"is_dttm",
Expand Down Expand Up @@ -143,6 +145,7 @@ class TableColumnInlineView( # pylint: disable=too-many-ancestors
"is_dttm": _("Is temporal"),
"python_date_format": _("Datetime Format"),
"type": _("Type"),
"business_type": _("Business Type"),
}
validators_columns = {
"python_date_format": [
Expand Down
2 changes: 2 additions & 0 deletions superset/datasets/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class DatasetRestApi(BaseSupersetModelRestApi):
"extra",
]
show_columns = show_select_columns + ["columns.type_generic"]
show_columns = show_select_columns + ["columns.business_type"]
add_model_schema = DatasetPostSchema()
edit_model_schema = DatasetPutSchema()
add_columns = ["database", "schema", "table_name", "owners"]
Expand All @@ -180,6 +181,7 @@ class DatasetRestApi(BaseSupersetModelRestApi):
"metrics",
"extra",
]
edit_columns = edit_columns + ["columns.business_type"]
openapi_spec_tag = "Datasets"
related_field_filters = {
"owners": RelatedFieldFilter("first_name", FilterRelatedOwners),
Expand Down
2 changes: 2 additions & 0 deletions superset/datasets/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class DatasetColumnsPutSchema(Schema):
id = fields.Integer()
column_name = fields.String(required=True, validate=Length(1, 255))
type = fields.String(validate=Length(1, 32))
business_type = fields.String(allow_none=True, Length=(1, 255))
verbose_name = fields.String(allow_none=True, Length=(1, 1024))
description = fields.String(allow_none=True)
expression = fields.String(allow_none=True)
Expand Down Expand Up @@ -131,6 +132,7 @@ class ImportV1ColumnSchema(Schema):
is_dttm = fields.Boolean(default=False, allow_none=True)
is_active = fields.Boolean(default=True, allow_none=True)
type = fields.String(allow_none=True)
business_type = fields.String(allow_none=True)
groupby = fields.Boolean()
filterable = fields.Boolean()
expression = fields.String(allow_none=True)
Expand Down
48 changes: 48 additions & 0 deletions superset/migrations/versions/6f139c533bea_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# 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.
"""adding_business_type.py

Revision ID: 6f139c533bea
Revises: 453530256cea
Create Date: 2021-05-27 16:10:59.567684

"""

import sqlalchemy as sa
from alembic import op


# revision identifiers, used by Alembic.
revision = '6f139c533bea'
down_revision = '453530256cea'

def upgrade():
with op.batch_alter_table("table_columns") as batch_op:
batch_op.add_column(
sa.Column("business_type", sa.VARCHAR(255), nullable=True,)
)
with op.batch_alter_table("columns") as batch_op:
batch_op.add_column(
sa.Column("business_type", sa.VARCHAR(255), nullable=True,)
)


def downgrade():
with op.batch_alter_table("table_columns") as batch_op:
batch_op.drop_column("business_type")
with op.batch_alter_table("columns") as batch_op:
batch_op.drop_column("business_type")
8 changes: 8 additions & 0 deletions tests/integration_tests/databases/commands_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": True,
"python_date_format": None,
"type": ds_type,
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -151,6 +152,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": False,
"python_date_format": None,
"type": "STRING" if example_db.backend == "hive" else "VARCHAR(16)",
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -165,6 +167,7 @@ def test_export_database_command(self, mock_g):
"type": "STRING"
if example_db.backend == "hive"
else "VARCHAR(255)",
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -177,6 +180,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": False,
"python_date_format": None,
"type": big_int_type,
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -189,6 +193,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": False,
"python_date_format": None,
"type": None,
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -201,6 +206,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": False,
"python_date_format": None,
"type": "STRING" if example_db.backend == "hive" else "VARCHAR(10)",
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -213,6 +219,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": False,
"python_date_format": None,
"type": big_int_type,
"business_type": None,
"verbose_name": None,
},
{
Expand All @@ -225,6 +232,7 @@ def test_export_database_command(self, mock_g):
"is_dttm": False,
"python_date_format": None,
"type": big_int_type,
"business_type": None,
"verbose_name": None,
},
],
Expand Down
Loading