Skip to content

Commit

Permalink
[8.12] [Security Solution] Exceptions flyout refreshes when it regain…
Browse files Browse the repository at this point in the history 
…s focus, causing configured fields to get cleared (#166550) (#172666) (#173131)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Security Solution] Exceptions flyout refreshes when it regains
focus, causing configured fields to get cleared (#166550)
(#172666)](#172666)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-12-12T09:14:57Z","message":"[Security
Solution] Exceptions flyout refreshes when it regains focus, causing
configured fields to get cleared (#166550) (#172666)\n\n##
Summary\r\n\r\nAddresses
https://github.com/elastic/kibana/issues/166550\r\n\r\nThese changes fix
the issue where user can experience data loss while\r\nadding rule
exceptions.\r\n\r\nThe main issue is that we keep conditions state
inside the child\r\ncomponent `ExceptionsConditions` which is rendered
conditionally based\r\non `isLoading` flag. This flag changes when
`useQuery` data gets stale\r\nand refetching is triggered. In this case
we would remove\r\n`ExceptionsConditions` component while loading data
and re-create it\r\nafter. Since conditions state stored inside
`ExceptionsConditions` we\r\nwill lose it.\r\n\r\nThis is a quick fix to
make sure our users are not frustrated. The state\r\nrefactoring will
come separately in the next release when we are going\r\nto address the
main
issue\r\nhttps://github.com/elastic/security-team/issues/8197\r\n\r\nTo
reproduce:\r\n1. Open \"add rule exception\" flyout\r\n2. Add exception
conditions\r\n3. Wait for 5 minutes (to avoid waiting this long you can
use
[this\r\napproach](https://github.com/elastic/kibana/issues/166550#issuecomment-1802941467))\r\n4.
Remove focus from the page (by switching to another app or
navigating\r\nto a different tab in a browser)\r\n5. Focus on the page
again\r\n\r\nWhen you are back to the page all exception conditions
should still be\r\nthere.\r\n\r\n---------\r\n\r\nCo-authored-by:
Vitalii Dmyterko
<[email protected]>","sha":"a0631cf8bd0ce820c1a3768f02e8384afc922b27","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:
SecuritySolution","backport:prev-minor","Team:Detection
Engine","v8.13.0"],"number":172666,"url":"https://github.com/elastic/kibana/pull/172666","mergeCommit":{"message":"[Security
Solution] Exceptions flyout refreshes when it regains focus, causing
configured fields to get cleared (#166550) (#172666)\n\n##
Summary\r\n\r\nAddresses
https://github.com/elastic/kibana/issues/166550\r\n\r\nThese changes fix
the issue where user can experience data loss while\r\nadding rule
exceptions.\r\n\r\nThe main issue is that we keep conditions state
inside the child\r\ncomponent `ExceptionsConditions` which is rendered
conditionally based\r\non `isLoading` flag. This flag changes when
`useQuery` data gets stale\r\nand refetching is triggered. In this case
we would remove\r\n`ExceptionsConditions` component while loading data
and re-create it\r\nafter. Since conditions state stored inside
`ExceptionsConditions` we\r\nwill lose it.\r\n\r\nThis is a quick fix to
make sure our users are not frustrated. The state\r\nrefactoring will
come separately in the next release when we are going\r\nto address the
main
issue\r\nhttps://github.com/elastic/security-team/issues/8197\r\n\r\nTo
reproduce:\r\n1. Open \"add rule exception\" flyout\r\n2. Add exception
conditions\r\n3. Wait for 5 minutes (to avoid waiting this long you can
use
[this\r\napproach](https://github.com/elastic/kibana/issues/166550#issuecomment-1802941467))\r\n4.
Remove focus from the page (by switching to another app or
navigating\r\nto a different tab in a browser)\r\n5. Focus on the page
again\r\n\r\nWhen you are back to the page all exception conditions
should still be\r\nthere.\r\n\r\n---------\r\n\r\nCo-authored-by:
Vitalii Dmyterko
<[email protected]>","sha":"a0631cf8bd0ce820c1a3768f02e8384afc922b27"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/172666","number":172666,"mergeCommit":{"message":"[Security
Solution] Exceptions flyout refreshes when it regains focus, causing
configured fields to get cleared (#166550) (#172666)\n\n##
Summary\r\n\r\nAddresses
https://github.com/elastic/kibana/issues/166550\r\n\r\nThese changes fix
the issue where user can experience data loss while\r\nadding rule
exceptions.\r\n\r\nThe main issue is that we keep conditions state
inside the child\r\ncomponent `ExceptionsConditions` which is rendered
conditionally based\r\non `isLoading` flag. This flag changes when
`useQuery` data gets stale\r\nand refetching is triggered. In this case
we would remove\r\n`ExceptionsConditions` component while loading data
and re-create it\r\nafter. Since conditions state stored inside
`ExceptionsConditions` we\r\nwill lose it.\r\n\r\nThis is a quick fix to
make sure our users are not frustrated. The state\r\nrefactoring will
come separately in the next release when we are going\r\nto address the
main
issue\r\nhttps://github.com/elastic/security-team/issues/8197\r\n\r\nTo
reproduce:\r\n1. Open \"add rule exception\" flyout\r\n2. Add exception
conditions\r\n3. Wait for 5 minutes (to avoid waiting this long you can
use
[this\r\napproach](https://github.com/elastic/kibana/issues/166550#issuecomment-1802941467))\r\n4.
Remove focus from the page (by switching to another app or
navigating\r\nto a different tab in a browser)\r\n5. Focus on the page
again\r\n\r\nWhen you are back to the page all exception conditions
should still be\r\nthere.\r\n\r\n---------\r\n\r\nCo-authored-by:
Vitalii Dmyterko
<[email protected]>","sha":"a0631cf8bd0ce820c1a3768f02e8384afc922b27"}}]}]
BACKPORT-->

Co-authored-by: Ievgen Sorokopud <[email protected]>
  • Loading branch information
@kibanamachine @e40pud
kibanamachine and e40pud authored Dec 12, 2023
1 parent 94bccfe commit 2ea119b
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 98 deletions.
200 changes: 103 additions & 97 deletions ...olution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,106 +500,112 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
</EuiTitle>
<EuiSpacer size="m" />
</FlyoutHeader>

<FlyoutBodySection className="builder-section">
<EuiSkeletonText data-test-subj="loadingAddExceptionFlyout" lines={4} isLoading={isLoading}>
{errorSubmitting != null && (
<>
<EuiCallOut
data-test-subj="addExceptionErrorCallOut"
title={i18n.SUBMIT_ERROR_TITLE}
{
// TODO: This is a quick fix to make sure that we do not lose conditions state on refetching index patterns via `useFetchIndexPatterns`
// which happens due to data being stale after 5 minutes (in `useFetchJobsSummaryQuery`, `useFetchModulesQuery` and `useFetchRecognizerQuery`)
// which makes useQuery triggering data refetch.
// To fix the issue properly, we will need to do refactoring and store conditions state in the parent component (`AddExceptionFlyout`)
// instead of keeping it in `ExceptionsConditions` which can be removed and recreated due to fetching steps described above.
// Refactoring ticket: https://github.com/elastic/security-team/issues/8197
}
{isLoading && <EuiSkeletonText data-test-subj="loadingAddExceptionFlyout" lines={4} />}
{errorSubmitting != null && (
<>
<EuiCallOut
data-test-subj="addExceptionErrorCallOut"
title={i18n.SUBMIT_ERROR_TITLE}
color="danger"
iconType="warning"
>
<EuiText>{i18n.SUBMIT_ERROR_DISMISS_MESSAGE}</EuiText>
<EuiSpacer size="s" />
<EuiButton
data-test-subj="addExceptionErrorDismissButton"
color="danger"
iconType="warning"
onClick={handleDismissError}
>
<EuiText>{i18n.SUBMIT_ERROR_DISMISS_MESSAGE}</EuiText>
<EuiSpacer size="s" />
<EuiButton
data-test-subj="addExceptionErrorDismissButton"
color="danger"
onClick={handleDismissError}
>
{i18n.SUBMIT_ERROR_DISMISS_BUTTON}
</EuiButton>
</EuiCallOut>
<EuiSpacer size="s" />
</>
)}
<ExceptionsFlyoutMeta
exceptionItemName={exceptionItemName}
onChange={setExceptionItemMeta}
/>
<EuiHorizontalRule />
<ExceptionsConditions
exceptionItemName={exceptionItemName}
allowLargeValueLists={allowLargeValueLists}
exceptionListItems={initialItems}
exceptionListType={listType}
indexPatterns={indexPatterns}
rules={rules}
selectedOs={selectedOs}
showOsTypeOptions={listType === ExceptionListTypeEnum.ENDPOINT && !hasAlertData}
isEdit={false}
onOsChange={setSelectedOs}
onExceptionItemAdd={setExceptionItemsToAdd}
onSetErrorExists={setConditionsValidationError}
getExtendedFields={getExtendedFields}
/>

{listType !== ExceptionListTypeEnum.ENDPOINT && !sharedListToAddTo?.length && (
<>
<EuiHorizontalRule />
<ExceptionsAddToRulesOrLists
rules={rules}
isBulkAction={isBulkAction}
selectedRadioOption={addExceptionToRadioSelection}
onListSelectionChange={setListsToAddExceptionTo}
onRuleSelectionChange={setSelectedRules}
onRadioChange={setRadioOption}
/>
</>
)}
<EuiHorizontalRule />
<ExceptionItemComments
accordionTitle={
<SectionHeader size="xs">
<h3>{i18n.COMMENTS_SECTION_TITLE(newComment ? 1 : 0)}</h3>
</SectionHeader>
}
initialIsOpen={!!newComment}
newCommentValue={newComment}
newCommentOnChange={setComment}
setCommentError={setCommentError}
/>
{listType !== ExceptionListTypeEnum.ENDPOINT && (
<>
<EuiHorizontalRule />
<ExceptionsExpireTime
expireTime={expireTime}
setExpireTime={setExpireTime}
setExpireError={setExpireError}
/>
</>
)}
{showAlertCloseOptions && (
<>
<EuiHorizontalRule />
<ExceptionItemsFlyoutAlertsActions
exceptionListType={listType}
shouldCloseSingleAlert={closeSingleAlert}
shouldBulkCloseAlert={bulkCloseAlerts}
disableBulkClose={disableBulkClose}
exceptionListItems={exceptionItems}
alertData={alertData}
alertStatus={alertStatus}
isAlertDataLoading={isAlertDataLoading ?? false}
onDisableBulkClose={setDisableBulkCloseAlerts}
onUpdateBulkCloseIndex={setBulkCloseIndex}
onBulkCloseCheckboxChange={setBulkCloseAlerts}
onSingleAlertCloseCheckboxChange={setCloseSingleAlert}
/>
</>
)}
</EuiSkeletonText>
{i18n.SUBMIT_ERROR_DISMISS_BUTTON}
</EuiButton>
</EuiCallOut>
<EuiSpacer size="s" />
</>
)}
<ExceptionsFlyoutMeta
exceptionItemName={exceptionItemName}
onChange={setExceptionItemMeta}
/>
<EuiHorizontalRule />
<ExceptionsConditions
exceptionItemName={exceptionItemName}
allowLargeValueLists={allowLargeValueLists}
exceptionListItems={initialItems}
exceptionListType={listType}
indexPatterns={indexPatterns}
rules={rules}
selectedOs={selectedOs}
showOsTypeOptions={listType === ExceptionListTypeEnum.ENDPOINT && !hasAlertData}
isEdit={false}
onOsChange={setSelectedOs}
onExceptionItemAdd={setExceptionItemsToAdd}
onSetErrorExists={setConditionsValidationError}
getExtendedFields={getExtendedFields}
/>

{listType !== ExceptionListTypeEnum.ENDPOINT && !sharedListToAddTo?.length && (
<>
<EuiHorizontalRule />
<ExceptionsAddToRulesOrLists
rules={rules}
isBulkAction={isBulkAction}
selectedRadioOption={addExceptionToRadioSelection}
onListSelectionChange={setListsToAddExceptionTo}
onRuleSelectionChange={setSelectedRules}
onRadioChange={setRadioOption}
/>
</>
)}
<EuiHorizontalRule />
<ExceptionItemComments
accordionTitle={
<SectionHeader size="xs">
<h3>{i18n.COMMENTS_SECTION_TITLE(newComment ? 1 : 0)}</h3>
</SectionHeader>
}
initialIsOpen={!!newComment}
newCommentValue={newComment}
newCommentOnChange={setComment}
setCommentError={setCommentError}
/>
{listType !== ExceptionListTypeEnum.ENDPOINT && (
<>
<EuiHorizontalRule />
<ExceptionsExpireTime
expireTime={expireTime}
setExpireTime={setExpireTime}
setExpireError={setExpireError}
/>
</>
)}
{showAlertCloseOptions && (
<>
<EuiHorizontalRule />
<ExceptionItemsFlyoutAlertsActions
exceptionListType={listType}
shouldCloseSingleAlert={closeSingleAlert}
shouldBulkCloseAlert={bulkCloseAlerts}
disableBulkClose={disableBulkClose}
exceptionListItems={exceptionItems}
alertData={alertData}
alertStatus={alertStatus}
isAlertDataLoading={isAlertDataLoading ?? false}
onDisableBulkClose={setDisableBulkCloseAlerts}
onUpdateBulkCloseIndex={setBulkCloseIndex}
onBulkCloseCheckboxChange={setBulkCloseAlerts}
onSingleAlertCloseCheckboxChange={setCloseSingleAlert}
/>
</>
)}
</FlyoutBodySection>
<EuiFlyoutFooter>
<FlyoutFooterGroup justifyContent="spaceBetween">
Expand Down
2 changes: 1 addition & 1 deletion ...lution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
</EuiTitle>
<EuiSpacer size="m" />
</FlyoutHeader>
{isLoading && <EuiSkeletonText data-test-subj="loadingEditExceptionFlyout" lines={4} />}
<FlyoutBodySection className="builder-section">
{isLoading && <EuiSkeletonText data-test-subj="loadingEditExceptionFlyout" lines={4} />}
<ExceptionsFlyoutMeta
exceptionItemName={exceptionItemName}
onChange={setExceptionItemMeta}
Expand Down

0 comments on commit 2ea119b

Please sign in to comment.