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.

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

[Tech Debt] Clean up draggables in security solution #212593

Open
christineweng opened this issue Feb 26, 2025 · 1 comment
Open

[Tech Debt] Clean up draggables in security solution #212593

christineweng opened this issue Feb 26, 2025 · 1 comment
Assignees
Labels
Team:Threat Hunting:Investigations Security Solution Investigations Team
Milestone

Comments

@christineweng
Copy link
Contributor

christineweng commented Feb 26, 2025

Background

With the introduction of unified table in timeline (PR, feature flag removed), draggable row renderers are not used anymore. Recently, this PR removed all the isDraggable usage. However, there are many legacy draggable components in the code base.

Goals

Current state

To understand how all the components work together, the following is a flow chart of key components. SecurityCellAction (with blue title) is the new cell actions that should be used. Components with yellow title are legacy components that should be refactored. Other components in circles are ones that depend on these legacy draggable components and thus will be affected by name change, prop change etc.

Image

  • There is redundancy in creating a data provider prop in DefaultDraggable, then deconstruct it in CellActionsWrapper to get field and value. We should just pass field and value through components
  • CellActionsWrapper and DraggableWrapper can be merged, since both are preparing data for the cell actions in some way
  • If data provider prop is removed, then all the props in red (id,fieldType, name, isAggregatable are not needed)

Challenges

Even though the exercise of cleaning up component names and removing props seem straightforward, the main challenges of doing so are

  • Scope of impact: many components rely on DefaultDraggable (like tables, network components) and DraggableBadge (event renderers, row renderers).
  • Downstream changes: most of the renders use DraggableBadge, despite only 31 direct uses, the renderers were built via prop drilling, and removing props will cause many file changes
  • Overall the effort to benefit ratio is not appealing
Example of prop drilling for 1 row renderer

Image

Proposed work plan

Step 1: remove `DraggableWrapper` and move the logic into `CellActionsWrapper`; remove the data provider logic

Image

Step 2: clean up `Default Draggable` by renaming and removing the unused props, clean up the impacted components

Image

Step 3: clean up `DraggableBadge` and all the down stream renderer components (most time consuming)

Image

@christineweng christineweng added this to the 9.2 milestone Feb 26, 2025
@christineweng christineweng self-assigned this Feb 26, 2025
@botelastic botelastic bot added the needs-team Issues missing a team label label Feb 26, 2025
@christineweng christineweng added the Team:Threat Hunting:Investigations Security Solution Investigations Team label Feb 26, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Threat Hunting:Investigations Security Solution Investigations Team
Projects
None yet
Development

No branches or pull requests

2 participants