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

fix: some bug fix related to tag and data element #1199

Merged
merged 3 commits into from
Mar 17, 2023
Merged

fix: some bug fix related to tag and data element #1199

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8afac36
fix: some tag ui bugs
jczhong84 Mar 17, 2023
b15be80
comments
jczhong84 Mar 17, 2023
2e3201f
remove column stats api
jczhong84 Mar 17, 2023
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
33 changes: 0 additions & 33 deletions querybook/server/datasources/data_element.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,6 @@
from app.auth.permission import verify_data_column_permission
from app.datasource import register
from lib.logger import get_logger
from lib.metastore import get_metastore_loader
from logic import data_element as logic
from const.data_element import DataElementAssociationDict

LOG = get_logger(__file__)


@register(
"/column/<int:column_id>/data_element/",
methods=["GET"],
)
def get_data_element_by_column_id(column_id: int) -> DataElementAssociationDict:
verify_data_column_permission(column_id)
associations = logic.get_data_element_associations_by_column_id(column_id=column_id)

if not associations:
return None

# check if there are more than 1 association type
association_types = set([r.type for r in associations])
if len(association_types) > 1:
LOG.error(
f"Column {column_id} has more than one data element associated with it"
)
return None

data_element = {}
for row in associations:
data_element["type"] = row.type.value
data_element[row.property_name] = (
row.data_element.to_dict() if row.data_element else row.primitive_type
)
return data_element


@register("/data_element/<int:data_element_id>/metastore_link/", methods=["GET"])
Expand Down
18 changes: 12 additions & 6 deletions querybook/server/datasources/metastore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
from lib.utils import mysql_cache
from logic import metastore as logic
from logic import admin as admin_logic
from logic import data_element as data_element_logic
from logic import tag as tag_logic
from models.metastore import (
DataTableWarning,
DataTableStatistics,
Expand Down Expand Up @@ -374,12 +376,16 @@ def get_column_by_table(table_id, column_name, with_table=False):

@register("/column/<int:column_id>/", methods=["GET"])
def get_column(column_id, with_table=False):
with DBSession() as session:
column = logic.get_column_by_id(column_id, session=session)
verify_data_table_permission(column.table_id, session=session)
column_dict = column.to_dict(with_table)

return column_dict
column = logic.get_column_by_id(column_id)
verify_data_table_permission(column.table_id)
column_dict = column.to_dict(with_table)

column_dict["stats"] = DataTableColumnStatistics.get_all(column_id=column_id)
column_dict["tags"] = tag_logic.get_tags_by_column_id(column_id=column_id)
column_dict[
"data_element_association"
] = data_element_logic.get_data_element_association_by_column_id(column_id)
return column_dict


@register("/column/<int:column_id>/", methods=["PUT"])
Expand Down
31 changes: 28 additions & 3 deletions querybook/server/logic/data_element.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Union
from app.db import with_session
from const.data_element import (
DataElementAssociationDict,
DataElementAssociationProperty,
DataElementAssociationTuple,
DataElementAssociationType,
Expand All @@ -24,12 +25,32 @@ def get_data_element_by_name(name: str, session=None):


@with_session
def get_data_element_associations_by_column_id(column_id: int, session=None) -> dict:
return (
def get_data_element_association_by_column_id(
column_id: int, session=None
) -> DataElementAssociationDict:
associations = (
session.query(DataElementAssociation)
.filter(DataElementAssociation.column_id == column_id)
.all()
)
if not associations:
return None

# check if there are more than 1 association type
association_types = set([r.type for r in associations])
if len(association_types) > 1:
LOG.error(
f"Column {column_id} has more than one data element associated with it"
)
return None

data_element = {}
for row in associations:
data_element["type"] = row.type.value
data_element[row.property_name] = (
row.data_element.to_dict() if row.data_element else row.primitive_type
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't serialize it here. Let it be serialized automatically when returning

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

api json encoder wont do automatic serialization for nested objects.

)
return data_element


@with_session
Expand Down Expand Up @@ -75,7 +96,11 @@ def create_data_element_association(
primitive_type: str = None,
session=None,
):
if data_element_tuple is None:
if (
not data_element_tuple
or not data_element_tuple.name
jczhong84 marked this conversation as resolved.
Show resolved Hide resolved
or not data_element_tuple.type
):
return None

if type(data_element_tuple) is str:
Expand Down
2 changes: 1 addition & 1 deletion querybook/server/logic/tag.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def create_or_update_tag(tag_name, meta={}, commit=True, session=None):
else:
tag = Tag.update(
id=tag.id,
fields={"count": tag.count + 1, "meta": meta},
fields={"count": tag.count + 1, "meta": {**tag.meta, **meta}},
skip_if_value_none=True,
commit=commit,
session=session,
Expand Down
16 changes: 4 additions & 12 deletions querybook/webapp/components/DataTableStats/DataTableColumnStats.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,19 @@
import * as React from 'react';

import { useResource } from 'hooks/useResource';
import { ITableColumnStats } from 'const/metastore';
import { isNumeric } from 'lib/utils/number';
import { TableColumnResource } from 'resource/table';
import { KeyContentDisplay } from 'ui/KeyContentDisplay/KeyContentDisplay';

import { TableStats } from './DataTableStatsCommon';

interface IProps {
columnId: number;
stats: ITableColumnStats[];
}

export const DataTableColumnStats: React.FunctionComponent<IProps> = ({
columnId,
stats,
}) => {
const { data: tableColumnStats } = useResource(
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we don't get tags, data elements, and stats individually anymore, can you remove the api and resource?

Copy link
Collaborator Author

@jczhong84 jczhong84 Mar 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

actually there are 3 /column/stats apis, two of them are also not used today. should we remove all of them?

@register("/column/stats/<int:column_id>/", methods=["GET"])
@register("/column/stats/<metastore_name>/", methods=["POST"])
@register("/column/stats/", methods=["POST"])

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh yeah lets remove all of them then

React.useCallback(
() => TableColumnResource.getStats(columnId),
[columnId]
)
);

const statsDOM = (tableColumnStats || []).map((tableColumnStat) => (
const statsDOM = (stats || []).map((tableColumnStat) => (
<KeyContentDisplay
key={tableColumnStat.id}
keyString={tableColumnStat.key}
Expand Down
7 changes: 6 additions & 1 deletion querybook/webapp/components/DataTableTags/DataTableTags.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ interface IProps {
tableId: number;
readonly?: boolean;
mini?: boolean;
showType?: boolean;
}

export const DataTableTags: React.FunctionComponent<IProps> = ({
tableId,
readonly = false,
mini = false,
showType = true,
}) => {
const isUserAdmin = useSelector(
(state: IStoreState) => state.user.myUserInfo.isAdmin
Expand Down Expand Up @@ -62,6 +64,7 @@ export const DataTableTags: React.FunctionComponent<IProps> = ({
key={tag.id}
isUserAdmin={isUserAdmin}
mini={mini}
showType={showType}
/>
));

Expand All @@ -82,7 +85,8 @@ export const TableTag: React.FC<{
readonly?: boolean;
deleteTag?: (tagName: string) => void;
mini?: boolean;
}> = ({ tag, readonly, deleteTag, isUserAdmin, mini }) => {
showType?: boolean;
}> = ({ tag, readonly, deleteTag, isUserAdmin, mini, showType = true }) => {
const tagMeta = tag.meta ?? {};
const tagRef = React.useRef<HTMLSpanElement>();
const [showConfigModal, setShowConfigModal] = React.useState(false);
Expand Down Expand Up @@ -151,6 +155,7 @@ export const TableTag: React.FC<{
onClick={handleTagClick}
ref={tagRef}
mini={mini}
showType={showType}
/>
</div>
);
Expand Down
2 changes: 1 addition & 1 deletion querybook/webapp/components/DataTableTags/TableTagSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ interface IProps {
const tagReactSelectStyle = makeReactSelectStyle(true, miniReactSelectStyles);

function isTagValid(val: string, existingTags: string[]) {
const regex = /^[a-z0-9]{1,255}$/i;
const regex = /^[a-z0-9_ ]{1,255}$/i;
const match = val.match(regex);
return Boolean(match && !existingTags.includes(val));
}
Expand Down
28 changes: 11 additions & 17 deletions querybook/webapp/components/DataTableViewColumn/DataTableColumnCard.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,19 @@
.Card {
margin: 16px 0px;

.DataTableColumnCard-top {
cursor: pointer;
.DataTableColumnCard-left {
display: flex;
flex-direction: row;
align-items: center;
max-width: 95%;

.column-type {
min-width: 80px;
word-break: break-all;
}
.DataTableColumnCard-header {
display: flex;
flex-direction: row;
align-items: center;
.column-name {
width: 180px;
margin-right: 12px;
}
}

.DataTableColumnCard-preview {
@include one-line-ellipsis();
width: 95%;
.column-type {
width: 100%;
word-break: break-all;
}
}

.EditableTextField .editor-wrapper {
Expand All @@ -31,7 +26,6 @@
}

.DataTableColumnCardNestedType {

.column-type {
min-width: 80px;
word-break: break-all;
Expand Down
Loading