Skip to content

Commit

Permalink
Locally track saved comments for reuse in speed grader.
Browse files Browse the repository at this point in the history
References OUT-2518

Test Plan:
- Ensure the NSR feature flag is enabled.
- Create a rubric with two free-form criteria.
- Align to an assessment.
- Grade for one student in speed grader, add comments, select
  "save comments for later" on one criteria but not the other.
- Save assessment, move to a different student to assess.
- Verify the comment that was previously saved can be selected,
  and the comment that wasn't saved is not in the selection drop down.

Change-Id: I0d35a07cd470585b865c86d9811abeb03888dd59
Reviewed-on: https://gerrit.instructure.com/164805
Reviewed-by: Neil Gupta <[email protected]>
Tested-by: Jenkins
QA-Review: Brian Watson <[email protected]>
Product-Review: Sidharth Oberoi <[email protected]>
Reviewed-by: Frank Murphy <[email protected]>
Product-Review: Matt Goodwin <[email protected]>
Reviewed-by: Augusto Callejas <[email protected]>
  • Loading branch information
AnIrishDuck committed Sep 24, 2018
1 parent 2273ef9 commit 281db2c
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 6 deletions.
4 changes: 3 additions & 1 deletion app/jsx/rubrics/Comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ const FreeFormComments = (props) => {

const options = savedComments.map((comment, ix) => (
// eslint-disable-next-line react/no-array-index-key
<option key={ix} value={ix.toString()}>{truncate(comment)}</option>
<option key={ix} value={ix.toString()} label={comment}>
{truncate(comment)}
</option>
))
const selector = [
(
Expand Down
4 changes: 2 additions & 2 deletions app/jsx/rubrics/Rubric.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import I18n from 'i18n!edit_rubric'

import Criterion from './Criterion'

import { getSavedComments } from './helpers'
import { rubricShape, rubricAssessmentShape, rubricAssociationShape } from './types'
import { roundIfWhole } from './Points'

Expand Down Expand Up @@ -52,7 +53,6 @@ const Rubric = (props) => {
const priorData = _.get(rubricAssessment, 'data', [])
const byCriteria = _.keyBy(priorData, (ra) => ra.criterion_id)
const criteriaById = _.keyBy(rubric.criteria, (c) => c.id)
const allComments = _.get(rubricAssociation, 'summary_data.saved_comments', {})
const hidePoints = _.get(rubricAssociation, 'hide_points', false)
const freeForm = rubric.free_form_criterion_comments

Expand Down Expand Up @@ -93,7 +93,7 @@ const Rubric = (props) => {
freeForm={freeForm}
isSummary={isSummary}
onAssessmentChange={assessing ? onCriteriaChange(criterion.id) : undefined}
savedComments={allComments[criterion.id]}
savedComments={getSavedComments(rubricAssociation, criterion.id)}
hidePoints={hidePoints}
hasPointsColumn={showPointsColumn()}
/>
Expand Down
28 changes: 27 additions & 1 deletion app/jsx/rubrics/__tests__/helpers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import { defaultCriteria, fillAssessment } from '../helpers'
import {
defaultCriteria,
fillAssessment,
getSavedComments,
updateAssociationData
} from '../helpers'
import { assessments, rubrics } from './fixtures'

describe('defaultCriteria', () => {
Expand Down Expand Up @@ -60,3 +65,24 @@ describe('fillAssessment', () => {
])
})
})

describe('updateAssociationData', () => {
it('updates saved comments', () => {
const assessment = {
score: 0,
data: [
{
...defaultCriteria('_1384'),
comments: 'for later',
saveCommentsForLater: true
},
defaultCriteria('7_391'),
]
}

const association = { }
expect(getSavedComments(association, '_1384')).toBeUndefined()
updateAssociationData(association, assessment)
expect(getSavedComments(association, '_1384')).toEqual(['for later'])
})
})
13 changes: 13 additions & 0 deletions app/jsx/rubrics/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,16 @@ export const fillAssessment = (rubric, partialAssessment) => {
)
}
}

const savedCommentPath = (id) => ['summary_data', 'saved_comments', id]
export const getSavedComments = (association, id) =>
_.get(association, savedCommentPath(id), undefined)

export const updateAssociationData = (association, assessment) => {
assessment.data
.filter(({ saveCommentsForLater }) => saveCommentsForLater)
.forEach(({ criterion_id: id, comments }) => {
const prior = getSavedComments(association, id) || []
_.set(association, savedCommentPath(id), _.uniq([...prior, comments]))
})
}
10 changes: 8 additions & 2 deletions public/javascripts/rubric_assessment.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import './jquery.templateData'
import './vendor/jquery.scrollTo'
import 'compiled/jquery.rails_flash_notifications' // eslint-disable-line
import Rubric from 'jsx/rubrics/Rubric'
import { fillAssessment } from 'jsx/rubrics/helpers'
import { fillAssessment, updateAssociationData } from 'jsx/rubrics/helpers'

// TODO: stop managing this in the view and get it out of the global scope submissions/show.html.erb
/*global rubricAssessment*/
Expand Down Expand Up @@ -258,7 +258,10 @@ window.rubricAssessment = {

updateRubricAssociation: function($rubric, data) {
var summary_data = data.summary_data;
if (summary_data && summary_data.saved_comments) {
if (ENV.nonScoringRubrics && this.currentAssessment !== undefined) {
const assessment = this.currentAssessment
updateAssociationData(this.currentAssociation, assessment)
} else if (summary_data && summary_data.saved_comments) {
for(var id in summary_data.saved_comments) {
var comments = summary_data.saved_comments[id],
$holder = $rubric.find("#criterion_" + id).find(".saved_custom_rating_holder").hide(),
Expand Down Expand Up @@ -286,6 +289,9 @@ window.rubricAssessment = {
render(currentAssessment)
}

const association = rubricAssessment.currentAssociation || rubricAssociation
rubricAssessment.currentAssociation = association

const render = (currentAssessment) => {
ReactDOM.render(<Rubric
allowExtraCredit={ENV.outcome_extra_credit_enabled}
Expand Down

0 comments on commit 281db2c

Please sign in to comment.