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

adapt Core userSettings service to no longer depends on the security plugin #181538

Merged
merged 7 commits into from
Apr 29, 2024
Merged

adapt Core userSettings service to no longer depends on the security plugin #181538

merged 7 commits into from
Apr 29, 2024

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Apr 24, 2024 •
edited
Loading

Summary

Part of #174578

Adapt Core's userSettings service to leverage Core's userProfile service instead of the security plugin

@pgayvallet pgayvallet mentioned this pull request Apr 24, 2024
Open
@pgayvallet pgayvallet changed the title [security-in-core] change userSettings to use new userProfile service [security-in-core] adapt userSettings to no longer depends on the security plugin Apr 24, 2024
@pgayvallet
Copy link
Contributor Author

/ci

@pgayvallet pgayvallet changed the title [security-in-core] adapt userSettings to no longer depends on the security plugin adapt Core userSettings service to no longer depends on the security plugin Apr 24, 2024
@pgayvallet pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v8.15.0 labels Apr 24, 2024
@pgayvallet
Copy link
Contributor Author

/ci

@pgayvallet
Copy link
Contributor Author

/ci

@pgayvallet
Copy link
Contributor Author

/ci

@pgayvallet
Copy link
Contributor Author

/ci

@pgayvallet pgayvallet marked this pull request as ready for review April 25, 2024 12:15
@pgayvallet pgayvallet requested review from a team as code owners April 25, 2024 12:15
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

pgayvallet
pgayvallet commented Apr 25, 2024
View reviewed changes
packages/core/user-settings/core-user-settings-server-internal/src/user_settings_service.ts
Comment on lines +31 to +33
export class UserSettingsService {
private logger: Logger;
private userProfile?: InternalUserProfileServiceStart;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, this is not a new file, it was moved from packages/core/user-settings/core-user-settings-server-internal/user_settings_service.ts, but I guess the file changed too much so it lost the info

packages/core/user-settings/core-user-settings-server/types.ts
Comment on lines +10 to +12
// keeping the empty service contract for now as we might re-use it very soon
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface UserSettingsServiceSetup {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As said in the comment: We may use the contract very soon to expose new APIs (given the per-user-settings discussions we're currently having) so I kept the contract and the public type package, to avoid having to remove everything and then re-add it later.

@jeramysoucy jeramysoucy self-requested a review April 25, 2024 14:16
TinaHeiligers
TinaHeiligers approved these changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM!

jeramysoucy
jeramysoucy approved these changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@pgayvallet pgayvallet enabled auto-merge (squash) April 29, 2024 06:13
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/core-user-settings-server 6 1 -5
@kbn/core-user-settings-server-internal 4 0 -4
total -9

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/core-user-settings-server-internal 1 2 +1
Unknown metric groups

API count

id before after diff
@kbn/core-user-settings-server 6 1 -5
@kbn/core-user-settings-server-internal 4 0 -4
total -9

ESLint disabled line counts

id before after diff
@kbn/core-user-settings-server 0 1 +1

Total ESLint disabled count

id before after diff
@kbn/core-user-settings-server 0 1 +1

History

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

@pgayvallet pgayvallet merged commit 8bf0c41 into elastic:main Apr 29, 2024
17 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeramysoucy jeramysoucy jeramysoucy approved these changes

@TinaHeiligers TinaHeiligers TinaHeiligers approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting 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 v8.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pgayvallet @elasticmachine @kibana-ci @TinaHeiligers @jeramysoucy @kibanamachine