Skip to content

Commit 3d398a6

Browse files
ianballouadamruzicka
ianballou
authored and
adamruzicka
committed
Refs #38991 - use PF5 table on dependency tab
1 parent 9603331 commit 3d398a6

File tree

4 files changed

+122
-70
lines changed

4 files changed

+122
-70
lines changed

‎app/views/foreman_tasks/api/tasks/details.json.rabl‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@ node(:depends_on) do
2222
if @task.execution_plan
2323
dynflow_uuids = ForemanTasks.dynflow.world.persistence.find_execution_plan_dependencies(@task.execution_plan.id)
2424
ForemanTasks::Task.where(external_id: dynflow_uuids).map do |task|
25-
{ id: task.id, action: task.action, humanized: task.humanized[:action] }
25+
{
26+
id: task.id,
27+
action: task.action,
28+
humanized: task.humanized[:action],
29+
state: task.state,
30+
result: task.result
31+
}
2632
end
2733
else
2834
[]
@@ -32,7 +38,13 @@ node(:blocks) do
3238
if @task.execution_plan
3339
dynflow_uuids = ForemanTasks.dynflow.world.persistence.find_blocked_execution_plans(@task.execution_plan.id)
3440
ForemanTasks::Task.where(external_id: dynflow_uuids).map do |task|
35-
{ id: task.id, action: task.action, humanized: task.humanized[:action] }
41+
{
42+
id: task.id,
43+
action: task.action,
44+
humanized: task.humanized[:action],
45+
state: task.state,
46+
result: task.result
47+
}
3648
end
3749
else
3850
[]

‎webpack/ForemanTasks/Components/TaskDetails/Components/Dependencies.js‎

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,65 +5,76 @@ import {
55
AlertVariant,
66
Grid,
77
GridItem,
8-
List,
9-
ListItem,
108
Title,
119
} from '@patternfly/react-core';
10+
import { Table, Thead, Tbody, Tr, Th, Td } from '@patternfly/react-table';
1211
import { translate as __ } from 'foremanReact/common/I18n';
1312

14-
const DependencyList = ({ title, tasks }) => {
15-
if (!tasks || tasks.length === 0) {
16-
return (
17-
<div>
18-
<Title headingLevel="h4" size="md">
19-
{title}
20-
</Title>
21-
<p className="text-muted">{__('None')}</p>
22-
</div>
23-
);
24-
}
25-
13+
const DependencyTable = ({ title, tasks }) => {
14+
const tableId = title.toLowerCase().replace(/\s+/g, '-');
2615
return (
2716
<div>
28-
<Title headingLevel="h4" size="md">
17+
<Title headingLevel="h4" size="md" ouiaId={`${tableId}-title`}>
2918
{title}
3019
</Title>
31-
<List isPlain>
32-
{tasks.map((task, index) => (
33-
<ListItem key={index}>
34-
<a href={`/foreman_tasks/tasks/${task.id}`}>
35-
{task.humanized || task.action}
36-
</a>
37-
</ListItem>
38-
))}
39-
</List>
20+
{tasks.length === 0 ? (
21+
<p className="text-muted">{__('None')}</p>
22+
) : (
23+
<Table aria-label={title} variant="compact" ouiaId={`${tableId}-table`}>
24+
<Thead>
25+
<Tr ouiaId={`${tableId}-table-header`}>
26+
<Th width={50}>{__('Action')}</Th>
27+
<Th width={25}>{__('State')}</Th>
28+
<Th width={25}>{__('Result')}</Th>
29+
</Tr>
30+
</Thead>
31+
<Tbody>
32+
{tasks.map(task => (
33+
<Tr key={task.id} ouiaId={`${tableId}-table-row-${task.id}`}>
34+
<Td>
35+
<a href={`/foreman_tasks/tasks/${task.id}`}>
36+
{task.humanized || task.action}
37+
</a>
38+
</Td>
39+
<Td>{task.state}</Td>
40+
<Td>{task.result}</Td>
41+
</Tr>
42+
))}
43+
</Tbody>
44+
</Table>
45+
)}
4046
</div>
4147
);
4248
};
4349

44-
DependencyList.propTypes = {
50+
DependencyTable.propTypes = {
4551
title: PropTypes.string.isRequired,
4652
tasks: PropTypes.array,
4753
};
4854

49-
DependencyList.defaultProps = {
55+
DependencyTable.defaultProps = {
5056
tasks: [],
5157
};
5258

5359
const Dependencies = ({ dependsOn, blocks }) => (
5460
<div>
55-
<Alert variant={AlertVariant.info} isInline title={__('Task dependencies')}>
61+
<Alert
62+
variant={AlertVariant.info}
63+
isInline
64+
title={__('Task dependencies')}
65+
ouiaId="task-dependencies-info-alert"
66+
>
5667
{__(
5768
'This task may have dependencies on other tasks or may be blocking other tasks from executing. Dependencies are established through task chaining relationships.'
5869
)}
5970
</Alert>
6071
<br />
6172
<Grid hasGutter>
6273
<GridItem span={6}>
63-
<DependencyList title={__('Depends on')} tasks={dependsOn} />
74+
<DependencyTable title={__('Depends on')} tasks={dependsOn} />
6475
</GridItem>
6576
<GridItem span={6}>
66-
<DependencyList title={__('Blocks')} tasks={blocks} />
77+
<DependencyTable title={__('Blocks')} tasks={blocks} />
6778
</GridItem>
6879
</Grid>
6980
</div>

‎webpack/ForemanTasks/Components/TaskDetails/Components/__tests__/Dependencies.test.js‎

Lines changed: 65 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,64 +1,92 @@
11
import React from 'react';
2-
import { shallow } from 'enzyme';
2+
import { mount } from 'enzyme';
33
import Dependencies from '../Dependencies';
44

55
describe('Dependencies', () => {
66
it('should render with no dependencies', () => {
7-
const wrapper = shallow(<Dependencies dependsOn={[]} blocks={[]} />);
7+
const wrapper = mount(<Dependencies dependsOn={[]} blocks={[]} />);
88
expect(wrapper.find('Alert')).toHaveLength(1);
9-
expect(wrapper.find('DependencyList')).toHaveLength(2);
9+
expect(wrapper.find('DependencyTable')).toHaveLength(2);
10+
expect(wrapper.find('Table')).toHaveLength(0);
11+
expect(wrapper.text()).toContain('None');
1012
});
1113

1214
it('should render with depends_on dependencies', () => {
1315
const dependsOn = [
14-
{ id: '123', action: 'Actions::FooBar', humanized: 'Foo Bar Action' },
15-
{ id: '456', action: 'Actions::BazQux', humanized: 'Baz Qux Action' },
16+
{
17+
id: '123',
18+
action: 'Actions::FooBar',
19+
humanized: 'Foo Bar Action',
20+
state: 'stopped',
21+
result: 'success',
22+
},
23+
{
24+
id: '456',
25+
action: 'Actions::BazQux',
26+
humanized: 'Baz Qux Action',
27+
state: 'running',
28+
result: 'pending',
29+
},
1630
];
17-
const wrapper = shallow(<Dependencies dependsOn={dependsOn} blocks={[]} />);
18-
expect(
19-
wrapper
20-
.find('DependencyList')
21-
.at(0)
22-
.prop('tasks')
23-
).toEqual(dependsOn);
31+
const wrapper = mount(<Dependencies dependsOn={dependsOn} blocks={[]} />);
32+
expect(wrapper.find('Table')).toHaveLength(1);
33+
expect(wrapper.find('Tbody').find('Tr')).toHaveLength(2);
34+
expect(wrapper.text()).toContain('Foo Bar Action');
35+
expect(wrapper.text()).toContain('Baz Qux Action');
36+
expect(wrapper.text()).toContain('stopped');
37+
expect(wrapper.text()).toContain('success');
2438
});
2539

2640
it('should render with blocks dependencies', () => {
2741
const blocks = [
28-
{ id: '789', action: 'Actions::Test', humanized: 'Test Action' },
42+
{
43+
id: '789',
44+
action: 'Actions::Test',
45+
humanized: 'Test Action',
46+
state: 'paused',
47+
result: 'warning',
48+
},
2949
];
30-
const wrapper = shallow(<Dependencies dependsOn={[]} blocks={blocks} />);
31-
expect(
32-
wrapper
33-
.find('DependencyList')
34-
.at(1)
35-
.prop('tasks')
36-
).toEqual(blocks);
50+
const wrapper = mount(<Dependencies dependsOn={[]} blocks={blocks} />);
51+
expect(wrapper.find('Table')).toHaveLength(1);
52+
expect(wrapper.find('Tbody').find('Tr')).toHaveLength(1);
53+
expect(wrapper.text()).toContain('Test Action');
54+
expect(wrapper.text()).toContain('paused');
55+
expect(wrapper.text()).toContain('warning');
3756
});
3857

3958
it('should render with both dependency types', () => {
4059
const dependsOn = [
41-
{ id: '123', action: 'Actions::Foo', humanized: 'Foo Action' },
60+
{
61+
id: '123',
62+
action: 'Actions::Foo',
63+
humanized: 'Foo Action',
64+
state: 'stopped',
65+
result: 'success',
66+
},
4267
];
4368
const blocks = [
44-
{ id: '456', action: 'Actions::Bar', humanized: 'Bar Action' },
45-
{ id: '789', action: 'Actions::Baz', humanized: 'Baz Action' },
69+
{
70+
id: '456',
71+
action: 'Actions::Bar',
72+
humanized: 'Bar Action',
73+
state: 'running',
74+
result: 'pending',
75+
},
76+
{
77+
id: '789',
78+
action: 'Actions::Baz',
79+
humanized: 'Baz Action',
80+
state: 'stopped',
81+
result: 'error',
82+
},
4683
];
47-
const wrapper = shallow(
84+
const wrapper = mount(
4885
<Dependencies dependsOn={dependsOn} blocks={blocks} />
4986
);
50-
expect(wrapper.find('DependencyList')).toHaveLength(2);
51-
expect(
52-
wrapper
53-
.find('DependencyList')
54-
.at(0)
55-
.prop('tasks')
56-
).toEqual(dependsOn);
57-
expect(
58-
wrapper
59-
.find('DependencyList')
60-
.at(1)
61-
.prop('tasks')
62-
).toEqual(blocks);
87+
expect(wrapper.find('Table')).toHaveLength(2);
88+
expect(wrapper.text()).toContain('Foo Action');
89+
expect(wrapper.text()).toContain('Bar Action');
90+
expect(wrapper.text()).toContain('Baz Action');
6391
});
6492
});

‎webpack/ForemanTasks/Components/TasksTable/Components/ConfirmModal/__test__/index.test.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,16 @@ jest.mock('../../../TasksTableSelectors', () => ({
4444
}));
4545

4646
// Create a mock store
47-
const createMockStore = (initialState = {}) =>
48-
configureStore({
47+
const createMockStore = (initialState = {}) => {
48+
return configureStore({
4949
reducer: {
5050
foremanTasks: (state = initialState, action) => state,
5151
},
5252
preloadedState: {
5353
foremanTasks: initialState,
5454
},
5555
});
56+
};
5657

5758
// Test wrapper component
5859
const TestWrapper = ({ children, store }) => (

0 commit comments

Comments
 (0)