-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Deprecate an entity #4633
Deprecate an entity #4633
Conversation
|
||
return ( | ||
<Modal | ||
title="Add Deprecation Data" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Let's change to "Deprecation Details"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and Committed.
refetch?: () => Promise<any>; | ||
}; | ||
|
||
export const AddDeprecatedDataModal = ({ urn, visible, onClose, refetch }: Props) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Let's change name to "AddDeprecationDetailsModal"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and Committed.
Cancel | ||
</Button> | ||
<Button form="addDeprecationForm" key="submit" htmlType="submit"> | ||
Add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: let's rename this to "Ok"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and Committed.
onClose(); | ||
}; | ||
|
||
const handleAdd = async (formData: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: let's rename to handleOk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and Committed.
<Form.Item name="note" label="Note"> | ||
<Input placeholder="Add Note" autoFocus /> | ||
</Form.Item> | ||
<Form.Item name="decommissionTime" label="Decommission Time"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Decomission Date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and Committed.
/> | ||
</Tooltip> | ||
</HeaderContainer> | ||
{entityData?.deprecation?.deprecated && entityData?.deprecation?.decommissionTime !== null && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Let's put this condition in a variable above^^ (line 227) that is called "isDeprecated" --> I think we can also remove the && entityData?.deprecation?.decommissionTime !== null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the logic and committed
datahub-web-react/src/app/entity/shared/containers/profile/header/EntityHeader.tsx
Show resolved
Hide resolved
/> | ||
</Tooltip> | ||
</HeaderContainer> | ||
{entityData?.deprecation?.deprecated && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets do this. above create a new variable
const hasDetails = (entityData?.deprecation?.note !== '' ||
entityData?.deprecation?.decommissionTime !== null)
then inside the content box we can do this:
!hasDetails && <current line 276-295> || 'No additional details'
then you can get rid of the long conditional on 269, as well as the line 303-310
entityData?.deprecation?.deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and committed my changes
Added a button to "deprecate" an entity from inside of the UI from the following entity profile page: dataset, chart, dashboard, data flow, data job, container, glossary term
Checklist