AdHocFiltersVariable: provide updateFilters method to allow updating filters without emitting SceneVariableValueChangedEvent

[gtk-grafana]

Use case: We have 2 ad-hoc variables at different scopes in the application, an "editing" variable at a lower scope, and a "submitted" variable at a higher scope. Both variables are interpolated by certain query providers. Our un-interpolated expression would look like this: ${AD_HOC_SUBMITTED} ${AD_HOC_EDITING}. The submitted variable is rendered on the top scene, and on every route, but the "editing" variable only exists for a single route. Both variables are rendered in different areas of the UI.

When users are done configuring the lower scope variable, it is submitted, and moved to the higher scope variable. The value in the editing variable is deleted.

However since the SceneVariableValueChangedEvent is emitted whenever an individual ad-hoc variable's expression has changed, this triggers duplicate queries in our application, even though the final interpolated expression doesn't change.

Usage:

// Move filter up
submittedVariable.updateFilters([ {key: 'key', operator: '=', value: 'val'} ], {skipPublish: true});

// Delete lower variable filters
editingVariable.updateFilters([], {
  skipPublish: true
});

Why this change:
Otherwise applications will either need to live with running duplicate queries, extend the base AdHocFiltersVariable class, or convert every query provider that consumes these variables to manual execution.

TL;DR:
This PR will allow the caller of updateFilters to override the default behavior and skip sending the SceneVariableValueChangedEvent, to allow setting the ad-hoc filters state without triggering queries.

Release notes

New AdHocFiltersVariable method updateFilters to allow updating filters state. Allows skipping emit of SceneVariableValueChangedEvent to prevent filter changes from notifying dependent scene objects.

📦 Published PR as canary version: 5.36.0--canary.1004.12431956361.0

✨ Test out this PR locally via:

npm install @grafana/[email protected]
npm install @grafana/[email protected]
# or 
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]

[Sergej-Vlasov]

Wondering if we should adjust updateFilters to:

 public updateFilters(
    update: Partial<AdHocFiltersVariableState>,
    options?: {
      skipPublish?: boolean;
    }
  )....

and then reuse that in the setState so we don't get out of sync on functions and have a single source of truth for updating

  public setState(update: Partial<AdHocFiltersVariableState>): void {
    this.updateFilters(update);
  }

[gtk-grafana]

Good catch, this change is implemented in cdffe34

[dprokop]

I disagree with this. updateFilters should update filters, not entire state of the object. What i think is needed here is to remove the setState overload all together. If someone want's to update filters (and control behavior of such updatr), then we have an API for that. If someone wants to update the state, then they can rely on the base class's setState (without side effect of the event being published). This is a breaking change, hence i suggested adding release notes :)

[gtk-grafana]

Thanks @dprokop, the setState overload has been reverted, but I kept the option to force the publish in the updateFilters.

[dprokop]

LGTM. I think the setState method overload in filters should be completely removed, it currently duplicates the updateFilters, but without the options.

Since we have the updateFilters function, it's this one that should be one for updating that particular state. The setState is inherited by default from SceneObjectBase and it should simply update the state with no effects.

I approve this work to not block it during xmas, but I think we can do the removal in this PR given that we are notifying about this change in the release notes.

[gtk-grafana]

@dprokop if we want to update and use this PR to introduce that breaking change after the holidays that's fine with me, I'm personally not really blocked as I extended the base AdHocFiltersVariable in Explore Logs to add this new updateFilters method until we can upgrade to a version of scenes that supports this functionality.

To make sure I'm understanding your request, the setState method on AdHocFiltersVariable should be completely removed, and everything should be handled in updateFilters? Then updateFilters can call this.setState instead of calling the SceneObjectBase super.setState?

[gtk-grafana]

I lied, I do need this to get unblocked, we can follow this up in another PR.

[scenes-repo-bot-access-token]

🚀 PR was released in v5.36.0 🚀