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

[Files] Allow option to disable delete action in mgt UI #155179

Merged
merged 19 commits into from
Apr 24, 2023

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Apr 18, 2023

This PR adds the possibility to disable behaviour of files in the Files management UI. It is now possible to disable

  • list - Files won't show in the management UI
  • delete - It won't be possible to delete the file from the management UI

Option added to the public registry: managementUiActions

files.registerFileKind({
      id: "...",
      allowedMimeTypes: "...",
      managementUiActions: { // NEW
        list: {
          enabled: false,
        },
        // If "list" is not enabled, this is not necessary :)
        // I leave it for documentation purpose
        delete: {
          enabled: false,
          reason: "Explain why the file cannot be deleted", // optional but nice for our users
        }
      },
    });

How to test

  • run kibana with the examples yarn start --run-examples
  • open the "Files" example an upload a file in each table (all UI actions, no listing and no delete action)
  • Navigate to the Management > Kibana > Files page and verify that the corresponding files are not shown or can't be deleted

Screenshots

Screenshot 2023-04-18 at 16 41 25

Screenshot 2023-04-20 at 13 36 29

Fixes #153756

@sebelga sebelga force-pushed the files/disable-delete-button branch from c5c618d to 52b5b5f Compare April 20, 2023 11:20
@sebelga sebelga marked this pull request as ready for review April 20, 2023 12:37
@sebelga sebelga requested a review from a team as a code owner April 20, 2023 12:37
@sebelga sebelga self-assigned this Apr 20, 2023
@sebelga sebelga added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Apr 20, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@sebelga sebelga added feature:Files release_note:skip Skip the PR/issue when compiling release notes labels Apr 20, 2023
@sebelga sebelga requested a review from Dosant April 20, 2023 12:38
Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! The functionality provided in these changes looks good to me! I pulled your branch and made the modifications to cases and the cases files no longer show in the file management UI 👍

I also tested that users can't delete through the file management UI when the files are listed

image

@sebelga
Copy link
Contributor Author

sebelga commented Apr 20, 2023

Thanks for the review @jonathan-buttner ! 👍

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

I am not sure what's wrong exactly, but files management doesn't seem to be working properly for me:

So I have a file that can't be deleted and two files that can. When I try to select files that can be deleted the selection state isn't correct and the delete button count / modal also doesn't reflect my selection:

delete.bug.mov

examples/files_example/public/components/app.tsx Outdated Show resolved Hide resolved
@sebelga
Copy link
Contributor Author

sebelga commented Apr 24, 2023

Thanks for the review @Dosant !

I am not sure what's wrong exactly, but files management doesn't seem to be working properly for me

Great catch. There was a memoization issue creating the state to be reinitialized on every render. I fixed it in fcd224c

Can you have another look?

@sebelga sebelga requested a review from Dosant April 24, 2023 12:43
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #37 / discover/group3 discover sidebar renders field groups should work when filters change

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/content-management-table-list 13 14 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dashboard 433.8KB 434.5KB +706.0B
filesManagement 86.2KB 87.6KB +1.4KB
graph 453.2KB 453.9KB +709.0B
maps 2.8MB 2.8MB +709.0B
visualizations 259.5KB 260.2KB +712.0B
total +4.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
files 10.5KB 10.6KB +99.0B
Unknown metric groups

API count

id before after diff
@kbn/content-management-table-list 20 21 +1
@kbn/shared-ux-file-types 70 71 +1
files 215 217 +2
total +4

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 395 398 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 475 478 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @sebelga

@sebelga sebelga enabled auto-merge (squash) April 24, 2023 14:18
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! re-tested. looks good

@sebelga sebelga merged commit 29a10fd into elastic:main Apr 24, 2023
@kibanamachine kibanamachine added v8.8.0 backport:skip This commit does not require backporting labels Apr 24, 2023
@sebelga sebelga deleted the files/disable-delete-button branch April 24, 2023 16:06
jonathan-buttner added a commit that referenced this pull request Apr 25, 2023
## Summary

Since #155179 was merged we can
now disable the deletion of Cases files in the Files page in Stack
management.



https://user-images.githubusercontent.com/1533137/234191683-8f768520-f842-413e-b922-200d01e2df28.mov

---------

Co-authored-by: Jonathan Buttner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting feature:Files release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Files] Disable delete button in Files Management
6 participants