Skip to content

Comments

[APM] Show missing permissions message to the user on the Services overview#56374

Merged
cauemarcondes merged 5 commits intoelastic:masterfrom
cauemarcondes:permission
Feb 7, 2020
Merged

[APM] Show missing permissions message to the user on the Services overview#56374
cauemarcondes merged 5 commits intoelastic:masterfrom
cauemarcondes:permission

Conversation

@cauemarcondes
Copy link
Contributor

@cauemarcondes cauemarcondes commented Jan 30, 2020

closes #50052

Checks whether a user has permission to read the APM index. It does it by calling a new API /api/apm/security/permissions, it returns hasPermission: true | false. Based on this response shows the permission page on top of any APM page.

It doesn't block the UI completely, as the user can still manually close the view the open the APM page.

@cauemarcondes cauemarcondes requested a review from a team January 30, 2020 12:28
cauemarcondes
cauemarcondes commented Jan 30, 2020
View reviewed changes
x-pack/legacy/plugins/apm/public/new-platform/plugin.tsx Outdated
Copy link
Contributor Author

@cauemarcondes cauemarcondes Jan 30, 2020

Choose a reason for hiding this comment

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

The Permission component was added here, as it should show part of the original page, such as the Breadcrumbs

@cauemarcondes
Copy link
Contributor Author

cauemarcondes commented Jan 30, 2020

@elasticmachine merge upstream

smith
smith requested changes Jan 31, 2020
View reviewed changes
Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

A few changes needed, but looks pretty nice.

x-pack/legacy/plugins/apm/public/components/app/Permission/index.tsx Outdated
Copy link
Contributor

@smith smith Jan 31, 2020

Choose a reason for hiding this comment

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

What's with this?

Copy link
Contributor Author

@cauemarcondes cauemarcondes Jan 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I changed it to iconColor={''} TS doesn't complain about it. But this is used to show the APM icon with color. Otherwise, it shows black and white. I will probably open a new issue because null should be valid as described here: https://elastic.github.io/eui/#/display/emptyprompt

@cauemarcondes cauemarcondes requested a review from smith January 31, 2020 09:58
@cauemarcondes
Copy link
Contributor Author

cauemarcondes commented Feb 3, 2020

@elasticmachine merge upstream

dgieselaar
dgieselaar approved these changes Feb 3, 2020
View reviewed changes
Copy link
Contributor

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

looking good, just a couple of questions & suggestions

dgieselaar
dgieselaar reviewed Feb 4, 2020
View reviewed changes
dgieselaar
dgieselaar reviewed Feb 5, 2020
View reviewed changes
@kibanamachine
Copy link
Contributor

kibanamachine commented Feb 6, 2020

💚 Build Succeeded

History

  • 💚 Build #24542 succeeded 881cd57aad7bdb831864ebc65ef556f034d0604d
  • 💚 Build #24293 succeeded bf2ea13bb344815b0c5e08140f34424bed1f913e
  • 💚 Build #23937 succeeded 7f4fc6d7e0fdd0eeb943c45ac15a405cf7e41baa
  • 💛 Build #23686 was flaky b1bbecc01b31132161e919abfb9b3447e5cbb9ff
  • 💛 Build #23499 was flaky e3bfcfd163eb4e76a328d15ba86a6406b1da45d7

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

@cauemarcondes cauemarcondes merged commit 07cb4c1 into elastic:master Feb 7, 2020
@cauemarcondes cauemarcondes deleted the permission branch February 7, 2020 08:02
@cauemarcondes cauemarcondes mentioned this pull request Feb 7, 2020
Merged
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Feb 7, 2020
@cauemarcondes
[APM] Show missing permissions message to the user on the Services ov…
56ef098 
…erview (elastic#56374)

* creating permission page

* pr comments

* creating permission page

* fixing pr comments

* removing breadcrumb and adding static label to dismiss the permission page
cauemarcondes added a commit that referenced this pull request Feb 7, 2020
@cauemarcondes
[APM] Show missing permissions message to the user on the Services ov…
fa8becb 
…erview (#56374) (#57082)

* creating permission page

* pr comments

* creating permission page

* fixing pr comments

* removing breadcrumb and adding static label to dismiss the permission page
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 7, 2020
@jloleysens
Merge branch 'console/feature/text-objects-in-saved-objects' of githu…
9e266c6 
…b.com:jloleysens/kibana into console/feature/text-objects-in-saved-objects

* 'console/feature/text-objects-in-saved-objects' of github.com:jloleysens/kibana: (103 commits)
  fix auto closing new vis modal when navigating to lens or when navigating away with browser history (elastic#56998)
  TS of esKuery\node_types  (elastic#56857)
  Kibana app migration: Move static code dependencies into kibana_legacy plugin, part 1 (elastic#56408)
  Retry ES API calls that fail with 410/Gone (elastic#56950)
  [APM] Show missing permissions message to the user on the Services overview (elastic#56374)
  Fixing flaky CI tests for custom appRoutes (elastic#55763)
  [State Management][Docs] State syncing utils docs (elastic#56479)
  [Index management] Remove index mapper setting in tests (elastic#57066)
  Exposed common EuiExpressions to separate components be able to reuse for building new for Alert Types  (elastic#56466)
  [SIEM] update url state between page if date is relative (elastic#56813)
  fix for chart_types test (elastic#57056)
  chore(NA): remove compress from dll minimizer (elastic#57023)
  [File upload] Migrate routing to NP & add route validation (elastic#52313)
  Adding docs for grouped nav advanced setting (elastic#57013)
  Use i18n titles for field formatters, human names for numeral locales (elastic#56348)
  [Maps] Remove EMS catalogue url from docs (elastic#57020)
  [Endpoint] ERT-82 ERT-83 ERT-84: Alert list API with pagination (elastic#56538)
  [DOCS] Adds Apple notarization info to install doc (elastic#57042)
  [ML] New Platform server shim: update results service routes to use new platform router (elastic#56886)
  Fix typo on detection engine rule (elastic#56993)
  ...
@cauemarcondes cauemarcondes mentioned this pull request Feb 10, 2020
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smith smith smith approved these changes

+1 more reviewer

@dgieselaar dgieselaar dgieselaar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:enhancement v7.7.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[APM] Show missing permissions message to the user on the Services overview

4 participants

@cauemarcondes @kibanamachine @smith @dgieselaar