elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.76k stars 8.16k forks source link

[Security Solution] Rework Update flyout to display all field updates and build Three-Way-Diff field component #171520

Open jpdjere opened 10 months ago

jpdjere commented 10 months ago

Epics: https://github.com/elastic/security-team/issues/1974 (internal), https://github.com/elastic/kibana/issues/174168 Design Discussion context: https://github.com/elastic/kibana/issues/178211 Design: Figma (internal) Miro board with UI components breakdown: https://miro.com/app/board/uXjVK0gqjjQ=/?share_link_id=554028328760 Screenshot of the Miro board (click to expand)

TODO (click to expand)

This is roughly the order I plan to implement these. **State management** (React Context to manage the state for components required for this task) - [ ] Make there is clear understanding of edge cases in the rules upgrade workflow - [ ] Research necessity of state persistence in local/session storage - [ ] Come up with initial React Context implementation - [ ] Integrate React Context with the components below - [ ] (If necessary) Reiterate in React Context after integration to fix/improve the implementation **Components: ComparisonSide** (left side, please refer to the [Miro board](https://miro.com/app/board/uXjVK0gqjjQ=/?share_link_id=554028328760) to see where it is) - [x] [[PR #188302](https://github.com/elastic/kibana/pull/188302): merged] Add `VersionPicker` component that displays a dropdown component to select which 2 versions to compare. - [x] [[PR #189384](https://github.com/elastic/kibana/pull/189384): merged] Add an `InlineDiffView` component that uses the existing `DiffView` component, but in inline diff mode. - [x] [[PR #189384](https://github.com/elastic/kibana/pull/189384): merged] Add a `ComparisonSide` component that combines `VersionPicker` and `InlineDiffView` - [x] Get rid of `resovedValue ?? merged_version` logic - [ ] Try to diff JSONs instead of individual subfields - [ ] Simplify generic types - [ ] Storybook: Sort simple cases before advanced ones - [ ] Storybook: Find a way to add explainer comments in Storybook **Components: FinalSide** (right side) - [ ] Add `EditButton` component that switches the edit mode on - [ ] [[PR #191499](https://github.com/elastic/kibana/pull/191499): merged, [PR #192342](https://github.com/elastic/kibana/pull/192342): merged, [PR #193261](https://github.com/elastic/kibana/pull/193261): merged] Add `FieldReadOnly` component that renders a suitable component / components from the Rule Details page based on the passed field name. - [ ] Add `FinalReadOnly` component that combines `EditButton` and `FieldReadOnly` - [ ] Add `SaveButton` component that saves user changes when they are valid and ready. - [ ] Add `Prefill` component that displays a dropdown with versions available for prefill. **Components: FieldEdit** FieldEdit components are components that render one or more UI elements from the Rule Editing page for every field. - [ ] **(In progress)** [[PR #193828](https://github.com/elastic/kibana/pull/193828): draft] Add FieldEdit components for fields in About section - [ ] Add FieldEdit components for fields in Definition section - [ ] Add FieldEdit components for fields in Schedule section - [ ] Add FieldEdit components for fields in Actions section - [ ] Add `FinalEdit` component that combines `SaveButton`, `Prefill` and FieldEdit components. - [ ] Add `FinalSide` component that displays either a `FinalReadOnly` or a `FinalEdit` component depending on the state. - [ ] Save user-typed changes in sessionStorage **Components: Basic layout** - [ ] Add a new tab component for the three-way diff and hide it behind the `prebuiltRulesCustomizationEnabled` feature flag - [ ] Add `RuleDetailsFlyoutHeader` component that displays a [badge](https://github.com/elastic/kibana/pull/186914) for Customized Elastic rules (needs isCustomized) - [ ] Add `PerformUpgradeButton` component that will trigger the upgrade. - [ ] Add `UpdateInfoBar` component that will display the number of updatable fields and a number of fields that have conflicts. (needs conflict) - [ ] Add `RuleDiffSection` component that will display a collapsible section with a label like "Definition", "About" or "Schedule". - [ ] Add `RuleFieldDiff` collapsible component that is expanded by default if conflict is SOLVABLE or NON_SOLVABLE. - [ ] Add `RuleDiffFieldHeader` component that displays the conflict bagde and Accepted/Auto-accepted label. (needs conflict) - [ ] Add a component that displays the "Edited" badge for the fields that were edited - [ ] Add a component that displays the rule version change **Testing** - [ ] Write a test plan - [ ] Write tests (will split this into more todos once the test plan is ready)

Summary

As part of the Prebuilt Rules customization epic, we need to:

  1. Update the Update tab from the flyout that pops up open when clicking on a rule in the Rule Updates table.
  2. Build a Three-Way-Diff field update component that will be used multiple times for each rule update in the Update tab from point 1, one for each rule field that has an update.

1. Update tab

image

The Updates tab currently displays a list of fields to be updated, in side-by-side view of the current and target version of each. This basic structure will be maintained, but for each field, the side-by-side diff component will be replaced by the Three-Way-Diff field component described in step 2.

The tab needs other changes:

2. Build a Three-Way-Diff field update component

The Three-Way-Diff field update component allows users to:

  1. see and understand the current value for a rule's field
  2. see what updates the next version of the rule (coming from the upstream Prebuilt Rules package) proposes for that field
  3. be offered a merged version of the current value and the target version of the field
  4. edit the "merged version" of the field as they prefer, write one from scratch if no proposal for a merged version is automatically made; or use the base, current, or target version as a starting point from which to create a new value for the field
  5. accept the new value for the field

The initial designs for the current component look like so:

Figure 1: image

Figure 2: image

Notice that this is only an initial iteration and does not include the specs written in this ticket.

Component description

We are thinking about a component that, by default, shows two columns:

Left column

The Left column will display, by default, an inline diff between the current and the target version of the field.

This column will additionally display two dropdowns that enable the user to select the two versions being compared. Alternatively, it could be just one, offering the combinations:

Selecting one of the options above will update the content of the Left column to display the diff between those two version.

Right column

The Right Column (Final Version) will contain:

A component displaying the Final Version of the field that will be saved when the upgrade is confirmed. This component can be switched between a read-only view (as seen on the first image above, at the top) and an editable component (as seen on Figure 1, above).

The Read-Only view can be switched to the editable component by clicking on the "Edit" button, and vice-versa by clicking on the "Save button".

The Final version will be displayed differently depending on the value of the conflict field from API response for that field:

The Right column will have a button located on top of the component that displays the Final Version that allows the user to fill in (prepopulate) the editable field, by offering the options:

Selecting any of the first two options will populate the merge proposal column with either the Current or Target version, respectively.

"Accepted" label

When the user has finished editing the field (prefilled, or edited manually, or just want to keep the proposed merged value), they should click on the "Save" button to "Accept" the changes.

Once the user clicks on the "Save" button, the field updates to display an "Accepted" label with a green tick to confirm that the change for the field was approved by the user and validated by the app.

(If the API response has the value of the conflict field set to NO or SOLVABLE, the field will appear from start as auto-accepted)

When the user clicks on "Save", if the change is validated, the accordion container of the field will collapse. If the accordion is reopened, the component should be in read-only view.

The user can "Un-Accept" a change by manually opening the accordion and clicking on the "Edit" button. That action will "Un-Accept" the field and will "block" the the update of the rule until the field is saved again, by clicking on the "Save" button (all fields to needs to be accepted in order to update the rule).

**EXPAND:** This is a UX similar to the file per file review in Github PR's and the feature to mark a file as "Reviewed": ### Before marking file as reviewed ![image](https://github.com/elastic/kibana/assets/5354282/6674219b-8683-414e-8960-4bfa64d2914a) ### After marking file as reviewed ![image](https://github.com/elastic/kibana/assets/5354282/a0ea1871-d79a-48e5-8a84-a03c8e1f9e1f)

ONLY once all fields have been confirmed (all have green ticks) the user can make the request to update the rule. (This however is not part of the spec for this component, but important context to understand its role)

**Other considerations:**

elasticmachine commented 10 months ago

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine commented 10 months ago

Pinging @elastic/security-solution (Team: SecuritySolution)

jpdjere commented 10 months ago

FYI @approksiu @ARWNightingale

banderror commented 7 months ago

This is a great writeup @jpdjere, very comprehensive and accurate 👍 Thank you!

I made a few small adjustments in the text (description is already updated):

-Selecting any of the first two fields will populate the **merge proposal** column
+Selecting any of the first two options will populate the **merge proposal** column
with either the Current or Target version, respectively.
+When the user finished editing the field (prefilled, or edited manually, or just want to
+keep the proposed merged value), they should explicitly accept the change for this field.
-Once the user selects a version, and accepts it via an **"Accept" button**
+Once the user clicks an **"Accept" button** 
located somewhere in the component (preferably near the third column?),
a **checkbox** gets automatically ticked located on the top right, to confirm
-that the change for the field was successfully selected.
+that the change for the field was approved by the user and validated by the app.

@jpdjere @ARWNightingale @approksiu I have a few other thoughts:

nikitaindik commented 2 months ago

Opened the "Add VersionPicker component" PR for review. Added a link to it to the todo list in description.

nikitaindik commented 1 month ago

Just merged the "Add ComparisonSide component" PR.