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 6 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
2 changes: 1 addition & 1 deletion web/src/__tests__/reducers/datasets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('datasets reducer', () => {
result: datasets,
totalCount: 16,
deletedDatasetName: '',
refreshTags: false
refreshTags: ''
})
})

Expand Down
26 changes: 23 additions & 3 deletions web/src/components/datasets/DatasetTags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ const DatasetTags: React.FC<IProps> = (props) => {
setTagDescription(event.target.value)
}

const tagData = useSelector((state: IState) => state.tags.tags)
const tagData = useSelector((state: IState) => state.tags.tags.sort((a,b) => a.name.localeCompare(b.name)))

const handleTagListChange = (event: any) => {
setListTag(event.target.value)
Expand All @@ -129,6 +129,7 @@ const DatasetTags: React.FC<IProps> = (props) => {
datasetField
? addDatasetFieldTag(namespace, datasetName, listTag, datasetField)
: addDatasetTag(namespace, datasetName, listTag)
setDialogOpen(false)
}

const handleDelete = (deletedTag: string) => {
Expand Down Expand Up @@ -243,7 +244,17 @@ const DatasetTags: React.FC<IProps> = (props) => {
</Grow>
)}
</Popper>
<Dialog open={isDialogOpen} onClose={closeDialog} fullWidth maxWidth='sm'>
<Dialog
open={isDialogOpen}
onClose={closeDialog}
fullWidth
maxWidth='sm'
onKeyDown={(event) => {
if (event.key === 'Escape') {
closeDialog()
}
}}
>
<DialogTitle>{i18next.t('dataset_tags.dialogtitle')}</DialogTitle>
<DialogContent>
<FormControl variant='outlined' size='small' fullWidth>
Expand Down Expand Up @@ -283,7 +294,16 @@ const DatasetTags: React.FC<IProps> = (props) => {
</Button>
</DialogActions>
</Dialog>
<Dialog open={openTagDesc} fullWidth maxWidth='sm'>
<Dialog
open={openTagDesc}
fullWidth
maxWidth='sm'
onKeyDown={(event) => {
if (event.key === 'Escape') {
handleTagDescClose()
}
}}
>
<DialogTitle>Select a Tag to change</DialogTitle>
<DialogContent>
<MQText subheading>Tag</MQText>
Expand Down
28 changes: 24 additions & 4 deletions web/src/store/actionCreators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,12 @@ export const deleteDatasetTag = (namespace: string, datasetName: string, tag: st
},
})

export const deleteDatasetTagSuccess = (datasetName: string) => ({
export const deleteDatasetTagSuccess = (namespace: string, datasetName: string, tag: string) => ({
type: actionTypes.DELETE_DATASET_TAG_SUCCESS,
payload: {
datasetName,
namespace,
tag,
},
})

Expand All @@ -140,10 +142,18 @@ export const deleteDatasetFieldTag = (
},
})

export const deleteDatasetFieldTagSuccess = (datasetName: string) => ({
export const deleteDatasetFieldTagSuccess = (
namespace: string,
datasetName: string,
field: string,
tag: string
) => ({
type: actionTypes.DELETE_DATASET_FIELD_TAG_SUCCESS,
payload: {
datasetName,
namespace,
tag,
field,
},
})

Expand All @@ -156,10 +166,12 @@ export const addDatasetTag = (namespace: string, datasetName: string, tag: strin
},
})

export const addDatasetTagSuccess = (datasetName: string) => ({
export const addDatasetTagSuccess = (namespace: string, datasetName: string, tag: string) => ({
type: actionTypes.ADD_DATASET_TAG_SUCCESS,
payload: {
datasetName,
namespace,
tag,
},
})

Expand All @@ -178,10 +190,18 @@ export const addDatasetFieldTag = (
},
})

export const addDatasetFieldTagSuccess = (datasetName: string) => ({
export const addDatasetFieldTagSuccess = (
namespace: string,
datasetName: string,
field: string,
tag: string
) => ({
type: actionTypes.ADD_DATASET_FIELD_TAG_SUCCESS,
payload: {
datasetName,
namespace,
field,
tag,
},
})

Expand Down
29 changes: 19 additions & 10 deletions web/src/store/reducers/datasets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export type IDatasetsState = {
totalCount: number
init: boolean
deletedDatasetName: string
refreshTags: boolean
refreshTags: string
Copy link
Member

Choose a reason for hiding this comment

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

Why would this be a string and not a boolean?

Copy link
Member Author

Choose a reason for hiding this comment

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

So the thinking is twofold - one to bring it into line with

}, [lineageDataset.name, datasets.refreshTags])

where we are refreshing based on the change in dataset name.

Second reason was for trouble shooting purposes. Based on the concatanation of namespace/datasetname/field/tag it was just easier to understand/diagnose what was happening.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can always change the dependency to

datasets.refreshTags === true

and change the reducer to set refreshTags to False when its updating then True on success.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's just go with what you have here.

}

export const initialState: IDatasetsState = {
Expand All @@ -41,7 +41,7 @@ export const initialState: IDatasetsState = {
result: [],
totalCount: 0,
deletedDatasetName: '',
refreshTags: false,
refreshTags: '',
}

export type IDatasetsAction = ReturnType<typeof fetchDatasetsSuccess> &
Expand Down Expand Up @@ -72,21 +72,30 @@ export default (state: IDatasetsState = initialState, action: IDatasetsAction):
case DELETE_DATASET_SUCCESS:
return { ...state, deletedDatasetName: payload.datasetName }
case DELETE_DATASET_TAG:
return { ...state, refreshTags: false }
return { ...state }
case DELETE_DATASET_TAG_SUCCESS:
return { ...state, refreshTags: true }
return {
...state,
refreshTags: `${payload.namespace}#${payload.datasetName}#${payload.tag}#d`,
}
case DELETE_DATASET_FIELD_TAG:
return { ...state, refreshTags: false }
return { ...state }
case DELETE_DATASET_FIELD_TAG_SUCCESS:
return { ...state, refreshTags: true }
return {
...state,
refreshTags: `${payload.namespace}#${payload.datasetName}#${payload.field}#${payload.tag}#d`,
}
case ADD_DATASET_TAG:
return { ...state, refreshTags: false }
return { ...state }
case ADD_DATASET_TAG_SUCCESS:
return { ...state, refreshTags: true }
return { ...state, refreshTags: `${payload.namespace}#${payload.datasetName}#${payload.tag}` }
case ADD_DATASET_FIELD_TAG:
return { ...state, refreshTags: false }
return { ...state }
case ADD_DATASET_FIELD_TAG_SUCCESS:
return { ...state, refreshTags: true }
return {
...state,
refreshTags: `${payload.namespace}#${payload.datasetName}#${payload.field}#${payload.tag}`,
}
default:
return state
}
Expand Down
40 changes: 22 additions & 18 deletions web/src/store/sagas/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,8 @@ export function* deleteDatasetTagSaga() {
while (true) {
try {
const { payload } = yield take(DELETE_DATASET_TAG)
const dataset: Dataset = yield call(
deleteDatasetTag,
payload.namespace,
payload.datasetName,
payload.tag
)
yield put(deleteDatasetTagSuccess(dataset.name))
yield call(deleteDatasetTag, payload.namespace, payload.datasetName, payload.tag)
yield put(deleteDatasetTagSuccess(payload.namespace, payload.datasetName, payload.tag))
} catch (e) {
yield put(applicationError('Something went wrong while removing tag from dataset'))
}
Expand All @@ -270,14 +265,21 @@ export function* deleteDatasetFieldTagSaga() {
while (true) {
try {
const { payload } = yield take(DELETE_DATASET_FIELD_TAG)
const dataset: Dataset = yield call(
yield call(
deleteDatasetFieldTag,
payload.namespace,
payload.datasetName,
payload.tag,
payload.field
)
yield put(deleteDatasetFieldTagSuccess(dataset.name))
yield put(
deleteDatasetFieldTagSuccess(
payload.namespace,
payload.datasetName,
payload.field,
payload.tag
)
)
} catch (e) {
yield put(applicationError('Something went wrong while removing tag from dataset field'))
}
Expand All @@ -288,13 +290,8 @@ export function* addDatasetTagSaga() {
while (true) {
try {
const { payload } = yield take(ADD_DATASET_TAG)
const dataset: Dataset = yield call(
addDatasetTag,
payload.namespace,
payload.datasetName,
payload.tag
)
yield put(addDatasetTagSuccess(dataset.name))
yield call(addDatasetTag, payload.namespace, payload.datasetName, payload.tag)
yield put(addDatasetTagSuccess(payload.namespace, payload.datasetName, payload.tag))
} catch (e) {
yield put(applicationError('Something went wrong while adding tag to dataset'))
}
Expand All @@ -305,14 +302,21 @@ export function* addDatasetFieldTagSaga() {
while (true) {
try {
const { payload } = yield take(ADD_DATASET_FIELD_TAG)
const dataset: Dataset = yield call(
yield call(
addDatasetFieldTag,
payload.namespace,
payload.datasetName,
payload.tag,
payload.field
)
yield put(addDatasetFieldTagSuccess(dataset.name))
yield put(
addDatasetFieldTagSuccess(
payload.namespace,
payload.datasetName,
payload.field,
payload.tag
)
)
} catch (e) {
yield put(applicationError('Something went wrong while adding tag to dataset field.'))
}
Expand Down
Loading