Repository: aurora Updated Branches: refs/heads/master 73e02c06b -> 53970139d
Refactor Job Page to make it more pluggable Reviewed at https://reviews.apache.org/r/63135/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/53970139 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/53970139 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/53970139 Branch: refs/heads/master Commit: 53970139d2be8901a0831b62d5d02768cb91f81b Parents: 73e02c0 Author: David McLaughlin <[email protected]> Authored: Thu Oct 19 14:37:56 2017 -0700 Committer: David McLaughlin <[email protected]> Committed: Thu Oct 19 14:37:56 2017 -0700 ---------------------------------------------------------------------- ui/src/main/js/components/JobHistory.js | 15 ++++ ui/src/main/js/components/JobOverview.js | 23 ++++++ ui/src/main/js/components/JobStatus.js | 27 +++++++ ui/src/main/js/components/Pagination.js | 6 +- ui/src/main/js/components/Tabs.js | 24 ++++--- .../js/components/__tests__/JobHistory-test.js | 18 +++++ .../js/components/__tests__/JobStatus-test.js | 37 ++++++++++ .../main/js/components/__tests__/Tabs-test.js | 22 +++--- ui/src/main/js/pages/Job.js | 75 +++----------------- ui/src/main/js/pages/__tests__/Job-test.js | 44 ++---------- 10 files changed, 163 insertions(+), 128 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/JobHistory.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/JobHistory.js b/ui/src/main/js/components/JobHistory.js new file mode 100644 index 0000000..9f00a7b --- /dev/null +++ b/ui/src/main/js/components/JobHistory.js @@ -0,0 +1,15 @@ +import React from 'react'; + +import PanelGroup from 'components/Layout'; +import { Tab } from 'components/Tabs'; +import TaskList from 'components/TaskList'; + +import { sort } from 'utils/Common'; +import { getLastEventTime, isActive } from 'utils/Task'; + +export default function ({ tasks }) { + const terminalTasks = sort(tasks.filter((t) => !isActive(t)), (t) => getLastEventTime(t), true); + return (<Tab id='history' name={`Job History (${terminalTasks.length})`}> + <PanelGroup><TaskList tasks={terminalTasks} /></PanelGroup> + </Tab>); +} http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/JobOverview.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/JobOverview.js b/ui/src/main/js/components/JobOverview.js new file mode 100644 index 0000000..4e9debc --- /dev/null +++ b/ui/src/main/js/components/JobOverview.js @@ -0,0 +1,23 @@ +import React from 'react'; + +import JobHistory from 'components/JobHistory'; +import JobStatus from 'components/JobStatus'; +import PanelGroup from 'components/Layout'; +import Loading from 'components/Loading'; +import Tabs from 'components/Tabs'; + +import { isNully } from 'utils/Common'; + +export default function JobOverview(props) { + if (isNully(props.tasks)) { + return <PanelGroup><Loading /></PanelGroup>; + } + + return (<Tabs + activeTab={props.queryParams.jobView} + className='job-overview' + onChange={props.onViewChange}> + {JobStatus(props)} + {JobHistory(props)} + </Tabs>); +} http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/JobStatus.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/JobStatus.js b/ui/src/main/js/components/JobStatus.js new file mode 100644 index 0000000..84e335c --- /dev/null +++ b/ui/src/main/js/components/JobStatus.js @@ -0,0 +1,27 @@ +import React from 'react'; + +import JobConfig from 'components/JobConfig'; +import PanelGroup from 'components/Layout'; +import Tabs, { Tab } from 'components/Tabs'; +import TaskList from 'components/TaskList'; + +import { isNully, sort } from 'utils/Common'; +import { isActive } from 'utils/Task'; + +export default function ({ configGroups, cronJob, onTaskViewChange, queryParams, tasks }) { + const activeTasks = sort(tasks.filter(isActive), (t) => t.assignedTask.instanceId); + const numberConfigs = isNully(cronJob) ? (isNully(configGroups) ? '' : configGroups.length) : 1; + return (<Tab id='status' name='Job Status'> + <PanelGroup> + <Tabs + activeTab={queryParams.taskView} + className='task-status-tabs' + onChange={onTaskViewChange}> + <Tab icon='th-list' id='tasks' name='Tasks'><TaskList tasks={activeTasks} /></Tab> + <Tab icon='info-sign' id='config' name={`Configuration (${numberConfigs})`}> + <JobConfig cronJob={cronJob} groups={configGroups} /> + </Tab> + </Tabs> + </PanelGroup> + </Tab>); +} http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/Pagination.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/Pagination.js b/ui/src/main/js/components/Pagination.js index 094ef2b..0d03fb8 100644 --- a/ui/src/main/js/components/Pagination.js +++ b/ui/src/main/js/components/Pagination.js @@ -95,7 +95,7 @@ export default class Pagination extends React.Component { const numPages = Math.ceil(filtered.length / numberPerPage); // The clickable page list. - const pagination = (numPages === 1 && hideIfSinglePage) ? '' : <PageNavigation + const pagination = (numPages === 1 && hideIfSinglePage) ? null : <PageNavigation currentPage={page} maxPages={maxPages || 8} numPages={Math.ceil(filtered.length / numberPerPage)} @@ -106,7 +106,9 @@ export default class Pagination extends React.Component { if (isTable) { return (<tbody> {elements} - {pagination ? <tr className='pagination-row'><td colSpan='100%'>{pagination}</td></tr> : ''} + {pagination + ? <tr className='pagination-row'><td colSpan='100%'>{pagination}</td></tr> + : null} </tbody>); } return <div>{elements}{pagination}</div>; http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/Tabs.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/Tabs.js b/ui/src/main/js/components/Tabs.js index e382aa7..a48c600 100644 --- a/ui/src/main/js/components/Tabs.js +++ b/ui/src/main/js/components/Tabs.js @@ -4,11 +4,16 @@ import Icon from 'components/Icon'; import { addClass } from 'utils/Common'; +// Wrapping tabs in his component helps simplify testing of Tabs with enzyme's shallow renderer. +export function Tab({ children, icon, id, name }) { + return <div>{children}</div>; +} + export default class Tabs extends React.Component { constructor(props) { super(props); this.state = { - active: props.activeTab || props.tabs[0].id + active: props.activeTab || React.Children.toArray(props.children)[0].props.id }; } @@ -24,17 +29,16 @@ export default class Tabs extends React.Component { const isActive = (t) => t.id === that.state.active; return (<div className={addClass('tabs', this.props.className)}> <ul className='tab-navigation'> - {this.props.tabs.map((t) => ( - <li - className={isActive(t) ? 'active' : ''} - key={t.name} - onClick={(e) => this.select(t)}> - {t.icon ? <Icon name={t.icon} /> : ''} - {t.name} - </li>))} + {React.Children.map(this.props.children, (t) => (<li + className={isActive(t.props) ? 'active' : ''} + key={t.props.name} + onClick={(e) => this.select(t.props)}> + {t.props.icon ? <Icon name={t.props.icon} /> : ''} + {t.props.name} + </li>))} </ul> <div className='active-tab'> - {this.props.tabs.find(isActive).content} + {React.Children.toArray(this.props.children).find((t) => isActive(t.props))} </div> </div>); } http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/__tests__/JobHistory-test.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/__tests__/JobHistory-test.js b/ui/src/main/js/components/__tests__/JobHistory-test.js new file mode 100644 index 0000000..13f7ecc --- /dev/null +++ b/ui/src/main/js/components/__tests__/JobHistory-test.js @@ -0,0 +1,18 @@ +import React from 'react'; +import { shallow } from 'enzyme'; + +import JobHistory from '../JobHistory'; +import TaskList from '../TaskList'; + +import { ScheduledTaskBuilder } from 'test-utils/TaskBuilders'; + +describe('JobHistory', () => { + it('Should render a task list only with terminal tasks', () => { + const tasks = [ + ScheduledTaskBuilder.status(ScheduleStatus.PENDING).build(), + ScheduledTaskBuilder.status(ScheduleStatus.FINISHED).build() + ]; + const el = shallow(JobHistory({tasks})); + expect(el.contains(<TaskList tasks={[tasks[1]]} />)).toBe(true); + }); +}); http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/__tests__/JobStatus-test.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/__tests__/JobStatus-test.js b/ui/src/main/js/components/__tests__/JobStatus-test.js new file mode 100644 index 0000000..78eaab4 --- /dev/null +++ b/ui/src/main/js/components/__tests__/JobStatus-test.js @@ -0,0 +1,37 @@ +import React from 'react'; +import { shallow } from 'enzyme'; + +import JobStatus from '../JobStatus'; +import TaskList from '../TaskList'; +import { Tab } from '../Tabs'; + +import { + ScheduledTaskBuilder, + TaskConfigBuilder, + createConfigGroup +} from 'test-utils/TaskBuilders'; + +describe('JobStatus', () => { + it('Should render a task list only with active tasks', () => { + const tasks = [ + ScheduledTaskBuilder.status(ScheduleStatus.PENDING).build(), + ScheduledTaskBuilder.status(ScheduleStatus.FINISHED).build() + ]; + const el = shallow(JobStatus({queryParams: {}, tasks})); + expect(el.contains(<TaskList tasks={[tasks[0]]} />)).toBe(true); + }); + + it('Should render the correct number of task configs', () => { + const configGroups = [ + createConfigGroup(TaskConfigBuilder, [0, 0]), + createConfigGroup(TaskConfigBuilder, [1, 10]) + ]; + const el = shallow(JobStatus({configGroups, queryParams: {}, tasks: []})); + expect(el.find(Tab).someWhere((t) => t.props().name === 'Configuration (2)')).toBe(true); + }); + + it('Should show one configuration when there is a cron job', () => { + const el = shallow(JobStatus({cronJob: {}, queryParams: {}, tasks: []})); + expect(el.find(Tab).someWhere((t) => t.props().name === 'Configuration (1)')).toBe(true); + }); +}); http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/components/__tests__/Tabs-test.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/components/__tests__/Tabs-test.js b/ui/src/main/js/components/__tests__/Tabs-test.js index 252e5e0..44445c7 100644 --- a/ui/src/main/js/components/__tests__/Tabs-test.js +++ b/ui/src/main/js/components/__tests__/Tabs-test.js @@ -1,39 +1,39 @@ import React from 'react'; import { shallow } from 'enzyme'; -import Tabs from '../Tabs'; +import Tabs, { Tab } from '../Tabs'; const DummyTab = ({ number }) => <span>Hello, {number}</span>; const tabs = [ - {id: 'one', name: 'one', content: <DummyTab number={1} />}, - {id: 'two', name: 'two', content: <DummyTab number={2} />}, - {id: 'three', name: 'three', content: <DummyTab number={3} />} + <Tab id='one' key={1} name='one'><DummyTab number={1} /></Tab>, + <Tab id='two' key={2} name='two'><DummyTab number={2} /></Tab>, + <Tab id='three' key={3} name='three'><DummyTab number={3} /></Tab> ]; describe('Tabs', () => { it('Should set the first tab to active by default', () => { - const el = shallow(<Tabs tabs={tabs} />); + const el = shallow(<Tabs>{tabs}</Tabs>); expect(el.contains(<DummyTab number={1} />)).toBe(true); expect(el.find(DummyTab).length).toBe(1); - expect(el.find('.active').key()).toBe('one'); + expect(el.find('.active').text()).toContain('one'); }); it('Should allow you to specify a default via props', () => { - const el = shallow(<Tabs activeTab='two' tabs={tabs} />); + const el = shallow(<Tabs activeTab='two'>{tabs}</Tabs>); expect(el.contains(<DummyTab number={2} />)).toBe(true); expect(el.find(DummyTab).length).toBe(1); - expect(el.find('.active').key()).toBe('two'); + expect(el.find('.active').text()).toContain('two'); }); it('Should switch tabs on click', () => { - const el = shallow(<Tabs tabs={tabs} />); + const el = shallow(<Tabs>{tabs}</Tabs>); expect(el.contains(<DummyTab number={1} />)).toBe(true); expect(el.find(DummyTab).length).toBe(1); - expect(el.find('.active').key()).toBe('one'); + expect(el.find('.active').text()).toContain('one'); el.find('li').at(2).simulate('click'); expect(el.contains(<DummyTab number={3} />)).toBe(true); - expect(el.find('.active').key()).toBe('three'); + expect(el.find('.active').text()).toContain('three'); }); }); http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/pages/Job.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/pages/Job.js b/ui/src/main/js/pages/Job.js index 070f1e4..591d097 100644 --- a/ui/src/main/js/pages/Job.js +++ b/ui/src/main/js/pages/Job.js @@ -2,23 +2,14 @@ import React from 'react'; import queryString from 'query-string'; import Breadcrumb from 'components/Breadcrumb'; -import JobConfig from 'components/JobConfig'; +import JobOverview from 'components/JobOverview'; import PanelGroup, { Container, StandardPanelTitle } from 'components/Layout'; -import Loading from 'components/Loading'; -import Tabs from 'components/Tabs'; -import TaskList from 'components/TaskList'; import { JobUpdateList } from 'components/UpdateList'; import UpdatePreview from 'components/UpdatePreview'; -import { isNully, sort } from 'utils/Common'; -import { getLastEventTime, isActive } from 'utils/Task'; +import { isNully } from 'utils/Common'; import { isInProgressUpdate } from 'utils/Update'; -export const TASK_CONFIG_TAB = 'config'; -export const TASK_LIST_TAB = 'tasks'; -export const JOB_STATUS_TAB = 'status'; -export const JOB_HISTORY_TAB = 'history'; - export default class Job extends React.Component { constructor(props) { super(props); @@ -115,17 +106,6 @@ export default class Job extends React.Component { </Container>); } - jobHistoryTab() { - const terminalTasks = sort( - this.state.tasks.filter((t) => !isActive(t)), (t) => getLastEventTime(t), true); - - return { - id: JOB_HISTORY_TAB, - name: `Job History (${terminalTasks.length})`, - content: <PanelGroup><TaskList tasks={terminalTasks} /></PanelGroup> - }; - } - setJobView(tabId) { const {match: {params: {role, environment, name}}} = this.props; this.props.history.push(`/beta/scheduler/${role}/${environment}/${name}?jobView=${tabId}`); @@ -136,49 +116,8 @@ export default class Job extends React.Component { this.props.history.push(`/beta/scheduler/${role}/${environment}/${name}?taskView=${tabId}`); } - jobStatusTab() { - const activeTasks = sort(this.state.tasks.filter(isActive), (t) => t.assignedTask.instanceId); - const numberConfigs = isNully(this.state.cronJob) - ? isNully(this.state.configGroups) ? '' : this.state.configGroups.length - : 1; - return { - id: JOB_STATUS_TAB, - name: 'Job Status', - content: (<PanelGroup> - <Tabs - activeTab={queryString.parse(this.props.location.search).taskView} - className='task-status-tabs' - onChange={(t) => this.setTaskView(t.id)} - tabs={[ - { - icon: 'th-list', - id: TASK_LIST_TAB, - name: 'Tasks', - content: <TaskList tasks={activeTasks} /> - }, - { - icon: 'info-sign', - id: TASK_CONFIG_TAB, - name: `Configuration (${numberConfigs})`, - content: <JobConfig cronJob={this.state.cronJob} groups={this.state.configGroups} /> - }]} /> - </PanelGroup>) - }; - } - - jobOverview() { - if (isNully(this.state.tasks)) { - return <PanelGroup><Loading /></PanelGroup>; - } - - return <Tabs - activeTab={queryString.parse(this.props.location.search).jobView} - className='job-overview' - onChange={(t) => this.setJobView(t.id)} - tabs={[this.jobStatusTab(), this.jobHistoryTab()]} />; - } - render() { + const that = this; return (<div className='job-page'> <Breadcrumb cluster={this.state.cluster} @@ -187,7 +126,13 @@ export default class Job extends React.Component { role={this.props.match.params.role} /> {this.updateInProgress()} <Container> - {this.jobOverview()} + <JobOverview + configGroups={this.state.configGroups} + cronJob={this.state.cronJob} + onTaskViewChange={(t) => that.setTaskView(t.id)} + onViewChange={(t) => that.setJobView(t.id)} + queryParams={queryString.parse(this.props.location.search)} + tasks={this.state.tasks} /> </Container> {this.updateHistory()} </div>); http://git-wip-us.apache.org/repos/asf/aurora/blob/53970139/ui/src/main/js/pages/__tests__/Job-test.js ---------------------------------------------------------------------- diff --git a/ui/src/main/js/pages/__tests__/Job-test.js b/ui/src/main/js/pages/__tests__/Job-test.js index 03ef20d..ad302d6 100644 --- a/ui/src/main/js/pages/__tests__/Job-test.js +++ b/ui/src/main/js/pages/__tests__/Job-test.js @@ -4,12 +4,9 @@ import { shallow } from 'enzyme'; import Job from '../Job'; import Breadcrumb from 'components/Breadcrumb'; -import Loading from 'components/Loading'; -import Tabs from 'components/Tabs'; import { JobUpdateList } from 'components/UpdateList'; import UpdatePreview from 'components/UpdatePreview'; -import { ScheduledTaskBuilder } from 'test-utils/TaskBuilders'; import { builderWithStatus } from 'test-utils/UpdateBuilders'; const params = { @@ -41,10 +38,11 @@ describe('Job', () => { }; }; - it('Should render Loading and fire off calls for data', () => { + it('Should fire off calls for data', () => { const api = apiSpy(); - expect(shallow(<Job api={api} match={{params: params}} />) - .contains(<Loading />)).toBe(true); + const apiProps = props(); + apiProps.api = api; + shallow(<Job {...apiProps} />); Object.keys(api).forEach((apiKey) => expect(api[apiKey].mock.calls.length).toBe(1)); }); @@ -85,38 +83,4 @@ describe('Job', () => { const el = shallow(<Job {...props()} updates={updates} />); expect(el.find(JobUpdateList).length).toBe(0); }); - - it('Should render task list with active tasks only on Job Status tab', () => { - const tasks = [ - ScheduledTaskBuilder.status(ScheduleStatus.PENDING).build(), - ScheduledTaskBuilder.status(ScheduleStatus.FINISHED).build() - ]; - const el = shallow(<Job {...props(tasks)} />); - const taskList = el.find(Tabs).props().tabs[0].content.props.children.props.tabs[0].content; - expect(taskList.props.tasks).toEqual([tasks[0]]); - }); - - it('Should render task list with terminal tasks only on Job History tab', () => { - const tasks = [ - ScheduledTaskBuilder.status(ScheduleStatus.PENDING).build(), - ScheduledTaskBuilder.status(ScheduleStatus.FINISHED).build() - ]; - const el = shallow(<Job {...props(tasks)} />); - expect(el.find(Tabs).props().tabs[1].name).toEqual('Job History (1)'); - const taskList = el.find(Tabs).props().tabs[1].content.props.children; - expect(taskList.props.tasks).toEqual([tasks[1]]); - }); - - it('Should set default active tabs based on URL query parameters', () => { - const tasks = [ - ScheduledTaskBuilder.status(ScheduleStatus.PENDING).build(), - ScheduledTaskBuilder.status(ScheduleStatus.FINISHED).build() - ]; - const p = props(tasks); - p.location.search = '?taskView=config&jobView=history'; - const el = shallow(<Job {...p} />); - expect(el.find(Tabs).props().activeTab).toEqual('history'); - const nestedTab = el.find(Tabs).props().tabs[0].content.props.children.props.activeTab; - expect(nestedTab).toEqual('config'); - }); });
