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.

Sign up for GitHub

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

Jump to bottom

Add flag to opt out from sub url tracking #55672

Merged
merged 9 commits into from
Jan 28, 2020
Merged

Add flag to opt out from sub url tracking #55672

merged 9 commits into from
Jan 28, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Jan 23, 2020 •
edited
Loading

This PR adds an additional flag to legacy nav links disableSubUrlTracking that mark them to not be included in the auto-sub url tracking of the legacy chrome. This is the first step of incrementally moving this logic into the applications itself.

You can test this by adding disableSubUrlTracking: true to one of the nav links specified in src/legacy/core_plugins/kibana/index.js

The doc builder fails locally for me, that's why the API checker test fails:
node scripts/check_core_api_changes.js --accept

node:82331) UnhandledPromiseRejectionWarning: Error: Internal Error: Unable to analyze the export "OverlayFlyoutStart" in
/Users/jreuter/Clones/kibana/yellow/target/types/core/public/overlays/flyout.d.ts

You have encountered a software defect. Please consider reporting the issue to the maintainers of this application.
    at ExportAnalyzer._getExportOfAstModule (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/ExportAnalyzer.ts:583:13)
    at ExportAnalyzer._getExportOfSpecifierAstModule (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/ExportAnalyzer.ts:573:39)
    at ExportAnalyzer._tryMatchImportDeclaration (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/ExportAnalyzer.ts:497:21)
    at ExportAnalyzer.fetchReferencedAstEntity (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/ExportAnalyzer.ts:345:35)
    at AstSymbolTable._analyzeChildTree (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/AstSymbolTable.ts:323:58)
    at AstSymbolTable._analyzeChildTree (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/AstSymbolTable.ts:360:12)
    at AstSymbolTable._analyzeChildTree (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/AstSymbolTable.ts:360:12)
    at AstSymbolTable._analyzeChildTree (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/AstSymbolTable.ts:360:12)
    at AstSymbolTable._analyzeChildTree (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/AstSymbolTable.ts:360:12)
    at AstSymbolTable.analyze (/Users/jreuter/Clones/kibana/yellow/node_modules/@microsoft/api-extractor/src/analyzer/AstSymbolTable.ts:156:12)
(node:82331) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 3)
(node:82331) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@flash1293 flash1293 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration v7.7.0 labels Jan 23, 2020
@flash1293 flash1293 marked this pull request as ready for review January 23, 2020 14:02
@flash1293 flash1293 requested a review from a team as a code owner January 23, 2020 14:02
@flash1293 flash1293 added Team:Visualizations Visualization editors, elastic-charts and infrastructure Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Jan 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

joshdover
joshdover reviewed Jan 23, 2020
View reviewed changes
Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

For fixing the core API docs, try doing this:

$ rm -rf target/
$ yarn build:types

And then run the check API / docs script.

src/legacy/ui/ui_nav_links/ui_nav_link.js
@@ -25,6 +25,7 @@ export class UiNavLink {
order = 0,
url,
subUrlBase,
disableSubUrlTracking,
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll also need to add this to src/legacy/ui/ui_apps/ui_app.js which is also a source of navLinks

@flash1293
Copy link
Contributor Author

@joshdover That did the trick, thanks 👍

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

pgayvallet
pgayvallet approved these changes Jan 28, 2020
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM for platform changes

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@flash1293 flash1293 merged commit 8c29802 into elastic:master Jan 28, 2020
flash1293 added a commit to flash1293/kibana that referenced this pull request Jan 28, 2020
@flash1293 @elasticmachine
Add flag to opt out from sub url tracking (elastic#55672)
1645ef3 
* introduce flag for disabling sub url tracking

* re-generate documentation and extend ui_app

* fix ui_nav_link test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@flash1293 flash1293 mentioned this pull request Jan 28, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 28, 2020
@gmmorris
Merge branch 'master' into alerting/details-ui
100aa8d 
* master: (21 commits)
  [SIEM][Detection Engine] critical blocker updates to latest ECS version
  [Monitoring] Fix inaccuracies in logstash pipeline listing metrics (elastic#55868)
  Resetting errors and removing duplicates (elastic#56054)
  Add flag to opt out from sub url tracking (elastic#55672)
  [SIEM][Detection Engine] critical bug, fixes duplicate tags
  [ML] Anomaly Detection: Fix persist/restore of refreshInterval in globalState. (elastic#56113)
  [ML] Single Metric Viewer: Fix annnotations refresh. (elastic#56107)
  adapt ObjectToConfigAdapter.getFlattenedPaths to consider arrays as final values (elastic#56105)
  Add Appender.receiveAllLevels option to fix LegacyAppender (elastic#55752)
  [ML] Process delimited files like semi-structured text (elastic#56038)
  Charts plugin (combining ui/color_maps and EuiUtils) (elastic#55469)
  fix tutorial documentation (elastic#55996)
  [ML] Fix persist/restore of time/refreshInterval in data visualizer. (elastic#56122)
  [Index Management] Fix errors with validation (elastic#56072)
  [Index Management] Add try/catch when parsing index filter from URI (elastic#56051)
  [NP] add HTTP resources testing strategies (elastic#54908)
  [ML] Single Metric Viewer: Fix brush update on short recent timespans. (elastic#56125)
  [Uptime] Add timeout for slow process to skipped functional tests (elastic#56065)
  refactor (elastic#56121)
  Move tests in dashboard into appropriate folders (elastic#55304)
  ...
flash1293 added a commit that referenced this pull request Jan 28, 2020
@flash1293
Add flag to opt out from sub url tracking (#55672) (#56169)
23757f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshdover joshdover joshdover left review comments

@pgayvallet pgayvallet pgayvallet approved these changes

Assignees
No one assigned
Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@flash1293 @elasticmachine @kibanamachine @pgayvallet @joshdover