Clone role button going to edit page #70768
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thomheymann
force-pushed
the
security/bugfix-clone-role-button
branch
from
e6647c6 to
892b724
Compare
thomheymann
force-pushed
the
security/bugfix-clone-role-button
branch
from
6113a50 to
5277c50
Compare
💚 Build SucceededBuild metricsSaved Objects .kibana field count
History
To update your PR or re-run it, just comment with: |
thomheymann
added
the
Team:Security
|
Pinging @elastic/kibana-security (Team:Security) |
pgayvallet
reviewed
Jul 8, 2020
Comment on lines
+48
to
+50
| history = (scopedHistoryMock.create({ | ||
| createHref: jest.fn((location) => location.pathname!), | ||
| }) as unknown) as ScopedHistory; |
There was a problem hiding this comment.
We don't usually add API to override core mock properties, as the same goal can be achieved by using the existing mocked methods.
const history: ScopedHistoryMock = scopedHistoryMock.create()
history.createHref.mockImplementation((location) => location.pathname!));However I see that our ScopedHistoryMock is not compatible with ScopedHistory, which complicates the process a little, see:
const h: ScopedHistory = scopedHistoryMock.create()
Type 'Mocked<Pick<ScopedHistory<unknown>, "replace" | "length" | "push" | "block" | "location" | "action" | "goBack" | "go" | "goForward" | "listen" | "createHref" | "createSubHistory">>' is missing the following properties from type 'ScopedHistory<unknown>': isActive, listeners, locationKeys, currentLocationKeyIndex, and 7 more.@joshdover wdyt? Is this change fine with you, or should we 'fix' the mock instead? I guess that would force us to create a IScopedHistory, but OTOH the mock cannot currently be properly used for testing (appart from using it for jest.mock('src/core/public'))
There was a problem hiding this comment.
I agree we should probably fix the mock and/or create an interface that doesn't require this private properties. Can be done in a follow up PR though if @thomheymann wants to keep this PR focused or have someone Platform do it.
There was a problem hiding this comment.
Ok. I'll create an issue for the mock fix, and revert this change then.
legrego
approved these changes
Jul 8, 2020
thomheymann
added a commit
to thomheymann/kibana
that referenced
this pull request
Jul 9, 2020
* Clone role button going to edit page * Added unit tests * Fixed types
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
kibanamachine
added
backport missing
thomheymann
added a commit
that referenced
this pull request
Jul 13, 2020
* Clone role button going to edit page * Added unit tests * Fixed types Co-authored-by: Elastic Machine <[email protected]>
|
7.x #71218 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #70219
Checklist