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(glossary): add ability to clone glossary term(name and documentation) from term profile menu #9170

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ export class GlossaryTermEntity implements Entity<GlossaryTerm> {
useEntityQuery={useGetGlossaryTermQuery as any}
headerActionItems={new Set([EntityActionItem.BATCH_ADD_GLOSSARY_TERM])}
headerDropdownItems={
new Set([EntityMenuItems.UPDATE_DEPRECATION, EntityMenuItems.MOVE, EntityMenuItems.DELETE])
new Set([
EntityMenuItems.UPDATE_DEPRECATION,
EntityMenuItems.CLONE,
EntityMenuItems.MOVE,
EntityMenuItems.DELETE,
])
}
isNameEditable
hideBrowseBar
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import styled from 'styled-components/macro';
import { EditOutlined } from '@ant-design/icons';
import { message, Button, Input, Modal, Typography, Form, Collapse } from 'antd';
import DOMPurify from 'dompurify';
import { useHistory } from 'react-router';
import {
useCreateGlossaryTermMutation,
useCreateGlossaryNodeMutation,
Expand All @@ -16,6 +17,7 @@ import DescriptionModal from '../components/legacy/DescriptionModal';
import { validateCustomUrnId } from '../../../shared/textUtil';
import { useGlossaryEntityData } from '../GlossaryEntityContext';
import { getGlossaryRootToUpdate, updateGlossarySidebar } from '../../../glossary/utils';
import { getEntityPath } from '../containers/profile/utils';

const StyledItem = styled(Form.Item)`
margin-bottom: 0;
Expand All @@ -33,6 +35,7 @@ interface Props {
entityType: EntityType;
onClose: () => void;
refetchData?: () => void;
clone?: boolean;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about a rename to isCloning? a little more obvious that it's a boolean flag than just clone if you ask me


function CreateGlossaryEntityModal(props: Props) {
Expand All @@ -43,15 +46,31 @@ function CreateGlossaryEntityModal(props: Props) {
const entityRegistry = useEntityRegistry();
const [stagedId, setStagedId] = useState<string | undefined>(undefined);
const [stagedName, setStagedName] = useState('');
const [selectedParentUrn, setSelectedParentUrn] = useState(entityData.urn);
const [selectedParentUrn, setSelectedParentUrn] = useState<string>(props.clone ? '' : entityData.urn);
const [documentation, setDocumentation] = useState('');
const [isDocumentationModalVisible, setIsDocumentationModalVisible] = useState(false);
const [createButtonDisabled, setCreateButtonDisabled] = useState(true);
const refetch = useRefetch();
const history = useHistory();

const [createGlossaryTermMutation] = useCreateGlossaryTermMutation();
const [createGlossaryNodeMutation] = useCreateGlossaryNodeMutation();

useEffect(() => {
if (props.clone && entityData.entityData) {
const { properties } = entityData.entityData;

if (properties?.name) {
setStagedName(properties.name);
form.setFieldValue('name', properties.name);
}

if (properties?.description) {
setDocumentation(properties.description);
}
}
}, [props.clone, entityData.entityData, form]);

function createGlossaryEntity() {
const mutation =
entityType === EntityType.GlossaryTerm ? createGlossaryTermMutation : createGlossaryNodeMutation;
Expand All @@ -67,7 +86,7 @@ function CreateGlossaryEntityModal(props: Props) {
},
},
})
.then(() => {
.then((res) => {
message.loading({ content: 'Updating...', duration: 2 });
setTimeout(() => {
analytics.event({
Expand All @@ -82,12 +101,21 @@ function CreateGlossaryEntityModal(props: Props) {
refetch();
if (isInGlossaryContext) {
// either refresh this current glossary node or the root nodes or root terms
const nodeToUpdate = entityData?.urn || getGlossaryRootToUpdate(entityType);
const nodeToUpdate = props.clone
? getGlossaryRootToUpdate(entityType)
: entityData?.urn || getGlossaryRootToUpdate(entityType);
updateGlossarySidebar([nodeToUpdate], urnsToUpdate, setUrnsToUpdate);
Comment on lines +104 to +106
Copy link
Collaborator

Choose a reason for hiding this comment

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

so i think you may have unconvered an existing bug here that can be fixed and work for both the clone case as well as all regular cases. instead of always updating the root when cloning, I think this line can be:
const nodeToUpdate = selectedParentUrn || getGlossaryRootToUpdate(entityType); because it's the parent that needs updating in the sidebar if there is a parent, otherwise we update the root

}
if (refetchData) {
refetchData();
}
if (props.clone) {
const redirectUrn =
entityType === EntityType.GlossaryTerm
? res.data?.createGlossaryTerm
: res.data?.createGlossaryNode;
history.push(getEntityPath(entityType, redirectUrn, entityRegistry, false, false));
}
}, 2000);
})
.catch((e) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
LinkOutlined,
MoreOutlined,
PlusOutlined,
CopyOutlined,
} from '@ant-design/icons';
import { Redirect } from 'react-router';
import { EntityType } from '../../../../types.generated';
Expand All @@ -32,6 +33,7 @@ export enum EntityMenuItems {
ADD_TERM_GROUP,
DELETE,
MOVE,
CLONE,
}

export const MenuIcon = styled(MoreOutlined)<{ fontSize?: number }>`
Expand Down Expand Up @@ -107,6 +109,7 @@ function EntityDropdown(props: Props) {

const [isCreateTermModalVisible, setIsCreateTermModalVisible] = useState(false);
const [isCreateNodeModalVisible, setIsCreateNodeModalVisible] = useState(false);
const [isCloneEntityModalVisible, setIsCloneEntityModalVisible] = useState<boolean>(false);
const [isDeprecationModalVisible, setIsDeprecationModalVisible] = useState(false);
const [isMoveModalVisible, setIsMoveModalVisible] = useState(false);

Expand Down Expand Up @@ -197,10 +200,21 @@ function EntityDropdown(props: Props) {
</MenuItem>
</StyledMenuItem>
)}
{menuItems.has(EntityMenuItems.CLONE) && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

how do you feel about moving this item down below the move and delete buttons? clone feels like an additional "nice to have" while move and delete feel more like required functionality.

<StyledMenuItem
key="4"
disabled={!entityData?.privileges?.canManageEntity}
onClick={() => setIsCloneEntityModalVisible(true)}
>
<MenuItem>
<CopyOutlined /> &nbsp;Clone
</MenuItem>
</StyledMenuItem>
)}
{!isDomainMoveHidden && menuItems.has(EntityMenuItems.MOVE) && (
<StyledMenuItem
data-testid="entity-menu-move-button"
key="4"
key="5"
disabled={isMoveDisabled(entityType, entityData, me.platformPrivileges)}
onClick={() => setIsMoveModalVisible(true)}
>
Expand All @@ -211,7 +225,7 @@ function EntityDropdown(props: Props) {
)}
{menuItems.has(EntityMenuItems.DELETE) && (
<StyledMenuItem
key="5"
key="6"
disabled={isDeleteDisabled(entityType, entityData, me.platformPrivileges)}
onClick={onDeleteEntity}
>
Expand Down Expand Up @@ -250,6 +264,14 @@ function EntityDropdown(props: Props) {
refetchData={refetchForNodes}
/>
)}
{isCloneEntityModalVisible && (
<CreateGlossaryEntityModal
entityType={entityType}
onClose={() => setIsCloneEntityModalVisible(false)}
refetchData={entityType === EntityType.GlossaryTerm ? refetchForTerms : refetchForNodes}
clone
/>
)}
{isDeprecationModalVisible && (
<UpdateDeprecationModal
urns={[urn]}
Expand Down
1 change: 1 addition & 0 deletions datahub-web-react/src/app/entity/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export type GenericEntityProperties = {
type?: EntityType;
name?: Maybe<string>;
properties?: Maybe<{
name?: Maybe<string>;
description?: Maybe<string>;
qualifiedName?: Maybe<string>;
sourceUrl?: Maybe<string>;
Expand Down
Loading