[Global pins] Correct the incorrect name change and add test cases (#6840)

## Motivation for features / changes

This PR fixes the renaming in #6831 and adds test cases to make the test
robust.

## Technical description of changes

* Tried to rename `disableSavingPins` to `removeSavedPinsOnDisable`, but
#6831 accidentally changed `removeAllPins` to
`removeSavedPinsOnDisable`.

## Screenshots of UI changes (or N/A)

## Detailed steps to verify changes work correctly (as executed by you)
* Added test cases + cl TAP presubmit passes
## Alternate designs / implementations considered (or N/A)

Author: roseayeon

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",

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$
       );
     },

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();
+      });
     });
   });
 });