Skip to content

Commit

Permalink
chore(frontend): Refactor RecurringRunDetails to functional component (
Browse files Browse the repository at this point in the history
…kubeflow#9939)

* Add alternative functional component for recurring run v2 details.

* Remove unnecessary recurringRunDetailsV2FCProps

* Add unit tests.
Move the file to FC folder.
Add updatebanner logic for error case
Simplify the getInitialToolBar() helper.

* Add new feature key "functional" to enable rendering functional
component. (only for validation test now)

* Remove handling error in useQuery.
Change feature flags.
Rename folder.

* Resolve eslint warning

* Avoid use recurringRun and experiment (object) as trigger for
useEffect().

* Remove unused import.

* Extract set() logic from useQuery.
Add documentation for error handle useEffect().
  • Loading branch information
jlyaoyuli authored Aug 30, 2023
1 parent ba2440a commit bc5fe57
Show file tree
Hide file tree
Showing 5 changed files with 639 additions and 3 deletions.
11 changes: 10 additions & 1 deletion frontend/src/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ export interface Feature {
export enum FeatureKey {
V2 = 'v2', // Please start using V2_ALPHA instead of V2, because we have switched to V2_ALPHA as V2 feature is enabled by default.
V2_ALPHA = 'v2_alpha',
FUNCTIONAL_COMPONENT = 'functional_component',
// We plan to refactor the class component to functional component.
// To avoid breacking current behavior, enable this feature to do the bugbash / validation test for functional components.
}

const FEATURE_V2 = {
Expand All @@ -21,7 +24,13 @@ const FEATURE_V2_ALPHA = {
active: true,
};

const features: Feature[] = [FEATURE_V2, FEATURE_V2_ALPHA];
const FEATURE_FUNCTIONAL_COMPONENT = {
name: FeatureKey.FUNCTIONAL_COMPONENT,
description: 'Use functional component',
active: false,
};

const features: Feature[] = [FEATURE_V2, FEATURE_V2_ALPHA, FEATURE_FUNCTIONAL_COMPONENT];

declare global {
var __FEATURE_FLAGS__: string;
Expand Down
8 changes: 7 additions & 1 deletion frontend/src/pages/RecurringRunDetailsRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import * as WorkflowUtils from 'src/lib/v2/WorkflowUtils';
import { PageProps } from './Page';
import RecurringRunDetails from './RecurringRunDetails';
import RecurringRunDetailsV2 from './RecurringRunDetailsV2';
import { RecurringRunDetailsV2FC } from 'src/pages/functional_components/RecurringRunDetailsV2FC';
import { FeatureKey, isFeatureEnabled } from 'src/features';

// This is a router to determine whether to show V1 or V2 recurring run details page.
export default function RecurringRunDetailsRouter(props: PageProps) {
Expand Down Expand Up @@ -76,7 +78,11 @@ export default function RecurringRunDetailsRouter(props: PageProps) {
if (getRecurringRunSuccess && v2RecurringRun && templateString) {
const isV2Pipeline = WorkflowUtils.isPipelineSpec(templateString);
if (isV2Pipeline) {
return <RecurringRunDetailsV2 {...props} />;
return isFeatureEnabled(FeatureKey.FUNCTIONAL_COMPONENT) ? (
<RecurringRunDetailsV2FC {...props} />
) : (
<RecurringRunDetailsV2 {...props} />
);
}
}

Expand Down
4 changes: 3 additions & 1 deletion frontend/src/pages/RecurringRunDetailsV2.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ describe('RecurringRunDetailsV2', () => {
};

jest.clearAllMocks();
jest.spyOn(features, 'isFeatureEnabled').mockReturnValue(true);
jest
.spyOn(features, 'isFeatureEnabled')
.mockImplementation(featureKey => featureKey === features.FeatureKey.V2_ALPHA);

getRecurringRunSpy.mockImplementation(() => fullTestV2RecurringRun);
getPipelineVersionSpy.mockImplementation(() => testPipelineVersion);
Expand Down
Loading

0 comments on commit bc5fe57

Please sign in to comment.