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

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

Merged
merged 4 commits into from
Sep 9, 2020

Conversation

cee-chen
Copy link
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
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
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:

@@ -67,44 +67,60 @@ export const callEnterpriseSearchConfigAPI = async ({
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
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
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
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
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
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
Contributor Author

cee-chen commented Sep 8, 2020

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

@brianmcgue
Copy link
Contributor

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

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.

🎉

@brianmcgue
Copy link
Contributor

Thank you for accepting my API modifications ☺️

@cee-chen
Copy link
Contributor Author

cee-chen commented Sep 9, 2020

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

@kibanamachine
Copy link
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