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: add certifiedby & certification details fields to the edit dataset columns fields #16454

Merged
merged 22 commits into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
00a17ed
add migration
pkdotson Aug 25, 2021
75fdca9
add backend and frontend for certified
pkdotson Aug 25, 2021
75ad447
update migration with batch
pkdotson Aug 27, 2021
03ac0c7
fix integration test and update Updating.md
pkdotson Aug 31, 2021
a0290ba
Merge branch 'master' of https://github.com/preset-io/superset into a…
pkdotson Aug 31, 2021
ba2059e
Update superset-frontend/src/datasource/DatasourceEditor.jsx
pkdotson Sep 9, 2021
b70d320
Update superset-frontend/src/datasource/DatasourceEditor.jsx
pkdotson Sep 9, 2021
22e8ce2
Update superset-frontend/src/datasource/DatasourceEditor.jsx
pkdotson Sep 9, 2021
393dac2
Merge branch 'master' of https://github.com/preset-io/superset into a…
pkdotson Sep 9, 2021
5c292e0
change method name
pkdotson Sep 9, 2021
dde96cf
add tooltip info
pkdotson Sep 13, 2021
b455c96
add mixin
pkdotson Sep 14, 2021
9119334
Merge branch 'master' of https://github.com/preset-io/superset into a…
pkdotson Sep 14, 2021
2d8264b
merge heads
pkdotson Sep 15, 2021
b9aecae
Merge branch 'master' into add-certified-columns
pkdotson Sep 15, 2021
aa1d81b
address comments
pkdotson Sep 20, 2021
96ef64c
fix select label styles
pkdotson Sep 20, 2021
c994637
Merge branch 'add-certified-columns' of https://github.com/preset-io/…
pkdotson Sep 20, 2021
29aa9bc
add extra field
pkdotson Sep 20, 2021
a3d71bd
fix test?
pkdotson Sep 20, 2021
a689c25
Merge branch 'master' of https://github.com/preset-io/superset into a…
pkdotson Sep 20, 2021
92b2552
add extra field to put schema
pkdotson Sep 21, 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
74 changes: 42 additions & 32 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,37 @@ assists people when migrating to a new version.
## Next

### Breaking Changes

pkdotson marked this conversation as resolved.
Show resolved Hide resolved
### Potential Downtime

### Deprecations

### Other

## 1.3.0

### Breaking Changes

- [15909](https://github.com/apache/incubator-superset/pull/15909): a change which
drops a uniqueness criterion (which may or may not have existed) to the tables table. This constraint was obsolete as it is handled by the ORM due to differences in how MySQL, PostgreSQL, etc. handle uniqueness for NULL values.
drops a uniqueness criterion (which may or may not have existed) to the tables table. This constraint was obsolete as it is handled by the ORM due to differences in how MySQL, PostgreSQL, etc. handle uniqueness for NULL values.

### Potential Downtime

- [14234](https://github.com/apache/superset/pull/14234): Adds the `limiting_factor` column to the `query` table. Give the migration includes a DDL operation on a heavily trafficed table, potential service downtime may be required.

-[16454](https://github.com/apache/superset/pull/16454): Adds the `extra` column to the `table_columns` table. Users using MySQL will either need to schedule downtime or use the percona toolkit (or similar) to perform the migration.

## 1.2.0

### Deprecations

- [13440](https://github.com/apache/superset/pull/13440): Dashboard/Charts reports and old Alerts is deprecated. The following config keys are deprecated:
- ENABLE_ALERTS
- SCHEDULED_EMAIL_DEBUG_MODE
- EMAIL_REPORTS_CRON_RESOLUTION
- EMAIL_ASYNC_TIME_LIMIT_SEC
- EMAIL_REPORT_BCC_ADDRESS
- EMAIL_REPORTS_USER
- ENABLE_ALERTS
- SCHEDULED_EMAIL_DEBUG_MODE
- EMAIL_REPORTS_CRON_RESOLUTION
- EMAIL_ASYNC_TIME_LIMIT_SEC
- EMAIL_REPORT_BCC_ADDRESS
- EMAIL_REPORTS_USER

### Other

Expand Down Expand Up @@ -84,19 +90,21 @@ drops a uniqueness criterion (which may or may not have existed) to the tables t
## 1.0.0

### Breaking Changes

- [11509](https://github.com/apache/superset/pull/12491): Dataset metadata updates check user ownership, only owners or an Admin are allowed.
- Security simplification (SIP-19), the following permission domains were simplified:
- [12072](https://github.com/apache/superset/pull/12072): `Query` with `can_read`, `can_write`
- [12036](https://github.com/apache/superset/pull/12036): `Database` with `can_read`, `can_write`.
- [12012](https://github.com/apache/superset/pull/12036): `Dashboard` with `can_read`, `can_write`.
- [12061](https://github.com/apache/superset/pull/12061): `Log` with `can_read`, `can_write`.
- [12000](https://github.com/apache/superset/pull/12000): `Dataset` with `can_read`, `can_write`.
- [12014](https://github.com/apache/superset/pull/12014): `Annotation` with `can_read`, `can_write`.
- [11981](https://github.com/apache/superset/pull/11981): `Chart` with `can_read`, `can_write`.
- [11853](https://github.com/apache/superset/pull/11853): `ReportSchedule` with `can_read`, `can_write`.
- [11856](https://github.com/apache/superset/pull/11856): `CssTemplate` with `can_read`, `can_write`.
- [11764](https://github.com/apache/superset/pull/11764): `SavedQuery` with `can_read`, `can_write`.
Old permissions will be automatically migrated to these new permissions and applied to all existing security Roles.

- [12072](https://github.com/apache/superset/pull/12072): `Query` with `can_read`, `can_write`
- [12036](https://github.com/apache/superset/pull/12036): `Database` with `can_read`, `can_write`.
- [12012](https://github.com/apache/superset/pull/12036): `Dashboard` with `can_read`, `can_write`.
- [12061](https://github.com/apache/superset/pull/12061): `Log` with `can_read`, `can_write`.
- [12000](https://github.com/apache/superset/pull/12000): `Dataset` with `can_read`, `can_write`.
- [12014](https://github.com/apache/superset/pull/12014): `Annotation` with `can_read`, `can_write`.
- [11981](https://github.com/apache/superset/pull/11981): `Chart` with `can_read`, `can_write`.
- [11853](https://github.com/apache/superset/pull/11853): `ReportSchedule` with `can_read`, `can_write`.
- [11856](https://github.com/apache/superset/pull/11856): `CssTemplate` with `can_read`, `can_write`.
- [11764](https://github.com/apache/superset/pull/11764): `SavedQuery` with `can_read`, `can_write`.
Old permissions will be automatically migrated to these new permissions and applied to all existing security Roles.

- [11499](https://github.com/apache/superset/pull/11499): Breaking change: `STORE_CACHE_KEYS_IN_METADATA_DB` config flag added (default=`False`) to write `CacheKey` records to the metadata DB. `CacheKey` recording was enabled by default previously.

Expand All @@ -111,42 +119,44 @@ drops a uniqueness criterion (which may or may not have existed) to the tables t
- [11244](https://github.com/apache/superset/pull/11244): The `REDUCE_DASHBOARD_BOOTSTRAP_PAYLOAD` feature flag has been removed after being set to True for multiple months.

- [11172](https://github.com/apache/superset/pull/11172): Turning
off language selectors by default as i18n is incomplete in most languages
and requires more work. You can easily turn on the languages you want
to expose in your environment in superset_config.py
off language selectors by default as i18n is incomplete in most languages
and requires more work. You can easily turn on the languages you want
to expose in your environment in superset_config.py

- [11172](https://github.com/apache/superset/pull/11172): Breaking change: SQL templating is turned off by default. To turn it on set `ENABLE_TEMPLATE_PROCESSING` to True on `FEATURE_FLAGS`

### Potential Downtime

- [11920](https://github.com/apache/superset/pull/11920): Undos the DB migration from [11714](https://github.com/apache/superset/pull/11714) to prevent adding new columns to the logs table. Deploying a sha between these two PRs may result in locking your DB.

- [11714](https://github.com/apache/superset/pull/11714): Logs
significantly more analytics events (roughly double?), and when
using DBEventLogger (default) could result in stressing the metadata
database more.
significantly more analytics events (roughly double?), and when
using DBEventLogger (default) could result in stressing the metadata
database more.

- [11098](https://github.com/apache/superset/pull/11098): includes a database migration that adds a `uuid` column to most models, and updates `Dashboard.position_json` to include chart UUIDs. Depending on number of objects, the migration may take up to 5 minutes, requiring planning for downtime.

### Deprecations

- [11155](https://github.com/apache/superset/pull/11155): The `FAB_UPDATE_PERMS` config parameter is no longer required as the Superset application correctly informs FAB under which context permissions should be updated.

## 0.38.0

* [10887](https://github.com/apache/superset/pull/10887): Breaking change: The custom cache backend changed in order to support the Flask-Caching factory method approach and thus must be registered as a custom type. See [here](https://flask-caching.readthedocs.io/en/latest/#custom-cache-backends) for specifics.
- [10887](https://github.com/apache/superset/pull/10887): Breaking change: The custom cache backend changed in order to support the Flask-Caching factory method approach and thus must be registered as a custom type. See [here](https://flask-caching.readthedocs.io/en/latest/#custom-cache-backends) for specifics.

* [10674](https://github.com/apache/superset/pull/10674): Breaking change: PUBLIC_ROLE_LIKE_GAMMA was removed is favour of the new PUBLIC_ROLE_LIKE so it can be set to whatever role you want.
- [10674](https://github.com/apache/superset/pull/10674): Breaking change: PUBLIC_ROLE_LIKE_GAMMA was removed is favour of the new PUBLIC_ROLE_LIKE so it can be set to whatever role you want.

* [10590](https://github.com/apache/superset/pull/10590): Breaking change: this PR will convert iframe chart into dashboard markdown component, and remove all `iframe`, `separator`, and `markup` slices (and support) from Superset. If you have important data in those slices, please backup manually.
- [10590](https://github.com/apache/superset/pull/10590): Breaking change: this PR will convert iframe chart into dashboard markdown component, and remove all `iframe`, `separator`, and `markup` slices (and support) from Superset. If you have important data in those slices, please backup manually.

* [10562](https://github.com/apache/superset/pull/10562): EMAIL_REPORTS_WEBDRIVER is deprecated use WEBDRIVER_TYPE instead.
- [10562](https://github.com/apache/superset/pull/10562): EMAIL_REPORTS_WEBDRIVER is deprecated use WEBDRIVER_TYPE instead.

* [10567](https://github.com/apache/superset/pull/10567): Default WEBDRIVER_OPTION_ARGS are Chrome-specific. If you're using FF, should be `--headless` only
- [10567](https://github.com/apache/superset/pull/10567): Default WEBDRIVER_OPTION_ARGS are Chrome-specific. If you're using FF, should be `--headless` only

* [10241](https://github.com/apache/superset/pull/10241): change on Alpha role, users started to have access to "Annotation Layers", "Css Templates" and "Import Dashboards".
- [10241](https://github.com/apache/superset/pull/10241): change on Alpha role, users started to have access to "Annotation Layers", "Css Templates" and "Import Dashboards".

* [10324](https://github.com/apache/superset/pull/10324): Facebook Prophet has been introduced as an optional dependency to add support for timeseries forecasting in the chart data API. To enable this feature, install Superset with the optional dependency `prophet` or directly `pip install fbprophet`.
- [10324](https://github.com/apache/superset/pull/10324): Facebook Prophet has been introduced as an optional dependency to add support for timeseries forecasting in the chart data API. To enable this feature, install Superset with the optional dependency `prophet` or directly `pip install fbprophet`.

* [10320](https://github.com/apache/superset/pull/10320): References to blacklst/whitelist language have been replaced with more appropriate alternatives. All configs refencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`.
- [10320](https://github.com/apache/superset/pull/10320): References to blacklst/whitelist language have been replaced with more appropriate alternatives. All configs refencing containing `WHITE`/`BLACK` have been replaced with `ALLOW`/`DENY`. Affected config variables that need to be updated: `TIME_GRAIN_BLACKLIST`, `VIZ_TYPE_BLACKLIST`, `DRUID_DATA_SOURCE_BLACKLIST`.

## 0.37.1

Expand Down
51 changes: 47 additions & 4 deletions superset-frontend/src/datasource/DatasourceEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ const ColumnButtonWrapper = styled.div`
${({ theme }) => `margin-bottom: ${theme.gridUnit * 2}px`}
`;

const StyledLabelWrapper = styled.div`
display: flex;
align-items: center;
span {
margin-right: ${({ theme }) => theme.gridUnit}px;
}
`;

const checkboxGenerator = (d, onChange) => (
<CheckboxControl value={d} onChange={onChange} />
);
Expand Down Expand Up @@ -235,6 +243,26 @@ function ColumnCollectionTable({
/>
}
/>
<Field
fieldKey="certified_by"
label={t('Certified By')}
control={
<TextControl
controlId="certified"
placeholder={t('Certified By')}
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
/>
}
/>
<Field
fieldKey="certification_details"
label={t('Certification Details')}
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
control={
<TextControl
controlId="certificationDetails"
placeholder={t('Certification Details')}
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
/>
}
/>
</Fieldset>
</FormContainer>
}
Expand All @@ -247,11 +275,27 @@ function ColumnCollectionTable({
}}
onChange={onChange}
itemRenderers={{
column_name: (v, onItemChange) =>
column_name: (v, onItemChange, _, record) =>
editableColumnName ? (
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
<StyledLabelWrapper>
{record.is_certified && (
<CertifiedIcon
certifiedBy={record.certified_by}
details={record.certification_details}
/>
)}
<EditableTitle canEdit title={v} onSaveTitle={onItemChange} />
</StyledLabelWrapper>
) : (
v
<StyledLabelWrapper>
{record.is_certified && (
<CertifiedIcon
certifiedBy={record.certified_by}
details={record.certification_details}
/>
)}
{v}
</StyledLabelWrapper>
),
type: d => (d ? <Label>{d}</Label> : null),
is_dttm: checkboxGenerator,
Expand Down Expand Up @@ -1072,7 +1116,6 @@ class DatasourceEditor extends React.PureComponent {
const { datasource, activeTabKey } = this.state;
const { metrics } = datasource;
const sortedMetrics = metrics?.length ? this.sortMetrics(metrics) : [];

return (
<DatasourceContainer>
{this.renderErrors()}
Expand Down
18 changes: 12 additions & 6 deletions superset-frontend/src/datasource/DatasourceModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,17 @@ interface DatasourceModalProps {
show: boolean;
}

function buildMetricExtraJsonObject(metric: Record<string, unknown>) {
function buildExtraJsonObjects(item: Record<string, unknown>) {
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
const certification =
metric?.certified_by || metric?.certification_details
item?.certified_by || item?.certification_details
? {
certified_by: metric?.certified_by,
details: metric?.certification_details,
certified_by: item?.certified_by,
details: item?.certification_details,
}
: undefined;
return JSON.stringify({
certification,
warning_markdown: metric?.warning_markdown,
warning_markdown: item?.warning_markdown,
});
}

Expand Down Expand Up @@ -109,7 +109,13 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
metrics: currentDatasource?.metrics?.map(
(metric: Record<string, unknown>) => ({
...metric,
extra: buildMetricExtraJsonObject(metric),
extra: buildExtraJsonObjects(metric),
}),
),
columns: currentDatasource?.columns?.map(
(column: Record<string, unknown>) => ({
...column,
extra: buildExtraJsonObjects(column),
}),
),
type: currentDatasource.type || currentDatasource.datasource_type,
Expand Down
35 changes: 34 additions & 1 deletion superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ class TableColumn(Model, BaseColumn):
is_dttm = Column(Boolean, default=False)
expression = Column(Text)
python_date_format = Column(String(255))
extra = Column(Text)

export_fields = [
"table_id",
Expand All @@ -200,6 +201,7 @@ class TableColumn(Model, BaseColumn):
"expression",
"description",
"python_date_format",
"extra",
]

update_from_object_fields = [s for s in export_fields if s not in ("table_id",)]
Expand Down Expand Up @@ -267,6 +269,28 @@ def get_sqla_col(self, label: Optional[str] = None) -> Column:
def datasource(self) -> RelationshipProperty:
return self.table

def get_extra_dict(self) -> Dict[str, Any]:
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
try:
return json.loads(self.extra)
except (TypeError, json.JSONDecodeError):
return {}

@property
def is_certified(self) -> bool:
return bool(self.get_extra_dict().get("certification"))

@property
def certified_by(self) -> Optional[str]:
return self.get_extra_dict().get("certification", {}).get("certified_by")

@property
def certification_details(self) -> Optional[str]:
return self.get_extra_dict().get("certification", {}).get("details")

@property
def warning_markdown(self) -> Optional[str]:
return self.get_extra_dict().get("warning_markdown")

def get_time_filter(
self,
start_dttm: DateTime,
Expand Down Expand Up @@ -374,8 +398,17 @@ def data(self) -> Dict[str, Any]:
"type",
"type_generic",
"python_date_format",
"is_certified",
"certified_by",
"certification_details",
"warning_markdown",
)
return {s: getattr(self, s) for s in attrs if hasattr(self, s)}

attr_dict = {s: getattr(self, s) for s in attrs if hasattr(self, s)}

attr_dict.update(super().data)

return attr_dict


class SqlMetric(Model, BaseMetric):
Expand Down
9 changes: 9 additions & 0 deletions superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):
"expression",
"is_dttm",
"python_date_format",
"extra",
]
add_columns = edit_columns
list_columns = [
Expand Down Expand Up @@ -129,6 +130,14 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):
),
True,
),
"extra": utils.markdown(
"Extra data to specify column metadata. Currently supports "
'certification data of the format: `{ "certification": "certified_by": '
'"Taylor Swift", "details": "This column is the source of truth." '
"} }`. This should be modified from the edit datasource model in "
"Explore to ensure correct formatting.",
True,
),
}
label_columns = {
"column_name": _("Column"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# 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_extra_column_to_columns_model

Revision ID: 181091c0ef16
Revises: 07071313dd52
Create Date: 2021-08-24 23:27:30.403308

"""

# revision identifiers, used by Alembic.
revision = "181091c0ef16"
down_revision = "07071313dd52"

import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

from superset.utils.core import generic_find_constraint_name


def upgrade():
with op.batch_alter_table("table_columns") as batch_op:
batch_op.add_column(sa.Column("extra", sa.Text(), nullable=True))


def downgrade():
with op.batch_alter_table("table_columns") as batch_op:
batch_op.drop_column("extra")
Loading