From 38919bd791b538bbb52ed2880f77d3350eb2537c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:06:27 +0200 Subject: [PATCH 01/17] memoize is selected observable --- .../public/components/file_picker/components/file_card.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx b/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx index 88c77f36a6c00..89d8e84c0397c 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React from 'react'; +import React, { useMemo } from 'react'; import type { FunctionComponent } from 'react'; import numeral from '@elastic/numeral'; import useObservable from 'react-use/lib/useObservable'; @@ -26,8 +26,8 @@ export const FileCard: FunctionComponent = ({ file }) => { const { kind, state, client } = useFilePickerContext(); const { euiTheme } = useEuiTheme(); const displayImage = isImage({ type: file.mimeType }); - - const isSelected = useObservable(state.watchFileSelected$(file.id), false); + const isSelected$ = useMemo(() => state.watchFileSelected$(file.id), [file.id, state]); + const isSelected = useObservable(isSelected$, false); const imageHeight = `calc(${euiTheme.size.xxxl} * 2)`; return ( From 2fcddfc4c5ba52b6d254b09cfb125f17e5c9c0c9 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:07:55 +0200 Subject: [PATCH 02/17] added on upload start and upload end cbs --- .../components/upload_file/upload_file.tsx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/files/public/components/upload_file/upload_file.tsx b/x-pack/plugins/files/public/components/upload_file/upload_file.tsx index d0ca3577b27a0..ae23739afbf2e 100644 --- a/x-pack/plugins/files/public/components/upload_file/upload_file.tsx +++ b/x-pack/plugins/files/public/components/upload_file/upload_file.tsx @@ -76,6 +76,16 @@ export interface Props { */ onError?: (e: Error) => void; + /** + * Will be called whenever an upload starts + */ + onUploadStart?: () => void; + + /** + * Will be called when attempt ends, in error otherwise + */ + onUploadEnd?: () => void; + /** * Whether to display the component in it's compact form. * @@ -105,6 +115,8 @@ export const UploadFile = ({ onError, fullWidth, allowClear, + onUploadEnd, + onUploadStart, compressed = false, kind: kindId, multiple = false, @@ -136,9 +148,12 @@ export const UploadFile = ({ }), uploadState.done$.subscribe((n) => n && onDone(n)), uploadState.error$.subscribe((e) => e && onError?.(e)), + uploadState.uploading$.subscribe((uploading) => + uploading ? onUploadStart?.() : onUploadEnd?.() + ), ]; return () => subs.forEach((sub) => sub.unsubscribe()); - }, [uploadState, onDone, onError]); + }, [uploadState, onDone, onError, onUploadStart, onUploadEnd]); useEffect(() => uploadState.dispose, [uploadState]); From f6ac530a7c66083de09f27eec9f9fd2744720094 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:23:03 +0200 Subject: [PATCH 03/17] listen for uploading state, also i18n for search field --- .../components/clear_filter_button.tsx | 6 +++++- .../file_picker/components/pagination.tsx | 9 +++++++- .../file_picker/components/search_field.tsx | 21 +++++++++++-------- .../file_picker/components/select_button.tsx | 3 ++- .../components/file_picker/i18n_texts.ts | 3 +++ 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/components/clear_filter_button.tsx b/x-pack/plugins/files/public/components/file_picker/components/clear_filter_button.tsx index 14356b9b02bd4..8ce3383c7e852 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/clear_filter_button.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/clear_filter_button.tsx @@ -12,6 +12,7 @@ import { css } from '@emotion/react'; import { useFilePickerContext } from '../context'; import { i18nTexts } from '../i18n_texts'; +import { useBehaviorSubject } from '../../use_behavior_subject'; interface Props { onClick: () => void; @@ -19,6 +20,7 @@ interface Props { export const ClearFilterButton: FunctionComponent = ({ onClick }) => { const { state } = useFilePickerContext(); + const isUploading = useBehaviorSubject(state.isUploading$); const query = useObservable(state.queryDebounced$); if (!query) { return null; @@ -30,7 +32,9 @@ export const ClearFilterButton: FunctionComponent = ({ onClick }) => { place-items: center; `} > - {i18nTexts.clearFilterButton} + + {i18nTexts.clearFilterButton} + ); }; diff --git a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx index bc2d0d444ba45..a49feadf8b005 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx @@ -15,5 +15,12 @@ export const Pagination: FunctionComponent = () => { const { state } = useFilePickerContext(); const page = useBehaviorSubject(state.currentPage$); const pageCount = useBehaviorSubject(state.totalPages$); - return ; + const isUploading = useBehaviorSubject(state.isUploading$); + return ( + {}} + pageCount={pageCount} + activePage={page} + /> + ); }; diff --git a/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx b/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx index 0235b03dd3fc1..196435a737254 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx @@ -7,7 +7,7 @@ import type { FunctionComponent } from 'react'; import React from 'react'; -import { EuiFieldSearch } from '@elastic/eui'; +import { EuiFieldSearch, EuiFormRow } from '@elastic/eui'; import { i18nTexts } from '../i18n_texts'; import { useFilePickerContext } from '../context'; import { useBehaviorSubject } from '../../use_behavior_subject'; @@ -17,14 +17,17 @@ export const SearchField: FunctionComponent = () => { const query = useBehaviorSubject(state.query$); const isLoading = useBehaviorSubject(state.isLoading$); const hasFiles = useBehaviorSubject(state.hasFiles$); + const isUploading = useBehaviorSubject(state.isUploading$); return ( - state.setQuery(ev.target.value)} - /> + + state.setQuery(ev.target.value)} + /> + ); }; diff --git a/x-pack/plugins/files/public/components/file_picker/components/select_button.tsx b/x-pack/plugins/files/public/components/file_picker/components/select_button.tsx index ac5e241c01d53..b6f1fa846fa48 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/select_button.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/select_button.tsx @@ -18,11 +18,12 @@ export interface Props { export const SelectButton: FunctionComponent = ({ onClick }) => { const { state } = useFilePickerContext(); + const isUploading = useBehaviorSubject(state.isUploading$); const selectedFiles = useBehaviorSubject(state.selectedFileIds$); return ( onClick(selectedFiles)} > {selectedFiles.length > 1 diff --git a/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts b/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts index 2670ecd71b084..162c9347f826c 100644 --- a/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts +++ b/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts @@ -34,6 +34,9 @@ export const i18nTexts = { searchFieldPlaceholder: i18n.translate('xpack.files.filePicker.searchFieldPlaceholder', { defaultMessage: 'my-file-*', }), + searchFieldLabel: i18n.translate('xpack.files.filePicker.searchFieldLabel', { + defaultMessage: 'Search for a file by name', + }), emptyFileGridPrompt: i18n.translate('xpack.files.filePicker.emptyGridPrompt', { defaultMessage: 'No files matched filter', }), From 1973a6d70da47fafc633ec6de27410f4f053b074 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:23:32 +0200 Subject: [PATCH 04/17] added uploading state --- .../components/file_picker/file_picker_state.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts index 697f1fc58188d..4a46588a2b0b0 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts +++ b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts @@ -38,6 +38,7 @@ export class FilePickerState { public readonly queryDebounced$ = this.query$.pipe(debounceTime(100)); public readonly currentPage$ = new BehaviorSubject(0); public readonly totalPages$ = new BehaviorSubject(undefined); + public readonly isUploading$ = new BehaviorSubject(false); /** * This is how we keep a deduplicated list of file ids representing files a user @@ -111,6 +112,7 @@ export class FilePickerState { page: number, query: undefined | string ): Observable<{ files: FileJSON[]; total: number }> => { + if (this.isUploading$.getValue()) throw new Error('Cannot fetch files while uploading'); if (this.abort) this.abort(); this.setIsLoading(true); this.loadingError$.next(undefined); @@ -156,6 +158,12 @@ export class FilePickerState { this.retry$.next(); }; + public resetFilters = (): void => { + this.setQuery(undefined); + this.setPage(0); + this.retry(); + }; + public hasFilesSelected = (): boolean => { return this.fileSet.size > 0; }; @@ -182,6 +190,10 @@ export class FilePickerState { this.currentPage$.next(page); }; + public setIsUploading = (value: boolean): void => { + this.isUploading$.next(value); + }; + public dispose = (): void => { for (const sub of this.subscriptions) sub.unsubscribe(); }; From 15fba280ec7ecd803e867f01171a4214a804e49b Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:49:55 +0200 Subject: [PATCH 05/17] updated the files example plugin --- .../files_example/public/components/app.tsx | 5 +++++ .../files_example/public/components/file_picker.tsx | 13 +++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/x-pack/examples/files_example/public/components/app.tsx b/x-pack/examples/files_example/public/components/app.tsx index afdf8be1f4f6e..9d10e33a8b23d 100644 --- a/x-pack/examples/files_example/public/components/app.tsx +++ b/x-pack/examples/files_example/public/components/app.tsx @@ -169,6 +169,11 @@ export const FilesExampleApp = ({ files, notifications }: FilesExampleAppDeps) = {showFilePickerModal && ( setShowFilePickerModal(false)} + onUpload={() => { + notifications.toasts.addSuccess({ + title: 'Uploaded files', + }); + }} onDone={(ids) => { notifications.toasts.addSuccess({ title: 'Selected files!', diff --git a/x-pack/examples/files_example/public/components/file_picker.tsx b/x-pack/examples/files_example/public/components/file_picker.tsx index 3c2178b299ea2..bc30ab92654d8 100644 --- a/x-pack/examples/files_example/public/components/file_picker.tsx +++ b/x-pack/examples/files_example/public/components/file_picker.tsx @@ -14,9 +14,18 @@ import { FilePicker } from '../imports'; interface Props { onClose: () => void; + onUpload: (ids: string[]) => void; onDone: (ids: string[]) => void; } -export const MyFilePicker: FunctionComponent = ({ onClose, onDone }) => { - return ; +export const MyFilePicker: FunctionComponent = ({ onClose, onDone, onUpload }) => { + return ( + onUpload(n.map(({ id }) => id))} + pageSize={50} + /> + ); }; From 442490411a2ca26ce5ad2f58e3ff90ce0abb08c1 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:50:26 +0200 Subject: [PATCH 06/17] rework the footer to include the compressed upload component --- .../file_picker/components/modal_footer.tsx | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/components/modal_footer.tsx b/x-pack/plugins/files/public/components/file_picker/components/modal_footer.tsx index d0d0e146d2c3b..643efec8abebd 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/modal_footer.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/modal_footer.tsx @@ -5,24 +5,72 @@ * 2.0. */ -import { EuiFlexGroup, EuiModalFooter } from '@elastic/eui'; +import { EuiModalFooter } from '@elastic/eui'; +import { css } from '@emotion/react'; import type { FunctionComponent } from 'react'; -import React from 'react'; +import React, { useCallback } from 'react'; +import { UploadFile } from '../../upload_file'; +import type { Props as FilePickerProps } from '../file_picker'; +import { useFilePickerContext } from '../context'; +import { i18nTexts } from '../i18n_texts'; import { Pagination } from './pagination'; import { SelectButton, Props as SelectButtonProps } from './select_button'; interface Props { + kind: string; onDone: SelectButtonProps['onClick']; + onUpload?: FilePickerProps['onUpload']; } -export const ModalFooter: FunctionComponent = ({ onDone }) => { +export const ModalFooter: FunctionComponent = ({ kind, onDone, onUpload }) => { + const { state } = useFilePickerContext(); + const onUploadStart = useCallback(() => state.setIsUploading(true), [state]); + const onUploadEnd = useCallback(() => state.setIsUploading(false), [state]); return ( - - - - +
+
+ { + state.selectFile(n.map(({ id }) => id)); + state.resetFilters(); + onUpload?.(n); + }} + onUploadStart={onUploadStart} + onUploadEnd={onUploadEnd} + kind={kind} + initialPromptText={i18nTexts.uploadFilePlaceholderText} + multiple + compressed + /> +
+
+ +
+
+ +
+
); }; From f67eb1b4e042d3256240ff2931288be6f0e1e6a6 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:50:41 +0200 Subject: [PATCH 07/17] prevent change page when uploading --- .../public/components/file_picker/components/pagination.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx index a49feadf8b005..ee036cdbdd338 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx @@ -18,7 +18,7 @@ export const Pagination: FunctionComponent = () => { const isUploading = useBehaviorSubject(state.isUploading$); return ( {}} + onPageClick={isUploading ? () => {} : state.setPage} pageCount={pageCount} activePage={page} /> From c28a59e2d691675c83ca37256c65759ab4b1d1af Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:51:16 +0200 Subject: [PATCH 08/17] revert change to search field --- .../file_picker/components/search_field.tsx | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx b/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx index 196435a737254..bb1fe3580d2bb 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/search_field.tsx @@ -7,7 +7,7 @@ import type { FunctionComponent } from 'react'; import React from 'react'; -import { EuiFieldSearch, EuiFormRow } from '@elastic/eui'; +import { EuiFieldSearch } from '@elastic/eui'; import { i18nTexts } from '../i18n_texts'; import { useFilePickerContext } from '../context'; import { useBehaviorSubject } from '../../use_behavior_subject'; @@ -19,15 +19,13 @@ export const SearchField: FunctionComponent = () => { const hasFiles = useBehaviorSubject(state.hasFiles$); const isUploading = useBehaviorSubject(state.isUploading$); return ( - - state.setQuery(ev.target.value)} - /> - + state.setQuery(ev.target.value)} + /> ); }; From 551baf021ce786137a2d7eb2d2ec18664d6149b9 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:51:54 +0200 Subject: [PATCH 09/17] added pass through the on upload callback so that consumers can respond to upload events --- .../files/public/components/file_picker/file_picker.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker.tsx b/x-pack/plugins/files/public/components/file_picker/file_picker.tsx index 72920b72a865d..6c95cd225ae27 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker.tsx +++ b/x-pack/plugins/files/public/components/file_picker/file_picker.tsx @@ -17,6 +17,7 @@ import { EuiFlexGroup, } from '@elastic/eui'; +import type { DoneNotification } from '../upload_file'; import { useBehaviorSubject } from '../use_behavior_subject'; import { useFilePickerContext, FilePickerContext } from './context'; @@ -43,13 +44,17 @@ export interface Props { * Will be called after a user has a selected a set of files */ onDone: (fileIds: string[]) => void; + /** + * When a user has succesfully uploaded some files this callback will be called + */ + onUpload?: (done: DoneNotification[]) => void; /** * The number of results to show per page. */ pageSize?: number; } -const Component: FunctionComponent = ({ onClose, onDone }) => { +const Component: FunctionComponent = ({ onClose, onDone, onUpload }) => { const { state, kind } = useFilePickerContext(); const hasFiles = useBehaviorSubject(state.hasFiles$); @@ -59,7 +64,7 @@ const Component: FunctionComponent = ({ onClose, onDone }) => { useObservable(state.files$); - const renderFooter = () => ; + const renderFooter = () => ; return ( Date: Tue, 25 Oct 2022 17:52:08 +0200 Subject: [PATCH 10/17] updated i18n --- .../files/public/components/file_picker/i18n_texts.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts b/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts index 162c9347f826c..59ea5457ec6c4 100644 --- a/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts +++ b/x-pack/plugins/files/public/components/file_picker/i18n_texts.ts @@ -34,9 +34,6 @@ export const i18nTexts = { searchFieldPlaceholder: i18n.translate('xpack.files.filePicker.searchFieldPlaceholder', { defaultMessage: 'my-file-*', }), - searchFieldLabel: i18n.translate('xpack.files.filePicker.searchFieldLabel', { - defaultMessage: 'Search for a file by name', - }), emptyFileGridPrompt: i18n.translate('xpack.files.filePicker.emptyGridPrompt', { defaultMessage: 'No files matched filter', }), @@ -46,4 +43,7 @@ export const i18nTexts = { clearFilterButton: i18n.translate('xpack.files.filePicker.clearFilterButtonLabel', { defaultMessage: 'Clear filter', }), + uploadFilePlaceholderText: i18n.translate('xpack.files.filePicker.uploadFilePlaceholderText', { + defaultMessage: 'Drag and drop to upload new files', + }), }; From 5734c5bb1d025db83624a09b6ace7a30d4335619 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:52:23 +0200 Subject: [PATCH 11/17] export DoneNotification type from upload files state --- x-pack/plugins/files/public/components/upload_file/index.tsx | 1 + .../plugins/files/public/components/upload_file/upload_state.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/files/public/components/upload_file/index.tsx b/x-pack/plugins/files/public/components/upload_file/index.tsx index 4901c46a78c91..8e9e89c33c799 100644 --- a/x-pack/plugins/files/public/components/upload_file/index.tsx +++ b/x-pack/plugins/files/public/components/upload_file/index.tsx @@ -9,6 +9,7 @@ import React, { lazy, Suspense } from 'react'; import { EuiLoadingSpinner } from '@elastic/eui'; import type { Props } from './upload_file'; +export type { DoneNotification } from './upload_state'; export type { Props as UploadFileProps }; const UploadFileContainer = lazy(() => import('./upload_file')); diff --git a/x-pack/plugins/files/public/components/upload_file/upload_state.ts b/x-pack/plugins/files/public/components/upload_file/upload_state.ts index 13c4cc020b05a..061f65115c799 100644 --- a/x-pack/plugins/files/public/components/upload_file/upload_state.ts +++ b/x-pack/plugins/files/public/components/upload_file/upload_state.ts @@ -43,7 +43,7 @@ interface FileState { type Upload = SimpleStateSubject; -interface DoneNotification { +export interface DoneNotification { id: string; kind: string; } From d6500a384e4c3f62d5c9e7ad10610ba394b97fd6 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 25 Oct 2022 17:58:28 +0200 Subject: [PATCH 12/17] added test for uploading state --- .../file_picker/file_picker_state.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts b/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts index 79eb5cbfa529d..0c3dc4b768717 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts +++ b/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts @@ -168,4 +168,20 @@ describe('FilePickerState', () => { expectObservable(filePickerState.loadingError$).toBe('a-b---c-', {}); }); }); + it('does not allow fetching files while an upload is in progress', () => { + getTestScheduler().run(({ expectObservable, cold }) => { + const files = [] as FileJSON[]; + filesClient.list.mockImplementation(() => of({ files }) as any); + const uploadInput = '---a|'; + const queryInput = ' -----a|'; + const upload$ = cold(uploadInput).pipe(tap(() => filePickerState.setIsUploading(true))); + const query$ = cold(queryInput).pipe(tap((q) => filePickerState.setQuery(q))); + expectObservable(merge(upload$, query$)).toBe('---a-a|'); + expectObservable(filePickerState.files$).toBe( + 'a----#-', + { a: [] }, + new Error('Cannot fetch files while uploading') + ); + }); + }); }); From b630442200f887e4dc1fcb59d5171f0a5460e774 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 26 Oct 2022 12:27:43 +0200 Subject: [PATCH 13/17] added common page and page size schemas --- x-pack/plugins/files/server/routes/common_schemas.ts | 3 +++ x-pack/plugins/files/server/routes/file_kind/list.ts | 5 +++-- x-pack/plugins/files/server/routes/file_kind/share/list.ts | 5 +++-- x-pack/plugins/files/server/routes/find.ts | 5 +++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/files/server/routes/common_schemas.ts b/x-pack/plugins/files/server/routes/common_schemas.ts index 6f9b6a5d651a0..449e4995ac5dd 100644 --- a/x-pack/plugins/files/server/routes/common_schemas.ts +++ b/x-pack/plugins/files/server/routes/common_schemas.ts @@ -42,4 +42,7 @@ export const fileAlt = schema.maybe( }) ); +export const page = schema.number({ min: 1, defaultValue: 1 }); +export const pageSize = schema.number({ min: 1, defaultValue: 100 }); + export const fileMeta = schema.maybe(schema.object({}, { unknowns: 'allow' })); diff --git a/x-pack/plugins/files/server/routes/file_kind/list.ts b/x-pack/plugins/files/server/routes/file_kind/list.ts index b9b389f41b7a9..96d1d8cc27273 100644 --- a/x-pack/plugins/files/server/routes/file_kind/list.ts +++ b/x-pack/plugins/files/server/routes/file_kind/list.ts @@ -7,6 +7,7 @@ import { schema } from '@kbn/config-schema'; import type { FileJSON, FileKind } from '../../../common/types'; import { CreateRouteDefinition, FILES_API_ROUTES } from '../api_routes'; +import * as cs from '../common_schemas'; import type { CreateHandler, FileKindRouter } from './types'; import { stringOrArrayOfStrings, @@ -24,8 +25,8 @@ const rt = { meta: schema.maybe(schema.object({}, { unknowns: 'allow' })), }), query: schema.object({ - page: schema.maybe(schema.number()), - perPage: schema.maybe(schema.number({ defaultValue: 100 })), + page: schema.maybe(cs.page), + perPage: schema.maybe(cs.pageSize), }), }; diff --git a/x-pack/plugins/files/server/routes/file_kind/share/list.ts b/x-pack/plugins/files/server/routes/file_kind/share/list.ts index edd58dbed7b6e..91a893d2dd31a 100644 --- a/x-pack/plugins/files/server/routes/file_kind/share/list.ts +++ b/x-pack/plugins/files/server/routes/file_kind/share/list.ts @@ -9,13 +9,14 @@ import { schema } from '@kbn/config-schema'; import { CreateRouteDefinition, FILES_API_ROUTES } from '../../api_routes'; import type { FileKind, FileShareJSON } from '../../../../common/types'; import { CreateHandler, FileKindRouter } from '../types'; +import * as cs from '../../common_schemas'; export const method = 'get' as const; const rt = { query: schema.object({ - page: schema.maybe(schema.number()), - perPage: schema.maybe(schema.number()), + page: schema.maybe(cs.page), + perPage: schema.maybe(cs.pageSize), forFileId: schema.maybe(schema.string()), }), }; diff --git a/x-pack/plugins/files/server/routes/find.ts b/x-pack/plugins/files/server/routes/find.ts index 9ec5ab681cb66..80a398189dae3 100644 --- a/x-pack/plugins/files/server/routes/find.ts +++ b/x-pack/plugins/files/server/routes/find.ts @@ -9,6 +9,7 @@ import type { CreateHandler, FilesRouter } from './types'; import { FileJSON } from '../../common'; import { FILES_MANAGE_PRIVILEGE } from '../../common/constants'; import { FILES_API_ROUTES, CreateRouteDefinition } from './api_routes'; +import { page, pageSize } from './common_schemas'; const method = 'post' as const; @@ -32,8 +33,8 @@ const rt = { meta: schema.maybe(schema.object({}, { unknowns: 'allow' })), }), query: schema.object({ - page: schema.maybe(schema.number()), - perPage: schema.maybe(schema.number({ defaultValue: 100 })), + page: schema.maybe(page), + perPage: schema.maybe(pageSize), }), }; From 05c3aab39235e411249934030b8bd7d0ef1f1a2d Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 26 Oct 2022 12:52:32 +0200 Subject: [PATCH 14/17] make sure the file$ does not collapse on error --- .../file_picker/file_picker_state.ts | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts index 4a46588a2b0b0..7300295f15ec2 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts +++ b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts @@ -8,7 +8,9 @@ import { map, tap, from, + EMPTY, switchMap, + catchError, Observable, shareReplay, debounceTime, @@ -17,8 +19,8 @@ import { BehaviorSubject, distinctUntilChanged, } from 'rxjs'; -import { FileJSON } from '../../../common'; -import { FilesClient } from '../../types'; +import type { FileJSON } from '../../../common'; +import type { FilesClient } from '../../types'; function naivelyFuzzify(query: string): string { return query.includes('*') ? query : `*${query}*`; @@ -57,23 +59,22 @@ export class FilePickerState { this.subscriptions = [ this.query$ .pipe( - tap(() => this.setIsLoading(true)), map((query) => Boolean(query)), distinctUntilChanged() ) .subscribe(this.hasQuery$), - this.requests$.pipe(tap(() => this.setIsLoading(true))).subscribe(), - this.internalIsLoading$ - .pipe(debounceTime(100), distinctUntilChanged()) - .subscribe(this.isLoading$), + this.internalIsLoading$.pipe(distinctUntilChanged()).subscribe(this.isLoading$), ]; } private readonly requests$ = combineLatest([ this.currentPage$.pipe(distinctUntilChanged()), - this.query$.pipe(distinctUntilChanged(), debounceTime(100)), + this.query$.pipe(distinctUntilChanged()), this.retry$, - ]); + ]).pipe( + tap(() => this.setIsLoading(true)), // set loading state as early as possible + debounceTime(100) + ); /** * File objects we have loaded on the front end, stored here so that it can @@ -136,6 +137,15 @@ export class FilePickerState { abortSignal: abortController.signal, }) ).pipe( + catchError((e) => { + if (e.name !== 'AbortError') { + this.setIsLoading(false); + this.loadingError$.next(e); + } else { + // If the request was aborted, we assume another request is now in progress + } + return EMPTY; + }), tap(() => { this.setIsLoading(false); this.abort = undefined; @@ -143,13 +153,7 @@ export class FilePickerState { shareReplay() ); - request$.subscribe({ - error: (e: Error) => { - if (e.name === 'AbortError') return; - this.setIsLoading(false); - this.loadingError$.next(e); - }, - }); + request$.subscribe(); return request$; }; From 42a21c07a59714c2a81bb0a7049861e7ce00ab37 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 26 Oct 2022 12:55:09 +0200 Subject: [PATCH 15/17] hide pagination if there are no files --- .../public/components/file_picker/components/pagination.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx index ee036cdbdd338..a1e1fadf18944 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx @@ -8,14 +8,19 @@ import React from 'react'; import type { FunctionComponent } from 'react'; import { EuiPagination } from '@elastic/eui'; +import useObservable from 'react-use/lib/useObservable'; import { useFilePickerContext } from '../context'; import { useBehaviorSubject } from '../../use_behavior_subject'; export const Pagination: FunctionComponent = () => { const { state } = useFilePickerContext(); const page = useBehaviorSubject(state.currentPage$); + const files = useObservable(state.files$, []); const pageCount = useBehaviorSubject(state.totalPages$); const isUploading = useBehaviorSubject(state.isUploading$); + if (files.length === 0) { + return null; + } return ( {} : state.setPage} From c0fada5e80c9d13c0aa6efd097313e96595722f3 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 26 Oct 2022 12:57:38 +0200 Subject: [PATCH 16/17] added a small test to check that pagination is hidden when there are no files --- .../components/file_picker/components/pagination.tsx | 1 + .../public/components/file_picker/file_picker.test.tsx | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx index a1e1fadf18944..397a1cda06e48 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/pagination.tsx @@ -23,6 +23,7 @@ export const Pagination: FunctionComponent = () => { } return ( {} : state.setPage} pageCount={pageCount} activePage={page} diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker.test.tsx b/x-pack/plugins/files/public/components/file_picker/file_picker.test.tsx index 14b621050a0ef..b9005c89503f3 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker.test.tsx +++ b/x-pack/plugins/files/public/components/file_picker/file_picker.test.tsx @@ -50,6 +50,7 @@ describe('FilePicker', () => { selectButton: `${baseTestSubj}.selectButton`, loadingSpinner: `${baseTestSubj}.loadingSpinner`, fileGrid: `${baseTestSubj}.fileGrid`, + paginationControls: `${baseTestSubj}.paginationControls`, }; return { @@ -126,4 +127,10 @@ describe('FilePicker', () => { expect(onDone).toHaveBeenCalledTimes(1); expect(onDone).toHaveBeenNthCalledWith(1, ['a', 'b']); }); + it('hides pagination if there are no files', async () => { + client.list.mockImplementation(() => Promise.resolve({ files: [] as FileJSON[], total: 2 })); + const { actions, testSubjects, exists } = await initTestBed(); + await actions.waitUntilLoaded(); + expect(exists(testSubjects.paginationControls)).toBe(false); + }); }); From e175ac1b99999cdef6df1bd8f3584a6fd00e8a20 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Wed, 26 Oct 2022 13:17:25 +0200 Subject: [PATCH 17/17] do not error in send request method --- .../public/components/file_picker/file_picker_state.test.ts | 6 +----- .../public/components/file_picker/file_picker_state.ts | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts b/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts index 0c3dc4b768717..3a5adb08a7af2 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts +++ b/x-pack/plugins/files/public/components/file_picker/file_picker_state.test.ts @@ -177,11 +177,7 @@ describe('FilePickerState', () => { const upload$ = cold(uploadInput).pipe(tap(() => filePickerState.setIsUploading(true))); const query$ = cold(queryInput).pipe(tap((q) => filePickerState.setQuery(q))); expectObservable(merge(upload$, query$)).toBe('---a-a|'); - expectObservable(filePickerState.files$).toBe( - 'a----#-', - { a: [] }, - new Error('Cannot fetch files while uploading') - ); + expectObservable(filePickerState.files$).toBe('a------', { a: [] }); }); }); }); diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts index 7300295f15ec2..708288f9cb4df 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts +++ b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts @@ -113,7 +113,7 @@ export class FilePickerState { page: number, query: undefined | string ): Observable<{ files: FileJSON[]; total: number }> => { - if (this.isUploading$.getValue()) throw new Error('Cannot fetch files while uploading'); + if (this.isUploading$.getValue()) return EMPTY; if (this.abort) this.abort(); this.setIsLoading(true); this.loadingError$.next(undefined);