add DateValidator to bulk edit

test plan:
 in assignment bulk edit,
 - ensure if you enter out-of-order availability dates
   (available from after available to, available from
    after due at, available until before due date),
   this is flagged as invalid
 - ensure if you enter a due date in a closed grading period,
   this is flagged as invalid
 - ensure if you enter a due date outside the term dates,
   this is flagged as invalid
 - ensure if you enter a due date in a section override that
   is outside the override's target section dates (and the
   section has "Students can only participate in the course between
   these dates" checked), this is flagged as invalid
 - ensure if any dates are flagged as invalid, the Save
   button is disabled

flag=assignment_bulk_edit
closes LA-934
closes LA-935

Change-Id: Iab15a4a988dc6de1a8ec289f5e624745e34cf44f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/235239
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jon Willesen <jonw+gerrit@instructure.com>
QA-Review: Jon Willesen <jonw+gerrit@instructure.com>
Product-Review: Lauren Williams <lcwilliams@instructure.com>
This commit is contained in:
Jeremy Stanley 2020-04-24 12:36:50 -06:00
parent d1df26bd0a
commit 9322a949f0
10 changed files with 356 additions and 54 deletions

View File

@ -101,10 +101,11 @@ export default class DateValidator {
const lockAt = data.lock_at
const unlockAt = data.unlock_at
const dueAt = data.due_at
const section = _.find(ENV.SECTION_LIST, {id: data.course_section_id})
const section_id = data.set_type === 'CourseSection' ? data.set_id : data.course_section_id
const section = _.find(ENV.SECTION_LIST, {id: section_id})
const currentDateRange = section ? this.getSectionRange(section) : this.dateRange
const datetimesToValidate = []
const forIndividualStudents = data.student_ids?.length
const forIndividualStudents = data.student_ids?.length || data.set_type === 'ADHOC'
if (currentDateRange.start_at && currentDateRange.start_at.date && !forIndividualStudents) {
datetimesToValidate.push({

View File

@ -239,8 +239,7 @@ export default class CreateAssignmentView extends DialogFormView
date_range: _.extend({}, validRange)
hasGradingPeriods: !!ENV.HAS_GRADING_PERIODS
gradingPeriods: GradingPeriodsAPI.deserializePeriods(ENV.active_grading_periods)
userIsAdmin: @currentUserIsAdmin(),
data
userIsAdmin: @currentUserIsAdmin()
)
errs = dateValidator.validateDatetimes(data)

View File

@ -17,7 +17,7 @@
*/
import React, {useCallback} from 'react'
import {bool, func, oneOf, string} from 'prop-types'
import {bool, func, oneOf, string, arrayOf, shape} from 'prop-types'
import tz from 'timezone'
import moment from 'moment-timezone'
import {DateTime} from '@instructure/ui-i18n'
@ -33,7 +33,8 @@ BulkDateInput.propTypes = {
updateAssignmentDate: func.isRequired,
timezone: string,
fancyMidnight: bool,
interaction: string
interaction: string,
messages: arrayOf(shape({type: string, text: string}))
}
BulkDateInput.defaultProps = {
@ -49,6 +50,7 @@ function formatDate(date) {
function BulkDateInput({
label,
selectedDateString,
messages,
dateKey,
assignmentId,
overrideId,
@ -98,6 +100,7 @@ function BulkDateInput({
onSelectedDateChange={handleSelectedDateChange}
timezone={timezone}
interaction={interaction}
messages={messages}
/>
)
}

View File

@ -17,11 +17,10 @@
*/
import I18n from 'i18n!assignments_bulk_edit'
import React, {useCallback, useEffect, useState} from 'react'
import React, {useCallback, useEffect, useState, useMemo} from 'react'
import {func, string} from 'prop-types'
import moment from 'moment-timezone'
import produce from 'immer'
import {List} from '@instructure/ui-elements'
import CanvasInlineAlert from 'jsx/shared/components/CanvasInlineAlert'
import LoadingIndicator from 'jsx/shared/LoadingIndicator'
import useFetchApi from 'jsx/shared/effects/useFetchApi'
@ -30,6 +29,8 @@ import BulkEditTable from './BulkEditTable'
import MoveDatesModal from './MoveDatesModal'
import useSaveAssignments from './hooks/useSaveAssignments'
import useMonitorJobCompletion from './hooks/useMonitorJobCompletion'
import DateValidator from 'coffeescripts/util/DateValidator'
import GradingPeriodsAPI from 'coffeescripts/api/gradingPeriodsApi'
import {originalDateField, canEditAll} from './utils'
BulkEdit.propTypes = {
@ -43,6 +44,19 @@ BulkEdit.defaultProps = {
}
export default function BulkEdit({courseId, onCancel, onSave}) {
const dateValidator = useMemo(
() =>
new DateValidator({
date_range: ENV.VALID_DATE_RANGE || {
start_at: {date: null, date_context: 'term'},
end_at: {date: null, date_context: 'term'}
},
hasGradingPeriods: !!ENV.HAS_GRADING_PERIODS,
gradingPeriods: GradingPeriodsAPI.deserializePeriods(ENV.active_grading_periods || []),
userIsAdmin: (ENV.current_user_roles || []).includes('admin')
}),
[]
)
const [assignments, setAssignments] = useState([])
const [loadingError, setLoadingError] = useState(null)
const [loading, setLoading] = useState(true)
@ -90,17 +104,52 @@ export default function BulkEdit({courseId, onCancel, onSave}) {
if (jobSuccess) clearOriginalDates()
}, [jobSuccess])
const setDateOnOverride = useCallback((override, dateFieldName, newDate) => {
const currentDate = override[dateFieldName]
const newDateISO = newDate?.toISOString() || null
if (currentDate === newDateISO || moment(currentDate).isSame(moment(newDateISO))) return
const originalField = originalDateField(dateFieldName)
if (!override.hasOwnProperty(originalField)) {
override[originalField] = override[dateFieldName]
useEffect(() => {
function recordJobErrors(errors) {
setAssignments(currentAssignments =>
produce(currentAssignments, draftAssignments => {
draftAssignments.forEach(draftAssignment => {
draftAssignment.all_dates.forEach(draftOverride => {
let error
if (draftOverride.base) {
error = errors.find(
e => e.assignment_id == draftAssignment.id && !e.assignment_override_id // eslint-disable-line eqeqeq
)
} else {
error = errors.find(e => e.assignment_override_id == draftOverride.id) // eslint-disable-line eqeqeq
}
if (error && error.errors) {
draftOverride.errors = {}
for (const dateKey in error.errors) {
draftOverride.errors[dateKey] = error.errors[dateKey][0].message
}
} else {
delete draftOverride.errors
}
})
})
})
)
}
override[dateFieldName] = newDateISO
}, [])
if (jobErrors && !jobErrors.hasOwnProperty('message')) recordJobErrors(jobErrors)
}, [jobErrors])
const setDateOnOverride = useCallback(
(override, dateFieldName, newDate) => {
const currentDate = override[dateFieldName]
const newDateISO = newDate?.toISOString() || null
if (currentDate === newDateISO || moment(currentDate).isSame(moment(newDateISO))) return
const originalField = originalDateField(dateFieldName)
if (!override.hasOwnProperty(originalField)) {
override[originalField] = override[dateFieldName]
}
override[dateFieldName] = newDateISO
override.persisted = false
override.errors = dateValidator.validateDatetimes(override)
},
[dateValidator]
)
const shiftDateOnOverride = useCallback(
(override, dateFieldName, nDays) => {
@ -154,6 +203,8 @@ export default function BulkEdit({courseId, onCancel, onSave}) {
delete override[originalField]
}
})
delete override.errors
delete override.persisted
})
)
},
@ -274,12 +325,9 @@ export default function BulkEdit({courseId, onCancel, onSave}) {
} else if (jobErrors) {
return (
<CanvasInlineAlert variant="error" liveAlert>
{I18n.t('There were errors saving the assignment dates:')}
<List>
{jobErrors.map(error => {
return <List.Item key={error}>{error}</List.Item>
})}
</List>
{jobErrors.hasOwnProperty('message')
? I18n.t('Error saving assignment dates: ') + jobErrors.message
: I18n.t('Invalid dates were found. Please correct them and try again.')}
</CanvasInlineAlert>
)
}

View File

@ -63,6 +63,12 @@ export default function BulkEditHeader({
)
})()
const validationErrorsExist = (() => {
return assignments.some(assignment =>
assignment.all_dates.some(override => Object.keys(override.errors || {}).length > 0)
)
})()
const selectedAssignmentsCount = assignments.filter(a => a.selected).length
return (
@ -114,7 +120,11 @@ export default function BulkEditHeader({
<Button
margin="0 0 0 small"
variant="primary"
interaction={startingSave || jobRunning || !anyAssignmentsEdited ? 'disabled' : 'enabled'}
interaction={
startingSave || jobRunning || !anyAssignmentsEdited || validationErrorsExist
? 'disabled'
: 'enabled'
}
onClick={onSave}
>
{startingSave || jobRunning ? I18n.t('Saving...') : I18n.t('Save')}

View File

@ -84,12 +84,20 @@ export default function BulkEditTable({
const allAssignmentsSelected =
someAssignmentsSelected && assignments.every(a => a.selected || !canEditAll(a))
function processErrors(errors, dateKey) {
if (!errors || !errors.hasOwnProperty(dateKey)) {
return []
}
return [{text: errors[dateKey], type: 'error'}]
}
function renderDateInput(assignmentId, dateKey, dates, overrideId = null) {
const label = DATE_INPUT_META[dateKey].label
return (
<BulkDateInput
label={label}
selectedDateString={dates[dateKey]}
messages={processErrors(dates.errors, dateKey)}
dateKey={dateKey}
assignmentId={assignmentId}
overrideId={overrideId}

View File

@ -43,7 +43,7 @@ function standardAssignmentResponse() {
base: true,
unlock_at: '2020-03-19T00:00:00Z',
due_at: '2020-03-20T03:00:00Z',
lock_at: '2020-03-21T00:00:00Z',
lock_at: '2020-04-11T00:00:00Z',
can_edit: true
},
{
@ -51,7 +51,7 @@ function standardAssignmentResponse() {
title: '2 students',
unlock_at: '2020-03-29T00:00:00Z',
due_at: '2020-03-30T00:00:00Z',
lock_at: '2020-03-31T00:00:00Z',
lock_at: '2020-04-21T00:00:00Z',
can_edit: true
}
]
@ -159,6 +159,57 @@ describe('Assignment Bulk Edit Dates', () => {
expect(onSave).toHaveBeenCalled()
})
it('disables save when local validation fails', async () => {
const {getByText, getAllByLabelText} = await renderBulkEditAndWait()
changeAndBlurInput(getAllByLabelText('Due At')[0], '2019-04-01')
expect(getByText('Unlock date cannot be after due date')).toBeInTheDocument()
expect(getByText('Save').closest('button').disabled).toBe(true)
})
it('clears the validation error when a bad edit is reverted', async () => {
const {queryByText, getAllByText, getAllByLabelText} = await renderBulkEditAndWait()
const theInput = getAllByLabelText('Due At')[0]
changeAndBlurInput(theInput, '2019-04-01')
expect(queryByText('Unlock date cannot be after due date')).toBeInTheDocument()
const revertButtons = getAllByText('Revert date changes').filter(elt => elt.closest('button'))
expect(revertButtons).toHaveLength(1)
fireEvent.click(revertButtons[0])
expect(queryByText('Unlock date cannot be after due date')).not.toBeInTheDocument()
})
it('validates against closed grading periods', async () => {
ENV.HAS_GRADING_PERIODS = true
ENV.active_grading_periods = [
{
start_date: '1970-01-01T00:00:00-07:00',
end_date: '2020-03-01T23:59:59-07:00',
close_date: '2020-03-01T23:59:59-07:00',
id: '1',
is_closed: true,
is_last: false,
permissions: {read: true, create: false, update: false, delete: false},
title: 'Closed'
},
{
start_date: '2020-03-01T23:59:59-06:00',
close_date: '3000-12-31T23:59:59-07:00',
end_date: '3000-12-31T23:59:59-07:00',
id: '2',
is_closed: false,
is_last: true,
permissions: {read: true, create: false, update: false, delete: false},
title: '5ever'
}
]
const {queryByText, getAllByLabelText} = await renderBulkEditAndWait()
changeAndBlurInput(getAllByLabelText('Available From')[0], '2020-01-01')
const theInput = getAllByLabelText('Due At')[0]
changeAndBlurInput(theInput, '2020-03-03')
expect(queryByText(/Please enter a due date on or after/)).not.toBeInTheDocument()
changeAndBlurInput(theInput, '2020-01-03')
expect(queryByText(/Please enter a due date on or after/)).toBeInTheDocument()
})
it('disables save when nothing has been edited', async () => {
const {getByText} = await renderBulkEditAndWait()
expect(getByText('Save').closest('button').disabled).toBe(true)
@ -171,7 +222,7 @@ describe('Assignment Bulk Edit Dates', () => {
const unlockAtInputs = getAllByLabelText('Available From')
expect(unlockAtInputs.map(i => i.value)).toEqual(['Thu Mar 19, 2020', 'Sun Mar 29, 2020', ''])
const lockAtInputs = getAllByLabelText('Available Until')
expect(lockAtInputs.map(i => i.value)).toEqual(['Sat Mar 21, 2020', 'Tue Mar 31, 2020', ''])
expect(lockAtInputs.map(i => i.value)).toEqual(['Sat Apr 11, 2020', 'Tue Apr 21, 2020', ''])
})
it('shows a message and no date default date fields if an assignment does not have default dates', async () => {
@ -192,7 +243,7 @@ describe('Assignment Bulk Edit Dates', () => {
it('modifies lock at date and enables save', async () => {
const {getByText, getByDisplayValue} = await renderBulkEditAndWait()
const overrideLockInput = getByDisplayValue('Tue Mar 31, 2020')
const overrideLockInput = getByDisplayValue('Tue Apr 21, 2020')
changeAndBlurInput(overrideLockInput, '2020-12-31')
expect(overrideLockInput.value).toBe('Thu Dec 31, 2020')
expect(getByText('Save').closest('button').disabled).toBe(false)
@ -225,7 +276,7 @@ describe('Assignment Bulk Edit Dates', () => {
const assignmentUnlockAt = getByDisplayValue('Thu Mar 19, 2020')
changeAndBlurInput(assignmentUnlockAt, '2020-06-15')
const overrideLockInput = getByDisplayValue('Tue Mar 31, 2020')
const overrideLockInput = getByDisplayValue('Tue Apr 21, 2020')
changeAndBlurInput(overrideLockInput, '')
const nullDueDate = getAllByLabelText('Due At')[2]
@ -235,7 +286,7 @@ describe('Assignment Bulk Edit Dates', () => {
expect(revertButtons).toHaveLength(3)
fireEvent.click(revertButtons[1])
expect(overrideLockInput.value).toBe('Tue Mar 31, 2020') // original value
expect(overrideLockInput.value).toBe('Tue Apr 21, 2020') // original value
expect(assignmentUnlockAt.value).toBe('Mon Jun 15, 2020') // not changed yet
expect(nullDueDate.value).toBe('Tue Jun 16, 2020') // not changed yet
// focus should be explicitly set to the lock at input
@ -523,7 +574,7 @@ describe('Assignment Bulk Edit Dates', () => {
})
it('displays an error if the job fails', async () => {
const {getByText} = await renderBulkEditAndSave()
const {getByText, getAllByLabelText} = await renderBulkEditAndSave()
fetch.mockResponseOnce(
JSON.stringify({
completion: 42,
@ -536,6 +587,10 @@ describe('Assignment Bulk Edit Dates', () => {
act(jest.runAllTimers)
await flushPromises()
expect(getByText(/some bad dates/)).toBeInTheDocument()
// save button is disabled due to error
expect(getByText('Save').closest('button').disabled).toBe(true)
// fix the error and save should be re-enabled
changeAndBlurInput(getAllByLabelText(/Due At/)[0], '2020-04-04')
expect(getByText('Save').closest('button').disabled).toBe(false)
})
@ -665,13 +720,13 @@ describe('Assignment Bulk Edit Dates', () => {
base: true,
unlock_at: '2020-03-21T00:00:00.000Z',
due_at: '2020-03-22T03:00:00.000Z', // time preservation
lock_at: '2020-03-23T00:00:00.000Z'
lock_at: '2020-04-13T00:00:00.000Z'
},
{
id: 'override_1',
unlock_at: '2020-03-31T00:00:00.000Z',
due_at: '2020-04-01T00:00:00.000Z',
lock_at: '2020-04-02T00:00:00.000Z'
lock_at: '2020-04-23T00:00:00.000Z'
}
]
}

View File

@ -21,21 +21,6 @@ import {useEffect, useRef, useState} from 'react'
import doFetchApi from 'jsx/shared/effects/doFetchApi'
import {extractFetchErrorMessage} from '../utils'
function pushFieldErrors(err, fieldName, acc) {
if (err[fieldName]) acc.push(...err[fieldName].map(e => e.message))
}
function extractJobErrorMessages(json) {
return json.results
.map(result => result.errors)
.reduce((acc, error) => {
pushFieldErrors(error, 'due_at', acc)
pushFieldErrors(error, 'unlock_at', acc)
pushFieldErrors(error, 'lock_at', acc)
return acc
}, [])
}
export default function useMonitorJobCompletion({progressUrl, pollingInterval = 2000}) {
const [jobCompletion, setJobCompletion] = useState(0)
const [jobRunning, setJobRunning] = useState(false)
@ -52,14 +37,18 @@ export default function useMonitorJobCompletion({progressUrl, pollingInterval =
function reportJobErrors(json) {
setJobRunning(false)
setJobErrors(extractJobErrorMessages(json))
setJobErrors(json.results)
}
async function reportFetchError(err) {
setJobRunning(false)
setJobErrors([
await extractFetchErrorMessage(err, I18n.t('There was an error retrieving job progress'))
])
// this is also the results format if the job fails due to an exception
setJobErrors({
message: await extractFetchErrorMessage(
err,
I18n.t('There was an error retrieving job progress')
)
})
}
function interpretCompletionResponse({json}) {

View File

@ -214,7 +214,7 @@ module DatesOverridable
[ due_at.present? ? CanvasSort::First : CanvasSort::Last, due_at.presence || CanvasSort::First ]
end
dates.map { |h| h.slice(:id, :due_at, :unlock_at, :lock_at, :title, :base) }
dates.map { |h| h.slice(:id, :due_at, :unlock_at, :lock_at, :title, :base, :set_type, :set_id) }
end
def due_date_hash
@ -228,6 +228,8 @@ module DatesOverridable
if @applied_overrides && override = @applied_overrides.find { |o| o.due_at == due_at }
hash[:override] = override
hash[:title] = override.title
hash[:set_type] = override.set_type
hash[:set_id] = override.set_id
end
hash

View File

@ -17,6 +17,7 @@
*/
import DateValidator from 'compiled/util/DateValidator'
import fakeENV from 'helpers/fakeENV'
const DATE_IN_CLOSED_PERIOD = '2015-07-23T03:59:59Z'
const DATE_IN_OPEN_PERIOD = '2015-09-23T03:59:59Z'
@ -240,6 +241,18 @@ QUnit.module('when applied to one or more individual students', hooks => {
ok(isValid(validator, data))
})
test('accepts all_dates format for student overrides', () => {
const data = generateData({
due_at: '2014-01-23T03:59:59Z',
student_ids: null,
set_type: 'ADHOC',
set_id: null
})
const validator = makeIndividualValidator()
ok(isValid(validator, data))
})
test('allows an unlock date before the prescribed start date', () => {
const data = generateData({
unlock_at: '2014-01-23T03:59:59Z'
@ -276,3 +289,177 @@ QUnit.module('when applied to one or more individual students', hooks => {
notOk(isValid(validator, data))
})
})
QUnit.module('term dates', hooks => {
let makeIndividualValidator
hooks.beforeEach(() => {
makeIndividualValidator = (params = {}) =>
createValidator({
dueDateRequiredForAccount: false,
gradingPeriods: null,
hasGradingPeriods: false,
postToSIS: true,
userIsAdmin: false,
...params
})
})
test('allows dates that are in range', () => {
const data = generateData({
unlock_at: '2015-03-03T03:59:59Z',
due_at: '2015-03-04T03:59:59Z',
lock_at: '2015-03-05T03:59:59Z',
student_ids: null
})
const validator = makeIndividualValidator()
ok(isValid(validator, data))
})
test('disallows a due date before the prescribed start date', () => {
const data = generateData({
due_at: '2014-01-23T03:59:59Z',
student_ids: null
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows an unlock date before the prescribed start date', () => {
const data = generateData({
unlock_at: '2014-01-23T03:59:59Z',
student_ids: null
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows a due date after the prescribed end date', () => {
const data = generateData({
due_at: '2017-01-23T03:59:59Z',
student_ids: null
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows a lock date after the prescribed end date', () => {
const data = generateData({
lock_at: '2017-01-23T03:59:59Z',
student_ids: null
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
})
QUnit.module('section dates', hooks => {
let makeIndividualValidator
hooks.beforeEach(() => {
makeIndividualValidator = (params = {}) =>
createValidator({
dueDateRequiredForAccount: false,
gradingPeriods: null,
hasGradingPeriods: false,
postToSIS: true,
userIsAdmin: false,
...params
})
fakeENV.setup({
SECTION_LIST: [
{
id: 123,
start_at: '2020-03-01T00:00:00Z',
end_at: '2020-07-01T00:00:00Z',
override_course_and_term_dates: true
}
]
})
})
hooks.afterEach(() => {
fakeENV.teardown()
})
test('allows dates that are in range', () => {
const data = generateData({
unlock_at: '2020-03-03T00:00:00Z',
due_at: '2020-03-04T00:00:00Z',
lock_at: '2020-03-05T00:00:00Z',
student_ids: null,
course_section_id: 123
})
const validator = makeIndividualValidator()
ok(isValid(validator, data))
})
test('accepts all_dates format for section overrides', () => {
const data = generateData({
unlock_at: '2020-03-03T00:00:00Z',
due_at: '2020-03-04T00:00:00Z',
lock_at: '2020-03-05T00:00:00Z',
student_ids: null,
set_type: 'CourseSection',
set_id: 123
})
const validator = makeIndividualValidator()
ok(isValid(validator, data))
})
test('matches the section id', () => {
const data = generateData({
unlock_at: '2020-03-03T00:00:00Z',
due_at: '2020-03-04T00:00:00Z',
lock_at: '2020-03-05T00:00:00Z',
student_ids: null,
course_section_id: 456
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows a due date before the prescribed start date', () => {
const data = generateData({
due_at: '2020-01-01T00:00:00Z',
student_ids: null,
course_section_id: 123
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows an unlock date before the prescribed start date', () => {
const data = generateData({
unlock_at: '2020-01-01T00:00:00Z',
student_ids: null,
course_section_id: 123
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows a due date after the prescribed end date', () => {
const data = generateData({
due_at: '2020-12-25T00:00:00Z',
student_ids: null,
course_section_id: 123
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
test('disallows a lock date after the prescribed end date', () => {
const data = generateData({
lock_at: '2020-12-25T00:00:00Z',
student_ids: null,
course_section_id: 123
})
const validator = makeIndividualValidator()
notOk(isValid(validator, data))
})
})