Btsymbala/trusted app deletion #77316
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
…to use it on the client.
…dded timestamping data.
kevinlog
reviewed
Sep 14, 2020
| ) | ||
| isOutdatedResourceState(currentPage, (data) => { | ||
| return ( | ||
| pageInfosEqual(currentPageInfo, data.paginationInfo) && data.timestamp >= freshDataTimestamp |
There was a problem hiding this comment.
For my understanding, the timestamp comparison is meant to decide when to refresh primarily in the case where we update data like in deletions or creations? I like the concept as it gives us another bit of control for when we update data.
There was a problem hiding this comment.
yes, that's the idea.
|
The approach LGTM, I like the ideas surrounding better control on when we refresh data, i.e. a url is changed vs. stale data |
x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/middleware.ts
Show resolved
Hide resolved
…hanged the modal message to indicate the trusted app name.
efreeti
added
Feature:Endpoint
|
Pinging @elastic/endpoint-management (Team:Endpoint Management) |
|
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
|
Pulled it down and the experience feels great! Creating and deleting trusted apps is easy 👍
looking more closely at the code next |
paul-tavares
approved these changes
Sep 17, 2020
x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/middleware.test.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/middleware.test.ts
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/management/pages/trusted_apps/store/reducer.test.ts
Show resolved
Hide resolved
...ugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.test.tsx
Outdated
Show resolved
Hide resolved
...ck/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.tsx
Show resolved
Hide resolved
kevinlog
approved these changes
Sep 17, 2020
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
distributable file count
History
To update your PR or re-run it, just comment with: |
efreeti
added a commit
to efreeti/kibana
that referenced
this pull request
Sep 23, 2020
* Moved the DeleteTrustedAppsRequestParams to common folder to be able to use it on the client. * Added trusted app deletion API to the client service layer. * Made default data type for async resource state. * Added guard for stale state. * Added timestamp to the list data to be used to refresh list when it's modified. * Separated out base type for resource state change actions. * Added action for outdating list data. * Moved the refresh condition inside the middleware case function and added timestamping data. * Added state, actions, reducers and middleware for deletion dialog. * Added actions column and deletion action. * Added trusted app deletion dialog. * Changed to not have deletonDialog as optional in store. * Changed the store to contain the full entry in the dialog state and changed the modal message to indicate the trusted app name. * Extracted notifications component and enhanced error display. * Added success message and unified messages a bit. * Complete coverage with tests. * Removed unused variable in translations. * Fixed tests because of outdated snapshots and inproper mocking of htmlIdGenerator. * Fixed code review comments. * Fixed type error.
efreeti
added a commit
that referenced
this pull request
Sep 24, 2020
* Moved the DeleteTrustedAppsRequestParams to common folder to be able to use it on the client. * Added trusted app deletion API to the client service layer. * Made default data type for async resource state. * Added guard for stale state. * Added timestamp to the list data to be used to refresh list when it's modified. * Separated out base type for resource state change actions. * Added action for outdating list data. * Moved the refresh condition inside the middleware case function and added timestamping data. * Added state, actions, reducers and middleware for deletion dialog. * Added actions column and deletion action. * Added trusted app deletion dialog. * Changed to not have deletonDialog as optional in store. * Changed the store to contain the full entry in the dialog state and changed the modal message to indicate the trusted app name. * Extracted notifications component and enhanced error display. * Added success message and unified messages a bit. * Complete coverage with tests. * Removed unused variable in translations. * Fixed tests because of outdated snapshots and inproper mocking of htmlIdGenerator. * Fixed code review comments. * Fixed type error.
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
Added functionality to delete trusted application list entry.
Checklist