-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Allow User to Cleanup Repository from UI #53047
Allow User to Cleanup Repository from UI #53047
Conversation
…g, added new repository request, type and telemetry metric.
@elasticmachine merge upstream |
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
Currently, hitting the |
… fixed a broken inport of RepositoryCleanup.
@elasticmachine merge upstream |
…ight now so we need to get those green. Added a functional test. Need to set up kbn-es to be able to set up a file repository before being able to run the functional tests.
💔 Build FailedHistory
To update your PR or re-run it, just comment with: |
…anup_Action_To_UI
…itory list table so that columns can be individually identified. Added functional test to allow checking the details of repositories.
…anup_Action_To_UI
💔 Build FailedTo update your PR or re-run it, just comment with: |
💚 Build SucceededTo update your PR or re-run it, just comment with: |
@elasticmachine merge upstream |
@alisonelizabeth I think your proposed UI/UX changes are spot-on. As long as we can still display info about the error in the callout so the user can figure out what went wrong, a danger callout will make it clearer to the user that there's a problem. |
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.
@cuff-links thanks for the quick turnaround on addressing my comments! Retested and works great. I left a few additional comments - mostly nits - but there were a few strings missing i18n that should be addressed before merging.
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
</div> | ||
) : ( | ||
<EuiCallOut | ||
title="Sorry, there was an error cleaning the repository." |
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.
i18n
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/snapshot_restore/public/app/services/http/repository_requests.ts
Show resolved
Hide resolved
async getRepoList() { | ||
const table = await testSubjects.find('repositoryTable'); | ||
const rows = await table.findAllByCssSelector('[data-test-subj="row"]'); | ||
return await mapAsync(rows, async row => { |
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.
could you use Promise.all
here instead of importing bluebird?
x-pack/legacy/plugins/snapshot_restore/__jest__/client_integration/helpers/http_requests.ts
Show resolved
Hide resolved
…Bird and used Promise.all(). Fixed all nits in PR comments.
jenkins test this |
@elasticmachine merge upstream |
@alisonelizabeth I think we are good now. |
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
…s/kibana into 43904_Add_Cleanup_Action_To_UI
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.
LGTM. Great job on this! I left one comment around your use of JSON.stringify()
, but going to go ahead and approve so I don't block you.
...t_restore/public/app/sections/home/repository_list/repository_details/repository_details.tsx
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Added repository cleanup button. Added logic for spinner while loading, added new repository request, type and telemetry metric. * Added additional bindings for server side to hit the cleanup endpoint. * fix cleanup request * Added data test subject to the code editors to differentiate them and fixed a broken inport of RepositoryCleanup. * Added files for a component integration test. The tests are failing right now so we need to get those green. Added a functional test. Need to set up kbn-es to be able to set up a file repository before being able to run the functional tests. * Added change to the way data-test-subjects were created for the repository list table so that columns can be individually identified. Added functional test to allow checking the details of repositories. * Removed the jest tests for repository details until we get jest fixed. * Fixed jest test to reflect updated test subjects. * Made changes per feedback in PR comments. * Fixed i10n issues using <FormattedMessage>. Removed reference to blueBird and used Promise.all(). Fixed all nits in PR comments. * Added i10n fixes for header. * Added i10n fixes for header. * Added name parameter for i18n strings. * Removed i18n string from JSON.stringify call since it's already a string. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
* master: (69 commits) [Graph] Fix various a11y issues (elastic#54097) Add ApplicationService app status management (elastic#50223) logs in one time (elastic#54447) Deprecate using `elasticsearch.ssl.certificate` without `elasticsearch.ssl.key` and vice versa (elastic#54392) [Optimizer] Fix a stack overflow with watch_cache when it attempts to delete very large folders. (elastic#54457) Security - Role Mappings UI (elastic#53620) [SIEM] [Detection engine] Permission II (elastic#54292) Allow User to Cleanup Repository from UI (elastic#53047) [Detection engine] Some UX for rule creation (elastic#54471) share specific instances of some ui packages (elastic#54079) [ML] APM modules configs for RUM Javascript and NodeJS (elastic#53792) [APM] Delay rendering invalid license notification (elastic#53924) [Graph] Improve error message on graph requests (elastic#54230) [ILM] Kibana should allow a min_age setting of 0ms in ILM policy phases (elastic#53719) Unit Tests for common/lib (elastic#53736) [Graph] Only show explorable fields (elastic#54101) remove linting rule exception for markdown (elastic#54232) [Monitoring] Fetch shard data more efficiently (elastic#54028) [Maps] Add hiddenLayers option to embeddable map input (elastic#54355) Pass termOrder and hasTermsAgg properties to serializeThresholdWatch function (elastic#54391) ...
* master: (69 commits) [Graph] Fix various a11y issues (elastic#54097) Add ApplicationService app status management (elastic#50223) logs in one time (elastic#54447) Deprecate using `elasticsearch.ssl.certificate` without `elasticsearch.ssl.key` and vice versa (elastic#54392) [Optimizer] Fix a stack overflow with watch_cache when it attempts to delete very large folders. (elastic#54457) Security - Role Mappings UI (elastic#53620) [SIEM] [Detection engine] Permission II (elastic#54292) Allow User to Cleanup Repository from UI (elastic#53047) [Detection engine] Some UX for rule creation (elastic#54471) share specific instances of some ui packages (elastic#54079) [ML] APM modules configs for RUM Javascript and NodeJS (elastic#53792) [APM] Delay rendering invalid license notification (elastic#53924) [Graph] Improve error message on graph requests (elastic#54230) [ILM] Kibana should allow a min_age setting of 0ms in ILM policy phases (elastic#53719) Unit Tests for common/lib (elastic#53736) [Graph] Only show explorable fields (elastic#54101) remove linting rule exception for markdown (elastic#54232) [Monitoring] Fetch shard data more efficiently (elastic#54028) [Maps] Add hiddenLayers option to embeddable map input (elastic#54355) Pass termOrder and hasTermsAgg properties to serializeThresholdWatch function (elastic#54391) ...
* Added repository cleanup button. Added logic for spinner while loading, added new repository request, type and telemetry metric. * Added additional bindings for server side to hit the cleanup endpoint. * fix cleanup request * Added data test subject to the code editors to differentiate them and fixed a broken inport of RepositoryCleanup. * Added files for a component integration test. The tests are failing right now so we need to get those green. Added a functional test. Need to set up kbn-es to be able to set up a file repository before being able to run the functional tests. * Added change to the way data-test-subjects were created for the repository list table so that columns can be individually identified. Added functional test to allow checking the details of repositories. * Removed the jest tests for repository details until we get jest fixed. * Fixed jest test to reflect updated test subjects. * Made changes per feedback in PR comments. * Fixed i10n issues using <FormattedMessage>. Removed reference to blueBird and used Promise.all(). Fixed all nits in PR comments. * Added i10n fixes for header. * Added i10n fixes for header. * Added name parameter for i18n strings. * Removed i18n string from JSON.stringify call since it's already a string. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
* Added repository cleanup button. Added logic for spinner while loading, added new repository request, type and telemetry metric. * Added additional bindings for server side to hit the cleanup endpoint. * fix cleanup request * Added data test subject to the code editors to differentiate them and fixed a broken inport of RepositoryCleanup. * Added files for a component integration test. The tests are failing right now so we need to get those green. Added a functional test. Need to set up kbn-es to be able to set up a file repository before being able to run the functional tests. * Added change to the way data-test-subjects were created for the repository list table so that columns can be individually identified. Added functional test to allow checking the details of repositories. * Removed the jest tests for repository details until we get jest fixed. * Fixed jest test to reflect updated test subjects. * Made changes per feedback in PR comments. * Fixed i10n issues using <FormattedMessage>. Removed reference to blueBird and used Promise.all(). Fixed all nits in PR comments. * Added i10n fixes for header. * Added i10n fixes for header. * Added name parameter for i18n strings. * Removed i18n string from JSON.stringify call since it's already a string. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
* Added repository cleanup button. Added logic for spinner while loading, added new repository request, type and telemetry metric. * Added additional bindings for server side to hit the cleanup endpoint. * fix cleanup request * Added data test subject to the code editors to differentiate them and fixed a broken inport of RepositoryCleanup. * Added files for a component integration test. The tests are failing right now so we need to get those green. Added a functional test. Need to set up kbn-es to be able to set up a file repository before being able to run the functional tests. * Added change to the way data-test-subjects were created for the repository list table so that columns can be individually identified. Added functional test to allow checking the details of repositories. * Removed the jest tests for repository details until we get jest fixed. * Fixed jest test to reflect updated test subjects. * Made changes per feedback in PR comments. * Fixed i10n issues using <FormattedMessage>. Removed reference to blueBird and used Promise.all(). Fixed all nits in PR comments. * Added i10n fixes for header. * Added i10n fixes for header. * Added name parameter for i18n strings. * Removed i18n string from JSON.stringify call since it's already a string. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
Closes #43904
This PR adds an additional button to the flyout for repositories to allow for cleaning up of unreferenced data.