Skip to content

Commit 78f5f83

Browse files
bmd3kbmd3k
authored
Cleanup: Remove remnants of feature flag route. (#6261)
In go/tbpr/5760 we introduced the FLAGS route. The idea was to build a new page within TensorBoard, devoted to setting internal feature flags. We pivoted and instead of having a FLAGS route we have a flags dialog that pops up with the showFlags query parameter, regardless of route. This PR removes the remants of the FLAGS route. it also renames the `feature_flag_page*.ts` files and `FeatureFlagPage*` TS objects to `feature_flag_dialog*.ts` and `FeatureFlagDialog*`, respectively. To Test: Made sure TensorBoard builds and runs and tests pass. Opened TensorBoard with showFlags query parameter and verified the dialog appears and works.
1 parent 4f5598b commit 78f5f83

14 files changed

+50
-59
lines changed

‎tensorboard/webapp/app_routing/internal_utils.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,6 @@ export function getDeepLinkGroup(routeKind: RouteKind): DeepLinkGroup | null {
179179
return DeepLinkGroup.DASHBOARD;
180180
case RouteKind.UNKNOWN:
181181
case RouteKind.NOT_SET:
182-
case RouteKind.FLAGS:
183182
return null;
184183
}
185184
}

‎tensorboard/webapp/app_routing/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ export enum RouteKind {
3535
// Temporary enum values until we can remove special cases in core_effects to
3636
// handle TensorBoard applications with no routes defined.
3737
NOT_SET,
38-
FLAGS,
3938
}
4039

4140
export const DEFAULT_EXPERIMENT_ID = 'defaultExperimentId';

‎tensorboard/webapp/feature_flag/views/BUILD

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ licenses(["notice"])
66

77
tf_sass_binary(
88
name = "style",
9-
src = "feature_flag_page_component.scss",
9+
src = "feature_flag_dialog_component.scss",
1010
strict_deps = False,
1111
deps = [
1212
"//tensorboard/webapp:angular_material_sass_deps",
@@ -21,7 +21,7 @@ tf_ng_module(
2121
"feature_flag_modal_trigger_module.ts",
2222
],
2323
deps = [
24-
":views",
24+
":feature_flag_dialog",
2525
"//tensorboard/webapp:app_state",
2626
"//tensorboard/webapp/angular:expect_angular_material_dialog",
2727
"//tensorboard/webapp/feature_flag/actions",
@@ -34,16 +34,16 @@ tf_ng_module(
3434
)
3535

3636
tf_ng_module(
37-
name = "views",
37+
name = "feature_flag_dialog",
3838
srcs = [
39-
"feature_flag_module.ts",
40-
"feature_flag_page_component.ts",
41-
"feature_flag_page_container.ts",
39+
"feature_flag_dialog_component.ts",
40+
"feature_flag_dialog_container.ts",
41+
"feature_flag_dialog_module.ts",
4242
"types.ts",
4343
],
4444
assets = [
4545
":style",
46-
"feature_flag_page_component.ng.html",
46+
"feature_flag_dialog_component.ng.html",
4747
],
4848
deps = [
4949
"//tensorboard/webapp:app_state",
@@ -65,12 +65,12 @@ tf_ts_library(
6565
name = "views_test",
6666
testonly = True,
6767
srcs = [
68+
"feature_flag_dialog_test.ts",
6869
"feature_flag_modal_trigger_container_test.ts",
69-
"feature_flag_page_test.ts",
7070
],
7171
deps = [
72+
":feature_flag_dialog",
7273
":feature_flag_modal_trigger",
73-
":views",
7474
"//tensorboard/webapp:app_state",
7575
"//tensorboard/webapp/angular:expect_angular_cdk_testing",
7676
"//tensorboard/webapp/angular:expect_angular_cdk_testing_testbed",

‎tensorboard/webapp/feature_flag/views/feature_flag_page_component.ng.html renamed to ‎tensorboard/webapp/feature_flag/views/feature_flag_dialog_component.ng.html

File renamed without changes.

‎tensorboard/webapp/feature_flag/views/feature_flag_page_component.scss renamed to ‎tensorboard/webapp/feature_flag/views/feature_flag_dialog_component.scss

File renamed without changes.

‎tensorboard/webapp/feature_flag/views/feature_flag_page_component.ts renamed to ‎tensorboard/webapp/feature_flag/views/feature_flag_dialog_component.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ import {FeatureFlags} from '../types';
1818
import {FeatureFlagStatus, FeatureFlagStatusEvent} from './types';
1919

2020
@Component({
21-
selector: 'feature-flag-page-component',
22-
styleUrls: ['feature_flag_page_component.css'],
23-
templateUrl: `feature_flag_page_component.ng.html`,
21+
selector: 'feature-flag-dialog-component',
22+
styleUrls: ['feature_flag_dialog_component.css'],
23+
templateUrl: `feature_flag_dialog_component.ng.html`,
2424
})
25-
export class FeatureFlagPageComponent {
25+
export class FeatureFlagDialogComponent {
2626
@Input() featureFlagStatuses!: FeatureFlagStatus<keyof FeatureFlags>[];
2727

2828
@Input() hasFlagsSentToServer: boolean = false;

‎tensorboard/webapp/feature_flag/views/feature_flag_page_container.ts renamed to ‎tensorboard/webapp/feature_flag/views/feature_flag_dialog_container.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,16 @@ import {
4040
} from './types';
4141

4242
@Component({
43-
selector: 'feature-flag-page',
44-
template: `<feature-flag-page-component
43+
selector: 'feature-flag-dialog',
44+
template: `<feature-flag-dialog-component
4545
[featureFlagStatuses]="featureFlags$ | async"
4646
[hasFlagsSentToServer]="hasFlagsSentToServer$ | async"
4747
[showFlagsFilter]="showFlagsFilter$ | async"
4848
(flagChanged)="onFlagChanged($event)"
4949
(allFlagsReset)="onAllFlagsReset()"
50-
></feature-flag-page-component>`,
50+
></feature-flag-dialog-component>`,
5151
})
52-
export class FeatureFlagPageContainer {
52+
export class FeatureFlagDialogContainer {
5353
constructor(private readonly store: Store<State>) {}
5454

5555
readonly showFlagsFilter$ = this.store.select(getOverriddenFeatureFlags).pipe(

‎tensorboard/webapp/feature_flag/views/feature_flag_module.ts renamed to ‎tensorboard/webapp/feature_flag/views/feature_flag_dialog_module.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,16 @@ import {CommonModule} from '@angular/common';
1616
import {NgModule} from '@angular/core';
1717
import {MatButtonModule} from '@angular/material/button';
1818
import {MatSelectModule} from '@angular/material/select';
19-
import {FeatureFlagPageComponent} from './feature_flag_page_component';
20-
import {FeatureFlagPageContainer} from './feature_flag_page_container';
19+
import {FeatureFlagDialogComponent} from './feature_flag_dialog_component';
20+
import {FeatureFlagDialogContainer} from './feature_flag_dialog_container';
2121

2222
/**
23-
* Provides the wrapper component that renders the main dashboard page.
23+
* Provides the wrapper component that renders the main dashboard dialog.
2424
*/
2525
@NgModule({
26-
declarations: [FeatureFlagPageComponent, FeatureFlagPageContainer],
26+
declarations: [FeatureFlagDialogComponent, FeatureFlagDialogContainer],
2727
imports: [CommonModule, MatButtonModule, MatSelectModule],
28-
exports: [FeatureFlagPageContainer],
29-
entryComponents: [FeatureFlagPageContainer],
28+
exports: [FeatureFlagDialogContainer],
29+
entryComponents: [FeatureFlagDialogContainer],
3030
})
31-
export class FeatureFlagPageModule {}
31+
export class FeatureFlagDialogModule {}

‎tensorboard/webapp/feature_flag/views/feature_flag_page_test.ts renamed to ‎tensorboard/webapp/feature_flag/views/feature_flag_dialog_test.ts

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,23 @@ import {
3030
} from '../store/feature_flag_selectors';
3131
import {buildFeatureFlagState, buildState} from '../store/testing';
3232
import {FeatureFlags} from '../types';
33-
import {FeatureFlagPageModule} from './feature_flag_module';
34-
import {FeatureFlagPageComponent} from './feature_flag_page_component';
33+
import {FeatureFlagDialogModule} from './feature_flag_dialog_module';
34+
import {FeatureFlagDialogComponent} from './feature_flag_dialog_component';
3535
import {
36-
FeatureFlagPageContainer,
36+
FeatureFlagDialogContainer,
3737
TEST_ONLY,
38-
} from './feature_flag_page_container';
38+
} from './feature_flag_dialog_container';
3939
import {FeatureFlagOverrideStatus} from './types';
4040

41-
describe('feature_flag_page_container', () => {
41+
describe('feature_flag_dialog_container', () => {
4242
let store: MockStore<State>;
4343
let dispatchSpy: jasmine.Spy;
44-
let fixture: ComponentFixture<FeatureFlagPageContainer>;
44+
let fixture: ComponentFixture<FeatureFlagDialogContainer>;
4545

4646
beforeEach(async () => {
4747
await TestBed.configureTestingModule({
48-
declarations: [FeatureFlagPageContainer],
49-
imports: [CommonModule, FeatureFlagPageModule],
48+
declarations: [FeatureFlagDialogContainer],
49+
imports: [CommonModule, FeatureFlagDialogModule],
5050
providers: [
5151
provideMockStore({
5252
initialState: buildState(
@@ -56,7 +56,7 @@ describe('feature_flag_page_container', () => {
5656
})
5757
),
5858
}),
59-
FeatureFlagPageContainer,
59+
FeatureFlagDialogContainer,
6060
],
6161
schemas: [NO_ERRORS_SCHEMA],
6262
}).compileComponents();
@@ -69,15 +69,15 @@ describe('feature_flag_page_container', () => {
6969
});
7070

7171
function createComponent() {
72-
fixture = TestBed.createComponent(FeatureFlagPageContainer);
72+
fixture = TestBed.createComponent(FeatureFlagDialogContainer);
7373
fixture.detectChanges();
7474
}
7575

7676
function getComponent(): HTMLElement {
77-
return fixture.nativeElement.querySelector('feature-flag-page-component');
77+
return fixture.nativeElement.querySelector('feature-flag-dialog-component');
7878
}
7979

80-
// Tests for feature_flag_page_container
80+
// Tests for feature_flag_dialog_container
8181
describe('onFlagChanged', () => {
8282
it('creates override status is set to enabled or disabled not set', () => {
8383
store.overrideSelector(getDefaultFeatureFlags, {
@@ -155,7 +155,7 @@ describe('feature_flag_page_container', () => {
155155
});
156156
});
157157

158-
// Tests for feature_flag_page_component
158+
// Tests for feature_flag_dialog_component
159159
it('creates rows for each feature flag', () => {
160160
store.overrideSelector(getDefaultFeatureFlags, {
161161
inColab: false,
@@ -192,44 +192,44 @@ describe('feature_flag_page_container', () => {
192192
describe('formatFlagValue', () => {
193193
it('converts true to "Enabled"', () => {
194194
const component = TestBed.createComponent(
195-
FeatureFlagPageComponent
195+
FeatureFlagDialogComponent
196196
).componentInstance;
197197
expect(component.formatFlagValue(true)).toEqual('- Enabled');
198198
});
199199

200200
it('converts false to "Disabled"', () => {
201201
const component = TestBed.createComponent(
202-
FeatureFlagPageComponent
202+
FeatureFlagDialogComponent
203203
).componentInstance;
204204
expect(component.formatFlagValue(false)).toEqual('- Disabled');
205205
});
206206

207207
it('converts null and undefined to "null"', () => {
208208
const component = TestBed.createComponent(
209-
FeatureFlagPageComponent
209+
FeatureFlagDialogComponent
210210
).componentInstance;
211211
expect(component.formatFlagValue(null)).toEqual('- null');
212212
expect(component.formatFlagValue(undefined)).toEqual('- null');
213213
});
214214

215215
it('serializes arrays', () => {
216216
const component = TestBed.createComponent(
217-
FeatureFlagPageComponent
217+
FeatureFlagDialogComponent
218218
).componentInstance;
219219
expect(component.formatFlagValue([])).toEqual('- []');
220220
});
221221

222222
it('serializes numbers and strings', () => {
223223
const component = TestBed.createComponent(
224-
FeatureFlagPageComponent
224+
FeatureFlagDialogComponent
225225
).componentInstance;
226226
expect(component.formatFlagValue(1)).toEqual('- 1');
227227
expect(component.formatFlagValue('foo')).toEqual('- foo');
228228
});
229229

230230
it('does not include hyphen when value has length 0', () => {
231231
const component = TestBed.createComponent(
232-
FeatureFlagPageComponent
232+
FeatureFlagDialogComponent
233233
).componentInstance;
234234
expect(component.formatFlagValue('')).toEqual('');
235235
});

‎tensorboard/webapp/feature_flag/views/feature_flag_modal_trigger_container.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {Store} from '@ngrx/store';
1818
import {State} from '../../app_state';
1919
import {featureFlagOverridesReset} from '../actions/feature_flag_actions';
2020
import {getShowFlagsEnabled} from '../store/feature_flag_selectors';
21-
import {FeatureFlagPageContainer} from './feature_flag_page_container';
21+
import {FeatureFlagDialogContainer} from './feature_flag_dialog_container';
2222

2323
const util = {
2424
reloadWindow: () => {
@@ -33,7 +33,7 @@ const util = {
3333
})
3434
export class FeatureFlagModalTriggerContainer implements OnInit {
3535
readonly showFeatureFlags$ = this.store.select(getShowFlagsEnabled);
36-
private featureFlagsDialog?: MatDialogRef<FeatureFlagPageContainer>;
36+
private featureFlagsDialog?: MatDialogRef<FeatureFlagDialogContainer>;
3737

3838
constructor(
3939
private readonly store: Store<State>,
@@ -43,7 +43,7 @@ export class FeatureFlagModalTriggerContainer implements OnInit {
4343
ngOnInit() {
4444
this.showFeatureFlags$.subscribe((showFeatureFlags: boolean) => {
4545
if (showFeatureFlags) {
46-
this.featureFlagsDialog = this.dialog.open(FeatureFlagPageContainer);
46+
this.featureFlagsDialog = this.dialog.open(FeatureFlagDialogContainer);
4747
this.featureFlagsDialog.afterClosed().subscribe(() => {
4848
// Reset the 'showFlags' flag when the dialog is closed to prevent the
4949
// dialog from appearing again after the page is refreshed.

0 commit comments

Comments
 (0)