Search aggregation: Support new non-proactive variation for the sidebar aggregation UI

[vovakulikov]

Screenshots

The area in the red rectangle will be hidden if all of the following criteria are true:

• Feature flag: disable-proactive-insight-aggregation enabled
• Any group by is active.
  • By default all of the group-by buttons will be inactive. When the user clicks a button the chart will render.

Dynamic content

• Buttons show an active state depending on the URL using useSyncedWithURLState
• Chart is pulled from API call TBD currently just pulling mock data
• Chart section visibility is dependent upon feature flag

Reqs

• Create a new feature flag disable-proactive-insight-aggregation false by default
• Active button needs to support "none" state if flag is active. See client/search-ui/src/results/aggregation/hooks.ts:aggregationModeDeserializer
• Hide the chart section in client/search-ui/src/results/sidebar/SearchAggregations.tsx if feature flag is enabled and no grouping is selected
• Currently, the app is using static mock data. Add a way to only pull this data based on feature flag Not needed. Vova's PR will be easy to integrate later.
  • Right now it hides all of the UI if search-aggregation-filters is disabled. This will obviously not make any API call. But we need to make sure the proactive call is also behind the new disable-proactive-insight-aggregation
    • tldr: Only make API call on page load if both flags are true

Developing

• Feature flag search-aggregation-filters need to be enabled
  • Go to site-admin/feature-flags create a boolean feature flag called search-aggregation-filters set to true
• Run any search and the aggregation card should be visible in the side bar

Testing

• Need unit tests asserting chart is only hidden when expected
  • feature flag enabled plus no grouping

Reviewing

• Feature flag search-aggregation-filters need to be enabled
  • Go to site-admin/feature-flags create a boolean feature flag called search-aggregation-filters set to true
• Search for anything. i.e. "hello" Confirm aggregation card shows in the side panel
• Enable disable-proactive-insight-aggregation
  • Go to site-admin/feature-flags create a boolean feature flag called disable-proactive-insight-aggregation set to true
• Run any search
• Confirm only buttons show in the side bar and explanatory text show

• Select a grouping mode by clicking one of the buttons
• Confirm chart displays

Context

Figma Desings

From Figma

Option with no proactive request to the BE (depending on how the prototype is going to turn out). The user needs to choose one of the options to the group.

According to this statement, one of the possible aggregation UI configurations here is to render aggregation controls only and not render the chart until users explicitly click on one of the aggregation controls and pick the aggregation type. This action should trigger the aggregation chart appearance and BE query run.

A separate user experimental feature flag should control this non-proactive UI configuration. So it should have its own user-set feature flag.

/cc @Joelkw @felixfbecker @vovakulikov @unclejustin

[felixfbecker]

@vovakulikov to avoid confusion I think it's maybe not the best idea to refer to all these states as "modes" because "modes" usually mean that they're mutually exclusive. But what is referred to as the "full UI mode" here and this new "non-proactive mode" are not mutually exclusive (as you have noted in the issue). I would suggest being more specific about what controls this state, is this just the feature flag for proactive insights? If so, what's the difference between this issue and #40088?

[unclejustin]

@AlicjaSuska could I get some guidance for what to show in this instance?

What's happening here?

This is the scenario where we disable proactively running the aggregations. If the flag is set to true, then the chart will be hidden, and no grouping mode will be set until the user clicks something. This is what it looks like currently before anything is selected.

I think we should have some sort of explanation here. Like "Select a grouping to aggregate this search" but, more human 😂

[felixfbecker]

@unclejustin I don't think that matches the design and it's clearer in the design:

• The title should be "Group by", not "Grouped by". That way it's clear it's an action the user can take here.
• The buttons shouldn't be a button group, they should be individual buttons, with no button selected yet. In the screenshot "Repo" is already selected, which makes it look broken. Having buttons where none is selected further communicates that the user needs to take an action.
• Margin between sections seems a bit too much?
• Side point: "Repo" should be "Repository"

[felixfbecker]

@vovakulikov some questions on the spec in the issue body:

• Feature flag search-aggregation-filters – what does "filters" here mean? What is being filtered?
• Feature flag disable-proactive-insight-aggregation – should this be proactive-aggregation-insights to avoid the double negative, with a default of true? Or is it not possible to distinguish between "not set" and false/can feature flags not have defaults?

[unclejustin]

@felixfbecker excellent points re: design! I was not intending to infer that this ready for design review. Just trying to get clarification as early as possible. What you see in my comment's screenshot is what is currently on main. Apologies for the confusion.

Feature flag search-aggregation-filters – what does "filters" here mean? What is being filtered?

Not sure. This is the feature flag that was already provided on main.

This is a good call out though since this isn't in any customer hands yet, now would be the time to fix it. It is being used to completely enable/disable the aggregation feature. I don't have a better name for it though so I am open to suggestions.

Feature flag disable-proactive-insight-aggregation – should this be...

My understanding is that this flag is defaulting to false. So we speak of it in terms of "disabling" the proactive aspect of this feature. I'm am certainly open to changing it though. I can see how enabling a feature flag to disable a feature can be a bit confusing.

Edit: I've updated my screenshot to remove confustion

[vovakulikov]

@felixfbecker I think part of this issue body was updated by someone else

Feature flag search-aggregation-filters – what does "filters" here mean? What is being filtered?

search-aggregation-filters if you have suggestions about naming they are welcomed. I created this flag with this name since this aggregation panel is placed in the filters sections and it actually provides filter functionality (the drill down filters by clicking on bar chart link)

[yowayb]

I know you're not yet at design review but doesn't the lack of a chart mean lack of any usable grouping, therefore don't even show grouping options? otherwise, agree with @felixfbecker no button-group

[felixfbecker]

@yowayb no, lack of a chart can mean that we timed out trying to provide a proactive insight (under a low timeout, to not overload the backend) but the user can still trigger it manually.