-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[App Search] Credentials: Add final Logic and server routes #81519
Conversation
Note: import reorder is required in for mocks to work correctly
+ some opinionated comments
describe('validates', () => { | ||
describe('admin keys', () => { |
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.
Note: this is copied/pasted from the new POST route tests. In theory we could DRY it out (since both routes inherit the same exact schema), but I didn't want to over-complicate it for now.
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.
Nice work, quick turn around on this! There's a couple of small changes we may want to consider here.
...prise_search/public/applications/app_search/components/credentials/credentials_logic.test.ts
Outdated
Show resolved
Hide resolved
...prise_search/public/applications/app_search/components/credentials/credentials_logic.test.ts
Outdated
Show resolved
Hide resolved
...enterprise_search/public/applications/app_search/components/credentials/credentials_logic.ts
Show resolved
Hide resolved
...enterprise_search/public/applications/app_search/components/credentials/credentials_logic.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/enterprise_search/server/routes/app_search/credentials.ts
Show resolved
Hide resolved
…ked being undefined vs a bool
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
…81519) * Add fullEngineAccessChecked logic * Add onEngineSelect logic * [Refactor] DRY out/simplify http mocks Note: import reorder is required in for mocks to work correctly * Add onApiTokenChange logic * Update flyout footer to use onApiTokenChange * Add new POST/PUT server routes + some opinionated comments * [PR feedback] tests copy, extra data tests * [PR feedback] Reuse fullEngineAccessChecked, fix fullEngineAccessChecked being undefined vs a bool
…81540) * Add fullEngineAccessChecked logic * Add onEngineSelect logic * [Refactor] DRY out/simplify http mocks Note: import reorder is required in for mocks to work correctly * Add onApiTokenChange logic * Update flyout footer to use onApiTokenChange * Add new POST/PUT server routes + some opinionated comments * [PR feedback] tests copy, extra data tests * [PR feedback] Reuse fullEngineAccessChecked, fix fullEngineAccessChecked being undefined vs a bool Co-authored-by: Kibana Machine <[email protected]>
PR sequence
This PR is part of a set of 3 PRs that implements the Credentials flyout form + functionality. It's broken up into PRs in the 400-800 line range so as not to open a 1700~ line PR.
List of all PRs (may be helpful for obtaining more context / QAing the entire final feature)
Summary
This PR achieves:
CredentialsLogic.values.fullEngineAccessChecked
CredentialsLogic.actions.onEngineSelect
CredentialsLogic.actions.onApiTokenChange
CredentialsLogic.actions.onApiTokenChange
calls:/api/app_search/credentials
/api/app_search/credentials/{name}
QA
This PR does not have easily testable changes in and of itself. It's easier to QA this against the
credentials-flyout-3
branch.Checklist