Skip to content
This repository has been archived by the owner on Mar 23, 2019. It is now read-only.

Bug 1520857- add support in tc-web for displaying information from the lastFire table #413

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
+288 −93
Open

Bug 1520857- add support in tc-web for displaying information from the lastFire table #413

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c57174f
Initial display of lastfire fires
Biboswan Jan 23, 2019
7a33df8
Improve responsiveness and overall ui with Grid
Biboswan Jan 24, 2019
5b5c97d
Remove unwanted class
Biboswan Jan 24, 2019
a154a50
Merge two hook graphql queries into one
Biboswan Jan 25, 2019
f55678d
Replace expansion panel with datatable
Biboswan Jan 25, 2019
c5f5a16
Add pagination
Biboswan Feb 6, 2019
87993e3
minor fix
Biboswan Feb 15, 2019
f98d385
Add toggle view for error
Biboswan Feb 15, 2019
4674382
Add drawer to display error info
Biboswan Feb 22, 2019
0156365
Minor tweak
Biboswan Feb 22, 2019
e238f6f
add link icon for taskids
Biboswan Feb 23, 2019
8bb249f
minor tweak
Biboswan Feb 25, 2019
2b28062
Remove 1-20 part and text rename
Biboswan Mar 13, 2019
aa51ef9
add scrollbar
Biboswan Mar 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 104 additions & 57 deletions src/components/HookForm/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { Component, Fragment } from 'react';
import { Link } from 'react-router-dom';
import { string, bool, func, oneOfType, object } from 'prop-types';
import { string, bool, func, oneOfType, object, array } from 'prop-types';
import classNames from 'classnames';
import { equals, assocPath } from 'ramda';
import cloneDeep from 'lodash.clonedeep';
Expand All @@ -11,6 +11,10 @@ import List from '@material-ui/core/List';
import ListItem from '@material-ui/core/ListItem';
import ListItemText from '@material-ui/core/ListItemText';
import ListSubheader from '@material-ui/core/ListSubheader';
import ExpansionPanel from '@material-ui/core/ExpansionPanel';
import ExpansionPanelDetails from '@material-ui/core/ExpansionPanelDetails';
import ExpansionPanelSummary from '@material-ui/core/ExpansionPanelSummary';
import ExpandMoreIcon from '@material-ui/icons/ExpandMore';
import TextField from '@material-ui/core/TextField';
import Switch from '@material-ui/core/Switch';
import FormGroup from '@material-ui/core/FormGroup';
Expand All @@ -21,15 +25,15 @@ import Typography from '@material-ui/core/Typography';
import FlashIcon from 'mdi-react/FlashIcon';
import PlusIcon from 'mdi-react/PlusIcon';
import DeleteIcon from 'mdi-react/DeleteIcon';
import LinkIcon from 'mdi-react/LinkIcon';
import ContentSaveIcon from 'mdi-react/ContentSaveIcon';
import { docs } from 'taskcluster-lib-urls';
import Label from '@mozilla-frontend-infra/components/Label';
import ErrorPanel from '../ErrorPanel';
import Button from '../Button';
import SpeedDial from '../SpeedDial';
import SpeedDialAction from '../SpeedDialAction';
import DialogAction from '../DialogAction';
import DateDistance from '../DateDistance';
import { HOOKS_LAST_FIRE_TYPE } from '../../utils/constants';
import { hook } from '../../utils/prop-types';
import removeKeys from '../../utils/removeKeys';

Expand Down Expand Up @@ -114,12 +118,16 @@ const initialHook = {
subheader: {
fontSize: theme.typography.pxToRem(16),
},
hookLastFires: {
width: '100%',
},
}))
/** A form to view/edit/create a hook */
export default class HookForm extends Component {
static defaultProps = {
isNewHook: false,
hook: initialHook,
hookLastFires: null,
onTriggerHook: null,
onCreateHook: null,
onUpdateHook: null,
Expand All @@ -131,6 +139,8 @@ export default class HookForm extends Component {
static propTypes = {
/** A GraphQL hook response. Not needed when creating a new hook */
hook: hook.isRequired,
/** Object */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a different comment here to describe the property.

hookLastFires: array,
/** Set to `true` when creating a new hook. */
isNewHook: bool,
/** Callback function fired when a hook is triggered. */
Expand All @@ -153,6 +163,8 @@ export default class HookForm extends Component {

state = {
hook: null,
hookLastFires: null,
lastFirePanelExpanded: '',
// eslint-disable-next-line react/no-unused-state
previousHook: null,
taskInput: '',
Expand All @@ -165,14 +177,18 @@ export default class HookForm extends Component {
};

static getDerivedStateFromProps(props, state) {
if (equals(props.hook, state.previousHook)) {
if (
equals(props.hook, state.previousHook) &&
equals(props.hookLastFires, state.hookLastFires)
) {
return null;
}

const hook = props.isNewHook ? initialHook : props.hook;

return {
hook: props.hook,
hookLastFires: props.hookLastFires,
previousHook: props.hook,
taskInput: JSON.stringify(
removeKeys(cloneDeep(hook.task), ['__typename']),
Expand Down Expand Up @@ -395,6 +411,12 @@ export default class HookForm extends Component {
),
});

handleLastFirePanelChange = panel => (event, expanded) => {
this.setState({
lastFirePanelExpanded: expanded ? panel : false,
});
};

render() {
const {
actionLoading,
Expand All @@ -412,10 +434,11 @@ export default class HookForm extends Component {
triggerSchemaInput,
triggerContextInput,
hook,
hookLastFires,
validation,
lastFirePanelExpanded,
} = this.state;
/* eslint-disable-next-line no-underscore-dangle */
const lastFireTypeName = !isNewHook && hook.status.lastFire.__typename;

return (
<Fragment>
Expand Down Expand Up @@ -508,61 +531,85 @@ export default class HookForm extends Component {
/>
</FormGroup>
</ListItem>
{!isNewHook && (
<Fragment>
<ListItem>
<ListItemText
primary="Last Fired"
secondary={
lastFireTypeName === HOOKS_LAST_FIRE_TYPE.NO_FIRE ? (
'n/a'
) : (
<DateDistance from={hook.status.lastFire.time} />
)
}
/>
</ListItem>
{lastFireTypeName === HOOKS_LAST_FIRE_TYPE.SUCCESSFUL_FIRE ? (
<ListItem
button
component={Link}
className={classes.listItemButton}
to={`/tasks/${hook.status.lastFire.taskId}`}>
<ListItemText
primary="Last Fired Result"
secondary={hook.status.lastFire.taskId}
/>
<LinkIcon />
</ListItem>
) : (
<ListItem>
<ListItemText
primary="Last Fired Result"
secondary={
lastFireTypeName === HOOKS_LAST_FIRE_TYPE.NO_FIRE ? (
'n/a'
) : (
<pre>
{JSON.stringify(hook.status.lastFire.error, null, 2)}
</pre>
)
}
/>
</ListItem>
)}
<ListItem>
<ListItem>
{hookLastFires ? (
<List className={classes.hookLastFires}>
<ListItemText
primary="Next Scheduled Fire"
secondary={
hook.status.nextScheduledDate ? (
<DateDistance from={hook.status.nextScheduledDate} />
) : (
'n/a'
)
primary={
<Typography variant="subtitle1">
Last Hook Fire Attempts
</Typography>
}
/>
</ListItem>
</Fragment>
{hookLastFires.map(
({ taskId, taskCreateTime, firedBy, result, error }) => (
<ListItem key={taskId}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the DataTable component in src/components/DataTable to display the result? The table will be similar to something like http://localhost:5080/provisioners/aws-provisioner-v1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a DataTable would be a good fit for the data. If result is success then expandable panel has nothing to show. If result is error then there is very large error text to show on opening the panel. In any column it could be trouble to fit the error text especially.

<Grid container direction="column">
<Grid
item
container
direction="row"
justify="space-between">
<Grid item>
<pre>{firedBy}</pre>
</Grid>
<Grid item>
<pre>
<DateDistance from={taskCreateTime} />
</pre>
</Grid>
</Grid>
<Grid item>
<ExpansionPanel
expanded={
result === 'ERROR' &&
lastFirePanelExpanded === taskId
}
onChange={this.handleLastFirePanelChange(taskId)}>
<ExpansionPanelSummary
expandIcon={error && <ExpandMoreIcon />}>
<Grid container justify="space-between">
<Grid item>
{(result === 'SUCCESS' && (
<Link to={`/tasks/${taskId}`}>
<Typography>{taskId}</Typography>
</Link>
)) || <Typography>{taskId}</Typography>}
</Grid>
<Grid item>
<Label mini status={result.toLowerCase()}>
{result}
</Label>
</Grid>
</Grid>
</ExpansionPanelSummary>
<ExpansionPanelDetails>
<ErrorPanel error={error} onClose={null} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably have errors be a separate column in the table and use simple text to display the error instead of having it in a error panel. We use error panels for when the server fails to return data or the user is trying to do something not allowed.

</ExpansionPanelDetails>
</ExpansionPanel>
</Grid>
</Grid>
</ListItem>
)
)}
</List>
) : (
''
)}
</ListItem>
{!isNewHook && (
<ListItem>
<ListItemText
primary="Next Scheduled Fire"
secondary={
hook.status.nextScheduledDate ? (
<DateDistance from={hook.status.nextScheduledDate} />
) : (
'n/a'
)
}
/>
</ListItem>
)}
<List>
<ListItem>
Expand Down
9 changes: 9 additions & 0 deletions src/views/Hooks/ViewHook/hookLastFires.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
query HookLastFires($hookGroupId: ID!, $hookId: ID!) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this file. See reason in other comment.

hookLastFires(hookGroupId: $hookGroupId, hookId: $hookId) {
taskId
firedBy
taskCreateTime
result
error
}
}
48 changes: 33 additions & 15 deletions src/views/Hooks/ViewHook/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { hot } from 'react-hot-loader';
import React, { Component, Fragment } from 'react';
import { graphql, withApollo } from 'react-apollo';
import { graphql, withApollo, compose } from 'react-apollo';
import Spinner from '@mozilla-frontend-infra/components/Spinner';
import Dashboard from '../../../components/Dashboard';
import HookForm from '../../../components/HookForm';
Expand All @@ -10,18 +10,32 @@ import createHookQuery from './createHook.graphql';
import deleteHookQuery from './deleteHook.graphql';
import updateHookQuery from './updateHook.graphql';
import triggerHookQuery from './triggerHook.graphql';
import hookLastFiresQuery from './hookLastFires.graphql';

@hot(module)
@withApollo
@graphql(hookQuery, {
skip: ({ match: { params } }) => !params.hookId,
options: ({ match: { params } }) => ({
variables: {
hookGroupId: params.hookGroupId,
hookId: decodeURIComponent(params.hookId),
},
@compose(
graphql(hookLastFiresQuery, {
skip: ({ match: { params } }) => !params.hookId,
options: ({ match: { params } }) => ({
variables: {
hookGroupId: params.hookGroupId,
hookId: decodeURIComponent(params.hookId),
},
}),
name: 'lastFiresData',
}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to create hookLastFires.graphql. You should be able to add everything in hook.graphql.

})
graphql(hookQuery, {
skip: ({ match: { params } }) => !params.hookId,
options: ({ match: { params } }) => ({
variables: {
hookGroupId: params.hookGroupId,
hookId: decodeURIComponent(params.hookId),
},
}),
name: 'hookData',
})
)
export default class ViewHook extends Component {
state = {
actionLoading: false,
Expand Down Expand Up @@ -128,13 +142,13 @@ export default class ViewHook extends Component {
};

render() {
const { isNewHook, data } = this.props;
const { isNewHook, hookData, lastFiresData } = this.props;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One you merge the last fires data in hook.graphql then everything will be under data.

const { error: err, dialogError, actionLoading, dialogOpen } = this.state;
const error = (data && data.error) || err;
const hookDataError = (hookData && hookData.error) || err;

return (
<Dashboard title={isNewHook ? 'Create Hook' : 'Hook'}>
<ErrorPanel error={error} />
<ErrorPanel error={hookDataError} />
{isNewHook ? (
<Fragment>
<HookForm
Expand All @@ -146,12 +160,16 @@ export default class ViewHook extends Component {
</Fragment>
) : (
<Fragment>
{!data.hook && data.loading && <Spinner loading />}
{data.hook && (
{!hookData.hook && hookData.loading && <Spinner loading />}
{hookData.hook && (
<HookForm
dialogError={dialogError}
actionLoading={actionLoading}
hook={data.hook}
hook={hookData.hook}
hookLastFires={lastFiresData.hookLastFires.sort(
(a, b) =>
new Date(b.taskCreateTime) - new Date(a.taskCreateTime)
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract logic from jsx:

const hookLastFires = // ...

// ...
hookLastFires={hookLastFires}

dialogOpen={dialogOpen}
onTriggerHook={this.handleTriggerHook}
onUpdateHook={this.handleUpdateHook}
Expand Down