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 -- table owner categories #2260

Merged
merged 10 commits into from
Sep 25, 2024
Merged
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 @@ -9,9 +9,11 @@ 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 InfoButton from 'components/InfoButton';
import { OwnerCategory } from 'interfaces/OwnerCategory';

import * as Constants from './constants';

Expand All @@ -33,6 +35,7 @@ export interface ComponentProps {
interface OwnerAvatarLabelProps extends AvatarLabelProps {
link?: string;
isExternal?: boolean;
additionalOwnerInfo?: any;
}

export interface StateFromProps {
Expand Down Expand Up @@ -238,21 +241,31 @@ 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;
renderOwnersSection = (section: OwnerCategory | null) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

we might want to consider doing some indication if there are no owners in a certain category. right now those titles look like floating text but isn't fully clear that it is an empty list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const { resourceType } = this.props;
const { itemProps } = this.state;

if (errorText) {
return (
<div className="owner-editor-component">
<span className="status-message">{errorText}</span>
</div>
// check if rendering an owner category that lacks any entries
let isEmptySection = false;

if (section) {
isEmptySection = Object.keys(itemProps).every(
(key) =>
itemProps[key].additionalOwnerInfo.owner_category.toLowerCase() !==
section.label.toLowerCase()
);
}

const ownerList = hasItems ? (
return (
<ul className="component-list">
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a css class to the <ul> here that sets margin-bottom: $spacer-1 (8px) but then sets it to 0 for the last-child? the larger sections have their own spacing so we want to skip it for the last item

{section ? (
<div>
<span className="title-3">{section.label}</span>
<InfoButton infoText={section.definition} />
</div>
) : null}
{isEmptySection ? <span className="body-3">None known</span> : null}

{Object.keys(itemProps).map((key) => {
const owner = itemProps[key];
const avatarLabel = React.createElement(AvatarLabel, owner);
Expand All @@ -274,7 +287,12 @@ export class OwnerEditor extends React.Component<
{avatarLabel}
</a>
);
} else {
} else if (
(section && // if section, only render owner that matches category
section.label.toLowerCase() ===
owner.additionalOwnerInfo.owner_category.toLowerCase()) ||
!section
) {
listItem = (
<Link
to={owner.link}
Expand All @@ -290,7 +308,37 @@ export class OwnerEditor extends React.Component<
return <li key={`list-item:${key}`}>{listItem}</li>;
})}
</ul>
) : null;
);
};

renderOwnersList = () => {
const sections = getOwnersSectionConfig().categories;

if (sections.length > 0) {
return (
<div>
{sections.map((section) => this.renderOwnersSection(section))}
</div>
);
}

return this.renderOwnersSection(null);
};

render() {
const { isEditing, readOnly } = this.props;
const { errorText, itemProps } = this.state;
const hasItems = Object.keys(itemProps).length > 0;

if (errorText) {
return (
<div className="owner-editor-component">
<span className="status-message">{errorText}</span>
</div>
);
}

const ownerList = hasItems ? this.renderOwnersList() : null;

return (
<div className="owner-editor-component">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,12 @@
}
}
}

ul.component-list {
margin-bottom: $spacer-1;

&:last-child {
margin-bottom: 0;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ const configDefault: AppConfig = {
maxNestedColumns: 500,
},
numberFormat: null,
ownersSection: {
categories: [],
},
productTour: {},
resourceConfig: {
[ResourceType.dashboard]: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
SortCriteria,
} from '../interfaces';

import { OwnerCategory } from '../interfaces/OwnerCategory';
import { Widget } from '../interfaces/Widgets';

/**
Expand Down Expand Up @@ -36,6 +37,7 @@ export interface AppConfig {
navTheme: 'dark' | 'light';
nestedColumns: NestedColumnConfig;
numberFormat: NumberFormatConfig | null;
ownersSection: OwnersSectionConfig;
productTour: ToursConfig;
resourceConfig: ResourceConfig;
searchPagination: SearchPagination;
Expand Down Expand Up @@ -84,6 +86,7 @@ export interface AppConfigCustom {
productTour?: ToursConfig;
searchPagination?: SearchPagination;
homePageWidgets?: HomePageWidgetsConfig;
ownersSection?: OwnersSectionConfig;
}

/**
Expand Down Expand Up @@ -604,3 +607,7 @@ export interface HomePageWidgetsConfig {
*/
widgets: Widget[];
}

export interface OwnersSectionConfig {
categories: OwnerCategory[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
HomePageWidgetsConfig,
TableLineageConfig,
DateFormatConfig,
OwnersSectionConfig,
} from './config-types';

const DEFAULT_DYNAMIC_NOTICES_ENABLED_FLAG = false;
Expand Down Expand Up @@ -646,3 +647,10 @@ export function getUserIdLabel(): string {
export function getDateConfiguration(): DateFormatConfig {
return AppConfig.date;
}

/**
* Returns the resource owners section configuration
*/
export function getOwnersSectionConfig(): OwnersSectionConfig {
return AppConfig.ownersSection;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright Contributors to the Amundsen project.
// SPDX-License-Identifier: Apache-2.0

export interface OwnerCategory {
label: string;
definition?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export interface User {
email: string;
profile_url: string;
user_id: string;
other_key_values?: Record<string, string>;
kristenarmes marked this conversation as resolved.
Show resolved Hide resolved
}

// Not a good name, not sure if we can consolidate yet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
const additionalOwnerInfo = other_key_values;

if (indexUsersEnabled()) {
isExternalLink = false;
Expand All @@ -32,6 +34,7 @@ export const mapStateToProps = (state: GlobalState) => {
label: display_name,
link: profileLink,
isExternal: isExternalLink,
additionalOwnerInfo,
};

return obj;
Expand Down
Loading