Suggestions feedback (#15723)

* use entity store for storing fqn

* fix feedbacks

* fix tests

* store cleanup

* cleanup

* review comments

* minor label change

* minor change

* fix flaky lineage cypress
This commit is contained in:
Karan Hotchandani 2024-03-28 21:34:16 +05:30 committed by GitHub
parent 884029f031
commit e22b8f7741
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 227 additions and 158 deletions

View File

@ -82,6 +82,30 @@ const deleteNode = (node) => {
verifyResponseStatusCode('@lineageDeleteApi', 200);
};
const deleteEdge = (fromNode, toNode) => {
interceptURL('DELETE', '/api/v1/lineage/**', 'lineageDeleteApi');
cy.get(`[data-testid="edge-${fromNode.fqn}-${toNode.fqn}"]`).click({
force: true,
});
if (
['Table', 'Topic'].indexOf(fromNode.entityType) > -1 &&
['Table', 'Topic'].indexOf(toNode.entityType) > -1
) {
cy.get('[data-testid="add-pipeline"]').click();
cy.get(
'[data-testid="add-edge-modal"] [data-testid="remove-edge-button"]'
).click();
} else {
cy.get('[data-testid="delete-button"]').click();
}
cy.get(
'[data-testid="delete-edge-confirmation-modal"] .ant-btn-primary'
).click();
verifyResponseStatusCode('@lineageDeleteApi', 200);
};
const applyPipelineFromModal = (fromNode, toNode, pipelineData) => {
interceptURL('PUT', '/api/v1/lineage', 'lineageApi');
cy.get(`[data-testid="edge-${fromNode.fqn}-${toNode.fqn}"]`).click({
@ -244,10 +268,10 @@ describe('Lineage verification', { tags: 'DataAssets' }, () => {
// Delete Nodes
for (let i = 0; i < LINEAGE_ITEMS.length; i++) {
if (i !== index) {
deleteNode(LINEAGE_ITEMS[i]);
cy.get(`[data-testid="lineage-node-${LINEAGE_ITEMS[i].fqn}"]`).should(
'not.exist'
);
deleteEdge(entity, LINEAGE_ITEMS[i]);
cy.get(
`[data-testid="edge-${entity.fqn}-${LINEAGE_ITEMS[i].fqn}"]`
).should('not.exist');
}
}

View File

@ -22,6 +22,7 @@ import SignUpPage from '../../pages/SignUp/SignUpPage';
import applicationRoutesClass from '../../utils/ApplicationRoutesClassBase';
import Appbar from '../AppBar/Appbar';
import LeftSidebar from '../MyData/LeftSidebar/LeftSidebar.component';
import applicationsClassBase from '../Settings/Applications/AppDetails/ApplicationsClassBase';
import './app-container.less';
const AppContainer = () => {
@ -29,7 +30,7 @@ const AppContainer = () => {
const { Header, Sider, Content } = Layout;
const { currentUser } = useApplicationStore();
const AuthenticatedRouter = applicationRoutesClass.getRouteElements();
const ApplicationExtras = applicationsClassBase.getApplicationExtension();
const isDirectionRTL = useMemo(() => i18n.dir() === 'rtl', [i18n]);
return (
@ -53,6 +54,7 @@ const AppContainer = () => {
<Layout>
<Content className="main-content">
<AuthenticatedRouter />
{ApplicationExtras && <ApplicationExtras />}
</Content>
</Layout>
</Layout>

View File

@ -11,13 +11,10 @@
* limitations under the License.
*/
import { Button } from 'antd';
import { t } from 'i18next';
import { lowerCase } from 'lodash';
import React, { Fragment, FunctionComponent, useMemo, useState } from 'react';
import EntityLink from '../../../utils/EntityLink';
import React, { Fragment, FunctionComponent, useState } from 'react';
import Searchbar from '../../common/SearchBarComponent/SearchBar.component';
import { useSuggestionsContext } from '../../Suggestions/SuggestionsProvider/SuggestionsProvider';
import SchemaTable from '../SchemaTable/SchemaTable.component';
import { Props } from './SchemaTab.interfaces';
@ -34,20 +31,11 @@ const SchemaTab: FunctionComponent<Props> = ({
tableConstraints,
}: Props) => {
const [searchText, setSearchText] = useState('');
const { selectedUserSuggestions } = useSuggestionsContext();
const handleSearchAction = (searchValue: string) => {
setSearchText(searchValue);
};
const columnSuggestions = useMemo(
() =>
selectedUserSuggestions?.filter(
(item) => EntityLink.getTableColumnName(item.entityLink) !== undefined
) ?? [],
[selectedUserSuggestions]
);
return (
<Fragment>
<div className="d-flex items-center justify-between">
@ -60,11 +48,6 @@ const SchemaTab: FunctionComponent<Props> = ({
onSearch={handleSearchAction}
/>
</div>
{columnSuggestions.length > 0 && (
<Button className="suggestion-pending-btn">
{columnSuggestions.length} {t('label.suggestion-pending')}
</Button>
)}
</div>
<SchemaTable
columnName={columnName}

View File

@ -60,6 +60,7 @@ const TableDescription = ({
<SuggestionsAlert
hasEditAccess={hasEditPermission}
maxLength={40}
showSuggestedBy={false}
suggestion={activeSuggestion}
/>
);
@ -86,7 +87,7 @@ const TableDescription = ({
return (
<Space
className="hover-icon-group w-full"
className="hover-icon-group w-full d-flex"
data-testid="description"
direction="vertical"
id={`field-description-${index}`}>

View File

@ -76,7 +76,7 @@ import AppSchedule from '../AppSchedule/AppSchedule.component';
import { ApplicationTabs } from '../MarketPlaceAppDetails/MarketPlaceAppDetails.interface';
import './app-details.less';
import { AppAction } from './AppDetails.interface';
import applicationSchemaClassBase from './ApplicationSchemaClassBase';
import applicationsClassBase from './ApplicationsClassBase';
const AppDetails = () => {
const { t } = useTranslation();
@ -93,7 +93,7 @@ const AppDetails = () => {
isRunLoading: false,
isSaveLoading: false,
});
const UiSchema = applicationSchemaClassBase.getJSONUISchema();
const UiSchema = applicationsClassBase.getJSONUISchema();
const fetchAppDetails = useCallback(async () => {
setLoadingState((prev) => ({ ...prev, isFetchLoading: true }));
@ -104,7 +104,7 @@ const AppDetails = () => {
});
setAppData(data);
const schema = await applicationSchemaClassBase.importSchema(fqn);
const schema = await applicationsClassBase.importSchema(fqn);
setJsonSchema(schema.default);
} catch (error) {

View File

@ -138,7 +138,7 @@ jest.mock('../AppSchedule/AppSchedule.component', () =>
))
);
jest.mock('./ApplicationSchemaClassBase', () => ({
jest.mock('./ApplicationsClassBase', () => ({
importSchema: jest.fn().mockReturnValue({ default: ['table'] }),
getJSONUISchema: jest.fn().mockReturnValue({}),
}));

View File

@ -11,7 +11,9 @@
* limitations under the License.
*/
class ApplicationSchemaClassBase {
import { FC } from 'react';
class ApplicationsClassBase {
public importSchema(fqn: string) {
return import(`../../../../utils/ApplicationSchemas/${fqn}.json`);
}
@ -21,9 +23,17 @@ class ApplicationSchemaClassBase {
public importAppLogo(appName: string) {
return import(`../../../../assets/svg/${appName}.svg`);
}
/**
* Used to pass extra elements from installed Apps.
*
* @return {FC | null} The application extension, or null if none exists.
*/
public getApplicationExtension(): FC | null {
return null;
}
}
const applicationSchemaClassBase = new ApplicationSchemaClassBase();
const applicationsClassBase = new ApplicationsClassBase();
export default applicationSchemaClassBase;
export { ApplicationSchemaClassBase };
export default applicationsClassBase;
export { ApplicationsClassBase };

View File

@ -12,7 +12,7 @@
*/
import { Avatar } from 'antd';
import React, { useCallback, useEffect, useState } from 'react';
import applicationSchemaClassBase from '../AppDetails/ApplicationSchemaClassBase';
import applicationsClassBase from '../AppDetails/ApplicationsClassBase';
const AppLogo = ({
logo,
@ -25,7 +25,7 @@ const AppLogo = ({
const fetchLogo = useCallback(async () => {
if (!logo) {
const data = await applicationSchemaClassBase.importAppLogo(appName);
const data = await applicationsClassBase.importAppLogo(appName);
const Icon = data.ReactComponent as React.ComponentType<
JSX.IntrinsicElements['svg']
>;

View File

@ -16,4 +16,5 @@ export interface SuggestionsAlertProps {
suggestion: Suggestion;
hasEditAccess?: boolean;
maxLength?: number;
showSuggestedBy?: boolean;
}

View File

@ -11,7 +11,7 @@
* limitations under the License.
*/
import { CheckOutlined, CloseOutlined } from '@ant-design/icons';
import { Button, Card, Space, Typography } from 'antd';
import { Button, Card, Typography } from 'antd';
import React from 'react';
import { useTranslation } from 'react-i18next';
import { ReactComponent as StarIcon } from '../../../assets/svg/ic-suggestions.svg';
@ -27,6 +27,7 @@ const SuggestionsAlert = ({
suggestion,
hasEditAccess = false,
maxLength,
showSuggestedBy = true,
}: SuggestionsAlertProps) => {
const { t } = useTranslation();
const { acceptRejectSuggestion } = useSuggestionsContext();
@ -37,60 +38,59 @@ const SuggestionsAlert = ({
}
return (
<Space
className="schema-description d-flex"
data-testid="asset-description-container"
direction="vertical"
size={12}>
<Card className="suggested-description-card card-padding-0">
<div className="suggested-alert-content">
<RichTextEditorPreviewer
markdown={suggestion.description ?? ''}
maxLength={maxLength}
/>
</div>
<div className="suggested-alert-footer d-flex justify-between">
<div className="d-flex items-center gap-2 ">
<StarIcon width={14} />
<Typography.Text className="text-grey-muted font-italic">
{t('label.suggested-by')}
</Typography.Text>
<UserPopOverCard userName={userName}>
<span>
<ProfilePicture
className="suggested-alert-footer-profile-pic"
name={userName}
width="20"
/>
</span>
</UserPopOverCard>
</div>
{hasEditAccess && (
<div className="d-flex justify-end gap-2">
<Button
ghost
data-testid="reject-suggestion"
icon={<CloseOutlined />}
size="small"
type="primary"
onClick={() =>
acceptRejectSuggestion(suggestion, SuggestionAction.Reject)
}
/>
<Button
data-testid="accept-suggestion"
icon={<CheckOutlined />}
size="small"
type="primary"
onClick={() =>
acceptRejectSuggestion(suggestion, SuggestionAction.Accept)
}
/>
</div>
<Card className="suggested-description-card card-padding-0">
<div className="suggested-alert-content">
<RichTextEditorPreviewer
markdown={suggestion.description ?? ''}
maxLength={maxLength}
/>
</div>
<div className="suggested-alert-footer d-flex justify-between">
<div className="d-flex items-center gap-2 ">
{showSuggestedBy && (
<>
<StarIcon width={14} />
<Typography.Text className="text-grey-muted font-italic">
{t('label.suggested-by')}
</Typography.Text>
<UserPopOverCard userName={userName}>
<span>
<ProfilePicture
className="suggested-alert-footer-profile-pic"
name={userName}
width="20"
/>
</span>
</UserPopOverCard>
</>
)}
</div>
</Card>
</Space>
{hasEditAccess && (
<div className="d-flex justify-end gap-2">
<Button
ghost
data-testid="reject-suggestion"
icon={<CloseOutlined />}
size="small"
type="primary"
onClick={() =>
acceptRejectSuggestion(suggestion, SuggestionAction.Reject)
}
/>
<Button
data-testid="accept-suggestion"
icon={<CheckOutlined />}
size="small"
type="primary"
onClick={() =>
acceptRejectSuggestion(suggestion, SuggestionAction.Accept)
}
/>
</div>
)}
</div>
</Card>
);
};

View File

@ -24,7 +24,7 @@ export interface SuggestionsContextType {
loadingAccept: boolean;
loadingReject: boolean;
allSuggestionsUsers: EntityReference[];
onUpdateActiveUser: (user: EntityReference) => void;
onUpdateActiveUser: (user?: EntityReference) => void;
fetchSuggestions: (entityFqn: string) => void;
acceptRejectSuggestion: (
suggestion: Suggestion,

View File

@ -119,7 +119,7 @@ const SuggestionsProvider = ({ children }: { children?: ReactNode }) => {
);
const onUpdateActiveUser = useCallback(
(user: EntityReference) => {
(user?: EntityReference) => {
setActiveUser(user);
},
[suggestionsByUser]

View File

@ -10,7 +10,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Button, Typography } from 'antd';
import { CheckOutlined, CloseOutlined } from '@ant-design/icons';
import { Button, Space, Typography } from 'antd';
import { t } from 'i18next';
import React from 'react';
import { SuggestionType } from '../../../generated/entity/feed/suggestion';
@ -24,6 +25,7 @@ const SuggestionsSlider = () => {
acceptRejectAllSuggestions,
loadingAccept,
loadingReject,
onUpdateActiveUser,
} = useSuggestionsContext();
return (
@ -33,11 +35,13 @@ const SuggestionsSlider = () => {
</Typography.Text>
<AvatarCarousel />
{selectedUserSuggestions.length > 0 && (
<>
<Space className="slider-btn-container m-l-xss">
<Button
ghost
className="text-xs text-primary font-medium"
data-testid="accept-all-suggestions"
icon={<CheckOutlined />}
loading={loadingAccept}
size="small"
type="primary"
onClick={() =>
acceptRejectAllSuggestions(
@ -45,15 +49,14 @@ const SuggestionsSlider = () => {
SuggestionAction.Accept
)
}>
<Typography.Text className="text-xs text-white">
{t('label.accept-all')}
</Typography.Text>
{t('label.accept-all')}
</Button>
<Button
ghost
className="text-xs text-primary font-medium"
data-testid="reject-all-suggestions"
icon={<CloseOutlined />}
loading={loadingReject}
size="small"
type="primary"
onClick={() =>
acceptRejectAllSuggestions(
@ -61,11 +64,18 @@ const SuggestionsSlider = () => {
SuggestionAction.Reject
)
}>
<Typography.Text className="text-xs text-primary">
{t('label.reject-all')}
</Typography.Text>
{t('label.reject-all')}
</Button>
</>
<Button
ghost
className="text-xs text-primary font-medium"
data-testid="close-suggestion"
icon={<CloseOutlined />}
type="primary"
onClick={() => onUpdateActiveUser()}>
{t('label.close')}
</Button>
</Space>
)}
</div>
);

View File

@ -10,7 +10,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { fireEvent, render, screen } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import React from 'react';
import AvatarCarousel from './AvatarCarousel';
@ -35,6 +35,7 @@ jest.mock('../../Suggestions/SuggestionsProvider/SuggestionsProvider', () => ({
{ id: '2', name: 'Avatar 2', type: 'user' },
],
acceptRejectSuggestion: jest.fn(),
selectedUserSuggestions: [],
onUpdateActiveUser: jest.fn(),
})),
__esModule: true,
@ -70,19 +71,10 @@ jest.mock('../../../rest/suggestionsAPI', () => ({
describe('AvatarCarousel', () => {
it('renders without crashing', () => {
render(<AvatarCarousel />);
render(<AvatarCarousel showArrows />);
expect(screen.getByText(/Avatar 1/i)).toBeInTheDocument();
expect(screen.getByText(/Avatar 2/i)).toBeInTheDocument();
expect(screen.getByTestId('prev-slide')).toBeDisabled();
});
it('disables the next button when on the last slide', () => {
render(<AvatarCarousel />);
const nextButton = screen.getByTestId('next-slide');
fireEvent.click(nextButton);
fireEvent.click(nextButton);
expect(nextButton).toBeDisabled();
});
});

View File

@ -11,16 +11,24 @@
* limitations under the License.
*/
import { LeftOutlined, RightOutlined } from '@ant-design/icons';
import { Button, Carousel } from 'antd';
import { Badge, Button, Carousel } from 'antd';
import classNames from 'classnames';
import React, { useCallback, useEffect, useState } from 'react';
import { useSuggestionsContext } from '../../Suggestions/SuggestionsProvider/SuggestionsProvider';
import UserPopOverCard from '../PopOverCard/UserPopOverCard';
import ProfilePicture from '../ProfilePicture/ProfilePicture';
import './avatar-carousel.less';
const AvatarCarousel = () => {
const { allSuggestionsUsers: avatarList, onUpdateActiveUser } =
useSuggestionsContext();
interface AvatarCarouselProps {
showArrows?: boolean;
}
const AvatarCarousel = ({ showArrows = false }: AvatarCarouselProps) => {
const {
allSuggestionsUsers: avatarList,
onUpdateActiveUser,
selectedUserSuggestions,
} = useSuggestionsContext();
const [currentSlide, setCurrentSlide] = useState(-1);
const prevSlide = useCallback(() => {
@ -43,48 +51,69 @@ const AvatarCarousel = () => {
onProfileClick(currentSlide);
}, [currentSlide]);
useEffect(() => {
if (selectedUserSuggestions.length === 0) {
setCurrentSlide(-1);
}
}, [selectedUserSuggestions]);
return (
<div className="avatar-carousel-container d-flex items-center">
<Button
className="carousel-arrow"
data-testid="prev-slide"
disabled={avatarList.length <= 1 || currentSlide <= 0}
icon={<LeftOutlined />}
size="small"
type="text"
onClick={prevSlide}
/>
{showArrows && (
<Button
className="carousel-arrow"
data-testid="prev-slide"
disabled={avatarList.length <= 1 || currentSlide <= 0}
icon={<LeftOutlined />}
size="small"
type="text"
onClick={prevSlide}
/>
)}
<Carousel
afterChange={(current) => setCurrentSlide(current)}
dots={false}
slidesToShow={avatarList.length < 3 ? avatarList.length : 3}>
{avatarList.map((avatar, index) => (
<UserPopOverCard
className=""
key={avatar.id}
userName={avatar?.name ?? ''}>
{avatarList.map((avatar, index) => {
const isActive = currentSlide === index;
const button = (
<Button
className={`p-0 m-r-xss avatar-item ${
currentSlide === index ? 'active' : ''
}`}
className={classNames('p-0 m-r-xss avatar-item', {
active: isActive,
})}
shape="circle"
onClick={() => setCurrentSlide(index)}>
<ProfilePicture name={avatar.name ?? ''} width="30" />
<ProfilePicture name={avatar.name ?? ''} width="28" />
</Button>
</UserPopOverCard>
))}
);
return (
<UserPopOverCard key={avatar.id} userName={avatar?.name ?? ''}>
{isActive ? ( // Show Badge only for active item
<Badge count={selectedUserSuggestions.length}>{button}</Badge>
) : (
button
)}
</UserPopOverCard>
);
})}
</Carousel>
<Button
className="carousel-arrow"
data-testid="next-slide"
disabled={
avatarList.length <= 1 || currentSlide === avatarList.length - 1
}
icon={<RightOutlined />}
size="small"
type="text"
onClick={nextSlide}
/>
{showArrows && (
<Button
className="carousel-arrow"
data-testid="next-slide"
disabled={
avatarList.length <= 1 || currentSlide === avatarList.length - 1
}
icon={<RightOutlined />}
size="small"
type="text"
onClick={nextSlide}
/>
)}
</div>
);
};

View File

@ -13,20 +13,42 @@
@import url('../../../styles/variables.less');
.avatar-item {
opacity: 0.4;
&.active {
opacity: 1;
}
position: relative;
&:hover {
border-color: @border-color !important;
}
&:focus {
border-color: @border-color !important;
}
&.ant-btn {
width: 28px;
height: 28px;
min-width: 28px;
}
}
.avatar-carousel-container {
.slick-slide {
width: 32px !important;
}
.slick-list {
overflow: visible !important;
}
.ant-badge-count {
right: 4px;
background-color: @red-3;
}
}
.slider-btn-container {
.ant-btn {
padding: 0 10px;
height: 30px;
}
.ant-btn.exit-suggestion {
color: @grey-4;
border-color: @grey-4;
padding: 0;
width: 30px;
}
}

View File

@ -205,7 +205,7 @@ const DescriptionV1 = ({
data-testid="asset-description-container"
direction="vertical"
size={16}>
<div className="d-flex justify-between">
<div className="d-flex justify-between flex-wrap">
<div className="d-flex items-center gap-2">
<Text className="right-panel-label">{t('label.description')}</Text>
{showActions && actionButtons}

View File

@ -24,7 +24,7 @@ import FormBuilder from '../../components/common/FormBuilder/FormBuilder';
import Loader from '../../components/common/Loader/Loader';
import TestSuiteScheduler from '../../components/DataQuality/AddDataQualityTest/components/TestSuiteScheduler';
import PageLayoutV1 from '../../components/PageLayoutV1/PageLayoutV1';
import applicationSchemaClassBase from '../../components/Settings/Applications/AppDetails/ApplicationSchemaClassBase';
import applicationSchemaClassBase from '../../components/Settings/Applications/AppDetails/ApplicationsClassBase';
import AppInstallVerifyCard from '../../components/Settings/Applications/AppInstallVerifyCard/AppInstallVerifyCard.component';
import IngestionStepper from '../../components/Settings/Services/Ingestion/IngestionStepper/IngestionStepper.component';
import { STEPS_FOR_APP_INSTALL } from '../../constants/Applications.constant';

View File

@ -48,7 +48,7 @@ jest.mock(
);
jest.mock(
'../../components/Settings/Applications/AppDetails/ApplicationSchemaClassBase',
'../../components/Settings/Applications/AppDetails/ApplicationsClassBase',
() => ({
importSchema: jest.fn().mockResolvedValue({}),
getJSONUISchema: jest.fn().mockReturnValue({}),

View File

@ -104,9 +104,6 @@ import TableConstraints from './TableConstraints/TableConstraints';
const TableDetailsPageV1: React.FC = () => {
const { isTourOpen, activeTabForTourDatasetPage, isTourPage } =
useTourProvider();
const FloatingButton = entityUtilClassBase.getEntityFloatingButton(
EntityType.TABLE
);
const { currentUser } = useApplicationStore();
const [tableDetails, setTableDetails] = useState<Table>();
const { tab: activeTab = EntityTabs.SCHEMA } =
@ -1075,8 +1072,6 @@ const TableDetailsPageV1: React.FC = () => {
onCancel={onThreadPanelClose}
/>
) : null}
{FloatingButton && <FloatingButton />}
</Row>
</PageLayoutV1>
);