Skip to content

Commit 0bc3a45

Browse files
committed
protect writing localstorage with feature flag
1 parent 4806818 commit 0bc3a45

File tree

4 files changed

+48
-15
lines changed

4 files changed

+48
-15
lines changed

tensorboard/webapp/feature_flag/store/feature_flag_selectors.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -159,4 +159,4 @@ export const getEnableGlobalPins = createSelector(
159159
(flags: FeatureFlags): boolean => {
160160
return flags.enableGlobalPins;
161161
}
162-
);
162+
);

tensorboard/webapp/metrics/effects/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ tf_ts_library(
4545
"//tensorboard/webapp/metrics/actions",
4646
"//tensorboard/webapp/metrics/data_source",
4747
"//tensorboard/webapp/metrics/store",
48+
"//tensorboard/webapp/testing:utils",
4849
"//tensorboard/webapp/types",
4950
"//tensorboard/webapp/util:dom",
5051
"//tensorboard/webapp/webapp_data_source:http_client_testing",

tensorboard/webapp/metrics/effects/index.ts

+24-12
Original file line numberDiff line numberDiff line change
@@ -265,19 +265,31 @@ export class MetricsEffects implements OnInitEffects {
265265

266266
private readonly addOrRemovePin$ = this.actions$.pipe(
267267
ofType(actions.cardPinStateToggled),
268-
withLatestFrom(this.getVisibleCardFetchInfos()),
269-
map(([{cardId, canCreateNewPins, wasPinned}, fetchInfos]) => {
270-
const card = fetchInfos.find((value) => value.id === cardId);
271-
// Saving only scalar pinned cards.
272-
if (!card || card.plugin !== PluginType.SCALARS) {
273-
return;
274-
}
275-
if (wasPinned) {
276-
this.savedPinsDataSource.removeScalarPin(card.tag);
277-
} else if (canCreateNewPins) {
278-
this.savedPinsDataSource.saveScalarPin(card.tag);
268+
withLatestFrom(
269+
this.getVisibleCardFetchInfos(),
270+
this.store.select(selectors.getEnableGlobalPins)
271+
),
272+
map(
273+
([
274+
{cardId, canCreateNewPins, wasPinned},
275+
fetchInfos,
276+
enableGlobalPins,
277+
]) => {
278+
if (!enableGlobalPins) {
279+
return;
280+
}
281+
const card = fetchInfos.find((value) => value.id === cardId);
282+
// Saving only scalar pinned cards.
283+
if (!card || card.plugin !== PluginType.SCALARS) {
284+
return;
285+
}
286+
if (wasPinned) {
287+
this.savedPinsDataSource.removeScalarPin(card.tag);
288+
} else if (canCreateNewPins) {
289+
this.savedPinsDataSource.saveScalarPin(card.tag);
290+
}
279291
}
280-
})
292+
)
281293
);
282294

283295
/**

tensorboard/webapp/metrics/effects/metrics_effects_test.ts

+22-2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
} from '../testing';
5252
import {CardId, TooltipSort} from '../types';
5353
import {CardFetchInfo, MetricsEffects, TEST_ONLY} from './index';
54+
import {buildMockState} from '../../testing/utils';
5455

5556
describe('metrics effects', () => {
5657
let metricsDataSource: MetricsDataSource;
@@ -73,8 +74,10 @@ describe('metrics effects', () => {
7374
MetricsEffects,
7475
provideMockStore({
7576
initialState: {
76-
...appStateFromMetricsState(buildMetricsState()),
77-
...coreTesting.createState(coreTesting.createCoreState()),
77+
...buildMockState({
78+
...appStateFromMetricsState(buildMetricsState()),
79+
...coreTesting.createState(coreTesting.createCoreState()),
80+
}),
7881
},
7982
}),
8083
],
@@ -791,6 +794,7 @@ describe('metrics effects', () => {
791794
saveScalarPinSpy = spyOn(savedPinsDataSource, 'saveScalarPin');
792795
removeScalarPinSpy = spyOn(savedPinsDataSource, 'removeScalarPin');
793796

797+
store.overrideSelector(selectors.getEnableGlobalPins, true);
794798
store.overrideSelector(TEST_ONLY.getCardFetchInfo, {
795799
id: 'card1',
796800
plugin: PluginType.SCALARS,
@@ -882,6 +886,22 @@ describe('metrics effects', () => {
882886
expect(saveScalarPinSpy).not.toHaveBeenCalled();
883887
expect(removeScalarPinSpy).not.toHaveBeenCalled();
884888
});
889+
890+
it('does not pin the card if getEnableGlobalPins is false', () => {
891+
store.overrideSelector(selectors.getEnableGlobalPins, false);
892+
store.refreshState();
893+
894+
actions$.next(
895+
actions.cardPinStateToggled({
896+
cardId: 'card1',
897+
wasPinned: false,
898+
canCreateNewPins: true,
899+
})
900+
);
901+
902+
expect(saveScalarPinSpy).not.toHaveBeenCalled();
903+
expect(removeScalarPinSpy).not.toHaveBeenCalled();
904+
});
885905
});
886906
});
887907
});

0 commit comments

Comments
 (0)