-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
Use documentation link service for snapshot restore #91596
Conversation
...ck/plugins/snapshot_restore/public/application/services/documentation/documentation_links.ts
Outdated
Show resolved
Hide resolved
612b22e
to
c395384
Compare
@elasticmachine merge upstream |
merge conflict between base and head |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @lcawl!
In the long run, I think https://github.com/elastic/kibana/blob/master/x-pack/plugins/snapshot_restore/public/application/services/documentation/documentation_links.ts should be removed and the documentation link service should be called directly from the app's pages.
I agree with this. I went ahead and opened a PR against your branch with these changes: lcawl#2. If you're comfortable with the changes, you can go ahead and merge it into your branch and I will review again.
befb48b
to
7bf27fc
Compare
That's great, thanks! I'll just address the merge conflicts, then merge. |
Merge pull request #2 from alisonelizabeth/suggested_changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest LGTM. I left one nit and also a suggestion about changing the cron documentation link, but nothing blocking.
@@ -420,7 +417,7 @@ export const PolicyStepLogistics: React.FunctionComponent<StepProps> = ({ | |||
defaultMessage="Use cron expression. {docLink}" | |||
values={{ | |||
docLink: ( | |||
<EuiLink href={documentationLinksService.getCronUrl()} target="_blank"> | |||
<EuiLink href={docLinks.links.watcher.cronSchedule} target="_blank"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For SLM, I think it might make more sense to point to these docs https://www.elastic.co/guide/en/elasticsearch/reference/current/cron-expressions.html rather than watcher.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've changed that URL
x-pack/plugins/snapshot_restore/public/application/lib/type_to_doc_url.ts
Outdated
Show resolved
Hide resolved
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
…_doc_url.ts Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
@@ -126,6 +127,8 @@ export class DocLinksService { | |||
addData: `${ELASTIC_WEBSITE_URL}guide/en/kibana/${DOC_LINK_VERSION}/connect-to-elasticsearch.html`, | |||
kibana: `${ELASTIC_WEBSITE_URL}guide/en/kibana/${DOC_LINK_VERSION}/index.html`, | |||
elasticsearch: { | |||
dateMathIndexNames: `${ELASTICSEARCH_DOCS}date-math-index-names.html`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Could alternatively live under date
with the other dateMath link, but no strong feelings on this one way or the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll make that change!
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
…ndition-for-hiding-recommded-allocation * 'master' of github.com:elastic/kibana: (117 commits) [coverage] ingest data in parallel (elastic#92074) [Lens] Drag and drop performance improvements (elastic#91641) A few more environment uiFilters fixes (elastic#92044) Enabling Uptime and Dashboard a11y test (elastic#91017) [Security Solution][Detections] Adds more granular validation for nested fields (elastic#92041) [Security Solution] [Detections] add overflow-wrap for description (elastic#91945) [Security Solution] [Detections] do not truncate filename in value list table in modal (elastic#91952) Skip flaky apm test elastic#91673 (elastic#92065) [docker] Default server.name to hostname (elastic#90799) Use documentation link service for snapshot restore (elastic#91596) [Security Solution] Clearing up all jest errors and warnings (elastic#91740) Add `@kbn/analytics` to UI Shared Deps (elastic#91810) [7.12][Telemetry] Add missing fields for security telemetry (elastic#91920) [Security Solution] Adds cypress-pipe (elastic#91550) [ML] Fix event rate chart annotation position (elastic#91899) [APM] Break down error table api removing the sparklines (elastic#89138) docs: update dependencies table bug (elastic#91964) [Time to Visualize] Stay in Edit Mode After Dashboard Quicksave (elastic#91729) Unskip Search Sessions Management UI test (elastic#90110) [Fleet] Handle long text in agent details page (elastic#91776) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx # x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx
…bana into task-manager/docs-monitoring * 'task-manager/docs-monitoring' of github.com:gmmorris/kibana: (29 commits) Update docs/developer/plugin-list.asciidoc Update docs/api/task-manager/health.asciidoc Update docs/api/task-manager/health.asciidoc [Lens] Load indexpatterns list from indexPattern Service (elastic#91984) [coverage] ingest data in parallel (elastic#92074) [Lens] Drag and drop performance improvements (elastic#91641) A few more environment uiFilters fixes (elastic#92044) Enabling Uptime and Dashboard a11y test (elastic#91017) [Security Solution][Detections] Adds more granular validation for nested fields (elastic#92041) [Security Solution] [Detections] add overflow-wrap for description (elastic#91945) [Security Solution] [Detections] do not truncate filename in value list table in modal (elastic#91952) Skip flaky apm test elastic#91673 (elastic#92065) [docker] Default server.name to hostname (elastic#90799) Use documentation link service for snapshot restore (elastic#91596) [Security Solution] Clearing up all jest errors and warnings (elastic#91740) Add `@kbn/analytics` to UI Shared Deps (elastic#91810) [7.12][Telemetry] Add missing fields for security telemetry (elastic#91920) [Security Solution] Adds cypress-pipe (elastic#91550) [ML] Fix event rate chart annotation position (elastic#91899) [APM] Break down error table api removing the sparklines (elastic#89138) ...
* master: (36 commits) [Uptime] Thumbnail full screen view steps navigation fix (elastic#91895) Implement ScopedHistory.block (elastic#91099) [Lens] Fix overlowing content on a chart for charts and table (elastic#92006) handle source column differences in embeddable as well (elastic#91987) [Vega] [Map] disable map rotation using right right click / touch rotation gesture (elastic#91996) [Lens] Load indexpatterns list from indexPattern Service (elastic#91984) [coverage] ingest data in parallel (elastic#92074) [Lens] Drag and drop performance improvements (elastic#91641) A few more environment uiFilters fixes (elastic#92044) Enabling Uptime and Dashboard a11y test (elastic#91017) [Security Solution][Detections] Adds more granular validation for nested fields (elastic#92041) [Security Solution] [Detections] add overflow-wrap for description (elastic#91945) [Security Solution] [Detections] do not truncate filename in value list table in modal (elastic#91952) Skip flaky apm test elastic#91673 (elastic#92065) [docker] Default server.name to hostname (elastic#90799) Use documentation link service for snapshot restore (elastic#91596) [Security Solution] Clearing up all jest errors and warnings (elastic#91740) Add `@kbn/analytics` to UI Shared Deps (elastic#91810) [7.12][Telemetry] Add missing fields for security telemetry (elastic#91920) [Security Solution] Adds cypress-pipe (elastic#91550) ...
Summary
Related to #88107
This PR replaces hard-coded links with keywords from the documentation link service.
It also fixes some links (e.g. replacing occurrences of https://www.elastic.co/guide/en/elasticsearch/reference/master/slm-api-put.html with https://www.elastic.co/guide/en/elasticsearch/reference/master/slm-api-put-policy.html)
In the long run, I think https://github.com/elastic/kibana/blob/master/x-pack/plugins/snapshot_restore/public/application/services/documentation/documentation_links.ts should be removed and the documentation link service should be called directly from the app's pages.
[This has been accomplished in the commits @alisonelizabeth added].
Screenshot
The links are used in the Snapshot and Restore UI in places like this: