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

[functional/security/test-user] use options object #126652

Merged
merged 2 commits into from
Mar 3, 2022
Merged

[functional/security/test-user] use options object #126652

merged 2 commits into from
Mar 3, 2022

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Mar 1, 2022 •
edited
Loading

While working on #126547 I needed to add an option to testUser.setRoles(). To do this I changed the call signature away using a "naked boolean" to an options object so that you can tell from the call-site what is happening, while also renaming the option so it's more intuitive what the default behavior is:

testUser.setRoles(roles, false);
// vs
testUser.setRoles(roles, { skipBrowserRefresh: true });

Previous uses of false were switched to { skipBrowserRefresh: true } and previous uses of true (the default) were removed.

@spalger spalger force-pushed the remove/test-user-naked-boolean branch 2 times, most recently from f1333c6 to 1d855e0 Compare March 1, 2022 22:27
@spalger spalger force-pushed the remove/test-user-naked-boolean branch from 1d855e0 to 7b51b5c Compare March 1, 2022 23:24
@spalger spalger added auto-backport Deprecated: Automatically backport this PR after it's merged release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.16.3 v8.0.0 v8.1.0 v8.2.0 labels Mar 2, 2022
@spalger spalger marked this pull request as ready for review March 2, 2022 00:57
@spalger spalger requested review from a team as code owners March 2, 2022 00:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

kertal
kertal approved these changes Mar 2, 2022
View reviewed changes
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

DataDiscovery.team.LGTM() === true

@kertal
Copy link
Member

kertal commented Mar 2, 2022

should also be back ported to 7.17, right?

pheyos
pheyos approved these changes Mar 2, 2022
View reviewed changes
Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

@spalger spalger added v7.17.2 and removed v7.16.3 labels Mar 2, 2022
nreese
nreese approved these changes Mar 2, 2022
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

changes to maps tests LGTM
code review

ymao1
ymao1 approved these changes Mar 2, 2022
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

Response Ops changes LGTM!

stratoula
stratoula approved these changes Mar 3, 2022
View reviewed changes
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Vis editors changes LGTM, code review only

@spalger
Copy link
Contributor Author

spalger commented Mar 3, 2022

@elasticmachine merge upstream

@spalger spalger enabled auto-merge (squash) March 3, 2022 13:49
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] OSS CI Group #6 / discover app css discover integration with data view editor use ccs to create a new data view

Metrics [docs]

✅ unchanged

History

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

@spalger spalger merged commit be1028c into elastic:main Mar 3, 2022
@spalger spalger deleted the remove/test-user-naked-boolean branch March 3, 2022 15:56
kibanamachine pushed a commit that referenced this pull request Mar 3, 2022
[functional/security/test-user] remove naked boolean (#126652)
49c768c 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit be1028c)
@kibanamachine kibanamachine mentioned this pull request Mar 3, 2022
Merged
kibanamachine pushed a commit that referenced this pull request Mar 3, 2022
[functional/security/test-user] remove naked boolean (#126652)
0d2ee25 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit be1028c)
@kibanamachine kibanamachine mentioned this pull request Mar 3, 2022
Merged
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
✅ 8.0
✅ 8.1
❌ 7.17 Backport failed because of merge conflicts

You might need to backport the following PRs to 7.17:
- Migrated ILM Tests to Use test_user (#121262)

Manual backport

To create the backport manually run:

node scripts/backport --pr 126652

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Mar 3, 2022
@kibanamachine
[functional/security/test-user] remove naked boolean (#126652) (#126807)
104b760 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit be1028c)

Co-authored-by: Spencer <spencer@elastic.co>
kibanamachine added a commit that referenced this pull request Mar 3, 2022
@kibanamachine
[functional/security/test-user] remove naked boolean (#126652) (#126808)
c9e2d49 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit be1028c)

Co-authored-by: Spencer <spencer@elastic.co>
lucasfcosta pushed a commit to lucasfcosta/kibana that referenced this pull request Mar 8, 2022
@kibanamachine @lucasfcosta
[functional/security/test-user] remove naked boolean (elastic#126652)
c46042d 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
lucasfcosta pushed a commit to lucasfcosta/kibana that referenced this pull request Mar 8, 2022
@kibanamachine @lucasfcosta
[functional/security/test-user] remove naked boolean (elastic#126652)
361707b 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@cuff-links cuff-links mentioned this pull request Mar 18, 2022
Closed
68 tasks
@kibanamachine kibanamachine mentioned this pull request Mar 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nreese nreese nreese approved these changes

@kertal kertal kertal approved these changes

@pheyos pheyos pheyos approved these changes

@ymao1 ymao1 ymao1 approved these changes

@stratoula stratoula stratoula approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.17.2 v8.0.0 v8.1.0 v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@spalger @elasticmachine @kertal @kibana-ci @kibanamachine @nreese @pheyos @ymao1 @stratoula