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

fix(metadata-sidebar): Fix metadata sidebar ai capabilities #3681

Merged
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
12 changes: 3 additions & 9 deletions src/elements/content-sidebar/MetadataSidebarRedesign.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { withErrorBoundary } from '../common/error-boundary';
import { withLogger } from '../common/logger';
import { ORIGIN_METADATA_SIDEBAR_REDESIGN, SIDEBAR_VIEW_METADATA } from '../../constants';
import { EVENT_JS_READY } from '../common/logger/constants';
import { useFeatureEnabled } from '../common/feature-checking';
import { mark } from '../../utils/performance';
import useSidebarMetadataFetcher, { STATUS } from './hooks/useSidebarMetadataFetcher';

Expand All @@ -41,7 +42,6 @@ const MARK_NAME_JS_READY = `${ORIGIN_METADATA_SIDEBAR_REDESIGN}_${EVENT_JS_READY
mark(MARK_NAME_JS_READY);

export interface ExternalProps {
isBoxAiSuggestionsEnabled: boolean;
isFeatureEnabled: boolean;
}

Expand All @@ -64,14 +64,7 @@ export interface MetadataSidebarRedesignProps extends PropsWithoutContext, Error
api: API;
}

function MetadataSidebarRedesign({
api,
elementId,
fileId,
isBoxAiSuggestionsEnabled,
onError,
isFeatureEnabled,
}: MetadataSidebarRedesignProps) {
function MetadataSidebarRedesign({ api, elementId, fileId, onError, isFeatureEnabled }: MetadataSidebarRedesignProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is isFeatureEnabled doing what useFeatureEnabled is doing? do we need both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I don't think so.

useFeatureEnabled is checking the features array. In our case we can check if metadata redesign is enabled, or if metadata aiSuggestions are enabled.

isFeatureEnabled is a prop that comes from host application in metadataSidebarProps

metadataSidebarProps: MetadataSidebarProps,

it's defined here

type ExternalProps = {
isFeatureEnabled: boolean,
selectedTemplateKey?: string,
templateFilters?: Array<string> | string,
};

maybe @bfoxx1906 or @jstoffan have more insight if it should be used 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Took a look, useFeatureEnabled isn't used anywhere else in BUIE. However, Seems like a use case for components that don't have the features array passed down, so it probably is needed here. Have we tested useFeatureEnabled works as expected here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, useFeatureEnabled works as expected. It's covered with a storybook story test (also in this PR). It was recommended by @jstoffan some time ago.

Now after re-reading this I understand initial confusion. As I mentioned isFeatereEnalbed is a prop indicating if user has Metadata Sidebar enabled for them.

const {
file,
handleCreateMetadataInstance,
Expand All @@ -84,6 +77,7 @@ function MetadataSidebarRedesign({
} = useSidebarMetadataFetcher(api, fileId, onError, isFeatureEnabled);

const { formatMessage } = useIntl();
const isBoxAiSuggestionsEnabled: boolean = useFeatureEnabled('metadata.aiSuggestions.enabled');

const [editingTemplate, setEditingTemplate] = React.useState<MetadataTemplateInstance | null>(null);
const [isUnsavedChangesModalOpen, setIsUnsavedChangesModalOpen] = React.useState<boolean>(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,16 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {
permissions: { [FIELD_PERMISSIONS_CAN_UPLOAD]: true },
};

const renderComponent = (props = {}) => {
const renderComponent = (props = {}, features = {}) => {
const defaultProps = {
api: {},
fileId: 'test-file-id-1',
elementId: 'element-1',
isBoxAiSuggestionsEnabled: true,
isFeatureEnabled: true,
onError: jest.fn(),
} satisfies MetadataSidebarRedesignProps;

render(<MetadataSidebarRedesign {...defaultProps} {...props} />);
render(<MetadataSidebarRedesign {...defaultProps} {...props} />, { wrapperProps: { features } });
};

beforeEach(() => {
Expand Down Expand Up @@ -154,7 +153,7 @@ describe('elements/content-sidebar/Metadata/MetadataSidebarRedesign', () => {
});

test('should correctly render empty state when AI feature is enabled', () => {
renderComponent();
renderComponent({}, { 'metadata.aiSuggestions.enabled': true });
expect(screen.getByRole('heading', { level: 2, name: 'Autofill Metadata with Box AI' })).toBeInTheDocument();
expect(
screen.getByText(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const defaultMetadataArgs = {
onError: fn,
};
const defaultMetadataSidebarProps: ComponentProps<typeof MetadataSidebarRedesign> = {
isBoxAiSuggestionsEnabled: true,
isFeatureEnabled: true,
onError: fn,
};
Expand Down Expand Up @@ -139,6 +138,10 @@ export const EmptyStateWithBoxAiEnabled: StoryObj<typeof MetadataSidebarRedesign
metadataSidebarProps: {
...defaultMetadataSidebarProps,
},
features: {
...mockFeatures,
'metadata.aiSuggestions.enabled': true,
},
},
};

Expand Down Expand Up @@ -188,7 +191,9 @@ export const MetadataInstanceEditorCancelChanges: StoryObj<typeof MetadataSideba
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const editButtons = await canvas.findAllByRole('button', { name: 'Edit' }, { timeout: 5000 });
// Edit buttons contains also template name
const editButton = await canvas.findByRole('button', { name: 'Edit My Template' }, { timeout: 5000 });
expect(editButton).toBeInTheDocument();

let headlines = await canvas.findAllByRole('heading', { level: 1 });
expect(headlines).toHaveLength(3);
Expand All @@ -197,7 +202,7 @@ export const MetadataInstanceEditorCancelChanges: StoryObj<typeof MetadataSideba
);

// go to edit mode - only edited template is visible
await userEvent.click(editButtons[0]);
await userEvent.click(editButton);

headlines = await canvas.findAllByRole('heading', { level: 1 });
expect(headlines).toHaveLength(1);
Expand Down Expand Up @@ -286,7 +291,6 @@ export const SwitchEditingTemplateInstances: StoryObj<typeof MetadataSidebarRede
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

// open and edit a new template
const addTemplateButton = await canvas.findByRole('button', { name: 'Add template' }, { timeout: 5000 });

Expand Down Expand Up @@ -335,3 +339,28 @@ export const SwitchEditingTemplateInstances: StoryObj<typeof MetadataSidebarRede
expect(templateMetadataOptionBAfterSwitch).toHaveAttribute('aria-disabled');
},
};

export const MetadataInstanceEditorAIEnabled: StoryObj<typeof MetadataSidebarRedesign> = {
args: {
features: {
...mockFeatures,
'metadata.aiSuggestions.enabled': true,
},
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const autofillWithBoxAI = await canvas.findAllByRole(
'button',
{ name: /Autofill .+ with Box AI/ },
{ timeout: 5000 },
);
expect(autofillWithBoxAI).toHaveLength(2);

const editButton = await canvas.findByRole('button', { name: 'Edit My Template' });
userEvent.click(editButton);

const autofillButton = await canvas.findByRole('button', { name: 'Autofill' });
expect(autofillButton).toBeInTheDocument();
},
};
22 changes: 15 additions & 7 deletions src/test-utils/testing-library.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
import React from 'react';
import { render } from '@testing-library/react';
import { render, type RenderOptions } from '@testing-library/react';

// Data Providers
import { TooltipProvider } from '@box/blueprint-web';
import { IntlProvider } from 'react-intl';
import { AutofillContextProvider } from '@box/metadata-editor';
import { FeatureProvider } from '../elements/common/feature-checking';

jest.unmock('react-intl');

const Wrapper = ({ children }) => (
<AutofillContextProvider isAiSuggestionsFeatureEnabled={false} fetchSuggestions={() => Promise.resolve([])}>
<TooltipProvider>
<IntlProvider locale="en">{children}</IntlProvider>
</TooltipProvider>
const Wrapper = ({ children, features = {}, isAiSuggestionsFeatureEnabled = false, fetchSuggestions = () => Promise.resolve([]) }) => (
<AutofillContextProvider isAiSuggestionsFeatureEnabled={isAiSuggestionsFeatureEnabled} fetchSuggestions={fetchSuggestions}>
<FeatureProvider features={features}>
<TooltipProvider>
<IntlProvider locale="en">{children}</IntlProvider>
</TooltipProvider>
</FeatureProvider>
</AutofillContextProvider>
);

const renderConnected = (element, options = {}) => render(element, { wrapper: Wrapper, ...options });
type RenderConnectedOptions = Omit<RenderOptions, 'wrapper'> & {
wrapperProps?: Record<string, unknown>;
};

const renderConnected = (element, options: RenderConnectedOptions = {}) =>
render(element, { wrapper: props => <Wrapper {...props} {...options.wrapperProps} />, ...options });

export * from '@testing-library/react';
export { renderConnected as render };
Loading