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

Add workplace search links to doc link service #118814

Merged
merged 36 commits into from
Nov 29, 2021
Merged

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Nov 16, 2021

Summary

Relates to #88107

This PR adds workplace search documentation URLs to the doc link service so that they can be tested and maintained. It removes the hard-coded links from to https://github.com/elastic/kibana/blob/main/x-pack/plugins/enterprise_search/public/applications/workplace_search/routes.ts and calls the doc link service from https://github.com/elastic/kibana/blob/main/x-pack/plugins/enterprise_search/public/applications/shared/doc_links/doc_links.ts instead.

@lcawl
Copy link
Contributor Author

lcawl commented Nov 19, 2021

@elasticmachine merge upstream

@lcawl lcawl marked this pull request as ready for review November 19, 2021 02:10
@lcawl lcawl requested review from a team November 19, 2021 02:10
@lcawl lcawl requested a review from a team as a code owner November 19, 2021 02:10
@lcawl lcawl requested a review from a team November 19, 2021 02:10
@lcawl lcawl added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v8.1.0 labels Nov 19, 2021
Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

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

Why was x-pack/plugins/enterprise_search/public/applications/shared/doc_links/doc_links.test.ts deleted? We have 100% test coverage everywhere else in our codebase and would like to have this covered as well.

@lcawl
Copy link
Contributor Author

lcawl commented Nov 19, 2021

Why was x-pack/plugins/enterprise_search/public/applications/shared/doc_links/doc_links.test.ts deleted? We have 100% test coverage everywhere else in our codebase and would like to have this covered as well.

I deleted that one since the only thing it was testing was the stem of the docs URLs and those no longer exist. I've re-added it with a smattering of URLs and the docLinksServiceMock: e3a0fe4

lcawl and others added 4 commits November 19, 2021 13:16
…h/components/api_logs/components/empty_state.test.tsx

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
…h/components/curations/components/empty_state.test.tsx

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
…h/components/curations/views/curations_settings/curations_settings.test.tsx

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
…h/components/curations/views/curations_settings/curations_settings.tsx

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
lcawl and others added 4 commits November 19, 2021 13:20
…h/components/engine_overview/engine_overview_empty.test.tsx

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
…h/components/documents/components/empty_state.test.tsx

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
@lcawl
Copy link
Contributor Author

lcawl commented Nov 22, 2021

@elasticmachine merge upstream

Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

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

Workplace Search changes LGTM

@scottybollinger
Copy link
Contributor

scottybollinger commented Nov 24, 2021

@lcawl looks like you have a merge conflict

@byronhulcher @JasonStoltz can one of you review this?

@lcawl
Copy link
Contributor Author

lcawl commented Nov 24, 2021

@elasticmachine update branch

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@lcawl
Copy link
Contributor Author

lcawl commented Nov 24, 2021

@elasticmachine merge upstream

Copy link
Contributor

@orhantoy orhantoy left a comment

Choose a reason for hiding this comment

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

LGTM

@lcawl
Copy link
Contributor Author

lcawl commented Nov 29, 2021

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

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.3MB 1.3MB +474.0B

Page load bundle

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

id before after diff
core 272.7KB 275.8KB +3.1KB
enterpriseSearch 9.4KB 16.2KB +6.8KB
total +9.9KB

History

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

@lcawl lcawl merged commit db74235 into elastic:main Nov 29, 2021
@lcawl lcawl deleted the search-url branch November 29, 2021 19:46
lcawl added a commit to lcawl/kibana that referenced this pull request Nov 29, 2021
Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants