Skip to content

Commit 7745cc1

Browse files
harsh-vadorharsh-vador
authored
fix(ui): Fix duplicate owners & tier field in project explorer card (#24297)
* fix(ui): fix duplicate owner field in overview section in explore card * nit * nit * hide tier row in overview section * refactor code
1 parent e18c848 commit 7745cc1

5 files changed

Lines changed: 187 additions & 58 deletions

File tree

‎openmetadata-ui/src/main/resources/ui/src/components/common/OverviewSection/CommonEntitySummaryInfoV1.interface.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,5 @@ export interface CommonEntitySummaryInfoV1Props {
2626
entityInfo: EntityInfoItemV1[];
2727
componentType: string;
2828
isDomainVisible?: boolean;
29+
excludedItems?: string[];
2930
}

‎openmetadata-ui/src/main/resources/ui/src/components/common/OverviewSection/CommonEntitySummaryInfoV1.test.tsx‎

Lines changed: 110 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
import { render, screen } from '@testing-library/react';
1414
import CommonEntitySummaryInfoV1 from './CommonEntitySummaryInfoV1';
15+
import { EntityInfoItemV1 } from './CommonEntitySummaryInfoV1.interface';
1516

1617
// Mock i18n
1718
jest.mock('react-i18next', () => ({
@@ -38,7 +39,7 @@ jest.mock('react-router-dom', () => ({
3839
),
3940
}));
4041

41-
const defaultItems = [
42+
const defaultItems: EntityInfoItemV1[] = [
4243
{ name: 'Type', value: 'Table', visible: ['explore'] },
4344
{ name: 'Rows', value: 1000, visible: ['explore'] },
4445
{ name: 'Columns', value: 15, visible: ['explore'] },
@@ -53,7 +54,7 @@ describe('CommonEntitySummaryInfoV1', () => {
5354
const { container } = render(
5455
<CommonEntitySummaryInfoV1
5556
componentType="explore"
56-
entityInfo={defaultItems as any}
57+
entityInfo={defaultItems}
5758
/>
5859
);
5960

@@ -72,32 +73,29 @@ describe('CommonEntitySummaryInfoV1', () => {
7273
});
7374

7475
it('filters out items not visible for the componentType', () => {
75-
const items = [
76+
const items: EntityInfoItemV1[] = [
7677
{ name: 'Visible', value: 'Yes', visible: ['explore'] },
7778
{ name: 'Hidden', value: 'No', visible: ['other'] },
7879
];
7980

8081
render(
81-
<CommonEntitySummaryInfoV1
82-
componentType="explore"
83-
entityInfo={items as any}
84-
/>
82+
<CommonEntitySummaryInfoV1 componentType="explore" entityInfo={items} />
8583
);
8684

8785
expect(screen.getByTestId('Visible-label')).toBeInTheDocument();
8886
expect(screen.queryByTestId('Hidden-label')).toBeNull();
8987
});
9088

9189
it('shows domain item when isDomainVisible is true regardless of visibility array', () => {
92-
const items = [
90+
const items: EntityInfoItemV1[] = [
9391
{ name: 'label.domain-plural', value: 'Domain A', visible: ['other'] },
9492
];
9593

9694
render(
9795
<CommonEntitySummaryInfoV1
9896
isDomainVisible
9997
componentType="explore"
100-
entityInfo={items as any}
98+
entityInfo={items}
10199
/>
102100
);
103101

@@ -108,7 +106,7 @@ describe('CommonEntitySummaryInfoV1', () => {
108106
});
109107

110108
it('renders internal link when isLink is true and isExternal is false', () => {
111-
const items = [
109+
const items: EntityInfoItemV1[] = [
112110
{
113111
name: 'Docs',
114112
value: 'OpenMetadata',
@@ -120,10 +118,7 @@ describe('CommonEntitySummaryInfoV1', () => {
120118
];
121119

122120
render(
123-
<CommonEntitySummaryInfoV1
124-
componentType="explore"
125-
entityInfo={items as any}
126-
/>
121+
<CommonEntitySummaryInfoV1 componentType="explore" entityInfo={items} />
127122
);
128123

129124
const link = screen.getByTestId('internal-link');
@@ -134,7 +129,7 @@ describe('CommonEntitySummaryInfoV1', () => {
134129
});
135130

136131
it('renders external link with icon when isExternal is true', () => {
137-
const items = [
132+
const items: EntityInfoItemV1[] = [
138133
{
139134
name: 'Website',
140135
value: 'OpenMetadata',
@@ -146,10 +141,7 @@ describe('CommonEntitySummaryInfoV1', () => {
146141
];
147142

148143
const { container } = render(
149-
<CommonEntitySummaryInfoV1
150-
componentType="explore"
151-
entityInfo={items as any}
152-
/>
144+
<CommonEntitySummaryInfoV1 componentType="explore" entityInfo={items} />
153145
);
154146

155147
const anchor = container.querySelector('a.summary-item-link');
@@ -160,38 +152,127 @@ describe('CommonEntitySummaryInfoV1', () => {
160152
});
161153

162154
it('renders dash when value is null or undefined', () => {
163-
const items = [
155+
const items: EntityInfoItemV1[] = [
164156
{ name: 'Empty', value: undefined, visible: ['explore'] },
165157
{ name: 'AlsoEmpty', value: null, visible: ['explore'] },
166158
];
167159

168160
const { getByTestId } = render(
169-
<CommonEntitySummaryInfoV1
170-
componentType="explore"
171-
entityInfo={items as any}
172-
/>
161+
<CommonEntitySummaryInfoV1 componentType="explore" entityInfo={items} />
173162
);
174163

175164
expect(getByTestId('Empty-value')).toHaveTextContent('-');
176165
expect(getByTestId('AlsoEmpty-value')).toHaveTextContent('-');
177166
});
178167

179168
it('supports labels with special characters in testids', () => {
180-
const items = [
169+
const items: EntityInfoItemV1[] = [
181170
{ name: 'Label & Value', value: 'Test', visible: ['explore'] },
182171
{ name: 'Label < > " \'', value: 'Again', visible: ['explore'] },
183172
];
184173

185174
render(
186-
<CommonEntitySummaryInfoV1
187-
componentType="explore"
188-
entityInfo={items as any}
189-
/>
175+
<CommonEntitySummaryInfoV1 componentType="explore" entityInfo={items} />
190176
);
191177

192178
expect(screen.getByTestId('Label & Value-value')).toHaveTextContent('Test');
193179
expect(screen.getByTestId('Label < > " \'-value')).toHaveTextContent(
194180
'Again'
195181
);
196182
});
183+
184+
it('excludes items specified in excludedItems prop', () => {
185+
const items: EntityInfoItemV1[] = [
186+
{ name: 'Type', value: 'Table', visible: ['explore'] },
187+
{ name: 'Owners', value: 'John Doe', visible: ['explore'] },
188+
{ name: 'Tier', value: 'Gold', visible: ['explore'] },
189+
{ name: 'Rows', value: 1000, visible: ['explore'] },
190+
];
191+
192+
render(
193+
<CommonEntitySummaryInfoV1
194+
componentType="explore"
195+
entityInfo={items}
196+
excludedItems={['Owners', 'Tier']}
197+
/>
198+
);
199+
200+
expect(screen.getByTestId('Type-label')).toBeInTheDocument();
201+
expect(screen.getByTestId('Rows-label')).toBeInTheDocument();
202+
expect(screen.queryByTestId('Owners-label')).toBeNull();
203+
expect(screen.queryByTestId('Tier-label')).toBeNull();
204+
});
205+
206+
it('renders all items when excludedItems is empty array', () => {
207+
const items: EntityInfoItemV1[] = [
208+
{ name: 'Type', value: 'Table', visible: ['explore'] },
209+
{ name: 'Owners', value: 'John Doe', visible: ['explore'] },
210+
];
211+
212+
render(
213+
<CommonEntitySummaryInfoV1
214+
componentType="explore"
215+
entityInfo={items}
216+
excludedItems={[]}
217+
/>
218+
);
219+
220+
expect(screen.getByTestId('Type-label')).toBeInTheDocument();
221+
expect(screen.getByTestId('Owners-label')).toBeInTheDocument();
222+
});
223+
224+
it('renders all items when excludedItems is not provided', () => {
225+
const items: EntityInfoItemV1[] = [
226+
{ name: 'Type', value: 'Table', visible: ['explore'] },
227+
{ name: 'Owners', value: 'John Doe', visible: ['explore'] },
228+
];
229+
230+
render(
231+
<CommonEntitySummaryInfoV1 componentType="explore" entityInfo={items} />
232+
);
233+
234+
expect(screen.getByTestId('Type-label')).toBeInTheDocument();
235+
expect(screen.getByTestId('Owners-label')).toBeInTheDocument();
236+
});
237+
238+
it('combines excludedItems with visibility filtering', () => {
239+
const items: EntityInfoItemV1[] = [
240+
{ name: 'Type', value: 'Table', visible: ['explore'] },
241+
{ name: 'Owners', value: 'John Doe', visible: ['explore'] },
242+
{ name: 'Hidden', value: 'Secret', visible: ['other'] },
243+
{ name: 'Tier', value: 'Gold', visible: ['explore'] },
244+
];
245+
246+
render(
247+
<CommonEntitySummaryInfoV1
248+
componentType="explore"
249+
entityInfo={items}
250+
excludedItems={['Owners', 'Tier']}
251+
/>
252+
);
253+
254+
expect(screen.getByTestId('Type-label')).toBeInTheDocument();
255+
expect(screen.queryByTestId('Owners-label')).toBeNull();
256+
expect(screen.queryByTestId('Hidden-label')).toBeNull();
257+
expect(screen.queryByTestId('Tier-label')).toBeNull();
258+
});
259+
260+
it('shows domain item when isDomainVisible is true even if in excludedItems', () => {
261+
const items: EntityInfoItemV1[] = [
262+
{ name: 'Type', value: 'Table', visible: ['explore'] },
263+
{ name: 'label.domain-plural', value: 'Domain A', visible: ['other'] },
264+
];
265+
266+
render(
267+
<CommonEntitySummaryInfoV1
268+
isDomainVisible
269+
componentType="explore"
270+
entityInfo={items}
271+
excludedItems={['label.domain-plural']}
272+
/>
273+
);
274+
275+
expect(screen.getByTestId('Type-label')).toBeInTheDocument();
276+
expect(screen.queryByTestId('label.domain-plural-label')).toBeNull();
277+
});
197278
});

‎openmetadata-ui/src/main/resources/ui/src/components/common/OverviewSection/CommonEntitySummaryInfoV1.tsx‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import Icon from '@ant-design/icons/lib/components/Icon';
1414
import classNames from 'classnames';
1515
import { isNil } from 'lodash';
16+
import { useMemo } from 'react';
1617
import { useTranslation } from 'react-i18next';
1718
import { Link } from 'react-router-dom';
1819
import { ReactComponent as IconExternalLink } from '../../../assets/svg/external-links.svg';
@@ -27,14 +28,23 @@ const CommonEntitySummaryInfoV1: React.FC<CommonEntitySummaryInfoV1Props> = ({
2728
entityInfo,
2829
componentType,
2930
isDomainVisible = false,
31+
excludedItems = [],
3032
}) => {
3133
const { t } = useTranslation();
3234

33-
const isItemVisible = (item: EntityInfoItemV1) => {
34-
const isDomain = isDomainVisible && item.name === t('label.domain-plural');
35+
const visibleEntityInfo = useMemo(() => {
36+
return entityInfo.filter((info) => {
37+
if (excludedItems.includes(info.name)) {
38+
return false;
39+
}
3540

36-
return (item.visible || []).includes(componentType) || isDomain;
37-
};
41+
const isDomain =
42+
isDomainVisible && info.name === t('label.domain-plural');
43+
const isVisibleInComponent = (info.visible ?? []).includes(componentType);
44+
45+
return isVisibleInComponent || isDomain;
46+
});
47+
}, [entityInfo, componentType, isDomainVisible, excludedItems]);
3848

3949
const renderInfoValue = (info: EntityInfoItemV1) => {
4050
if (!info.isLink) {
@@ -64,7 +74,7 @@ const CommonEntitySummaryInfoV1: React.FC<CommonEntitySummaryInfoV1Props> = ({
6474

6575
return (
6676
<div className="overview-section">
67-
{entityInfo.filter(isItemVisible).map((info) => (
77+
{visibleEntityInfo.map((info) => (
6878
<div className="overview-row" key={info.name}>
6979
<span
7080
className={classNames('overview-label')}

‎openmetadata-ui/src/main/resources/ui/src/components/common/OverviewSection/OverviewSection.test.tsx‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,42 @@ jest.mock('../SectionWithEdit/SectionWithEdit', () => {
3737
));
3838
});
3939

40+
// Mock CommonEntitySummaryInfoV1 component
41+
jest.mock('./CommonEntitySummaryInfoV1', () => {
42+
return jest
43+
.fn()
44+
.mockImplementation(({ entityInfo, excludedItems = [], componentType }) => {
45+
const filteredInfo = entityInfo.filter(
46+
(item: { name: string; visible?: string[] }) => {
47+
if (excludedItems.includes(item.name)) {
48+
return false;
49+
}
50+
51+
return !componentType || (item.visible ?? []).includes(componentType);
52+
}
53+
);
54+
55+
return (
56+
<div className="overview-section">
57+
{filteredInfo.map((info: { name: string; value: unknown }) => (
58+
<div className="overview-row" key={info.name}>
59+
<span
60+
className="overview-label"
61+
data-testid={`${info.name}-label`}>
62+
{info.name}
63+
</span>
64+
<span
65+
className="overview-value text-grey-body"
66+
data-testid={`${info.name}-value`}>
67+
{String(info.value)}
68+
</span>
69+
</div>
70+
))}
71+
</div>
72+
);
73+
});
74+
});
75+
4076
const entityInfoV1 = [
4177
{ name: 'Type', value: 'Table', visible: ['explore'] },
4278
{ name: 'Rows', value: 1000, visible: ['explore'] },
@@ -116,12 +152,25 @@ describe('OverviewSection', () => {
116152
expect(values).toHaveLength(5);
117153
});
118154

119-
it('should hide when nothing visible for given componentType', () => {
155+
it('should exclude Owners and Tier items via excludedItems prop', () => {
156+
const infoWithExcluded = [
157+
{ name: 'Type', value: 'Table', visible: ['explore'] },
158+
{ name: 'Owners', value: 'John Doe', visible: ['explore'] },
159+
{ name: 'Tier', value: 'Gold', visible: ['explore'] },
160+
{ name: 'Rows', value: 1000, visible: ['explore'] },
161+
];
162+
120163
render(
121-
<OverviewSection componentType="other" entityInfoV1={entityInfoV1} />
164+
<OverviewSection
165+
componentType="explore"
166+
entityInfoV1={infoWithExcluded}
167+
/>
122168
);
123169

124-
expect(screen.queryByTestId('section-with-edit')).toBeNull();
170+
expect(screen.getByTestId('Type-label')).toBeInTheDocument();
171+
expect(screen.getByTestId('Rows-label')).toBeInTheDocument();
172+
expect(screen.queryByTestId('Owners-label')).not.toBeInTheDocument();
173+
expect(screen.queryByTestId('Tier-label')).not.toBeInTheDocument();
125174
});
126175

127176
it('should handle single item', () => {

0 commit comments

Comments
 (0)