Update RHS after creating outcome

closes OUT-4687

flag=improved_outcomes_management

test plan:
NOTE: Make sure the Course you choose have less than 8 outcomes
  associated to the root group. Once the outcome is created,
  it will trigger a refetch and the outcome will be added to the
  selected group (in this case the root level group) and in the
  proper order.  If the created outcome's group & title causes
  the outcome to be listed at the bottom of the list, you will need
  to use the scrolling feature to locate it.  If you search for the
  outcome, the search causing its own refetch which is different from
  what is outlined in this PS.
- Turn on FF
- Navigation to a Course -> Outcomes
- Do not click on a group from the LHS
- Click on the Create button
- Create an outcome by filling out the form and selecting
  the root group
- Click save
- Notice the outcome is not shown in the RHS.
- Refresh the page.
- Click on the root group from the LHS
- Click on the Create button again
- Create another outcome by filling out the form and selecting
  the root group
- Click save
- Notice that newly created outcome is in the list of outcomes in
  the RHS.
- Add a few groups including nested groups.
- Click on a parent group of a nested group.
- Click on the create button and create an outcome. This time
  selecting a nested group of the parent.
- Click save.
- Notice the newly created outcome is in the list of outcomes in
  the RHS.
- Conduct the same tests in an Account

Change-Id: Ifbd746a2ba08990b8b30e853d3a439c18e6e031c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/271816
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Martin Yosifov <martin.yosifov@instructure.com>
QA-Review: Martin Yosifov <martin.yosifov@instructure.com>
Product-Review: Ben Friedman <ben.friedman@instructure.com>
This commit is contained in:
Chrystal Langston 2021-08-18 09:21:32 -04:00
parent 334df70ff4
commit f25f3df70f
11 changed files with 273 additions and 30 deletions

View File

@ -42,7 +42,7 @@ import useCanvasContext from '@canvas/outcomes/react/hooks/useCanvasContext'
import {useMutation} from 'react-apollo'
import OutcomesRceField from './shared/OutcomesRceField'
const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
const CreateOutcomeModal = ({isOpen, onCloseHandler, onSuccess}) => {
const {contextType, contextId, friendlyDescriptionFF, isMobileView} = useCanvasContext()
const [title, titleChangeHandler] = useInput()
const [displayName, displayNameChangeHandler] = useInput()
@ -51,14 +51,16 @@ const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
const [showTitleError, setShowTitleError] = useState(false)
const [setOutcomeFriendlyDescription] = useMutation(SET_OUTCOME_FRIENDLY_DESCRIPTION_MUTATION)
const [createLearningOutcome] = useMutation(CREATE_LEARNING_OUTCOME)
const {rootId, collections} = useManageOutcomes('OutcomeManagementPanel')
const [targetGroup, setTargetGroup] = useState(null)
const {rootId, collections, addNewGroup} = useManageOutcomes('OutcomeManagementPanel')
const [selectedGroup, setSelectedGroup] = useState(null)
const [selectedGroupAncestorIds, setSelectedGroupAncestorIds] = useState([])
useEffect(() => {
if (rootId && collections[rootId] && !targetGroup) {
setTargetGroup(collections[rootId])
if (rootId && collections[rootId] && !selectedGroup) {
setSelectedGroup(collections[rootId])
setSelectedGroupAncestorIds([rootId])
}
}, [collections, rootId, targetGroup])
}, [collections, rootId, selectedGroup, selectedGroupAncestorIds])
const invalidTitle = titleValidator(title)
const invalidDisplayName = displayNameValidator(displayName)
@ -75,17 +77,13 @@ const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
onCloseHandler()
}
const handleSetTargetGroup = ({targetGroup}) => {
setTargetGroup(targetGroup)
}
const onCreateOutcomeHandler = () => {
;(async () => {
try {
const createLearningOutcomeResult = await createLearningOutcome({
variables: {
input: {
groupId: targetGroup.id,
groupId: selectedGroup.id,
title,
displayName,
description
@ -112,6 +110,11 @@ const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
}
})
}
onSuccess({selectedGroupAncestorIds})
// resetting selectedGroup to null otherwise it will be maintained
// and will cause the group to not be loaded in the GroupSelectedDrillDown
// when opening the create modal again
setSelectedGroup(null)
showFlashAlert({
message: I18n.t('Outcome "%{title}" was successfully created.', {title}),
@ -206,7 +209,15 @@ const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
<Text size="medium" weight="bold">
{isMobileView ? I18n.t('Select a location') : I18n.t('Location')}
</Text>
<TargetGroupSelector setTargetGroup={handleSetTargetGroup} />
<TargetGroupSelector
groupId={selectedGroup?.id}
setTargetGroup={({targetGroup, targetAncestorsIds}) => {
setSelectedGroupAncestorIds(targetAncestorsIds)
setSelectedGroup(targetGroup)
}}
onGroupCreated={addNewGroup}
modalName="CreateOutcomeModal"
/>
</View>
</Modal.Body>
<Modal.Footer>
@ -218,7 +229,7 @@ const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
color="primary"
margin="0 x-small 0 0"
interaction={
!invalidTitle && !invalidDisplayName && targetGroup ? 'enabled' : 'disabled'
!invalidTitle && !invalidDisplayName && selectedGroup ? 'enabled' : 'disabled'
}
onClick={onCreateOutcomeHandler}
>
@ -230,9 +241,14 @@ const CreateOutcomeModal = ({isOpen, onCloseHandler}) => {
)
}
CreateOutcomeModal.defaultProps = {
onSuccess: () => {}
}
CreateOutcomeModal.propTypes = {
isOpen: PropTypes.bool.isRequired,
onCloseHandler: PropTypes.func.isRequired
onCloseHandler: PropTypes.func.isRequired,
onSuccess: PropTypes.func
}
export default CreateOutcomeModal

View File

@ -87,10 +87,11 @@ describe('OutcomeManagementPanel', () => {
contextType = 'Account',
contextId = '1',
canManage = true,
mocks = accountMocks({childGroupsCount: 0})
mocks = accountMocks({childGroupsCount: 0}),
renderer = rtlRender
} = {}
) => {
return rtlRender(
return renderer(
<OutcomesContext.Provider
value={{
env: {contextType, contextId, canManage, isMobileView, rootIds: [ACCOUNT_GROUP_ID]}
@ -936,6 +937,44 @@ describe('OutcomeManagementPanel', () => {
})
})
describe('After an outcome is added', () => {
const createdOutcomeProps = (props = {}, ancestorIds = ['2']) => ({
createdOutcomeGroupIds: ancestorIds,
...props
})
it('it does not trigger a refetch if an outcome is created but not in the currently selected group', async () => {
const {getByText, queryByText, rerender} = render(<OutcomeManagementPanel />, {
...groupDetailDefaultProps
})
await act(async () => jest.runOnlyPendingTimers())
fireEvent.click(getByText('Course folder 0'))
await act(async () => jest.runOnlyPendingTimers())
render(<OutcomeManagementPanel {...createdOutcomeProps()} />, {
...groupDetailDefaultProps,
renderer: rerender
})
await act(async () => jest.runOnlyPendingTimers())
expect(queryByText(/Newly Created Outcome/)).not.toBeInTheDocument()
})
it('it does trigger a refetch if an outcome is created is in currently selected group in RHS', async () => {
const {getByText, queryByText, rerender} = render(<OutcomeManagementPanel />, {
...groupDetailDefaultProps
})
await act(async () => jest.runOnlyPendingTimers())
fireEvent.click(getByText('Course folder 0'))
await act(async () => jest.runOnlyPendingTimers())
render(<OutcomeManagementPanel {...createdOutcomeProps({}, ['200'])} />, {
...groupDetailDefaultProps,
renderer: rerender
})
await act(async () => jest.runOnlyPendingTimers())
expect(queryByText(/Newly Created Outcome/)).toBeInTheDocument()
})
})
describe('mobile', () => {
beforeEach(() => {
isMobileView = true

View File

@ -17,6 +17,7 @@
*/
import React, {useCallback, useState, useEffect} from 'react'
import PropTypes from 'prop-types'
import {Flex} from '@instructure/ui-flex'
import {Spinner} from '@instructure/ui-spinner'
import {Text} from '@instructure/ui-text'
@ -41,7 +42,7 @@ import OutcomeMoveModal from './OutcomeMoveModal'
import ManageOutcomesBillboard from './ManageOutcomesBillboard'
import GroupActionDrillDown from '../shared/GroupActionDrillDown'
const OutcomeManagementPanel = ({importNumber}) => {
const OutcomeManagementPanel = ({importNumber, createdOutcomeGroupIds}) => {
const {isCourse, isMobileView, canManage} = useCanvasContext()
const {setContainerRef, setLeftColumnRef, setDelimiterRef, setRightColumnRef, onKeyDownHandler} =
useResize()
@ -75,7 +76,8 @@ const OutcomeManagementPanel = ({importNumber}) => {
const {group, loading, loadMore, removeLearningOutcomes, readLearningOutcomes} = useGroupDetail({
id: selectedGroupId,
searchString: debouncedSearchString
searchString: debouncedSearchString,
rhsGroupIdsToRefetch: createdOutcomeGroupIds
})
const selectedOutcomes = readLearningOutcomes(selectedOutcomeIds)
@ -421,4 +423,13 @@ const OutcomeManagementPanel = ({importNumber}) => {
)
}
OutcomeManagementPanel.defaultProps = {
createdOutcomeGroupIds: []
}
OutcomeManagementPanel.propTypes = {
createdOutcomeGroupIds: PropTypes.arrayOf(PropTypes.string),
importNumber: PropTypes.number
}
export default OutcomeManagementPanel

View File

@ -45,8 +45,9 @@ export const groupCollectionShape = PropTypes.shape({
export const outcomeEdgesNodeShape = PropTypes.shape({
_id: PropTypes.string.isRequired,
title: PropTypes.string.isRequired,
isImported: PropTypes.bool.isRequired,
description: PropTypes.string
isImported: PropTypes.bool,
description: PropTypes.string,
displayName: PropTypes.string
})
export const outcomeEdgeShape = PropTypes.shape({

View File

@ -34,7 +34,7 @@ import CreateOutcomeModal from './CreateOutcomeModal'
import useCanvasContext from '@canvas/outcomes/react/hooks/useCanvasContext'
import useModal from '@canvas/outcomes/react/hooks/useModal'
const ManagementHeader = ({handleFileDrop, handleAddOutcomes}) => {
const ManagementHeader = ({handleFileDrop, handleAddOutcomes, onSuccessfulCreateOutcome}) => {
const [isFindOutcomeModalOpen, openFindOutcomeModal, closeFindOutcomeModal] = useModal()
const [isCreateOutcomeModalOpen, openCreateOutcomeModal, closeCreateOutcomeModal] = useModal()
const {isMobileView, canManage, canImport} = useCanvasContext()
@ -112,14 +112,20 @@ const ManagementHeader = ({handleFileDrop, handleAddOutcomes}) => {
<CreateOutcomeModal
isOpen={isCreateOutcomeModalOpen}
onCloseHandler={closeCreateOutcomeModal}
onSuccess={onSuccessfulCreateOutcome}
/>
</div>
)
}
ManagementHeader.defaultProps = {
onSuccessfulCreateOutcome: () => {}
}
ManagementHeader.propTypes = {
handleFileDrop: PropTypes.func.isRequired,
handleAddOutcomes: PropTypes.func.isRequired
handleAddOutcomes: PropTypes.func.isRequired,
onSuccessfulCreateOutcome: PropTypes.func
}
export default ManagementHeader

View File

@ -54,6 +54,7 @@ export const OutcomeManagementWithoutGraphql = ({breakpoints}) => {
const [importRef, setImportRef] = useState(null)
const [importNumber, setImportNumber] = useState(0)
const [isImporting, setIsImporting] = useState(false)
const [createdOutcomeGroupIds, setCreatedOutcomeGroupIds] = useState([])
const [selectedIndex, setSelectedIndex] = useState(() => {
const tabs = {'#mastery_scale': 1, '#mastery_calculation': 2}
return window.location.hash in tabs ? tabs[window.location.hash] : 0
@ -149,6 +150,10 @@ export const OutcomeManagementWithoutGraphql = ({breakpoints}) => {
})
}
const onSuccessfulCreateOutcome = ({selectedGroupAncestorIds}) => {
setCreatedOutcomeGroupIds(selectedGroupAncestorIds)
}
const isMobileView = !breakpoints?.tablet
const onAddOutcomes = addedOutcomes => {
@ -160,7 +165,11 @@ export const OutcomeManagementWithoutGraphql = ({breakpoints}) => {
return (
<OutcomesContext.Provider value={getContext(isMobileView)}>
{improvedManagement && (
<ManagementHeader handleAddOutcomes={onAddOutcomes} handleFileDrop={onFileDrop} />
<ManagementHeader
handleFileDrop={onFileDrop}
handleAddOutcomes={onAddOutcomes}
onSuccessfulCreateOutcome={onSuccessfulCreateOutcome}
/>
)}
<Tabs onRequestTabChange={handleTabChange}>
<Tabs.Panel
@ -170,7 +179,12 @@ export const OutcomeManagementWithoutGraphql = ({breakpoints}) => {
id="management"
>
{improvedManagement ? (
!isImporting && <OutcomeManagementPanel importNumber={importNumber} />
!isImporting && (
<OutcomeManagementPanel
importNumber={importNumber}
createdOutcomeGroupIds={createdOutcomeGroupIds}
/>
)
) : (
<OutcomePanel />
)}

View File

@ -35,10 +35,12 @@ jest.useFakeTimers()
describe('CreateOutcomeModal', () => {
let onCloseHandlerMock
let onSuccessMock
let cache
const defaultProps = (props = {}) => ({
isOpen: true,
onCloseHandler: onCloseHandlerMock,
onSuccess: onSuccessMock,
...props
})
@ -65,6 +67,7 @@ describe('CreateOutcomeModal', () => {
beforeEach(() => {
onCloseHandlerMock = jest.fn()
onSuccessMock = jest.fn()
cache = createCache()
})
@ -142,7 +145,7 @@ describe('CreateOutcomeModal', () => {
expect(getByText('Create').closest('button')).toHaveAttribute('disabled')
})
it('calls onCloseHandler on Create button click', async () => {
it('calls onCloseHandler & onSuccess on Create button click', async () => {
const {getByLabelText, getByText} = render(<CreateOutcomeModal {...defaultProps()} />, {
mocks: [...smallOutcomeTree('Account')]
})
@ -173,6 +176,38 @@ describe('CreateOutcomeModal', () => {
expect(within(getByRole('dialog')).getByText('Create').closest('button')).not.toBeDisabled()
})
it('calls onSuccess if create request succeeds', async () => {
const {getByText, getByLabelText} = render(<CreateOutcomeModal {...defaultProps()} />, {
mocks: [
...smallOutcomeTree('Account'),
setFriendlyDescriptionOutcomeMock({
inputDescription: 'Friendly Description value'
}),
createLearningOutcomeMock({
title: 'Outcome 123',
displayName: 'Display name',
description: '',
groupId: '100'
})
]
})
await act(async () => jest.runOnlyPendingTimers())
fireEvent.change(getByLabelText('Name'), {target: {value: 'Outcome 123'}})
fireEvent.change(getByLabelText('Friendly Name'), {target: {value: 'Display name'}})
fireEvent.change(getByLabelText('Friendly description (for parent/student display)'), {
target: {value: 'Friendly Description value'}
})
fireEvent.click(getByText('Account folder 0'))
fireEvent.click(getByText('Create'))
await act(async () => jest.runOnlyPendingTimers())
await waitFor(() => {
expect(onSuccessMock).toHaveBeenCalledTimes(1)
expect(onSuccessMock).toHaveBeenCalledWith({
selectedGroupAncestorIds: ['100', '1']
})
})
})
it('displays flash confirmation with proper message if create request succeeds', async () => {
const showFlashAlertSpy = jest.spyOn(FlashAlert, 'showFlashAlert')
const {getByText, getByLabelText} = render(<CreateOutcomeModal {...defaultProps()} />, {

View File

@ -56,6 +56,7 @@ describe('ManagementHeader', () => {
})
afterEach(() => {
showImportOutcomesModal.mockRestore()
jest.clearAllMocks()
})

View File

@ -519,7 +519,92 @@ export const groupDetailMocks = ({
__typename: 'LearningOutcomeGroup'
}
}
}
},
// for testing graphqls refetch in index.js
newData: jest.fn(() => ({
data: {
group: {
_id: groupId,
description: `${groupDescription} 4`,
title: `Refetched ${title}`,
outcomesCount: 3,
outcomes: {
pageInfo: {
hasNextPage: withMorePage,
endCursor: 'Mx',
__typename: 'PageInfo'
},
edges: [
{
canUnlink,
_id: '1',
node: {
_id: '1',
description: '',
title: `Refetched Outcome 1 - ${title}`,
displayName: '',
canEdit,
contextId,
contextType,
friendlyDescription: null,
__typename: 'LearningOutcome'
},
group: {
_id: groupId,
title: `Refetched ${title}`,
__typename: 'LearningOutcomeGroup'
},
__typename: 'ContentTag'
},
{
canUnlink,
_id: '2',
node: {
_id: '2',
description: '',
title: `Refetched Outcome 2 - ${title}`,
displayName: '',
canEdit,
contextId,
contextType,
friendlyDescription: null,
__typename: 'LearningOutcome'
},
group: {
_id: groupId,
title: `Refetched ${title}`,
__typename: 'LearningOutcomeGroup'
},
__typename: 'ContentTag'
},
{
canUnlink,
_id: '11',
node: {
_id: '11',
description: '',
title: `Newly Created Outcome - ${title}`,
displayName: '',
canEdit,
contextId,
contextType,
friendlyDescription: null,
__typename: 'LearningOutcome'
},
group: {
_id: groupId,
title: `Refetched ${title}`,
__typename: 'LearningOutcomeGroup'
},
__typename: 'ContentTag'
}
],
__typename: 'ContentTagConnection'
},
__typename: 'LearningOutcomeGroup'
}
}
}))
},
{
request: {

View File

@ -128,6 +128,25 @@ describe('groupDetailHook', () => {
])
})
it('refetches when id is in rhsGroupIdsToRefetch', async () => {
mocks = [...groupDetailMocks(), ...groupDetailMocks({groupId: '200'})]
const {result, rerender} = renderHook(
id => useGroupDetail({id, rhsGroupIdsToRefetch: ['200']}),
{wrapper, initialProps: '1'}
)
await act(async () => jest.runAllTimers())
expect(result.current.group.title).toBe('Group 1')
expect(outcomeTitles(result)).toEqual(['Outcome 1 - Group 1', 'Outcome 2 - Group 1'])
act(() => rerender('200'))
await act(async () => jest.runAllTimers())
expect(result.current.group.title).toBe('Refetched Group 200')
expect(outcomeTitles(result)).toEqual([
'Refetched Outcome 1 - Group 200',
'Refetched Outcome 2 - Group 200',
'Newly Created Outcome - Group 200'
])
})
it('should not load group info if ACCOUNT_GROUP_ID passed as id', async () => {
const {result} = renderHook(() => useGroupDetail({id: ACCOUNT_GROUP_ID}), {wrapper})
expect(result.current.loading).toBe(false)

View File

@ -22,7 +22,7 @@ import useCanvasContext from './useCanvasContext'
import I18n from 'i18n!OutcomeManagement'
import {showFlashAlert} from '@canvas/alerts/react/FlashAlert'
import {SEARCH_GROUP_OUTCOMES} from '@canvas/outcomes/graphql/Management'
import {uniqWith, uniqBy, isEqual} from 'lodash'
import {uniqWith, uniqBy, uniq, isEqual} from 'lodash'
import {gql} from '@canvas/apollo'
const useAbortController = dependencies => {
@ -60,7 +60,8 @@ const useGroupDetail = ({
query = SEARCH_GROUP_OUTCOMES,
loadOutcomesIsImported = false,
searchString = '',
id
id,
rhsGroupIdsToRefetch = []
}) => {
const {contextType, contextId, rootIds} = useCanvasContext()
searchString = useSearchString(searchString)
@ -68,9 +69,14 @@ const useGroupDetail = ({
const queryVars = {outcomesContextType: contextType, outcomesContextId: contextId}
const client = useApolloClient()
const allVariables = useRef([])
const refetchGroupIds = useRef([])
if (searchString) queryVars.searchQuery = searchString
useEffect(() => {
refetchGroupIds.current = uniq(refetchGroupIds.current.concat(rhsGroupIdsToRefetch))
}, [rhsGroupIdsToRefetch])
const skip = !id || rootIds.includes(id)
const variables = {
id,
@ -78,7 +84,7 @@ const useGroupDetail = ({
...queryVars
}
const {loading, error, data, fetchMore} = useQuery(query, {
const {loading, error, data, fetchMore, refetch} = useQuery(query, {
variables,
skip,
context: {
@ -91,6 +97,15 @@ const useGroupDetail = ({
}
})
// To handle refetching of groups when an outcome is created. This will ensure that
// all groups including parent, grandparent, etc... are refetched.
useEffect(() => {
if (refetchGroupIds.current.includes(id)) {
refetchGroupIds.current = refetchGroupIds.current.filter(groupId => groupId !== id)
refetch()
}
}, [id, refetch])
// this will handle the case when we click in group 1, wait for group 1
// load to finish, then click in a different group
// it'll issue a query to load the second group
@ -224,7 +239,8 @@ const useGroupDetail = ({
error,
loadMore,
removeLearningOutcomes,
readLearningOutcomes
readLearningOutcomes,
refetch
}
}