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

Support arbitrary asset upload/deletion for Library Components [FC-0062] #1430

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
@@ -0,0 +1,95 @@
/* eslint-disable no-nested-ternary */
/* eslint-disable import/prefer-default-export */
import React from 'react';
import {
Button,
Dropzone,
} from '@openedx/paragon';
import { Delete } from '@openedx/paragon/icons';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { FormattedMessage, FormattedNumber, useIntl } from '@edx/frontend-platform/i18n';

import { LoadingSpinner } from '../../generic/Loading';
import DeleteModal from '../../generic/delete-modal/DeleteModal';
import { useLibraryContext } from '../common/context';
import { getXBlockAssetsApiUrl } from '../data/api';
import { useDeleteXBlockAsset, useInvalidateXBlockAssets, useXBlockAssets } from '../data/apiHooks';
import messages from './messages';

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

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

// For listing assets:
const { data: assets, isLoading: areAssetsLoading } = useXBlockAssets(usageKey);
const refreshAssets = useInvalidateXBlockAssets(usageKey);

// For uploading assets:
const handleProcessUpload = React.useCallback(async ({
fileData, requestConfig, handleError,
}: { fileData: FormData, requestConfig: any, handleError: any }) => {
const uploadData = new FormData();
const file = fileData.get('file') as File;
uploadData.set('content', file); // Paragon calls this 'file' but our API needs it called 'content'
// TODO: make the filename unique if is already exists in assets list, to avoid overwriting.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think overwriting will be a common and desired use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, maybe a future improvement would be just to show a confirmation dialog/warning in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment updated ^

const uploadUrl = `${getXBlockAssetsApiUrl(usageKey)}static/${encodeURI(file.name)}`;
const client = getAuthenticatedHttpClient();
try {
await client.put(uploadUrl, uploadData, requestConfig);
} catch (error) {
handleError(error);
return;

Check warning on line 47 in src/library-authoring/component-info/ComponentAdvancedAssets.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/component-info/ComponentAdvancedAssets.tsx#L46-L47

Added lines #L46 - L47 were not covered by tests
}
refreshAssets();
}, [usageKey]);

// For deleting assets:
const deleter = useDeleteXBlockAsset(usageKey);
const [filePathToDelete, setConfirmDeleteAsset] = React.useState<string>('');
const deleteFile = React.useCallback(() => {
deleter.mutateAsync(filePathToDelete); // Don't wait for this before clearing the modal on the next line
setConfirmDeleteAsset('');
}, [filePathToDelete, usageKey]);

return (
<>
<ul>
{ areAssetsLoading ? <li><LoadingSpinner /></li> : null }
{ assets?.map(a => (
<li key={a.path}>
<a href={a.url}>{a.path}</a>{' '}
(<FormattedNumber value={a.size} notation="compact" unit="byte" unitDisplay="narrow" />)
<Button variant="link" size="sm" iconBefore={Delete} onClick={() => { setConfirmDeleteAsset(a.path); }} title={intl.formatMessage(messages.advancedDetailsAssetsDeleteButton)}>
<span className="sr-only"><FormattedMessage {...messages.advancedDetailsAssetsDeleteButton} /></span>
</Button>
</li>
)) }
</ul>
{ assets !== undefined && !readOnly // Wait until assets have loaded before displaying add button:
? (
<Dropzone
style={{ height: '200px' }}
onProcessUpload={handleProcessUpload}
onUploadProgress={() => {}}

Check warning on line 79 in src/library-authoring/component-info/ComponentAdvancedAssets.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/component-info/ComponentAdvancedAssets.tsx#L79

Added line #L79 was not covered by tests
/>
)
: null }

<DeleteModal
isOpen={filePathToDelete !== ''}
close={() => { setConfirmDeleteAsset(''); }}

Check warning on line 86 in src/library-authoring/component-info/ComponentAdvancedAssets.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/component-info/ComponentAdvancedAssets.tsx#L86

Added line #L86 was not covered by tests
variant="warning"
title={intl.formatMessage(messages.advancedDetailsAssetsDeleteFileTitle)}
description={`Are you sure you want to delete ${filePathToDelete}?`}
onDeleteSubmit={deleteFile}
btnState="default"
/>
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../data/api.mocks';
import { LibraryProvider, SidebarBodyComponentId } from '../common/context';
import { ComponentAdvancedInfo } from './ComponentAdvancedInfo';
import { getXBlockAssetsApiUrl } from '../data/api';

mockContentLibrary.applyMock();
mockLibraryBlockMetadata.applyMock();
Expand Down Expand Up @@ -68,6 +69,58 @@ describe('<ComponentAdvancedInfo />', () => {
expect(await screen.findByText(/\(12M\)/)).toBeInTheDocument(); // size of the above file
expect(await screen.findByText(/static\/data\.csv/)).toBeInTheDocument();
expect(await screen.findByText(/\(8K\)/)).toBeInTheDocument(); // size of the above file
expect(await screen.findByText(/Drag and drop your file here or click to upload/)).toBeInTheDocument();
});

it('should delete static assets of the block', async () => {
const { axiosMock } = initializeMocks();

render();

const url = `${getXBlockAssetsApiUrl(mockLibraryBlockMetadata.usageKeyPublished)}${encodeURIComponent('static/image1.png')}`;
axiosMock.onDelete(url).reply(200);

const expandButton = await screen.findByRole('button', { name: /Advanced details/ });
fireEvent.click(expandButton);

expect(await screen.findByText(/static\/image1\.png/)).toBeInTheDocument();

// Click on delete button
const deleteButtons = await screen.findAllByTitle('Delete this file');
expect(deleteButtons.length).toEqual(2);
fireEvent.click(deleteButtons[0]);

// Show the pop up and click on delete
expect(await screen.findByText(/Are you sure you want to delete static\/image1\.png/)).toBeInTheDocument();
const deleteButton = await screen.findByRole('button', { name: /delete/i });
fireEvent.click(deleteButton);

await waitFor(() => expect(axiosMock.history.delete[0].url).toEqual(url));
});

it('should add asset in Dropzone', async () => {
const { axiosMock } = initializeMocks();
render();

const url = `${getXBlockAssetsApiUrl(mockLibraryBlockMetadata.usageKeyPublished)}static/image3.png`;
axiosMock.onPut(url).reply(200);

const expandButton = await screen.findByRole('button', { name: /Advanced details/ });
fireEvent.click(expandButton);

const dropzone = await screen.findByText(/Drag and drop your file here or click to upload/);
expect(dropzone).toBeInTheDocument();

const file = new File(['file'], 'image3.png', {
type: 'image/png',
});
Object.defineProperty(dropzone, 'files', {
value: [file],
});

fireEvent.drop(dropzone);

await waitFor(() => expect(axiosMock.history.put[0].url).toEqual(url));
});

it('should display the OLX source of the block (when expanded)', async () => {
Expand All @@ -80,11 +133,12 @@ describe('<ComponentAdvancedInfo />', () => {
await waitFor(() => expect(screen.getByText(olxPart)).toBeInTheDocument());
});

it('does not display "Edit OLX" button when the library is read-only', async () => {
it('does not display "Edit OLX" button and assets dropzone when the library is read-only', async () => {
render(mockXBlockOLX.usageKeyHtml, mockContentLibrary.libraryIdReadOnly);
const expandButton = await screen.findByRole('button', { name: /Advanced details/ });
fireEvent.click(expandButton);
expect(screen.queryByRole('button', { name: /Edit OLX/ })).not.toBeInTheDocument();
expect(screen.queryByText(/Drag and drop your file here or click to upload/)).not.toBeInTheDocument();
});

it('can edit the OLX', async () => {
Expand Down
15 changes: 3 additions & 12 deletions src/library-authoring/component-info/ComponentAdvancedInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ import {
OverlayTrigger,
Tooltip,
} from '@openedx/paragon';
import { FormattedMessage, FormattedNumber, useIntl } from '@edx/frontend-platform/i18n';
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';

import { LoadingSpinner } from '../../generic/Loading';
import { CodeEditor, EditorAccessor } from '../../generic/CodeEditor';
import { useLibraryContext } from '../common/context';
import {
useUpdateXBlockOLX,
useXBlockAssets,
useXBlockOLX,
} from '../data/apiHooks';
import messages from './messages';
import { ComponentAdvancedAssets } from './ComponentAdvancedAssets';

const ComponentAdvancedInfoInner: React.FC<Record<never, never>> = () => {
const intl = useIntl();
Expand All @@ -31,7 +31,6 @@ const ComponentAdvancedInfoInner: React.FC<Record<never, never>> = () => {
}

const { data: olx, isLoading: isOLXLoading } = useXBlockOLX(usageKey);
const { data: assets, isLoading: areAssetsLoading } = useXBlockAssets(usageKey);
const editorRef = React.useRef<EditorAccessor | undefined>(undefined);
const [isEditingOLX, setEditingOLX] = React.useState(false);
const olxUpdater = useUpdateXBlockOLX(usageKey);
Expand Down Expand Up @@ -101,15 +100,7 @@ const ComponentAdvancedInfoInner: React.FC<Record<never, never>> = () => {
);
})()}
<h3 className="h5"><FormattedMessage {...messages.advancedDetailsAssets} /></h3>
<ul>
{ areAssetsLoading ? <li><LoadingSpinner /></li> : null }
{ assets?.map(a => (
<li key={a.path}>
<a href={a.url}>{a.path}</a>{' '}
(<FormattedNumber value={a.size} notation="compact" unit="byte" unitDisplay="narrow" />)
</li>
)) }
</ul>
<ComponentAdvancedAssets />
</>
);
};
Expand Down
10 changes: 10 additions & 0 deletions src/library-authoring/component-info/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ const messages = defineMessages({
defaultMessage: 'Assets (Files)',
description: 'Heading for files attached to the component',
},
advancedDetailsAssetsDeleteFileTitle: {
id: 'course-authoring.library-authoring.component.advanced.assets.delete-file-title',
defaultMessage: 'Delete File',
description: 'Title for confirmation dialog when deleting a file',
},
advancedDetailsAssetsDeleteButton: {
id: 'course-authoring.library-authoring.component.advanced.assets.delete-btn',
defaultMessage: 'Delete this file',
description: 'screen reader description of the delete button for each static asset file',
},
advancedDetailsOLX: {
id: 'course-authoring.library-authoring.component.advanced.olx',
defaultMessage: 'OLX Source',
Expand Down
8 changes: 8 additions & 0 deletions src/library-authoring/data/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,14 @@ export async function getXBlockAssets(usageKey: string): Promise<{ path: string;
return data.files;
}

/**
* Delete a single asset file
*/
// istanbul ignore next
export async function deleteXBlockAsset(usageKey: string, path: string): Promise<void> {
await getAuthenticatedHttpClient().delete(getXBlockAssetsApiUrl(usageKey) + encodeURIComponent(path));
}

/**
* Get the collection metadata.
*/
Expand Down
23 changes: 23 additions & 0 deletions src/library-authoring/data/apiHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type Query,
type QueryClient,
} from '@tanstack/react-query';
import { useCallback } from 'react';

import { getLibraryId } from '../../generic/key-utils';
import {
Expand Down Expand Up @@ -42,6 +43,7 @@ import {
updateComponentCollections,
removeComponentsFromCollection,
publishXBlock,
deleteXBlockAsset,
} from './api';

export const libraryQueryPredicate = (query: Query, libraryId: string): boolean => {
Expand Down Expand Up @@ -397,6 +399,27 @@ export const useXBlockAssets = (usageKey: string) => (
})
);

/** Refresh the list of assets (static files) attached to a library component */
export const useInvalidateXBlockAssets = (usageKey: string) => {
const client = useQueryClient();
return useCallback(() => {
client.invalidateQueries({ queryKey: xblockQueryKeys.xblockAssets(usageKey) });
}, [usageKey]);
};

/**
* Use this mutation to delete an asset file from a library
*/
export const useDeleteXBlockAsset = (usageKey: string) => {
const queryClient = useQueryClient();
return useMutation({
mutationFn: (path: string) => deleteXBlockAsset(usageKey, path),
onSettled: () => {
queryClient.invalidateQueries({ queryKey: xblockQueryKeys.xblockAssets(usageKey) });
},
});
};

/**
* Get the metadata for a collection in a library
*/
Expand Down