From bf12883a09b1ddc7708e8c40d79c627c7fd9a910 Mon Sep 17 00:00:00 2001 From: Ben Dye Date: Wed, 11 Sep 2024 13:22:40 -0400 Subject: [PATCH 01/11] add reference to other key values Signed-off-by: Ben Dye --- .../js/pages/TableDetailPage/TableOwnerEditor/index.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/amundsen_application/static/js/pages/TableDetailPage/TableOwnerEditor/index.tsx b/frontend/amundsen_application/static/js/pages/TableDetailPage/TableOwnerEditor/index.tsx index cc8eeeb0c1..5a6403eaed 100644 --- a/frontend/amundsen_application/static/js/pages/TableDetailPage/TableOwnerEditor/index.tsx +++ b/frontend/amundsen_application/static/js/pages/TableDetailPage/TableOwnerEditor/index.tsx @@ -19,9 +19,11 @@ export const mapStateToProps = (state: GlobalState) => { const ownerObj = state.tableMetadata.tableOwners.owners; const items = Object.keys(ownerObj).reduce((obj, ownerId) => { // eslint-disable-next-line @typescript-eslint/naming-convention - const { profile_url, user_id, display_name } = ownerObj[ownerId]; + const { profile_url, user_id, display_name, other_key_values } = + ownerObj[ownerId]; let profileLink = profile_url; let isExternalLink = true; + let additionalOwnerInfo = other_key_values; if (indexUsersEnabled()) { isExternalLink = false; @@ -32,6 +34,7 @@ export const mapStateToProps = (state: GlobalState) => { label: display_name, link: profileLink, isExternal: isExternalLink, + additionalOwnerInfo: additionalOwnerInfo, }; return obj; From 4b42b7ca60a6faa7bb6feb2a29c8166f6467308b Mon Sep 17 00:00:00 2001 From: Ben Dye Date: Mon, 16 Sep 2024 14:16:26 -0400 Subject: [PATCH 02/11] render owner category InfoButtons when configured Signed-off-by: Ben Dye --- .../js/components/OwnerEditor/index.tsx | 142 +++++++++++++++--- 1 file changed, 123 insertions(+), 19 deletions(-) diff --git a/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx b/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx index 25773b19d0..c301df1fbd 100644 --- a/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx +++ b/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx @@ -3,19 +3,21 @@ import * as React from 'react'; import { Link } from 'react-router-dom'; -import { Modal } from 'react-bootstrap'; +import { Modal, OverlayTrigger, Popover } from 'react-bootstrap'; import AvatarLabel, { AvatarLabelProps } from 'components/AvatarLabel'; + import LoadingSpinner from 'components/LoadingSpinner'; import { ResourceType, UpdateMethod, UpdateOwnerPayload } from 'interfaces'; import { logClick, logAction } from 'utils/analytics'; -import { getUserIdLabel } from 'config/config-utils'; +import { getUserIdLabel, getOwnersSectionConfig } from 'config/config-utils'; import { EditableSectionChildProps } from 'components/EditableSection'; import * as Constants from './constants'; import './styles.scss'; +import InfoButton from 'components/InfoButton'; export interface DispatchFromProps { onUpdateList: ( @@ -33,6 +35,7 @@ export interface ComponentProps { interface OwnerAvatarLabelProps extends AvatarLabelProps { link?: string; isExternal?: boolean; + additionalOwnerInfo?: any; // TODO should ownerCategory be a separate field? So we don't require OSS users to know the right key to use within additionalOwnerInfo? } export interface StateFromProps { @@ -238,25 +241,87 @@ export class OwnerEditor extends React.Component< ); }; - render() { - const { isEditing, readOnly, resourceType } = this.props; - const { errorText, itemProps } = this.state; - const hasItems = Object.keys(itemProps).length > 0; + renderOwnersList = () => { + const { resourceType } = this.props; + const { itemProps } = this.state; + + // TODO reuse the existing code for rendering each owner, refactor to a shareable method/function + + // Render owner list grouped by category, if categories configured + if (getOwnersSectionConfig().categories.length > 0) { + const sections = getOwnersSectionConfig().categories; + + console.log(`itemProps: ${JSON.stringify(itemProps)}`); + + console.log(`sections: ${JSON.stringify(sections)}`); + + // TODO confirm an owner added via UI edit button (i) adds immediately to the owners list without page refresh + // (that's current behavior) and (ii) is added as "configured" in Lyft's Amundsen + + // TODO confirm when the config is not provided, keeps prior behavior - if (errorText) { return ( -
- {errorText} +
+ {sections.map((section, index) => ( +
    + {section.label} + + + {Object.keys(itemProps).map((key) => { + const owner = itemProps[key]; + const avatarLabel = React.createElement(AvatarLabel, owner); + + console.log(`${JSON.stringify(owner)}`); + + let listItem: React.ReactNode; + + if (owner.link === undefined) { + listItem = avatarLabel; + } else if (owner.isExternal) { + listItem = ( + + {avatarLabel} + + ); + } else if ( + section.label.toLowerCase() === + owner.additionalOwnerInfo.owner_category.toLowerCase() + ) { + listItem = ( + + {avatarLabel} + + ); + } + return
  • {listItem}
    • ; + })} +
+ ))}
); } - const ownerList = hasItems ? ( + // Render default owner list + return (
    {Object.keys(itemProps).map((key) => { const owner = itemProps[key]; const avatarLabel = React.createElement(AvatarLabel, owner); + console.log(`${JSON.stringify(owner)}`); + let listItem: React.ReactNode; if (owner.link === undefined) { @@ -276,21 +341,60 @@ export class OwnerEditor extends React.Component< ); } else { listItem = ( - +
    + {Object.entries(owner.additionalOwnerInfo).map( + ([key, value]) => ( +

    + {key}: {value} +

    + ) + )} +
    + + } > - {avatarLabel} - + + {avatarLabel} + + ); } - return
  • {listItem}
    • ; })}
- ) : null; + ); + }; + + render() { + const { isEditing, readOnly, resourceType } = this.props; + const { errorText, itemProps } = this.state; + const hasItems = Object.keys(itemProps).length > 0; + + if (errorText) { + return ( +
+ {errorText} +
+ ); + } + + // TODO if popover works, refactor to put it on external owner too, DRY + // TODO overlay is triggering when no text + // TODO don't render the info button if no ownership context info available? There should always be context + // for lyft owners though, we always have e.g. the update time + + const ownerList = hasItems ? this.renderOwnersList() : null; return (
From 535f4bc7134b10d9daaa5cbe72142eac47b781cb Mon Sep 17 00:00:00 2001 From: Ben Dye Date: Mon, 16 Sep 2024 14:20:31 -0400 Subject: [PATCH 03/11] cleanup experiment Signed-off-by: Ben Dye --- .../js/components/OwnerEditor/index.tsx | 33 ++++--------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx b/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx index c301df1fbd..29f7ae2866 100644 --- a/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx +++ b/frontend/amundsen_application/static/js/components/OwnerEditor/index.tsx @@ -341,33 +341,14 @@ export class OwnerEditor extends React.Component< ); } else { listItem = ( - -
- {Object.entries(owner.additionalOwnerInfo).map( - ([key, value]) => ( -

- {key}: {value} -

- ) - )} -
- - } + - - {avatarLabel} - - + {avatarLabel} + ); } return
  • {listItem}
    • ; From 01e93b2c4525a78994c633390a44524ab117bb8c Mon Sep 17 00:00:00 2001 From: Ben Dye Date: Tue, 17 Sep 2024 13:12:12 -0400 Subject: [PATCH 04/11] WIP backup Signed-off-by: Ben Dye --- .../static/js/components/EditableSection/index.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontend/amundsen_application/static/js/components/EditableSection/index.tsx b/frontend/amundsen_application/static/js/components/EditableSection/index.tsx index e526e53cff..315fd8546d 100644 --- a/frontend/amundsen_application/static/js/components/EditableSection/index.tsx +++ b/frontend/amundsen_application/static/js/components/EditableSection/index.tsx @@ -5,6 +5,8 @@ import * as React from 'react'; import { OverlayTrigger, Popover } from 'react-bootstrap'; +import InfoButton from 'components/InfoButton'; + import { logClick } from 'utils/analytics'; import * as Constants from './constants'; @@ -18,8 +20,11 @@ export interface EditableSectionProps { editText?: string; /* Should be used when readOnly=true to link to the source where users can edit the given metadata */ editUrl?: string; + infoButtonText?: string; // TODO make a subclass for ownerssection instead } + + interface EditableSectionState { isEditing: boolean; } @@ -138,6 +143,7 @@ export class EditableSection extends React.Component< }); }); + // TODO make the infoText a prop, don't show it if no none provided return (