ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Prevent 404 error in Direct Share tray 

fixes LA-325
flag=direct_share

Test plan
- In the direct share import tray
  select a course and then a module
  for that course
- Then search for a different course
  and select one from the searched list
- Verify that you don't get the "something
  went wrong" picture

Change-Id: I70b7614da7d9003c76bf3ebe3ff1290e4de98575
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/219207
Tested-by: Jenkins
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jon Willesen <jonw+gerrit@instructure.com>
QA-Review: Carl Kibler <ckibler@instructure.com>
Product-Review: Mysti Lilla <mysti@instructure.com>
This commit is contained in:
Mysti Lilla 2019-12-02 17:30:57 -07:00
parent 0dd1a45d95
commit 3849cfad1c
10 changed files with 188 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
app/jsx/content_shares/CourseImportPanel.js
View File

@ -46,7 +46,7 @@ export default function CourseImportPanel({contentShare, onClose, onImport}) {
migration_type: 'canvas_cartridge_importer',
settings: {
content_export_id: contentShare.content_export.id,
insert_into_module_id: selectedModule?.id,
insert_into_module_id: selectedModule?.id || null,
insert_into_module_type: contentShare.content_type,
insert_into_module_position: selectedPosition
}
@ -56,6 +56,11 @@ export default function CourseImportPanel({contentShare, onClose, onImport}) {
onImport(contentShare)
}
function handleSelectedCourse(course) {
setSelectedModule(null)
setSelectedCourse(course)
}
return (
<>
<DirectShareOperationStatus
@ -66,8 +71,8 @@ export default function CourseImportPanel({contentShare, onClose, onImport}) {
/>
<CourseAndModulePicker
selectedCourseId={selectedCourse?.id}
setSelectedCourse={setSelectedCourse}
selectedModuleId={selectedModule?.id}
setSelectedCourse={handleSelectedCourse}
selectedModuleId={selectedModule?.id || null}
setSelectedModule={setSelectedModule}
setModuleItemPosition={setSelectedPosition}
/>

42
app/jsx/content_shares/__tests__/CourseImportPanel.test.js
View File

@ -20,7 +20,9 @@ import React from 'react'
import {render, fireEvent, act} from '@testing-library/react'
import fetchMock from 'fetch-mock'
import useManagedCourseSearchApi from 'jsx/shared/effects/useManagedCourseSearchApi'
import useModuleCourseSearchApi from 'jsx/shared/effects/useModuleCourseSearchApi'
import useModuleCourseSearchApi, {
useCourseModuleItemApi
} from 'jsx/shared/effects/useModuleCourseSearchApi'
import CourseImportPanel from '../CourseImportPanel'
import {mockShare} from 'jsx/content_shares/__tests__/test-utils'
@ -43,7 +45,10 @@ describe('CourseImportPanel', () => {
beforeEach(() => {
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: 'abc', name: 'abc'}, {id: 'cde', name: 'cde'}])
success([
{id: 'abc', name: 'abc'},
{id: 'cde', name: 'cde'}
])
})
})
@ -93,7 +98,10 @@ describe('CourseImportPanel', () => {
workflow_state: 'running'
})
useModuleCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: '1', name: 'Module 1'}, {id: '2', name: 'Module 2'}])
success([
{id: '1', name: 'Module 1'},
{id: '2', name: 'Module 2'}
])
})
const {getByText, getByLabelText, queryByText} = render(
<CourseImportPanel contentShare={share} onImport={onImport} />
@ -121,6 +129,34 @@ describe('CourseImportPanel', () => {
expect(onImport.mock.calls[0][0]).toBe(share)
})
it('deletes the module and removes the position selector when a new course is selected', () => {
const share = mockShare()
useModuleCourseSearchApi.mockImplementationOnce(({success}) => {
success([
{id: '1', name: 'Module 1'},
{id: '2', name: 'Module 2'}
])
})
const {getByText, getByLabelText, queryByText} = render(
<CourseImportPanel contentShare={share} />
)
const courseSelector = getByText(/select a course/i)
fireEvent.click(courseSelector)
fireEvent.click(getByText('abc'))
fireEvent.click(getByText(/select a module/i))
fireEvent.click(getByText(/Module 1/))
expect(getByText(/Position/)).toBeInTheDocument()
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: 'ghi', name: 'foo'}])
})
useCourseModuleItemApi.mockClear()
const input = getByLabelText(/select a course/i)
fireEvent.change(input, {target: {value: 'fo'}})
fireEvent.click(getByText('foo'))
expect(queryByText(/Position/)).not.toBeInTheDocument()
expect(useCourseModuleItemApi).not.toHaveBeenCalled()
})
describe('errors', () => {
beforeEach(() => {
jest.spyOn(console, 'error').mockImplementation()

2
app/jsx/shared/components/ConfirmActionButtonBar.js
View File

@ -19,7 +19,7 @@
import React from 'react'
import {bool, string, func} from 'prop-types'
import {Button} from '@instructure/ui-buttons'
import {Flex} from '@instructure/ui-layout'
import {Flex} from '@instructure/ui-flex'
ConfirmActionButtonBar.propTypes = {
// only buttons with labels will be displayed

22
app/jsx/shared/direct_share/CourseAndModulePicker.js
View File

@ -52,25 +52,25 @@ export default function CourseAndModulePicker({
itemSearchFunction={useManagedCourseSearchApi}
/>
</Flex.Item>
<Flex.Item padding="small">
{selectedCourseId && (
{selectedCourseId && (
<Flex.Item padding="small">
<SearchItemSelector
onItemSelected={setSelectedModule}
renderLabel={I18n.t('Select a Module (optional)')}
itemSearchFunction={useModuleCourseSearchApi}
contextId={selectedCourseId}
contextId={selectedCourseId || null}
/>
)}
</Flex.Item>
<Flex.Item padding="small">
{selectedCourseId && selectedModuleId && (
</Flex.Item>
)}
{selectedCourseId && selectedModuleId && (
<Flex.Item padding="small">
<ModulePositionPicker
courseId={selectedCourseId}
moduleId={selectedModuleId}
courseId={selectedCourseId || null}
moduleId={selectedModuleId || null}
setModuleItemPosition={setModuleItemPosition}
/>
)}
</Flex.Item>
</Flex.Item>
)}
</Flex>
</>
)

11
app/jsx/shared/direct_share/DirectShareCoursePanel.js
View File

@ -52,7 +52,7 @@ export default function DirectShareCoursePanel({sourceCourseId, contentSelection
select: contentSelection,
settings: {
source_course_id: sourceCourseId,
insert_into_module_id: selectedModule?.id,
insert_into_module_id: selectedModule?.id || null,
insert_into_module_type: contentSelection ? Object.keys(contentSelection)[0] : null,
insert_into_module_position: selectedPosition
}
@ -61,6 +61,11 @@ export default function DirectShareCoursePanel({sourceCourseId, contentSelection
)
}
function handleSelectedCourse(course) {
setSelectedModule(null)
setSelectedCourse(course)
}
return (
<>
<DirectShareOperationStatus
@ -71,8 +76,8 @@ export default function DirectShareCoursePanel({sourceCourseId, contentSelection
/>
<CourseAndModulePicker
selectedCourseId={selectedCourse?.id}
setSelectedCourse={setSelectedCourse}
selectedModuleId={selectedModule?.id}
setSelectedCourse={handleSelectedCourse}
selectedModuleId={selectedModule?.id || null}
setSelectedModule={setSelectedModule}
setModuleItemPosition={setSelectedPosition}
/>

76
app/jsx/shared/direct_share/__tests__/CourseAndModulePicker.test.js
View File

@ -17,9 +17,11 @@
*/
import React from 'react'
import {render} from '@testing-library/react'
import {render, fireEvent} from '@testing-library/react'
import useManagedCourseSearchApi from 'jsx/shared/effects/useManagedCourseSearchApi'
import useModuleCourseSearchApi from 'jsx/shared/effects/useModuleCourseSearchApi'
import useModuleCourseSearchApi, {
useCourseModuleItemApi
} from 'jsx/shared/effects/useModuleCourseSearchApi'
import CourseAndModulePicker from '../CourseAndModulePicker'
jest.mock('jsx/shared/effects/useManagedCourseSearchApi')
@ -39,15 +41,73 @@ describe('CourseAndModulePicker', () => {
if (ariaLive) ariaLive.remove()
})
it('enables the module selector when a course is selected', () => {
it('shows course selector by default', () => {
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: 'abc', name: 'abc'}, {id: 'cde', name: 'cde'}])
success([
{id: 'abc', name: 'abc'},
{id: 'cde', name: 'cde'}
])
})
const setCourse = jest.fn()
const {getByText} = render(<CourseAndModulePicker setSelectedCourse={setCourse} />)
const selector = getByText(/select a course/i)
fireEvent.click(selector)
fireEvent.click(getByText('abc'))
expect(setCourse).toHaveBeenLastCalledWith({id: 'abc', name: 'abc'})
})
it('shows the course and module selector when a course is given', () => {
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([
{id: 'abc', name: 'abc'},
{id: 'cde', name: 'cde'}
])
})
useModuleCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: '1', name: 'Module 1'}, {id: '2', name: 'Module 2'}])
success([
{id: '1', name: 'Module 1'},
{id: '2', name: 'Module 2'}
])
})
const {getByText} = render(<CourseAndModulePicker selectedCourseId="abc" />)
expect(getByText(/select a course/i)).toBeInTheDocument()
expect(getByText(/select a module/i)).toBeInTheDocument()
const setModule = jest.fn()
const {getByText} = render(
<CourseAndModulePicker selectedCourseId="abc" setSelectedModule={setModule} />
)
const selector = getByText(/select a module/i)
fireEvent.click(selector)
fireEvent.click(getByText('Module 1'))
expect(setModule).toHaveBeenLastCalledWith({id: '1', name: 'Module 1'})
})
it('shows the position selector when a module is given', () => {
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([
{id: 'abc', name: 'abc'},
{id: 'cde', name: 'cde'}
])
})
useModuleCourseSearchApi.mockImplementationOnce(({success}) => {
success([
{id: '1', name: 'Module 1'},
{id: '2', name: 'Module 2'}
])
})
useCourseModuleItemApi.mockImplementationOnce(({success}) => {
success([
{id: 'a', title: 'Item 1', position: '5'},
{id: 'b', title: 'Item 2', position: '6'}
])
})
const setPosition = jest.fn()
const {getByTestId} = render(
<CourseAndModulePicker
selectedCourseId="abc"
selectedModuleId="1"
setModuleItemPosition={setPosition}
/>
)
const selector = getByTestId('select-position')
fireEvent.change(selector, {target: {value: 'top'}})
expect(setPosition).toHaveBeenLastCalledWith(1)
})
})

39
app/jsx/shared/direct_share/__tests__/DirectShareCoursePanel.test.js
View File

@ -20,9 +20,13 @@ import React from 'react'
import {render, fireEvent, act} from '@testing-library/react'
import fetchMock from 'fetch-mock'
import useManagedCourseSearchApi from 'jsx/shared/effects/useManagedCourseSearchApi'
import useModuleCourseSearchApi, {
useCourseModuleItemApi
} from 'jsx/shared/effects/useModuleCourseSearchApi'
import DirectShareCoursePanel from '../DirectShareCoursePanel'
jest.mock('jsx/shared/effects/useManagedCourseSearchApi')
jest.mock('jsx/shared/effects/useModuleCourseSearchApi')
describe('DirectShareCoursePanel', () => {
let ariaLive
@ -40,7 +44,10 @@ describe('DirectShareCoursePanel', () => {
beforeEach(() => {
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: 'abc', name: 'abc'}, {id: 'cde', name: 'cde'}])
success([
{id: 'abc', name: 'abc'},
{id: 'cde', name: 'cde'}
])
})
})
@ -121,6 +128,36 @@ describe('DirectShareCoursePanel', () => {
expect(getByText('Close')).toBeInTheDocument()
})
it('deletes the module and removes the position selector when a new course is selected', () => {
useModuleCourseSearchApi.mockImplementationOnce(({success}) => {
success([
{id: '1', name: 'Module 1'},
{id: '2', name: 'Module 2'}
])
})
const {getByText, getByLabelText, queryByText} = render(
<DirectShareCoursePanel
sourceCourseId="42"
contentSelection={{discussion_topics: ['1123']}}
/>
)
const courseSelector = getByText(/select a course/i)
fireEvent.click(courseSelector)
fireEvent.click(getByText('abc'))
fireEvent.click(getByText(/select a module/i))
fireEvent.click(getByText(/Module 1/))
expect(getByText(/Position/)).toBeInTheDocument()
useManagedCourseSearchApi.mockImplementationOnce(({success}) => {
success([{id: 'ghi', name: 'foo'}])
})
useCourseModuleItemApi.mockClear()
const input = getByLabelText(/select a course/i)
fireEvent.change(input, {target: {value: 'fo'}})
fireEvent.click(getByText('foo'))
expect(queryByText(/Position/)).not.toBeInTheDocument()
expect(useCourseModuleItemApi).not.toHaveBeenCalled()
})
describe('errors', () => {
beforeEach(() => {
jest.spyOn(console, 'error').mockImplementation()

4
app/jsx/shared/helpers/SelectPosition.js
View File

@ -20,10 +20,10 @@ import React from 'react'
import {string, func, bool, arrayOf, node, shape} from 'prop-types'
import I18n from 'i18n!selectPosition'
import ConnectorIcon from '../../move_item/ConnectorIcon'
import {Text} from '@instructure/ui-elements'
import {Text} from '@instructure/ui-text'
import {FormField} from '@instructure/ui-form-field'
import {View} from '@instructure/ui-layout'
import {ScreenReaderContent} from '@instructure/ui-a11y'
import {ScreenReaderContent} from '@instructure/ui-a11y-content'
import {positions} from '../../move_item/positions'
import {itemShape} from '../../move_item/propTypes'

1
package.json
View File

@ -52,6 +52,7 @@
"@instructure/ui-svg-images": "6",
"@instructure/ui-table": "6",
"@instructure/ui-tabs": "6",
"@instructure/ui-text": "6",
"@instructure/ui-text-input": "6",
"@instructure/ui-themeable": "6",
"@instructure/ui-themes": "6",

12
yarn.lock
View File

@ -2331,6 +2331,18 @@
classnames "^2"
prop-types "^15"
"@instructure/ui-text@6":
version "6.15.0"
resolved "https://registry.yarnpkg.com/@instructure/ui-text/-/ui-text-6.15.0.tgz#0bceef3fb9a2b5640b54d5f00664a37cee4ce4ba"
integrity sha512-KXF73rRC/RkIcryWnHGNnQJi3JcGcGWjJLm0kaDljXCQBS25ypvURg9OCiSYhvyInFB7fLQFP1anAAY2UyPISQ==
dependencies:
"@babel/runtime" "^7.5.0"
"@instructure/console" "^6.15.0"
"@instructure/ui-react-utils" "^6.15.0"
"@instructure/ui-themeable" "^6.15.0"
classnames "^2"
prop-types "^15"
"@instructure/ui-themeable@6", "@instructure/ui-themeable@^6.15.0", "@instructure/ui-themeable@^6.9.0":
version "6.15.0"
resolved "https://registry.yarnpkg.com/@instructure/ui-themeable/-/ui-themeable-6.15.0.tgz#8ef3f08020731385d9df5d0213c10692e3a1cbd3"