-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[SR] Allow custom index pattern to be used instead of selectable list when choosing indices to restore #41534
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
Changes from all commits
199f4b7
a577553
7ee6173
f5aa8d8
e23da95
8629d89
8f215b9
3c7442a
f67724c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import { | |
| EuiSpacer, | ||
| EuiSwitch, | ||
| EuiTitle, | ||
| EuiComboBox, | ||
| } from '@elastic/eui'; | ||
| import { Option } from '@elastic/eui/src/components/selectable/types'; | ||
| import { RestoreSettings } from '../../../../../common/types'; | ||
|
|
@@ -31,10 +32,9 @@ export const RestoreSnapshotStepLogistics: React.FunctionComponent<StepProps> = | |
| errors, | ||
| }) => { | ||
| const { | ||
| core: { | ||
| i18n: { FormattedMessage }, | ||
| }, | ||
| core: { i18n }, | ||
| } = useAppDependencies(); | ||
| const { FormattedMessage } = i18n; | ||
| const { | ||
| indices: snapshotIndices, | ||
| includeGlobalState: snapshotIncludeGlobalState, | ||
|
|
@@ -55,11 +55,27 @@ export const RestoreSnapshotStepLogistics: React.FunctionComponent<StepProps> = | |
| (index): Option => ({ | ||
| label: index, | ||
| checked: | ||
| isAllIndices || (restoreIndices && restoreIndices.includes(index)) ? 'on' : undefined, | ||
| isAllIndices || | ||
| typeof restoreIndices === 'string' || | ||
| (Array.isArray(restoreIndices) && restoreIndices.includes(index)) | ||
| ? 'on' | ||
| : undefined, | ||
| }) | ||
| ) | ||
| ); | ||
|
|
||
| // State for using selectable indices list or custom patterns | ||
| // Users with more than 100 indices will probably want to use an index pattern to select | ||
| // them instead, so we'll default to showing them the index pattern input. | ||
| const [selectIndicesMode, setSelectIndicesMode] = useState<'list' | 'custom'>( | ||
| typeof restoreIndices === 'string' || snapshotIndices.length > 100 ? 'custom' : 'list' | ||
| ); | ||
|
|
||
| // State for custom patterns | ||
| const [restoreIndexPatterns, setRestoreIndexPatterns] = useState<string[]>( | ||
| typeof restoreIndices === 'string' ? restoreIndices.split(',') : [] | ||
| ); | ||
|
|
||
| // State for setting renaming indices patterns | ||
| const [isRenamingIndices, setIsRenamingIndices] = useState<boolean>( | ||
| Boolean(renamePattern || renameReplacement) | ||
|
|
@@ -147,7 +163,10 @@ export const RestoreSnapshotStepLogistics: React.FunctionComponent<StepProps> = | |
| updateRestoreSettings({ indices: undefined }); | ||
| } else { | ||
| updateRestoreSettings({ | ||
| indices: [...(cachedRestoreSettings.indices || [])], | ||
| indices: | ||
| selectIndicesMode === 'custom' | ||
| ? restoreIndexPatterns.join(',') | ||
| : [...(cachedRestoreSettings.indices || [])], | ||
| }); | ||
| } | ||
| }} | ||
|
|
@@ -156,89 +175,163 @@ export const RestoreSnapshotStepLogistics: React.FunctionComponent<StepProps> = | |
| <Fragment> | ||
| <EuiSpacer size="m" /> | ||
| <EuiFormRow | ||
| className="snapshotRestore__restoreForm__stepLogistics__indicesFieldWrapper" | ||
| label={ | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.selectIndicesLabel" | ||
| defaultMessage="Select indices" | ||
| /> | ||
| selectIndicesMode === 'list' ? ( | ||
| <EuiFlexGroup justifyContent="spaceBetween"> | ||
| <EuiFlexItem grow={false}> | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.selectIndicesLabel" | ||
| defaultMessage="Select indices" | ||
| /> | ||
| </EuiFlexItem> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiLink | ||
| onClick={() => { | ||
| setSelectIndicesMode('custom'); | ||
| updateRestoreSettings({ indices: restoreIndexPatterns.join(',') }); | ||
| }} | ||
| > | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.indicesToggleCustomLink" | ||
| defaultMessage="Use index patterns" | ||
| /> | ||
| </EuiLink> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| ) : ( | ||
| <EuiFlexGroup justifyContent="spaceBetween"> | ||
| <EuiFlexItem grow={false}> | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.indicesPatternLabel" | ||
| defaultMessage="Index patterns" | ||
| /> | ||
| </EuiFlexItem> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiLink | ||
| onClick={() => { | ||
| setSelectIndicesMode('list'); | ||
| updateRestoreSettings({ indices: cachedRestoreSettings.indices }); | ||
| }} | ||
| > | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.indicesToggleListLink" | ||
| defaultMessage="Select indices" | ||
| /> | ||
| </EuiLink> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| ) | ||
| } | ||
| helpText={ | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.selectIndicesHelpText" | ||
| defaultMessage="{count} {count, plural, one {index} other {indices}} will be restored. {selectOrDeselectAllLink}" | ||
| values={{ | ||
| count: restoreIndices && restoreIndices.length, | ||
| selectOrDeselectAllLink: | ||
| restoreIndices && restoreIndices.length > 0 ? ( | ||
| <EuiLink | ||
| onClick={() => { | ||
| indicesOptions.forEach((option: Option) => { | ||
| option.checked = undefined; | ||
| }); | ||
| updateRestoreSettings({ indices: [] }); | ||
| setCachedRestoreSettings({ | ||
| ...cachedRestoreSettings, | ||
| indices: [], | ||
| }); | ||
| }} | ||
| > | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.deselectAllIndicesLink" | ||
| defaultMessage="Deselect all" | ||
| /> | ||
| </EuiLink> | ||
| ) : ( | ||
| <EuiLink | ||
| onClick={() => { | ||
| indicesOptions.forEach((option: Option) => { | ||
| option.checked = 'on'; | ||
| }); | ||
| updateRestoreSettings({ indices: [...snapshotIndices] }); | ||
| setCachedRestoreSettings({ | ||
| ...cachedRestoreSettings, | ||
| indices: [...snapshotIndices], | ||
| }); | ||
| }} | ||
| > | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.selectAllIndicesLink" | ||
| defaultMessage="Select all" | ||
| /> | ||
| </EuiLink> | ||
| ), | ||
| }} | ||
| /> | ||
| selectIndicesMode === 'list' ? ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a good alternative atm, but i'm not sure if it would have been immediately clear to me that the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gchaps Could you help me with the wording for the toggle links? Currently they are: Maybe they could be:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had the same thought when I looked at the copy. How about "Use index pattern" and "Choose individual indices"? In terms of prior art, in the Rollup Job Wizard we use "Create cron expression" and "Create basic interval" to switch between two UIs for creating what eventually becomes a cron expression, though I'm not convinced "Create" is the right verb to use here. I think I would prefer "Use".
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jen-huang Based on the suggestion from @cjcenizal , what about: Use index pattern
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @gchaps! Is there a reason to use "Select index" instead of "Select indices"? Is singular better than plural in these cases?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cjcenizal If the user can select more than one index, then "Select indices" is better. |
||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.selectIndicesHelpText" | ||
| defaultMessage="{count} {count, plural, one {index} other {indices}} will be restored. {selectOrDeselectAllLink}" | ||
| values={{ | ||
| count: restoreIndices && restoreIndices.length, | ||
| selectOrDeselectAllLink: | ||
| restoreIndices && restoreIndices.length > 0 ? ( | ||
| <EuiLink | ||
| onClick={() => { | ||
| indicesOptions.forEach((option: Option) => { | ||
| option.checked = undefined; | ||
| }); | ||
| updateRestoreSettings({ indices: [] }); | ||
| setCachedRestoreSettings({ | ||
| ...cachedRestoreSettings, | ||
| indices: [], | ||
| }); | ||
| }} | ||
| > | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.deselectAllIndicesLink" | ||
| defaultMessage="Deselect all" | ||
| /> | ||
| </EuiLink> | ||
| ) : ( | ||
| <EuiLink | ||
| onClick={() => { | ||
| indicesOptions.forEach((option: Option) => { | ||
| option.checked = 'on'; | ||
| }); | ||
| updateRestoreSettings({ indices: [...snapshotIndices] }); | ||
| setCachedRestoreSettings({ | ||
| ...cachedRestoreSettings, | ||
| indices: [...snapshotIndices], | ||
| }); | ||
| }} | ||
| > | ||
| <FormattedMessage | ||
| id="xpack.snapshotRestore.restoreForm.stepLogistics.selectAllIndicesLink" | ||
| defaultMessage="Select all" | ||
| /> | ||
| </EuiLink> | ||
| ), | ||
| }} | ||
| /> | ||
| ) : null | ||
| } | ||
| isInvalid={Boolean(errors.indices)} | ||
| error={errors.indices} | ||
| > | ||
| <EuiSelectable | ||
| allowExclusions={false} | ||
| options={indicesOptions} | ||
| onChange={options => { | ||
| const newSelectedIndices: string[] = []; | ||
| options.forEach(({ label, checked }) => { | ||
| if (checked === 'on') { | ||
| newSelectedIndices.push(label); | ||
| {selectIndicesMode === 'list' ? ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor nit: we have three parallel branches that depend upon |
||
| <EuiSelectable | ||
| allowExclusions={false} | ||
| options={indicesOptions} | ||
| onChange={options => { | ||
| const newSelectedIndices: string[] = []; | ||
| options.forEach(({ label, checked }) => { | ||
| if (checked === 'on') { | ||
| newSelectedIndices.push(label); | ||
| } | ||
| }); | ||
| setIndicesOptions(options); | ||
| updateRestoreSettings({ indices: [...newSelectedIndices] }); | ||
| setCachedRestoreSettings({ | ||
| ...cachedRestoreSettings, | ||
| indices: [...newSelectedIndices], | ||
| }); | ||
| }} | ||
| searchable | ||
| height={300} | ||
| > | ||
| {(list, search) => ( | ||
| <EuiPanel paddingSize="s" hasShadow={false}> | ||
| {search} | ||
| {list} | ||
| </EuiPanel> | ||
| )} | ||
| </EuiSelectable> | ||
| ) : ( | ||
| <EuiComboBox | ||
| options={snapshotIndices.map(index => ({ label: index }))} | ||
| placeholder={i18n.translate( | ||
| 'xpack.snapshotRestore.restoreForm.stepLogistics.indicesPatternPlaceholder', | ||
| { | ||
| defaultMessage: 'Enter index patterns, i.e. logstash-*', | ||
| } | ||
| )} | ||
| selectedOptions={restoreIndexPatterns.map(pattern => ({ label: pattern }))} | ||
| onCreateOption={(pattern: string) => { | ||
| if (!pattern.trim().length) { | ||
| return; | ||
| } | ||
| }); | ||
| setIndicesOptions(options); | ||
| updateRestoreSettings({ indices: [...newSelectedIndices] }); | ||
| setCachedRestoreSettings({ | ||
| ...cachedRestoreSettings, | ||
| indices: [...newSelectedIndices], | ||
| }); | ||
| }} | ||
| searchable | ||
| height={300} | ||
| > | ||
| {(list, search) => ( | ||
| <EuiPanel paddingSize="s" hasShadow={false}> | ||
| {search} | ||
| {list} | ||
| </EuiPanel> | ||
| )} | ||
| </EuiSelectable> | ||
| const newPatterns = [...restoreIndexPatterns, pattern]; | ||
| setRestoreIndexPatterns(newPatterns); | ||
| updateRestoreSettings({ | ||
| indices: newPatterns.join(','), | ||
| }); | ||
| }} | ||
| onChange={(patterns: Array<{ label: string }>) => { | ||
| const newPatterns = patterns.map(({ label }) => label); | ||
| setRestoreIndexPatterns(newPatterns); | ||
| updateRestoreSettings({ | ||
| indices: newPatterns.join(','), | ||
| }); | ||
| }} | ||
| /> | ||
| )} | ||
| </EuiFormRow> | ||
| </Fragment> | ||
| )} | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the UX consideration we're making here. I think it warrants a comment, e.g.
Also, the Index Pattern Wizard shows you a preview of matching indices to your index pattern. Do you think users would benefit from something similar here? Even if so, I would not consider it a blocker for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW if we do decide to fetch matching indices at some point, #42045 contains a suggestion on how to make that request more efficient.