[Security Solution] turn on newDataViewPickerEnabled feature flag

[PhilippeOberti]

Summary

This PR turns on the newDataViewPickerEnabled feature flag.

Checklist

• Unit or functional tests were updated or added to match the most common scenarios
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
• Review the backport guidelines and apply applicable backport:* labels.

#234088

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: c32ec93

Failed CI Steps

• Jest Tests #7
• Jest Tests #7
• FTR Configs #1
• FTR Configs #4
• Serverless AI Assistant - Security Solution Cypress Tests #1

Test Failures

• [job] [logs] FTR Configs #4 / Cloud Security Posture Cloud Posture Rules Page - Counters "before each" hook for "Shows posture score when there are findings"
• [job] [logs] Jest Tests #7 / EmbeddedMapComponent On mount, selects existing Kibana data views that match any selected index pattern
• [job] [logs] Jest Tests #7 / EmbeddedMapComponent On mount, selects existing Kibana data views that match any selected index pattern
• [job] [logs] Jest Tests #7 / EmbeddedMapComponent On rerender with new selected patterns, selects existing Kibana data views that match any selected index pattern
• [job] [logs] Jest Tests #7 / EmbeddedMapComponent On rerender with new selected patterns, selects existing Kibana data views that match any selected index pattern
• [job] [logs] Jest Tests #7 / EmbeddedMapComponent renders IndexPatternsMissingPrompt
• [job] [logs] Jest Tests #7 / EmbeddedMapComponent renders IndexPatternsMissingPrompt

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
securitySolution	10.5MB	10.5MB	-39.0B

History

• 💔 Build #340106 failed c672673
• 💔 Build #340095 failed c12261b
• 💔 Build #338204 failed e51c30e
• 💔 Build #338193 failed 74e6c7e

[elasticmachine]

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

[jkelas]

Why this change?
Moving setShowBuildingBlockAlerts (which dispatches) into render introduces a side-effect during render and may cause potential loops.

[PhilippeOberti]

I spend over an entire day troubleshooting an issue where this Cypress test was failing. For some (still) unknown reason, the table was not showing the building block alerts.

Debugging this was extremely challenging as the current code in this index.tsx file is a bit of a nightmare (no offense). The page is re-rendering so many times before even showing anything on the screen, it's impossible to know what happens and why... Changing that useEffect fixed the issue without changing any of the behavior. I know it's not ideal but after spending hours on this I was running out of ideas... :(

[PhilippeOberti]

If you prefer me not changing this file, then I'l revert the change and will skip the Cypress test (which is owned by our team)

[jkelas]

I am against this change, as it's an anti-pattern.

I think that ideally you could improve it this way: let the useDataTableFilters hook accept an override for building_block_type.

You would then skip calling the setShowBuildingBlockAlerts in the index.ts file, because that logic would already be included in the hook. The calling of the hook would look something like this:

    const { showBuildingBlockAlerts, showOnlyThreatIndicatorAlerts } = useDataTableFilters(
      TableId.alertsOnRuleDetailsPage,
      {
        buildingBlockOverride: rule?.building_block_type != null,
      }
    );

The code in the hook would look like this:

export type UseDataTableFilters = (
  tableId: TableId,
  options?: {
    buildingBlockOverride?: boolean;
  }
) => {
  showBuildingBlockAlerts: boolean;
  setShowBuildingBlockAlerts: (value: boolean) => void;
  showOnlyThreatIndicatorAlerts: boolean;
  setShowOnlyThreatIndicatorAlerts: (value: boolean) => void;
};

export const useDataTableFilters: UseDataTableFilters = (tableId: TableId, options) => {
  const dispatch = useDispatch();

  const getTable = useMemo(() => dataTableSelectors.getTableByIdSelector(), []);

  const { showOnlyThreatIndicatorAlerts, showBuildingBlockAlerts: stateShowBuildingBlockAlerts } =
    useShallowEqualSelector(
      (state) =>
        (getTable(state, tableId) ?? tableDefaults).additionalFilters ??
        tableDefaults.additionalFilters
    );

  // Use override value if provided, otherwise use state value
  const showBuildingBlockAlerts = options?.buildingBlockOverride ?? stateShowBuildingBlockAlerts;
...

Please try this approach. If it doesn't work, then you would have to skip the test.

I agree that the code in this file might benefit from a refactoring, but that's something for a different time. If you wish, please create a ticket for us and describe the issue and the need for refactoring this as a way to unskip the test.

[PhilippeOberti]

I spent a little bit more time and decided to revert the change and skip our Cypress test.

The funny thing is with the useEffect, the Cypress test is stuck in an infinite re-rendering loop. Removing the useEffect fixes that... 🤣

[tiansivive]

Entity Analytics changes LGTM

[NicholasPeretti]

This looks great!
Works nice on my machine as well 🚀

[NicholasPeretti]

Very nice changes in the test file! 🚀

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: a2d7a75

Failed CI Steps

• Detection Engine - Security Solution Cypress Tests #4

Test Failures

• [job] [logs] Detection Engine - Security Solution Cypress Tests #4 / Changing alert status privileges - ESS User is readonly "before each" hook for "should not allow users to bulk change the alert status" "before each" hook for "should not allow users to bulk change the alert status"

Metrics [docs]

✅ unchanged

History

• 💔 Build #346394 failed 5d718bb
• 💔 Build #346353 failed 77b7cc2
• 💛 Build #345497 was flaky b7ea0ec
• 💔 Build #345431 failed 3c97362
• 💚 Build #342774 succeeded 04c0b59