Skip to content
This repository was archived by the owner on Oct 11, 2023. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/actions/incidentActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ const postIncidentFetchArgs = (ticketId, ticketSystem) => {
]
}

export const updateIncidentCreationInput = (input) => ({
export const updateTicketNavigationInput = (input, history) => ({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

history appears to go unused in any calls to updateTicketNavigationInput and within the function body itself. Is it necessary?

type: UPDATE_INCIDENT_CREATION_INPUT,
input
})
Expand Down
10 changes: 6 additions & 4 deletions src/components/Home.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ export const Home = ({ ticket }) => {
if (ticket && ticket.originId) {
return <Redirect to={`/tickets/${ticket.originId}`} />
}
return <Redirect to={'/search'} />
return null
}

export const mapStateToProps = (state) => {
// this gross thing finds all actual tickets, omitting the refresh metadata, and then picks the most recently visited one out
// the sort function works because the dates are UTC and thus lexically sortable to start with
return {
ticket: (state.tickets && state.tickets.map) ? Object.values(state.tickets.map).filter(ticket => ticket && ticket.lastRefresh)
.sort((a, b) => { return (a.lastRefresh > b.lastRefresh) ? -1 : 1 })[0]
: undefined
ticket: (state.tickets && state.tickets.map)
? Object.values(state.tickets.map)
.filter(ticket => ticket && ticket.lastRefresh)
.sort((a, b) => { return (a.lastRefresh > b.lastRefresh) ? -1 : 1 })[0]
: undefined
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,46 +3,54 @@ import PropTypes from 'prop-types'
import { connect } from 'react-redux'
import { TextField } from 'material-ui'
import FlatButtonStyled from 'components/elements/FlatButtonStyled'
import { updateIncidentCreationInput } from 'actions/incidentActions'
import { updateTicketNavigationInput } from 'actions/incidentActions'
import Paper from 'material-ui/Paper'

export const onSubmit = (input, history, dispatch) => () => {
export const onSubmit = (input, history) => () => {
if (input) {
dispatch(updateIncidentCreationInput(''))
history.push(/tickets/ + input)
history.push('/tickets/' + input)
}
}

export const CreateIncident = ({input, creationError, history, dispatch}) => <form
id='incident-search'
export const GoToTicketForm = ({input, creationError, history, dispatch}) => <form
id='incident-navigation'
onSubmit={onSubmit(input, history)}
style={{padding: '16px'}}
>
<TextField
hintText='Ticket Id of primary ticket'
hintText='Ticket Id'
floatingLabelText='Ticket Id'
onChange={(event, newValue) => dispatch(updateIncidentCreationInput(newValue))}
onChange={(event, newValue) => dispatch(updateTicketNavigationInput(newValue))}
value={input}
errorText={creationError}
/>
<FlatButtonStyled
label='Submit'
onTouchTap={onSubmit(input, history, dispatch)}
onTouchTap={onSubmit(input, history)}
/>
</form>

export const mapStateToProps = (state) => {
export const mapStateToProps = (state, ownProps) => {
return {
input: state.incidents.creation.input,
ticketSystem: state.tickets.systems[1],
creationError: state.incidents.creation.error ? state.incidents.creation.error.message : ''
creationError: state.incidents.creation.error ? state.incidents.creation.error.message : '',
...ownProps
}
}

CreateIncident.propTypes = {
GoToTicketForm.propTypes = {
input: PropTypes.string,
creationError: PropTypes.string,
history: PropTypes.object.isRequired,
dispatch: PropTypes.func.isRequired
}

export default connect(mapStateToProps)(CreateIncident)
export const ConnectedGoToTicketForm = connect(mapStateToProps)(GoToTicketForm)

export const GoToTicket = ({history}) => <Paper zDepth={1}>
<span>Go To Ticket:</span>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[This will almost certainly be less relevant given our discussion at the standup]

The <form> tag results in a line break... which makes the form harder to understand and creates an opportunity to evangelize styles.

To get "Go To Ticket" on the same line as the form:

// After the includes, near the top of the file: 
const styles = {
  inlineForm: { display: 'inline', padding: '16px' }
}
...
export const GoToTicketForm = ({input, creationError, history, dispatch}) => <form
  id='incident-navigation'
  onSubmit={onSubmit(input, history)}
  style={styles.inlineForm}      // <-- Access the styles object to apply the styles to the element
>
...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From our conversation earlier and the wireframe up top, Toolbars might give us what we need here: http://www.material-ui.com/#/components/toolbar

<ConnectedGoToTicketForm history={history} />
</Paper>

export default GoToTicket
7 changes: 6 additions & 1 deletion src/components/Incident/Ticket.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Redirect } from 'react-router'
import { Route } from 'react-router-dom'
import PropTypes from 'prop-types'
import React, { Component } from 'react'

import DisplayIncident from 'components/Incident/DisplayIncident'
import LoadingMessage from 'components/elements/LoadingMessage'
import ErrorMessage from 'components/elements/ErrorMessage'
Expand All @@ -26,11 +27,15 @@ class Ticket extends Component {
const { dispatch, incident, ticketId, ticket, ticketSystem, preferences, incidentIsFetching, incidentIsError } = this.props
dispatch(incidentActions.fetchIncidentIfNeeded(incident, ticketId, ticket, ticketSystem, preferences, incidentIsFetching, incidentIsError))
dispatch(fetchEventTypes())
dispatch(incidentActions.updateTicketNavigationInput(''))
}

componentDidUpdate () {
componentDidUpdate (previousProps) {
const { dispatch, incident, ticketId, ticket, ticketSystem, preferences, incidentIsFetching, incidentIsError } = this.props
dispatch(incidentActions.fetchIncidentIfNeeded(incident, ticketId, ticket, ticketSystem, preferences, incidentIsFetching, incidentIsError))
if (previousProps.ticketId !== ticketId) {
dispatch(incidentActions.updateTicketNavigationInput(''))
}
}

render () {
Expand Down
4 changes: 2 additions & 2 deletions src/components/MainComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Router, Route } from 'react-router-dom'
import { PersistGate } from 'redux-persist/lib/integration/react'

import createBrowserHistory from 'history/createBrowserHistory'
import CreateIncident from 'components/Search/CreateIncident'
import GoToTicket from 'components/Incident/GoToTicket'
import Ticket from 'components/Incident/Ticket'
import EnsureLoggedInContainer from 'components/Auth/EnsureLoggedIn'
import incidentRedirect from 'components/Incident/IncidentRedirect'
Expand All @@ -32,9 +32,9 @@ export default class MainComponent extends React.Component {
<div>
{ isChromeExtensionBackground() ? <Notifications /> : null }
<TopNav />
<GoToTicket history={history} />
<Route exact path='/' component={Home} />
<Route exact path='/extension.html' component={Home} />
<Route path='/search' component={CreateIncident} />
<Route path='/tickets/:ticketId' component={Ticket} />
<Route path='/incidents/:incidentId' component={incidentRedirect} />
<Route path='/debug' render={() => <Debug />} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,34 @@
import { expect } from 'chai'
import React from 'react'
import { TextField } from 'material-ui'
import Paper from 'material-ui/Paper'

import { GetMockDispatch, GetDispatchRecorder } from 'test/helpers/mockDispatch'
import { GetMockHistory, GetHistoryRecorder } from 'test/helpers/mockHistory'
import createComponent from 'test/helpers/shallowRenderHelper'

import { CreateIncident, mapStateToProps, onSubmit } from 'components/Search/CreateIncident'
import {
GoToTicketForm,
mapStateToProps,
onSubmit,
ConnectedGoToTicketForm,
GoToTicket
} from 'components/Incident/GoToTicket'
import FlatButtonStyled from 'components/elements/FlatButtonStyled'

const setup = (input, creationError) => {
let props = {
dispatch: () => null,
input,
creationError
}
describe('GoToTicket', function () {
describe('GoToTicketForm render output', function () {
const setup = (input, creationError) => {
let props = {
dispatch: () => null,
input,
creationError
}

return createComponent(CreateIncident, props)
}
return createComponent(GoToTicketForm, props)
}

describe('CreateIncident', function testCreateIncident () {
describe('Rendered component', function () {
beforeEach(function createIncidentInit () {
beforeEach(function () {
this.testError = 'Test Error'
this.testInput = '10'

Expand All @@ -31,7 +38,7 @@ describe('CreateIncident', function testCreateIncident () {
this.withInput = setup(this.testInput, '')
})

it('Should render a form with text field and FlatButtonStyled', function createIncidentRenderForm () {
it('Should render a form with text field and FlatButtonStyled', function () {
expect(this.defaultCase.type).to.equal('form')
expect(this.defaultCase.props.children[0].type).to.equal(TextField)
expect(this.defaultCase.props.children[1].type).to.equal(FlatButtonStyled)
Expand All @@ -43,12 +50,12 @@ describe('CreateIncident', function testCreateIncident () {
expect(this.withInput.props.children[1].type).to.equal(FlatButtonStyled)
})

it('Should pass props.input to TextField value', function createIncidentDisplayInput () {
it('Should pass props.input to TextField value', function () {
expect(this.defaultCase.props.children[0].props.value).to.equal('')
expect(this.withInput.props.children[0].props.value).to.equal(this.testInput)
})

it('Should pass props.creationError to TextField errorText', function createIncidentDisplayCreationError () {
it('Should pass props.creationError to TextField errorText', function () {
expect(this.defaultCase.props.children[0].props.errorText).to.equal('')
expect(this.withError.props.children[0].props.errorText).to.equal(this.testError)
})
Expand All @@ -59,23 +66,13 @@ describe('CreateIncident', function testCreateIncident () {
const input = 'testInput'
const historyRecord = GetHistoryRecorder()
const history = GetMockHistory(historyRecord)
const dispatchRecord = GetDispatchRecorder()
const dispatch = GetMockDispatch(dispatchRecord)

onSubmit(input, history, dispatch)()
onSubmit(input, history)()
describe('History', function () {
it('Should push a new url based on input', function () {
expect(historyRecord[0]).to.equal('/tickets/testInput')
})
})

describe('Dispatched actions', function () {
it('Should dispatch an updateIncidentCreationInput action', function () {
expect(dispatchRecord.action).to.not.be.null
expect(dispatchRecord.action.type).to.equal('UPDATE_INCIDENT_CREATION_INPUT')
expect(dispatchRecord.action.input).to.equal('')
})
})
})

context('When input is not truthy', function () {
Expand All @@ -85,59 +82,74 @@ describe('CreateIncident', function testCreateIncident () {
const dispatchRecord = GetDispatchRecorder()
const dispatch = GetMockDispatch(dispatchRecord)

onSubmit(input, history, dispatch)()
onSubmit(input, history)()
describe('History', function () {
it('Should not be changed', function () {
expect(historyRecord.length).to.equal(0)
})
})

describe('Dispatched actions', function () {
it('Should not have dispatched any actions', function () {
expect(dispatchRecord.action).to.be.undefined
})
})
})
})
})

const inputState = {
tickets: {
map: {
1: {id: 100}
},
systems: {
1: {id: 1},
2: {id: 2}
}
},
incidents: {
creation: {
input: 'test input',
error: {
message: 'test error message'
describe('mapStateToProps', function () {
const inputState = {
tickets: {
map: {
1: {id: 100}
},
systems: {
1: {id: 1},
2: {id: 2}
}
},
incidents: {
creation: {
input: 'test input',
error: {
message: 'test error message'
}
}
}
}
}
}

const expectedResult = {
input: 'test input',
ticketSystem: {
id: 1
},
creationError: 'test error message',
incidentActions: {
TestKey: 'TestValue'
}
}

describe('CreateIncidentMapStateToProps', () => {
it('Should correctly generate an args object from state', () => {

const result = mapStateToProps(inputState)

expect(result.input).to.equal(expectedResult.input)
expect(result.ticketSystem.id).to.equal(expectedResult.ticketSystem.id)
expect(result.creationError).to.equal(expectedResult.creationError)
it('Should correctly generate an args object from state', () => {
const expectedResult = {
input: 'test input',
ticketSystem: {
id: 1
},
creationError: 'test error message',
incidentActions: {
TestKey: 'TestValue'
}
}

expect(result.input).to.equal(expectedResult.input)
expect(result.ticketSystem.id).to.equal(expectedResult.ticketSystem.id)
expect(result.creationError).to.equal(expectedResult.creationError)
})
})

describe('GoToTicket render output', function () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to change anything here... Taking advantage of another soapbox opportunity

These tests are manually verifying aspects of the component but they're not handling scenarios where, for example, a child could be added after result.props.children[1]

const result = createComponent(GoToTicket)

it('Should be a Paper', function () {
expect(result.type).to.equal(Paper)
})

it('Should have a label span as its first child', function () {
expect(result.props.children[0].type).to.equal('span')
})

it('Should have ConnectedGoToTicketForm as its second child', function () {
expect(result.props.children[1].type).to.equal(ConnectedGoToTicketForm)
})
})
})





20 changes: 9 additions & 11 deletions test/components/homeTest.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use strict'
import { expect } from 'chai'
import React from 'react'

import createComponent from 'test/helpers/shallowRenderHelper'
import GetMockStore from 'test/helpers/mockReduxStore'

import { Home, mapStateToProps } from 'components/Home'

const setup = (props, children) => createComponent(Home, props, children)
Expand All @@ -16,17 +17,14 @@ const ticketState = { ticket: {
lastRefresh: '2017-12-29T20:19:32.407Z'
}}

describe('Home Container Component', function test () {
beforeEach(() => {
this.noTicketsExistState = setup(noTicketState, children)
this.ticketsExistState = setup(ticketState, children)
})

it('Should redirect to search if no tickets', () => {
expect(this.noTicketsExistState.props.to).to.equal('/search')
describe('Home Container Component', function () {
it('Should return null if no tickets', function () {
const noTicketsExistResult = setup(noTicketState, children)
expect(noTicketsExistResult).to.be.null
})
it('Should redirect to most recent ticket if there are tickets', () => {
expect(this.ticketsExistState.props.to).to.equal('/tickets/5507')
it('Should redirect to most recent ticket if there are tickets', function () {
const ticketsExistResult = setup(ticketState, children)
expect(ticketsExistResult.props.to).to.equal('/tickets/5507')
})
})

Expand Down
Loading