Skip to content

Commit

Permalink
Fix Getty AAT QA lookups
Browse files Browse the repository at this point in the history
Fixes #576

Includes:
* Fix a couple dozen typos in the lookup config (`getty_att*` to `getty_aat*`)
* Log info, rather than error, messages to the console when lookups lack defaults. Defaults are not required so these are not errors.
* Avoid creating a new Swagger client on every QA request; instead create the client in the component constructor.
* Increase parity/decrease drift between `InputLookupQA` and `InputListLOC` components by including `id` and `selectHintOnEnter` in the typeahead properties.
* Fix typo in `InputLookupQA` component that was setting the subauthority to the same value as the authority.
  • Loading branch information
mjgiarlo committed May 30, 2019
1 parent 9b24f0b commit f111e37
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 53 deletions.
4 changes: 2 additions & 2 deletions __tests__/components/editor/InputListLOC.test.js
Expand Up @@ -103,11 +103,11 @@ describe('<InputList />', () => {
}
}
}
const errorSpy = jest.spyOn(console, 'error').mockReturnValue(null)
const infoSpy = jest.spyOn(console, 'info').mockReturnValue(null)
const wrapper2 = shallow(<InputList.WrappedComponent {...plProps} handleSelectedChange={mockFormDataFn} />)

expect(wrapper2.state('defaults')).toEqual([])
expect(errorSpy).toBeCalledWith(`no defaults defined in property template: ${JSON.stringify(plProps.propertyTemplate)}`)
expect(infoSpy).toBeCalledWith(`no defaults defined in property template: ${JSON.stringify(plProps.propertyTemplate)}`)
})
})

Expand Down
9 changes: 7 additions & 2 deletions __tests__/components/editor/InputLookupQA.test.js
Expand Up @@ -71,6 +71,11 @@ describe('<InputLookupQA />', () => {
const mockFormDataFn = jest.fn()
const wrapper = shallow(<InputLookupQA.WrappedComponent {...plProps} handleSelectedChange={mockFormDataFn} />)

it('has a lookupClient', () => {
// The Swagger constructor returns a promise
expect(wrapper.instance().lookupClient).toBeInstanceOf(Promise)
})

it('uses the propertyLabel from the template as the form control label', () => {
expect(wrapper.find('#lookupComponent').props().placeholder).toMatch('Name Lookup')
})
Expand Down Expand Up @@ -125,11 +130,11 @@ describe('<InputLookupQA />', () => {
}
}

const errorSpy = jest.spyOn(console, 'error').mockReturnValue(null)
const infoSpy = jest.spyOn(console, 'info').mockReturnValue(null)
const wrapper2 = shallow(<InputLookupQA.WrappedComponent {...plProps} handleSelectedChange={mockFormDataFn} />)

expect(wrapper2.state('defaults')).toEqual([])
expect(errorSpy).toBeCalledWith(`no defaults defined in property template: ${JSON.stringify(plProps.propertyTemplate)}`)
expect(infoSpy).toBeCalledWith(`no defaults defined in property template: ${JSON.stringify(plProps.propertyTemplate)}`)
})

it('sets the async typeahead component defaultSelected attribute', () => {
Expand Down
3 changes: 2 additions & 1 deletion src/components/editor/InputListLOC.jsx
Expand Up @@ -20,7 +20,8 @@ class InputListLOC extends Component {
if (defaults.length > 0) {
this.setPayLoad(defaults)
} else {
console.error(`no defaults defined in property template: ${JSON.stringify(this.props.propertyTemplate)}`)
// Property templates do not require defaults but we like to know when this happens
console.info(`no defaults defined in property template: ${JSON.stringify(this.props.propertyTemplate)}`)
}

this.state = {
Expand Down
24 changes: 12 additions & 12 deletions src/components/editor/InputLookupQA.jsx
Expand Up @@ -17,12 +17,14 @@ class InputLookupQA extends Component {
const defaults = defaultValuesFromPropertyTemplate(this.props.propertyTemplate)

if (defaults.length === 0)
console.error(`no defaults defined in property template: ${JSON.stringify(this.props.propertyTemplate)}`)
// Property templates do not require defaults but we like to know when this happens
console.info(`no defaults defined in property template: ${JSON.stringify(this.props.propertyTemplate)}`)

this.state = {
isLoading: false,
defaults: defaults
}
this.lookupClient = Swagger({ url: 'src/lib/apidoc.json' })
}

// Render menu function to be used by typeahead
Expand Down Expand Up @@ -91,10 +93,12 @@ class InputLookupQA extends Component {
JSON.parse(this.props.propertyTemplate.repeatable)

const typeaheadProps = {
id: 'lookupComponent',
required: isMandatory,
multiple: isRepeatable,
placeholder: this.props.propertyTemplate.propertyLabel,
useCache: true,
selectHintOnEnter: true,
isLoading: this.state.isLoading,
options: this.state.options,
selected: this.state.selected,
Expand All @@ -104,17 +108,15 @@ class InputLookupQA extends Component {

return (
<div>
<AsyncTypeahead id="lookupComponent"

renderMenu={(results, menuProps) => this.renderMenuFunc(results, menuProps)}
<AsyncTypeahead renderMenu={(results, menuProps) => this.renderMenuFunc(results, menuProps)}

onSearch={query => {
this.setState({ isLoading: true })
Swagger({ url: "src/lib/apidoc.json" }).then(client => {
this.lookupClient.then(client => {
//create array of promises based on the lookup config array that is sent in
const lookupPromises = lookupConfigs.map(lookupConfig => {
authority = lookupConfig.authority
subauthority = lookupConfig.authority
subauthority = lookupConfig.subauthority
language = lookupConfig.language
//return the 'promise'
//Since we don't want promise.all to fail if
Expand All @@ -138,13 +140,12 @@ class InputLookupQA extends Component {
})

Promise.all(lookupPromises).then(values => {
let i
for (i = 0; i < values.length; i++) {
for (let i = 0; i < values.length; i++) {
//If undefined, add info - note if error, error object returned in object
//which allows attaching label and uri for authority
if (values[i]) {
values[i]["authLabel"] = lookupConfigs[i].label
values[i]["authURI"] = lookupConfigs[i].uri
values[i].authLabel = lookupConfigs[i].label
values[i].authURI = lookupConfigs[i].uri
}
}

Expand All @@ -163,8 +164,7 @@ class InputLookupQA extends Component {
reduxPath: this.props.reduxPath
}
this.props.handleSelectedChange(payload)
}
}
}}

{...typeaheadProps}

Expand Down

0 comments on commit f111e37

Please sign in to comment.