Skip to content

[Enterprise Search] Update config data endpoint to v2#76970

Merged
cee-chen merged 4 commits intoelastic:masterfrom
cee-chen:config-data-v2
Sep 9, 2020
Merged

[Enterprise Search] Update config data endpoint to v2#76970
cee-chen merged 4 commits intoelastic:masterfrom
cee-chen:config-data-v2

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Sep 8, 2020

Summary

This PR is a follow-up to #75616. When that PR was opened, the actual endpoint changes had yet to be implemented, and I was mostly guesstimating the new data based on hypothetical specs. Now that the inestimable @brianmcgue has given us an actual endpoint with much better organized data, we have to update our data structure, tests, and types accordingly.

QA

Checklist

- Update endpoint to v2
- Update data accordingly to new API structures
- Update types accordingly
@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 8, 2020
@cee-chen cee-chen requested review from a team and scottybollinger September 8, 2020 18:46
Copy link
Copy Markdown
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.

LGTM!

Copy link
Copy Markdown
Contributor Author

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Some FYIs:

publicUrl: stripTrailingSlash(data?.settings?.external_url),
readOnlyMode: !!data?.settings?.read_only_mode,
ilmEnabled: !!data?.settings?.ilm_enabled,
isFederatedAuth: !!data?.settings?.is_federated_auth, // i.e., not standard auth
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Brian moved this to a top-level setting, not just nested under a Workplace Search specific key - I agree with him in this case since it affects both AS & WS.

So just an FYI @scottybollinger that the location of isFederatedAuth has moved, if that's okay

Comment on lines +80 to +88
workplaceSearch: {
customApiSource: {
maxDocumentByteSize:
data?.settings?.configured_limits?.workplace_search?.custom_api_source
?.document_size_in_bytes,
totalFields:
data?.settings?.configured_limits?.workplace_search?.custom_api_source?.total_fields,
},
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@scottybollinger Not sure if Workplace Search actually uses this configured limit data anywhere in the UI but I figured I might as well add it!

Comment on lines +72 to +79
appSearch: {
engine: {
maxDocumentByteSize:
data?.settings?.configured_limits?.app_search?.engine?.document_size_in_bytes,
maxEnginesPerMetaEngine:
data?.settings?.configured_limits?.app_search?.engine?.source_engines_per_meta_engine,
},
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note to self:

There's a couple other configured limits for App Search that Bmac added to this endpoint:

Screen Shot 2020-09-08 at 11 49 39 AM

I left them out for now because the App Search UI doesn't indicate using/displaying those values (at least in AppLogic), but they're there if we ever need to ✨

groups: data?.settings?.workplace_search?.fp_account.groups || [],
isAdmin: !!data?.settings?.workplace_search?.fp_account?.is_admin,
canCreatePersonalSources: !!data?.settings?.workplace_search?.fp_account
account: {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@scottybollinger - FYI, Brian renamed fpAccount to just account and I followed suit, mostly because I think the distinction is no longer really necessary in the new Kibana world (theoretically since it's client-facing only, it shouldn't need to hold references to our old code names).

That being said, this change only affects the initial data/config endpoint & doesn't preclude you from using fpAccount in the rest of Workplace Search if your team strongly prefers to do so - just a thought that you could probably clean that var name up moving forward if you wanted to.

isCurated: !!data?.settings?.workplace_search?.fp_account.is_curated,
viewedOnboardingPage: !!data?.settings?.workplace_search?.fp_account
.viewed_onboarding_page,
canCreateInvitations: !!data?.current_user?.workplace_search?.account
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@scottybollinger FYI - canCreateInvitations moved from being a top-level WS key to nested under account

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Sep 8, 2020

Will merge this in tomorrow if @brianmcgue confirms roleType is intentional :shipit:

@brianmcgue
Copy link
Copy Markdown
Contributor

roleType was unintentional! https://github.com/elastic/ent-search/pull/2099 should fix.

Copy link
Copy Markdown
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.

🎉

@brianmcgue
Copy link
Copy Markdown
Contributor

Thank you for accepting my API modifications ☺️

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Sep 9, 2020

@brianmcgue They were amazing!! You're a much better API developer than me 😆

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@cee-chen cee-chen merged commit a0defb8 into elastic:master Sep 9, 2020
@cee-chen cee-chen deleted the config-data-v2 branch September 9, 2020 17:01
cee-chen pushed a commit that referenced this pull request Sep 9, 2020
* Update our internal config/app data to v2 specs

- Update endpoint to v2
- Update data accordingly to new API structures
- Update types accordingly

* Fix failing type check for other endpoints that use IAccount

* Update role type casing

- ent search was fixed from camel to snake
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 10, 2020
* master: (25 commits)
  [bugfix] Replace panel flyout opens 2 flyouts (elastic#76931)
  clean up test (elastic#76887)
  [Enterprise Search] Update shared API request handler (elastic#77112)
  [Maps] convert ESAggSource to TS (elastic#76999)
  Add plugin status API - take 2 (elastic#76732)
  Adds lens as a readable saved object for read-only dashboard users (elastic#77067)
  Skip checking for the reserved realm (elastic#76687)
  Reporting/diagnostics (elastic#74314)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  [ML] Add decision path charts to exploration results table (elastic#73561)
  Bump eventemitter3 from 4.0.0 to 4.0.7 (elastic#77016)
  [Ingest Pipelines] Add descriptions for ingest processors K-S (elastic#76981)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 10, 2020
* master: (41 commits)
  [bugfix] Replace panel flyout opens 2 flyouts (elastic#76931)
  clean up test (elastic#76887)
  [Enterprise Search] Update shared API request handler (elastic#77112)
  [Maps] convert ESAggSource to TS (elastic#76999)
  Add plugin status API - take 2 (elastic#76732)
  Adds lens as a readable saved object for read-only dashboard users (elastic#77067)
  Skip checking for the reserved realm (elastic#76687)
  Reporting/diagnostics (elastic#74314)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  [ML] Add decision path charts to exploration results table (elastic#73561)
  Bump eventemitter3 from 4.0.0 to 4.0.7 (elastic#77016)
  [Ingest Pipelines] Add descriptions for ingest processors K-S (elastic#76981)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 10, 2020
…rok/new-patterns-component-use-array

* 'master' of github.com:elastic/kibana: (39 commits)
  [APM] Always load esarchives from common (elastic#77139)
  [Ingest Manager] Handle Legacy ES client errors (elastic#76865)
  [Docs] URL Drilldown (elastic#76529)
  [bugfix] Replace panel flyout opens 2 flyouts (elastic#76931)
  clean up test (elastic#76887)
  [Enterprise Search] Update shared API request handler (elastic#77112)
  [Maps] convert ESAggSource to TS (elastic#76999)
  Add plugin status API - take 2 (elastic#76732)
  Adds lens as a readable saved object for read-only dashboard users (elastic#77067)
  Skip checking for the reserved realm (elastic#76687)
  Reporting/diagnostics (elastic#74314)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  ...

# Conflicts:
#	src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_array.ts
rylnd added a commit to rylnd/kibana that referenced this pull request Sep 10, 2020
* master: (38 commits)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  [ML] Add decision path charts to exploration results table (elastic#73561)
  Bump eventemitter3 from 4.0.0 to 4.0.7 (elastic#77016)
  [Ingest Pipelines] Add descriptions for ingest processors K-S (elastic#76981)
  [Metrics UI] Replace Snapshot API with Metrics API (elastic#76253)
  legacy utils cleanup (elastic#76608)
  [ML] Account for "properties" layer in find_file_structure mappings (elastic#77035)
  fixed typo
  Upgrade to Kea 2.2 (elastic#77047)
  a11y tests on spaces home page including feature control  (elastic#76515)
  [ML] Transforms list: persist pagination through refresh interval (elastic#76786)
  [ML] Replace all use of date_histogram interval with fixed_interval (elastic#76876)
  ...
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 v7.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants