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

Update dataset/dataset field tagging experience #2761

Merged
merged 22 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5bb09ac
add key down escape for dialogs
davidsharp7 Mar 7, 2024
f543b32
add truthy condition of refreshTags
davidsharp7 Mar 7, 2024
864974e
fix freshTags state
davidsharp7 Mar 7, 2024
2b6f971
fix failing test
davidsharp7 Mar 7, 2024
486e42e
Merge branch 'MarquezProject:main' into bug/fix_tag_state
davidsharp7 Mar 7, 2024
9aef43f
add tag sort order and update conflict insert sql
davidsharp7 Mar 11, 2024
8203687
add tag integration ON CONFLICT test
davidsharp7 Mar 15, 2024
efbdc09
Merge branch 'MarquezProject:main' into bug/fix_tag_state
davidsharp7 Mar 15, 2024
a690558
Merge branch 'MarquezProject:main' into bug/fix_tag_state
davidsharp7 Mar 21, 2024
d114c17
Merge branch 'main' into bug/fix_tag_state
wslulciuc Mar 31, 2024
abd7b31
draft changes to tagging
davidsharp7 Apr 2, 2024
a34bf7d
Merge branch 'bug/fix_tag_state' of https://github.com/davidsharp7/ma…
davidsharp7 Apr 2, 2024
8f7b10a
add missing freesolo
davidsharp7 Apr 2, 2024
38d29ab
add field tag toggle
davidsharp7 Apr 3, 2024
c975369
Merge branch 'main' into bug/fix_tag_state
davidsharp7 Apr 3, 2024
2b37984
add multi select/fix fresh
davidsharp7 Apr 4, 2024
938b507
fix failing test
davidsharp7 Apr 5, 2024
aa7327d
fix DOM underline error
davidsharp7 Apr 5, 2024
80e1ac5
remove selectOption for tag update
davidsharp7 Apr 13, 2024
5d5c449
add auto highlight prop to aid user selection
davidsharp7 Apr 15, 2024
ef2de7b
change tag deletion updated based on feedback
davidsharp7 Apr 16, 2024
3bb97d4
Merge branch 'main' into bug/fix_tag_state
davidsharp7 Apr 16, 2024
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
2 changes: 1 addition & 1 deletion api/src/main/java/marquez/db/DatasetFieldDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void deleteDatasetVersionFieldTag(

@SqlUpdate(
"INSERT INTO dataset_fields_tag_mapping (dataset_field_uuid, tag_uuid, tagged_at) "
+ "VALUES (:rowUuid, :tagUuid, :taggedAt)")
wslulciuc marked this conversation as resolved.
Show resolved Hide resolved
+ "VALUES (:rowUuid, :tagUuid, :taggedAt) ON CONFLICT DO NOTHING")
void updateTags(UUID rowUuid, UUID tagUuid, Instant taggedAt);

@SqlBatch(
Expand Down
34 changes: 34 additions & 0 deletions api/src/test/java/marquez/api/TagResourceIntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,38 @@ public void testApp_testDatasetTagFieldDelete() {
// assert that only SENSITIVE remains
assertThat(taggedDatasetFieldDelete.getFields().get(0).getTags()).containsExactly("SENSITIVE");
}

@Test
public void testApp_testDatasetTagFieldConflict() {
Copy link
Member

Choose a reason for hiding this comment

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

❤️

// Create Namespace
createNamespace(NAMESPACE_NAME);
// create a source
createSource(DB_TABLE_SOURCE_NAME);
// Create Dataset
MARQUEZ_CLIENT.createDataset(NAMESPACE_NAME, DB_TABLE_NAME, DB_TABLE_META);

// tag dataset field
Dataset taggedDatasetField =
MARQUEZ_CLIENT.tagFieldWith(
NAMESPACE_NAME,
DB_TABLE_NAME,
DB_TABLE_META.getFields().get(0).getName(),
"TESTFIELDTAG");
// assert the tag TESTFIELDTAG has been added to field at position 0
assertThat(taggedDatasetField.getFields().get(0).getTags()).contains("TESTFIELDTAG");
// assert a total of two tags exist on the field
assertThat(taggedDatasetField.getFields().get(0).getTags()).hasSize(2);

// tag dataset field again to test ON CONFLICT DO NOTHING
Dataset taggedDatasetField2 =
MARQUEZ_CLIENT.tagFieldWith(
NAMESPACE_NAME,
DB_TABLE_NAME,
DB_TABLE_META.getFields().get(0).getName(),
"TESTFIELDTAG");
// assert the tag TESTFIELDTAG has been added to field at position 0
assertThat(taggedDatasetField2.getFields().get(0).getTags()).contains("TESTFIELDTAG");
// assert a total of two tags exist on the field
assertThat(taggedDatasetField2.getFields().get(0).getTags()).hasSize(2);
}
}
24 changes: 20 additions & 4 deletions web/src/components/datasets/DatasetDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import * as Redux from 'redux'
import { Box, Button, Tab, Tabs, createTheme } from '@mui/material'
import { Box, Button, Switch, Tab, Tabs, createTheme } from '@mui/material'
import { CircularProgress } from '@mui/material'
import { DatasetVersion } from '../../types/api'
import { IState } from '../../store/reducers'
Expand Down Expand Up @@ -31,7 +31,7 @@ import IconButton from '@mui/material/IconButton'
import Io from '../io/Io'
import MqStatus from '../core/status/MqStatus'
import MqText from '../core/text/MqText'
import React, { ChangeEvent, FunctionComponent, useEffect } from 'react'
import React, { ChangeEvent, FunctionComponent, useEffect, useState } from 'react'

interface StateProps {
lineageDataset: LineageDataset
Expand Down Expand Up @@ -79,6 +79,7 @@ const DatasetDetailPage: FunctionComponent<IProps> = (props) => {
const i18next = require('i18next')
const theme = createTheme(useTheme())
const [_, setSearchParams] = useSearchParams()
const [showTags, setShowTags] = useState(false)

// unmounting
useEffect(
Expand All @@ -91,7 +92,13 @@ const DatasetDetailPage: FunctionComponent<IProps> = (props) => {

useEffect(() => {
fetchDatasetVersions(lineageDataset.namespace, lineageDataset.name)
}, [lineageDataset.name, datasets.refreshTags])
}, [lineageDataset.name])

useEffect(() => {
if (showTags === true) {
fetchDatasetVersions(lineageDataset.namespace, lineageDataset.name)
}
}, [showTags])

// if the dataset is deleted then redirect to datasets end point
useEffect(() => {
Expand Down Expand Up @@ -194,7 +201,7 @@ const DatasetDetailPage: FunctionComponent<IProps> = (props) => {
</Tabs>
</Box>
</Box>
<Box display={'flex'} alignItems={'center'}>
<Box display={'flex'} alignItems={'center'} justifyContent={'space-between'}>
{facetsStatus && (
<Box mr={1}>
<MqStatus label={'Quality'} color={facetsStatus} />
Expand All @@ -203,6 +210,14 @@ const DatasetDetailPage: FunctionComponent<IProps> = (props) => {
<MqText heading font={'mono'}>
{name}
</MqText>
<Box ml={1} display={'flex'} alignItems={'center'}>
<MqText subheading>{i18next.t('datasets.show_field_tags')}</MqText>
<Switch
checked={showTags}
onChange={() => setShowTags(!showTags)}
inputProps={{ 'aria-label': 'toggle show tags' }}
/>
</Box>
</Box>
<Box mb={2}>
<MqText subdued>{description}</MqText>
Expand All @@ -213,6 +228,7 @@ const DatasetDetailPage: FunctionComponent<IProps> = (props) => {
datasetFields={firstVersion.fields}
facets={firstVersion.facets}
run={firstVersion.createdByRun}
showTags={showTags}
/>
)}
{tabIndex === 1 && <Io />}
Expand Down
112 changes: 40 additions & 72 deletions web/src/components/datasets/DatasetInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@ import * as Redux from 'redux'
import { Box, Table, TableBody, TableCell, TableHead, TableRow } from '@mui/material'
import { Field, Run } from '../../types/api'
import { IState } from '../../store/reducers'

import { connect, useSelector } from 'react-redux'
import { fetchJobFacets, resetFacets } from '../../store/actionCreators'
import Collapse from '@mui/material/Collapse'
import DatasetTags from './DatasetTags'
import KeyboardArrowDownIcon from '@mui/icons-material/KeyboardArrowDown'
import MqEmpty from '../core/empty/MqEmpty'
import MqJsonView from '../core/json-view/MqJsonView'
import MqText from '../core/text/MqText'
import React, { FunctionComponent, useEffect, useState } from 'react'
import React, { FunctionComponent, useEffect } from 'react'

export interface DispatchProps {
fetchJobFacets: typeof fetchJobFacets
Expand All @@ -36,22 +33,18 @@ type DatasetInfoProps = {
datasetFields: Field[]
facets?: object
run?: Run
showTags?: boolean
} & JobFacetsProps &
DispatchProps

const DatasetInfo: FunctionComponent<DatasetInfoProps> = (props) => {
const { datasetFields, facets, run, fetchJobFacets, resetFacets } = props
const { datasetFields, facets, run, fetchJobFacets, resetFacets, showTags } = props
const i18next = require('i18next')
const dsNamespace = useSelector(
(state: IState) => state.datasetVersions.result.versions[0].namespace
)
const dsName = useSelector((state: IState) => state.datasetVersions.result.versions[0].name)

const loadCollapsedState = () => {
const storedState = localStorage.getItem(`dsi_${dsNamespace}_${dsName}`)
return storedState ? JSON.parse(storedState) : []
}

useEffect(() => {
run && fetchJobFacets(run.id)
}, [run])
Expand All @@ -62,27 +55,6 @@ const DatasetInfo: FunctionComponent<DatasetInfoProps> = (props) => {
},
[]
)
const [expandedRows, setExpandedRows] = useState<number[]>(loadCollapsedState)

const toggleRow = (index: number) => {
setExpandedRows((prevExpandedRows) => {
const newExpandedRows = prevExpandedRows.includes(index)
? prevExpandedRows.filter((rowIndex) => rowIndex !== index)
: [...prevExpandedRows, index]

localStorage.setItem(`dsi_${dsNamespace}_${dsName}`, JSON.stringify(newExpandedRows))

return newExpandedRows
})
}

useEffect(() => {
for (const key in localStorage) {
if (key !== `dsi_${dsNamespace}_${dsName}`) {
localStorage.removeItem(key)
}
}
}, [dsNamespace, dsName])

return (
<Box>
Expand All @@ -102,53 +74,49 @@ const DatasetInfo: FunctionComponent<DatasetInfoProps> = (props) => {
{i18next.t('dataset_info_columns.name')}
</MqText>
</TableCell>
<TableCell align='left'>
<MqText subheading inline>
{i18next.t('dataset_info_columns.type')}
</MqText>
</TableCell>
<TableCell align='left'>
<MqText subheading inline>
{i18next.t('dataset_info_columns.description')}
</MqText>
</TableCell>
<TableCell align='left'></TableCell>
{!showTags && (
<TableCell align='left'>
<MqText subheading inline>
{i18next.t('dataset_info_columns.type')}
</MqText>
</TableCell>
)}
{!showTags && (
<TableCell align='left'>
<MqText subheading inline>
{i18next.t('dataset_info_columns.description')}
</MqText>
</TableCell>
)}
{showTags && (
<TableCell align='left'>
<MqText subheading inline>
{i18next.t('dataset_tags.tags')}
</MqText>
</TableCell>
)}
</TableRow>
</TableHead>
<TableBody>
{datasetFields.map((field, index) => {
{datasetFields.map((field) => {
return (
<React.Fragment key={field.name}>
<TableRow
sx={{ cursor: 'pointer' }}
onClick={() => toggleRow(index)}
className='expandable-row'
>
<TableRow sx={{ cursor: 'pointer' }}>
<TableCell align='left'>{field.name}</TableCell>
<TableCell align='left'>{field.type}</TableCell>
<TableCell align='left'>{field.description || 'no description'}</TableCell>
<TableCell align='right'>
<KeyboardArrowDownIcon
sx={{
rotate: expandedRows.includes(index) ? '180deg' : 0,
transition: 'rotate .3s',
}}
/>
</TableCell>
</TableRow>
<TableRow>
<TableCell colSpan={4} style={{ padding: 0, border: 'none' }}>
<Collapse in={expandedRows.includes(index)} timeout='auto'>
<Box p={2}>
<DatasetTags
namespace={dsNamespace}
datasetName={dsName}
datasetTags={field.tags}
datasetField={field.name}
/>
</Box>
</Collapse>
</TableCell>
{!showTags && <TableCell align='left'>{field.type}</TableCell>}
{!showTags && (
<TableCell align='left'>{field.description || 'no description'}</TableCell>
)}
{showTags && (
<TableCell align='left'>
<DatasetTags
namespace={dsNamespace}
datasetName={dsName}
datasetTags={field.tags}
datasetField={field.name}
/>
</TableCell>
)}
</TableRow>
</React.Fragment>
)
Expand Down
Loading
Loading