diff --git a/tensorboard/webapp/metrics/BUILD b/tensorboard/webapp/metrics/BUILD index b0e6ce3175..dfc66bd267 100644 --- a/tensorboard/webapp/metrics/BUILD +++ b/tensorboard/webapp/metrics/BUILD @@ -110,6 +110,7 @@ tf_ts_library( "//tensorboard/webapp/angular:expect_angular_core_testing", "//tensorboard/webapp/angular:expect_angular_platform_browser_animations", "//tensorboard/webapp/angular:expect_angular_platform_browser_dynamic_testing", + "//tensorboard/webapp/feature_flag", "//tensorboard/webapp/metrics/store:metrics_initial_state_provider", "//tensorboard/webapp/metrics/store:types", "//tensorboard/webapp/metrics/views", diff --git a/tensorboard/webapp/metrics/effects/index.ts b/tensorboard/webapp/metrics/effects/index.ts index 730a1514e8..c03211c3a9 100644 --- a/tensorboard/webapp/metrics/effects/index.ts +++ b/tensorboard/webapp/metrics/effects/index.ts @@ -333,7 +333,7 @@ export class MetricsEffects implements OnInitEffects { }) ); - private readonly removeSavedPinsOnDisable$ = this.actions$.pipe( + private readonly removeAllPins$ = this.actions$.pipe( ofType(actions.metricsClearAllPinnedCards), withLatestFrom( this.store.select(selectors.getEnableGlobalPins), @@ -356,7 +356,7 @@ export class MetricsEffects implements OnInitEffects { }) ); - private readonly disableSavingPins$ = this.actions$.pipe( + private readonly removeSavedPinsOnDisable$ = this.actions$.pipe( ofType(actions.metricsEnableSavingPinsToggled), withLatestFrom( this.store.select(selectors.getEnableGlobalPins), @@ -423,6 +423,10 @@ export class MetricsEffects implements OnInitEffects { /** * Subscribes to: metricsClearAllPinnedCards. */ + this.removeAllPins$, + /** + * Subscribes to: metricsEnableSavingPinsToggled. + */ this.removeSavedPinsOnDisable$ ); }, diff --git a/tensorboard/webapp/metrics/effects/metrics_effects_test.ts b/tensorboard/webapp/metrics/effects/metrics_effects_test.ts index bf0bb695f3..3c8fb4ca1d 100644 --- a/tensorboard/webapp/metrics/effects/metrics_effects_test.ts +++ b/tensorboard/webapp/metrics/effects/metrics_effects_test.ts @@ -918,6 +918,22 @@ describe('metrics effects', () => { expect(saveScalarPinSpy).not.toHaveBeenCalled(); expect(removeScalarPinSpy).not.toHaveBeenCalled(); }); + + it('does not pin the card if getMetricsSavingPinsEnabled is false', () => { + store.overrideSelector(selectors.getMetricsSavingPinsEnabled, false); + store.refreshState(); + + actions$.next( + actions.cardPinStateToggled({ + cardId: 'card1', + wasPinned: false, + canCreateNewPins: true, + }) + ); + + expect(saveScalarPinSpy).not.toHaveBeenCalled(); + expect(removeScalarPinSpy).not.toHaveBeenCalled(); + }); }); describe('loadSavedPins', () => { @@ -984,6 +1000,19 @@ describe('metrics effects', () => { expect(actualActions).toEqual([]); }); + + it('does not load saved pins if getMetricsSavingPinsEnabled is false', () => { + getSavedScalarPinsSpy = spyOn( + savedPinsDataSource, + 'getSavedScalarPins' + ).and.returnValue(['tagA', 'tagB']); + store.overrideSelector(selectors.getMetricsSavingPinsEnabled, false); + store.refreshState(); + + actions$.next(TEST_ONLY.initAction()); + + expect(actualActions).toEqual([]); + }); }); describe('removeAllPins', () => { @@ -1021,6 +1050,62 @@ describe('metrics effects', () => { expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); }); + + it('does not remove pins if getMetricsSavingPinsEnabled is false', () => { + store.overrideSelector(selectors.getMetricsSavingPinsEnabled, false); + store.refreshState(); + + actions$.next(actions.metricsClearAllPinnedCards()); + + expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); + }); + }); + + describe('removeSavedPinsOnDisable', () => { + let removeAllScalarPinsSpy: jasmine.Spy; + + beforeEach(() => { + removeAllScalarPinsSpy = spyOn( + savedPinsDataSource, + 'removeAllScalarPins' + ); + store.overrideSelector(selectors.getEnableGlobalPins, true); + store.overrideSelector(selectors.getMetricsSavingPinsEnabled, false); + store.refreshState(); + }); + + it('removes all pins by calling metricsEnableSavingPinsToggled method', () => { + actions$.next(actions.metricsEnableSavingPinsToggled()); + + expect(removeAllScalarPinsSpy).toHaveBeenCalled(); + }); + + it('does not remove pins if getEnableGlobalPins is false', () => { + store.overrideSelector(selectors.getEnableGlobalPins, false); + store.refreshState(); + + actions$.next(actions.metricsEnableSavingPinsToggled()); + + expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); + }); + + it('does not remove pins if getShouldPersistSettings is false', () => { + store.overrideSelector(selectors.getShouldPersistSettings, false); + store.refreshState(); + + actions$.next(actions.metricsEnableSavingPinsToggled()); + + expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); + }); + + it('does not remove pins if getMetricsSavingPinsEnabled is true', () => { + store.overrideSelector(selectors.getMetricsSavingPinsEnabled, true); + store.refreshState(); + + actions$.next(actions.metricsEnableSavingPinsToggled()); + + expect(removeAllScalarPinsSpy).not.toHaveBeenCalled(); + }); }); }); });