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

[TIP] Restrict content visibility for non-enterprise users #138097

Merged
merged 1 commit into from
Aug 9, 2022
Merged

[TIP] Restrict content visibility for non-enterprise users #138097

merged 1 commit into from
Aug 9, 2022

Conversation

lgestc
Copy link
Contributor

@lgestc lgestc commented Aug 4, 2022

Summary

I think this unit test sums it up best.

In short, this will restrict the nested components visiblity and render the fallback instead if
the user is below enterprise tier.

Checklist

Delete any items that are not applicable to this PR.

@lgestc lgestc added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team: Protections Experience labels Aug 4, 2022
@lgestc lgestc requested a review from a team as a code owner August 4, 2022 11:39
@maxcold
Copy link
Contributor

maxcold commented Aug 4, 2022

am I correct that we keep our page in the navigation, but render nothing when not on enterprise? This approach is great if in the future we want to render "upgrade your license" fallback, but I think if we go with no fallback, then we would like to hide the app completely as with the feature flag - basically no nav item, no page available via direct link (redirect to Security Overview or smth). While we are waiting for the decision on the "upgrade your license block" and have our feature flag I think we can go ahead and merge it, but can happen that we will need to update the logic once more

@lgestc
Copy link
Contributor Author

lgestc commented Aug 4, 2022

am I correct that we keep our page in the navigation, but render nothing when not on enterprise? This approach is great if in the future we want to render "upgrade your license" fallback, but I think if we go with no fallback, then we would like to hide the app completely as with the feature flag - basically no nav item, no page available via direct link (redirect to Security Overview or smth). While we are waiting for the decision on the "upgrade your license block" and have our feature flag I think we can go ahead and merge it, but can happen that we will need to update the logic once more

According to the comment in parent issue (on mobile, cannot paste it) - we are not going to hide anything in the nav. :)

I will implement the specific fallback once the design is ready.

@PhilippeOberti
Copy link
Contributor

PhilippeOberti commented Aug 4, 2022

@maxcold @lgmys looking at this comment from the Security Solution, it seems they don't care about having the plugin load but show nothing...

Changes are implemented. I used a Redirect to the base route (goes to Get Started) when the Threat Intelligence feature isn't enabled

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
threatIntelligence 115 118 +3

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
threatIntelligence 5 6 +1

Async chunks

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

id before after diff
securitySolution 5.6MB 5.6MB +20.0B
threatIntelligence 62.5KB 62.3KB -111.0B
total -91.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
threatIntelligence 0 1 +1

Page load bundle

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

id before after diff
threatIntelligence 4.8KB 5.2KB +410.0B
Unknown metric groups

API count

id before after diff
threatIntelligence 5 6 +1

History

  • 💔 Build #63141 failed 406c5d96e211fc4298dfe0a7cfb222b091b2eefe
  • 💔 Build #63133 failed e5a1f5bad7270d9ff1d336490223514d566c54d2
  • 💔 Build #63129 failed e5839507c67acd775cbbf44d2b63bfe196a8c878

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

@maxcold
Copy link
Contributor

maxcold commented Aug 8, 2022

@lgmys makes sense, as we are going with the fallback component "upgrade your license" the approach in the PR is good!

@lgestc lgestc merged commit 335d63a into elastic:main Aug 9, 2022
@lgestc lgestc deleted the add-enterprise-guard branch August 9, 2022 08:01
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 9, 2022
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team: Protections Experience v8.4.0 v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants