diff --git a/tensorboard/webapp/metrics/effects/card_interaction_effects.ts b/tensorboard/webapp/metrics/effects/card_interaction_effects.ts index 1a01380598..53a13c42c5 100644 --- a/tensorboard/webapp/metrics/effects/card_interaction_effects.ts +++ b/tensorboard/webapp/metrics/effects/card_interaction_effects.ts @@ -91,36 +91,38 @@ export class CardInteractionEffects implements OnInitEffects { this.getPreviousCardInteractions$, this.store.select(getCardMetadataMap) ), - tap(([, cardInteractions, previousCardInteractions, metadataMap]) => { - const nextCardInteractions = { - pins: makeUnique([ - ...cardInteractions.pins, - ...previousCardInteractions.pins, - ]), - clicks: makeUnique([ - ...cardInteractions.clicks, - ...previousCardInteractions.clicks, - ]), - tagFilters: Array.from( - new Set([ - ...cardInteractions.tagFilters, - ...previousCardInteractions.tagFilters, - ]) - ), - }; + tap( + ([, newCardInteractions, previousCardInteractions, metadataMap]) => { + const nextCardInteractions = { + pins: makeUnique([ + ...previousCardInteractions.pins, + ...newCardInteractions.pins, + ]), + clicks: makeUnique([ + ...previousCardInteractions.clicks, + ...newCardInteractions.clicks, + ]), + tagFilters: Array.from( + new Set([ + ...previousCardInteractions.tagFilters, + ...newCardInteractions.tagFilters, + ]) + ), + }; - this.store.dispatch( - metricsPreviousCardInteractionsChanged({ - cardInteractions: nextCardInteractions, - }) - ); + this.store.dispatch( + metricsPreviousCardInteractionsChanged({ + cardInteractions: nextCardInteractions, + }) + ); - function makeUnique(cardMetadata: CardIdWithMetadata[]) { - return Array.from( - new Set(cardMetadata.map(({cardId}) => cardId)) - ).map((cardId) => ({...metadataMap[cardId], cardId})); + function makeUnique(cardMetadata: CardIdWithMetadata[]) { + return Array.from( + new Set(cardMetadata.map(({cardId}) => cardId)) + ).map((cardId) => ({...metadataMap[cardId], cardId})); + } } - }) + ) ); }, {dispatch: false} diff --git a/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts b/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts index 5a66e159a3..ee3ea5f3ee 100644 --- a/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts +++ b/tensorboard/webapp/metrics/effects/card_interactions_effects_test.ts @@ -201,9 +201,9 @@ describe('CardInteractions Effects', () => { expect(dispatchedActions).toEqual([ actions.metricsPreviousCardInteractionsChanged({ cardInteractions: { - pins: [card1a, card2a], - clicks: [card1b, card2b], - tagFilters: ['foo', 'bar'], + pins: [card2a, card1a], + clicks: [card2b, card1b], + tagFilters: ['bar', 'foo'], }, }), ]); diff --git a/tensorboard/webapp/metrics/store/metrics_reducers.ts b/tensorboard/webapp/metrics/store/metrics_reducers.ts index 9e1de90e0b..0007ec86ea 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers.ts @@ -775,7 +775,7 @@ const reducer = createReducer( return { ...state, tagFilter, - cardInteractions: nextNewCardInteractions, + newCardInteractions: nextNewCardInteractions, }; }), on(actions.metricsChangeTooltipSort, (state, {sort}) => { @@ -1192,7 +1192,7 @@ const reducer = createReducer( cardToPinnedCopy: nextCardToPinnedCopy, cardToPinnedCopyCache: nextCardToPinnedCopyCache, pinnedCardToOriginal: nextPinnedCardToOriginal, - cardInteractions: nextNewCardInteractions, + newCardInteractions: nextNewCardInteractions, previousCardInteractions: nextPreviousCardInteractions, }; }), @@ -1539,10 +1539,10 @@ const reducer = createReducer( } ), on(actions.metricsCardClicked, (state, {cardId}) => { - const nextClicksIds = new Set( - state.newCardInteractions.clicks.map(({cardId}) => cardId) - ); - nextClicksIds.add(cardId); + const nextClicksIds = state.newCardInteractions.clicks + .map(({cardId}) => cardId) + .filter((id) => id !== cardId) + .concat(cardId); const nextNewCardInteractions = { ...state.newCardInteractions, diff --git a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts index be7ee66410..18f388adfa 100644 --- a/tensorboard/webapp/metrics/store/metrics_reducers_test.ts +++ b/tensorboard/webapp/metrics/store/metrics_reducers_test.ts @@ -28,6 +28,7 @@ import { TagMetadata as DataSourceTagMetadata, } from '../data_source'; import { + appStateFromMetricsState, buildDataSourceTagMetadata, buildMetricsSettingsState, buildMetricsState, @@ -2938,6 +2939,53 @@ describe('metrics reducers', () => { return reducers(beforeState, action); }).toThrow(); }); + + it('adds an entry to newCardInteractions', () => { + const state = buildMetricsState({ + cardMetadataMap: { + card1: { + tag: 'foo', + runId: null, + plugin: PluginType.SCALARS, + }, + }, + }); + const state2 = reducers( + state, + actions.cardPinStateToggled({ + cardId: 'card1', + wasPinned: false, + canCreateNewPins: true, + }) + ); + + expect(state2.newCardInteractions.pins).toEqual([ + { + cardId: 'card1', + plugin: PluginType.SCALARS, + runId: null, + tag: 'foo', + }, + ]); + + const state3 = reducers( + state2, + actions.cardPinStateToggled({ + cardId: 'card1', + wasPinned: false, + canCreateNewPins: true, + }) + ); + + expect(state3.newCardInteractions.pins).toEqual([ + { + cardId: 'card1', + plugin: PluginType.SCALARS, + runId: null, + tag: 'foo', + }, + ]); + }); }); describe('metricsCardStateUpdated', () => { @@ -3042,6 +3090,20 @@ describe('metrics reducers', () => { const nextState = reducers(state, action); expect(nextState.tagFilter).toBe('foobar'); }); + + it('adds entry to newCardInteractions', () => { + const state = buildMetricsState({}); + const state2 = reducers( + state, + actions.metricsTagFilterChanged({tagFilter: 'foo'}) + ); + expect(state2.newCardInteractions.tagFilters).toEqual(['foo']); + const state3 = reducers( + state2, + actions.metricsTagFilterChanged({tagFilter: 'foo'}) + ); + expect(state3.newCardInteractions.tagFilters).toEqual(['foo']); + }); }); describe('metricsTagGroupExpansionChanged', () => { @@ -4717,4 +4779,129 @@ describe('metrics reducers', () => { }); }); }); + + describe('#metricsPreviousCardInteractionsChanged', () => { + it('sets previousCardInteractions', () => { + const cardInteractions = { + pins: [ + { + cardId: 'card1', + runId: null, + plugin: PluginType.SCALARS, + tag: 'bar', + }, + ], + clicks: [ + { + cardId: 'card2', + runId: null, + plugin: PluginType.SCALARS, + tag: 'baz', + }, + ], + tagFilters: ['foo'], + }; + const state = buildMetricsState({}); + const state2 = reducers( + state, + actions.metricsPreviousCardInteractionsChanged({ + cardInteractions, + }) + ); + + expect(state2.previousCardInteractions).toEqual(cardInteractions); + }); + }); + + describe('#metricsCardClicked', () => { + it('adds entry to newCardInteractions', () => { + const state = buildMetricsState({ + cardMetadataMap: { + card1: { + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + card2: { + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + }, + }); + const state2 = reducers( + state, + actions.metricsCardClicked({cardId: 'card1'}) + ); + expect(state2.newCardInteractions.clicks).toEqual([ + { + cardId: 'card1', + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + ]); + const state3 = reducers( + state2, + actions.metricsCardClicked({cardId: 'card2'}) + ); + expect(state3.newCardInteractions.clicks).toEqual([ + { + cardId: 'card1', + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + { + cardId: 'card2', + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + ]); + }); + + it('cannot add duplicate entries to newCardInteractions', () => { + const state = buildMetricsState({ + cardMetadataMap: { + card1: { + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + card2: { + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + }, + }); + const state2 = reducers( + state, + actions.metricsCardClicked({cardId: 'card1'}) + ); + const state3 = reducers( + state2, + actions.metricsCardClicked({cardId: 'card2'}) + ); + const state4 = reducers( + state3, + actions.metricsCardClicked({cardId: 'card1'}) + ); + expect(state4.newCardInteractions.clicks).toEqual([ + { + cardId: 'card2', + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + { + cardId: 'card1', + plugin: PluginType.SCALARS, + tag: 'foo', + runId: null, + }, + ]); + }); + }); });