Skip to content

[SR] Allow custom index pattern to be used instead of selectable list when choosing indices to restore#41534

Merged
jen-huang merged 9 commits intoelastic:masterfrom
jen-huang:fix/sr-restore-indices
Jul 31, 2019
Merged

[SR] Allow custom index pattern to be used instead of selectable list when choosing indices to restore#41534
jen-huang merged 9 commits intoelastic:masterfrom
jen-huang:fix/sr-restore-indices

Conversation

@jen-huang
Copy link
Contributor

Summary

This PR is an enhancement to #39193. The original work overlooked that fact that when restoring a snapshot, an index pattern can be used to choose which indices to restore instead of index names. This is useful for users with with a large amount of indices and/or time series indices.

With this PR, there is a now a toggle link when the user turns off All Indices. By default, if the snapshot to restore has <= 100 indices, the selectable list will display. If the snapshot to restore has > 100 indices, the custom pattern input will display. The custom pattern input does not do validation against the available indices, so ES error will surface on Review page if it is invalid.

Screenshots

Selectable list input with toggle link to custom pattern input

image

Custom pattern input with toggle link to selectable list input

image

Review page using custom pattern input value rollup*,kibana*,test_index

image
image

Review page surfacing an error with invalid index name in custom pattern input

image

@jen-huang jen-huang added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v7.3.0 v7.4.0 labels Jul 18, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@jen-huang jen-huang requested a review from gchaps July 18, 2019 23:01
id="xpack.snapshotRestore.repositoryDetails.reverifyButtonLabel"
defaultMessage="Re-verify repository"
id="xpack.snapshotRestore.repositoryDetails.verifyButtonLabel"
defaultMessage="Verify repository"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a small copy change @gchaps requested

@jen-huang
Copy link
Contributor Author

jen-huang commented Jul 18, 2019

@gchaps Would you please review the copy for the toggle links and the validation error message? Thanks! (The error message under Index Pattern input, not the ES one on review page.)

@elasticmachine
Copy link
Contributor

💔 Build Failed

@jen-huang jen-huang changed the title [SR] Allow user to use custom input instead of selectable list when choosing indices to restore [SR] Allow custom index pattern to be used instead of selectable list when choosing indices to restore Jul 19, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@gchaps
Copy link
Contributor

gchaps commented Jul 19, 2019

We typically use this format for validation on fields:

An index is required.

Make sure this format is consistent with the rest of the UI (i.e. that the validation on the Name field is "Name is required."_

Also, for the message on the review page, is it possible to change the heading to "Unable to restore snapshot."? I'd prefer not to use the word "execute".

Do we need to update the S & R documentation?

@jen-huang
Copy link
Contributor Author

@gchaps Thanks for the feedback!

The SR doc link doesn't seem to include detailed information about the Restore wizard, so I don't think that needs to be updated. I do remember we worked on a detailed walkthrough for the wizard that includes screenshots of all steps - will that be published? If so, there is probably 1 screenshot that should be updated.

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

tested locally and LGTM. i left a couple comments about the UX, let me know what you think.

)}
</EuiSelectable>
) : (
<EuiFieldText
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about using the EuiComboBox here instead?

Copy link
Contributor Author

@jen-huang jen-huang Jul 23, 2019

Choose a reason for hiding this comment

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

I think this is a good idea, I'll play around with the combo box placeholder text to make sure the user understands the kind of patterns they can enter

),
}}
/>
selectIndicesMode === 'list' ? (
Copy link
Contributor

Choose a reason for hiding this comment

The 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 Custom pattern/Show indices link is used to toggle views if i wasn't familiar with the PR changes.

Copy link
Contributor Author

@jen-huang jen-huang Jul 23, 2019

Choose a reason for hiding this comment

The 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: Custom pattern and Select indices. You can see them in the screenshots in PR description. I agree with Alison that the current wording doesn't correlate very strongly to the fact that the links themselves toggle between a text input view and a selectable list view.

Maybe they could be: Enter index patterns and Select from indices list instead? Help!! 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The 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".

image

image

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Select index

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

@jen-huang
Copy link
Contributor Author

Updated toggle link and validation copies, and changed index pattern input to combo box:

image

image

image

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cjcenizal cjcenizal added the Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI label Jul 24, 2019
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Code LGTM! Didn't test locally. I found one string which I think we should internationalize and one comment I'd love to see added before merging but the rest of my comments are optional.


// State for using selectable indices list or custom patterns
const [selectIndicesMode, setSelectIndicesMode] = useState<'list' | 'custom'>(
typeof restoreIndices === 'string' || snapshotIndices.length > 100 ? 'custom' : 'list'
Copy link
Contributor

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.

// 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.

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.

Copy link
Contributor

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.

options.forEach(({ label, checked }) => {
if (checked === 'on') {
newSelectedIndices.push(label);
{selectIndicesMode === 'list' ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: we have three parallel branches that depend upon selectIndicesMode, do you think it would make sense to reduce this to a single branch that switches between an IndicesSelect component and an IndexPatternInput component? That would also reduce the size of this file, which is getting lengthy.

) : (
<EuiComboBox
options={snapshotIndices.map(index => ({ label: index }))}
placeholder="Enter index patterns or indices, i.e. logstash-*"
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 this needs to be internationalized.

@jen-huang jen-huang added v7.3.0 and removed v7.3.0 labels Jul 30, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@jen-huang jen-huang added v7.3.1 and removed v7.3.0 labels Jul 31, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jen-huang jen-huang merged commit 200c189 into elastic:master Jul 31, 2019
@jen-huang jen-huang deleted the fix/sr-restore-indices branch July 31, 2019 04:25
jen-huang added a commit to jen-huang/kibana that referenced this pull request Jul 31, 2019
… when choosing indices to restore (elastic#41534)

* Allow user to use custom input instead of selectable list

* Small copy change, fix indices toggle link

* Remove unused translation

* Adjust copy and change index pattern input to combo box

* Address PR feedback
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 31, 2019
…-or-edit-existing-rollup-job

* 'master' of github.com:elastic/kibana: (114 commits)
  [ML] Fixing empty index pattern list (elastic#42299)
  [Markdown] Shim new platform - cleanup plugin (elastic#41760)
  [Code] Enable hierarchicalDocumentSymbolSupport for java language server (elastic#42233)
  Add New Platform mocks for data plugin (elastic#42261)
  Http server route handler implementation (elastic#41894)
  [SR] Allow custom index pattern to be used instead of selectable list when choosing indices to restore (elastic#41534)
  [Code] distributed Code abstraction (elastic#41374)
  [SIEM] Sets page titles to the current page you are on  (elastic#42157)
  Saved Objects export API stable type order (elastic#42310)
  cancellation of interpreter execution (elastic#40238)
  [SIEM] Fixes a crash when Machine Learning influencers is an undefined value (elastic#42198)
  Changed the job to work with a dedicated index (elastic#42297)
  FTR: fix testSubjects.missingOrFail (elastic#42290)
  Increase retry timeout to prevent flaky tests (elastic#42291)
  Spaces - make space a hidden saved object type (elastic#41688)
  Allow applications to register feature privileges which are excluded from the base privileges (elastic#41300)
  Disable flaky log column reorder test (elastic#42285)
  Fixing add element in element reducer (elastic#42276)
  Fix infinite loop (elastic#42228)
  [Maps][File upload] Remove geojson deep clone logic, handle on maps side (elastic#41835)
  ...
jen-huang added a commit that referenced this pull request Jul 31, 2019
… when choosing indices to restore (#41534) (#42320)

* Allow user to use custom input instead of selectable list

* Small copy change, fix indices toggle link

* Remove unused translation

* Adjust copy and change index pattern input to combo box

* Address PR feedback
jen-huang added a commit that referenced this pull request Jul 31, 2019
… when choosing indices to restore (#41534) (#42319)

* Allow user to use custom input instead of selectable list

* Small copy change, fix indices toggle link

* Remove unused translation

* Adjust copy and change index pattern input to combo box

* Address PR feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.3.1 v7.4.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants