Skip to content

Commit

Permalink
fix: groupBy manual applyMode (#1010)
Browse files Browse the repository at this point in the history
  • Loading branch information
joannaWebDev authored Dec 19, 2024
1 parent eb2f87c commit fc15e94
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
20 changes: 19 additions & 1 deletion packages/scenes/src/variables/groupby/GroupByVariable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { SceneVariableSet } from '../sets/SceneVariableSet';
import userEvent from '@testing-library/user-event';
import { TestContextProvider } from '../../../utils/test/TestContextProvider';
import { FiltersRequestEnricher } from '../../core/types';
import { allActiveGroupByVariables } from './findActiveGroupByVariablesByUid';

// 11.1.2 - will use SafeSerializableSceneObject
// 11.1.1 - will NOT use SafeSerializableSceneObject
Expand Down Expand Up @@ -259,8 +260,25 @@ describe.each(['11.1.2', '11.1.1'])('GroupByVariable', (v) => {
});
});

it('does NOT call addActivationHandler when applyMode is manual', async () => {
allActiveGroupByVariables.clear();

const { variable } = setupTest({
applyMode: 'manual',
});

const addActivationHandlerSpy = jest.spyOn(variable, 'addActivationHandler');

await act(async () => {
await lastValueFrom(variable.validateAndUpdate());
});

expect(addActivationHandlerSpy).not.toHaveBeenCalled();
expect(allActiveGroupByVariables.size).toBe(0);
});

// TODO enable once this repo is using @grafana/[email protected]
it.skip('shows groups and orders according to first occurence of a group item', async () => {
it.skip('shows groups and orders according to first occurrence of a group item', async () => {
const { runRequest } = setupTest({
getTagKeysProvider: async () => ({
replace: true,
Expand Down
10 changes: 6 additions & 4 deletions packages/scenes/src/variables/groupby/GroupByVariable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,13 @@ export class GroupByVariable extends MultiValueVariable<GroupByVariableState> {
noValueOnClear: true,
});

this.addActivationHandler(() => {
allActiveGroupByVariables.add(this);
if (this.state.applyMode === 'auto') {
this.addActivationHandler(() => {
allActiveGroupByVariables.add(this);

return () => allActiveGroupByVariables.delete(this);
});
return () => allActiveGroupByVariables.delete(this);
});
}
}

/**
Expand Down

0 comments on commit fc15e94

Please sign in to comment.