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

[Workplace Search] Role Mappings to Kibana #93123

Merged

Conversation

scottybollinger
Copy link
Contributor

@scottybollinger scottybollinger commented Mar 1, 2021

Summary

This PR converts the Workplace Search Role Mappings component tree to Kibana

coverage
s1
s2
s3

Checklist

closes https://github.com/elastic/workplace-search-team/issues/1193

- Update all paths
- Change router to use children instead of render props
- Remove legacy app chrome
- Update all paths
- Use global flash messages
- Update all paths
- Use global flash messages
- Add types to fix errors
- Use React Router Hooks instead of legacy withRouter HOC
This is not needed because the ID is actually passed in the URL itself and is not a requirement in the body of the request
It appeared that the server sometimes sent flash messages with the API response, but I checked the Rails server code and there is no `flashMessages` sent back from the server so I am omitting that from the `RoleMappingsServerDetails` interface as well.
I did this for RoleMappings but forgot this one
Because the shared role mapping components work for both App Search and Workplace Search, the more generic string is used here because App Search has different role names.
In the case of a new role mapping, the server is called at the ‘/new’ route and the server responds without a roleMapping prop, as it has not yet been created.
@scottybollinger scottybollinger added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 1, 2021
@scottybollinger scottybollinger requested a review from a team March 1, 2021 21:09
@scottybollinger scottybollinger changed the title Scottybollinger/ws role mapping [Workplace Search] Role Mappings to Kibana Mar 1, 2021
@scottybollinger
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1281 1294 +13

Async chunks

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

id before after diff
enterpriseSearch 1.9MB 2.0MB +32.1KB
triggersActionsUi 1.6MB 1.5MB -23.9KB
total +8.2KB

Page load bundle

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

id before after diff
triggersActionsUi 104.0KB 104.1KB +82.0B
Unknown metric groups

async chunk count

id before after diff
triggersActionsUi 41 42 +1

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

Copy link
Contributor

@yakhinvadim yakhinvadim left a comment

Choose a reason for hiding this comment

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

This looks great! Congratulations on migrating the last big component! 🎉

@scottybollinger scottybollinger merged commit 5788a6b into elastic:master Mar 2, 2021
@scottybollinger scottybollinger deleted the scottybollinger/ws-role-mapping branch March 2, 2021 17:23
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #93269

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 2, 2021
* Add routes for role mapings

* Initial copy/paste

* Update RoleMappingsRouter

- Update all paths
- Change router to use children instead of render props
- Remove legacy app chrome

* Update RoleMappings

- Update all paths
- Use global flash messages

* Update RoleMapping

- Update all paths
- Use global flash messages
- Add types to fix errors
- Use React Router Hooks instead of legacy withRouter HOC

* Fix path in index and add route helper

* Update paths in RoleMappingsLogic

* Remove history in favor of KibanaLogic.navigateToUrl

* Add Role type

* Remove ID prop

This is not needed because the ID is actually passed in the URL itself and is not a requirement in the body of the request

* Replace contextual flash messages with global

It appeared that the server sometimes sent flash messages with the API response, but I checked the Rails server code and there is no `flashMessages` sent back from the server so I am omitting that from the `RoleMappingsServerDetails` interface as well.

* Replace Rails http with kibana http

* Fix route path

* Add route and update global navigation

* Add breadcrumb/page title

* Update flash messages in RoleMapping

I did this for RoleMappings but forgot this one

* Use explicit AttributeName type instead of string

* Add i18n

* Fix type issue

Because the shared role mapping components work for both App Search and Workplace Search, the more generic string is used here because App Search has different role names.

* Add tests for components and router

* Add optional to interface

In the case of a new role mapping, the server is called at the ‘/new’ route and the server responds without a roleMapping prop, as it has not yet been created.

* Add tests for RoleMappingsLogic

Co-authored-by: Kibana Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 2, 2021
* master: (199 commits)
  Convert Canvas docs to MDX for use in Elastic Docs (elastic#91969)
  [Bazel] More resilient Workspace Status (elastic#93244)
  [Discover] Change icon of saved search in open search panel and embeddable selection (elastic#93001)
  [Workplace Search] Role Mappings to Kibana (elastic#93123)
  [Fleet] Use type-only imports where possible (elastic#92979)
  [Lens] Set pie chart slices sorted clockwise (elastic#92617)
  Remove ms label from CPU load on status page (elastic#92836)
  [App Search] Migrate Create Meta Engine View (elastic#92127)
  [Time to Visualize] Disable Visualize URL Tracker When Linked to OriginatingApp (elastic#92917)
  [ILM] Allow multiple searchable snapshot actions (elastic#92789)
  Improve consistency for display of management items (elastic#92694)
  skip flaky suite (elastic#93152)
  skip flaky suite (elastic#93152)
  [ILM] Refactor edit_policy client integration tests into separate feature files (elastic#92826)
  Add developer documentation about the building blocks we offer plugin developers (elastic#92743)
  [Security Solution] Case ui enhancement (elastic#91863)
  [Security Solution] [Detections] Updates warning message when no indices match provided index patterns (elastic#93094)
  Collect agent telemetry even when fleet server is disabled. (elastic#93198)
  [Lens] Fix runtime validation error message (elastic#93195)
  [Lens] Remove warning about ordinal x-domain (elastic#93049)
  ...
kibanamachine added a commit that referenced this pull request Mar 2, 2021
* Add routes for role mapings

* Initial copy/paste

* Update RoleMappingsRouter

- Update all paths
- Change router to use children instead of render props
- Remove legacy app chrome

* Update RoleMappings

- Update all paths
- Use global flash messages

* Update RoleMapping

- Update all paths
- Use global flash messages
- Add types to fix errors
- Use React Router Hooks instead of legacy withRouter HOC

* Fix path in index and add route helper

* Update paths in RoleMappingsLogic

* Remove history in favor of KibanaLogic.navigateToUrl

* Add Role type

* Remove ID prop

This is not needed because the ID is actually passed in the URL itself and is not a requirement in the body of the request

* Replace contextual flash messages with global

It appeared that the server sometimes sent flash messages with the API response, but I checked the Rails server code and there is no `flashMessages` sent back from the server so I am omitting that from the `RoleMappingsServerDetails` interface as well.

* Replace Rails http with kibana http

* Fix route path

* Add route and update global navigation

* Add breadcrumb/page title

* Update flash messages in RoleMapping

I did this for RoleMappings but forgot this one

* Use explicit AttributeName type instead of string

* Add i18n

* Fix type issue

Because the shared role mapping components work for both App Search and Workplace Search, the more generic string is used here because App Search has different role names.

* Add tests for components and router

* Add optional to interface

In the case of a new role mapping, the server is called at the ‘/new’ route and the server responds without a roleMapping prop, as it has not yet been created.

* Add tests for RoleMappingsLogic

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Scotty Bollinger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants