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: Show published OLX in Library Content Picker [FC-0062] #1534

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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 @@ -12,6 +12,7 @@ import {
mockXBlockAssets,
mockXBlockOLX,
} from '../data/api.mocks';
import * as apiHooks from '../data/apiHooks';
import { LibraryProvider, SidebarBodyComponentId } from '../common/context';
import { ComponentAdvancedInfo } from './ComponentAdvancedInfo';
import { getXBlockAssetsApiUrl } from '../data/api';
Expand All @@ -25,6 +26,7 @@ const setOLXspy = mockSetXBlockOLX.applyMock();
const render = (
usageKey: string = mockLibraryBlockMetadata.usageKeyPublished,
libraryId: string = mockContentLibrary.libraryId,
showOnlyPublished: boolean = false,
) => baseRender(
<ComponentAdvancedInfo />,
{
Expand All @@ -35,6 +37,7 @@ const render = (
id: usageKey,
type: SidebarBodyComponentId.ComponentInfo,
}}
showOnlyPublished={showOnlyPublished}
>
{children}
</LibraryProvider>
Expand Down Expand Up @@ -124,13 +127,31 @@ describe('<ComponentAdvancedInfo />', () => {
});

it('should display the OLX source of the block (when expanded)', async () => {
const usageKey = mockXBlockOLX.usageKeyHtml;
const spy = jest.spyOn(apiHooks, 'useXBlockOLX');

render(mockXBlockOLX.usageKeyHtml);
const expandButton = await screen.findByRole('button', { name: /Advanced details/ });
fireEvent.click(expandButton);
// Because of syntax highlighting, the OLX will be borken up by many different tags so we need to search for
// just a substring:
const olxPart = /This is a text component which uses/;
await waitFor(() => expect(screen.getByText(olxPart)).toBeInTheDocument());
expect(spy).toHaveBeenCalledWith(usageKey, undefined);
});

it('should display the published OLX source of the block (when expanded)', async () => {
const usageKey = mockXBlockOLX.usageKeyHtml;
const spy = jest.spyOn(apiHooks, 'useXBlockOLX');

render(usageKey, undefined, true);
const expandButton = await screen.findByRole('button', { name: /Advanced details/ });
fireEvent.click(expandButton);
// Because of syntax highlighting, the OLX will be borken up by many different tags so we need to search for
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Because of syntax highlighting, the OLX will be borken up by many different tags so we need to search for
// Because of syntax highlighting, the OLX will be broken up by many different tags so we need to search for

// just a substring:
const olxPart = /This is a text component which uses/;
await waitFor(() => expect(screen.getByText(olxPart)).toBeInTheDocument());
expect(spy).toHaveBeenCalledWith(usageKey, 'published');
});

it('does not display "Edit OLX" button and assets dropzone when the library is read-only', async () => {
Expand Down
11 changes: 9 additions & 2 deletions src/library-authoring/component-info/ComponentAdvancedInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,22 @@ import { ComponentAdvancedAssets } from './ComponentAdvancedAssets';

const ComponentAdvancedInfoInner: React.FC<Record<never, never>> = () => {
const intl = useIntl();
const { readOnly, sidebarComponentInfo } = useLibraryContext();
const {
readOnly,
sidebarComponentInfo,
showOnlyPublished,
} = useLibraryContext();

const usageKey = sidebarComponentInfo?.id;
// istanbul ignore if: this should never happen in production
if (!usageKey) {
throw new Error('sidebarComponentUsageKey is required to render ComponentAdvancedInfo');
}

const { data: olx, isLoading: isOLXLoading } = useXBlockOLX(usageKey);
const { data: olx, isLoading: isOLXLoading } = useXBlockOLX(
usageKey,
showOnlyPublished ? 'published' : undefined,
);
const editorRef = React.useRef<EditorAccessor | undefined>(undefined);
const [isEditingOLX, setEditingOLX] = React.useState(false);
const olxUpdater = useUpdateXBlockOLX(usageKey);
Expand Down
4 changes: 2 additions & 2 deletions src/library-authoring/data/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ export async function createCollection(libraryId: string, collectionData: Create
* Fetch the OLX for the given XBlock.
*/
// istanbul ignore next
export async function getXBlockOLX(usageKey: string): Promise<string> {
const { data } = await getAuthenticatedHttpClient().get(getXBlockOLXApiUrl(usageKey));
export async function getXBlockOLX(usageKey: string, version?: string): Promise<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of version?: string maybe we can use version?: VersionSpec ?

export type VersionSpec = 'published' | 'draft' | number;

const { data } = await getAuthenticatedHttpClient().get(getXBlockOLXApiUrl(usageKey), { params: { version } });
return data.olx;
}

Expand Down
4 changes: 2 additions & 2 deletions src/library-authoring/data/apiHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,10 @@ export const useCreateLibraryCollection = (libraryId: string) => {
};

/** Get the OLX source of a library component */
export const useXBlockOLX = (usageKey: string) => (
export const useXBlockOLX = (usageKey: string, version?: string) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of version?: string maybe we can use version?: VersionSpec ?

export type VersionSpec = 'published' | 'draft' | number;

useQuery({
queryKey: xblockQueryKeys.xblockOLX(usageKey),
queryFn: () => getXBlockOLX(usageKey),
queryFn: () => getXBlockOLX(usageKey, version),
enabled: !!usageKey,
})
);
Expand Down