Only allow non-negative points

refs OUT-2240

test plan:
  - enable the non-scoring rubric feature flag if not enabled
  - load the accounts rubrics page
  - click the "Learning Mastery" tab
  - confirm that when attempting to save ratings with
    negative points, an error message is displayed below
    the points field

Change-Id: Ic41b013cbc6499e3c1783fed2283fe6ed02ba2f0
Reviewed-on: https://gerrit.instructure.com/153069
Reviewed-by: Michael Brewer-Davis <mbd@instructure.com>
Reviewed-by: Matt Berns <mberns@instructure.com>
Tested-by: Jenkins
QA-Review: Michael Brewer-Davis <mbd@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>

app/jsx/rubrics/ProficiencyTable.js

@@ -142,7 +142,7 @@ export default class ProficiencyTable extends React.Component {
   handlePointsChange = _.memoize((index) => (value) => {
     const parsed = NumberHelper.parse(value)
     let rows = this.state.rows
-    if (!this.invalidPoints(parsed)) {
+    if (!this.invalidPoints(parsed) && parsed >= 0) {
       rows = rows.removeIn([index, 'pointsError'])
     }
     rows = rows.setIn([index, 'points'], parsed)
@@ -164,7 +164,7 @@ export default class ProficiencyTable extends React.Component {
   })
 
   isStateValid = () => !this.state.rows.some(row =>
-    this.invalidPoints(row.get('points')) || this.invalidDescription(row.get('description')))
+    this.invalidPoints(row.get('points')) || row.get('points') < 0 || this.invalidDescription(row.get('description')))
 
 
   stateToConfig = () => ({
@@ -208,6 +208,12 @@ export default class ProficiencyTable extends React.Component {
           r = r.set('focusField', 'points')
           firstError = false
         }
+      } else if (row.get('points') < 0) {
+        r = r.set('pointsError', I18n.t('Negative points'))
+        if (firstError) {
+          r = r.set('focusField', 'points')
+          firstError = false
+        }
       }
       return r
     })

app/jsx/rubrics/__tests__/ProficiencyTable.test.js

@@ -73,6 +73,42 @@ describe('default proficiency', () => {
     })
   })
 
+  it('setting blank description sets error', () => {
+    const wrapper = mount(<ProficiencyTable {...defaultProps()}/>)
+    return promise.then(() => {
+      wrapper.instance().handleDescriptionChange(0)("")
+      wrapper.find('Button').last().simulate('click')
+      expect(wrapper.find('ProficiencyRating').first().prop('descriptionError')).toBe('Missing required description')
+    })
+  })
+
+  it('setting blank points sets error', () => {
+    const wrapper = mount(<ProficiencyTable {...defaultProps()}/>)
+    return promise.then(() => {
+      wrapper.instance().handlePointsChange(0)("")
+      wrapper.find('Button').last().simulate('click')
+      expect(wrapper.find('ProficiencyRating').first().prop('pointsError')).toBe('Invalid points')
+    })
+  })
+
+  it('setting invalid points sets error', () => {
+    const wrapper = mount(<ProficiencyTable {...defaultProps()}/>)
+    return promise.then(() => {
+      wrapper.instance().handlePointsChange(0)("1.1.1")
+      wrapper.find('Button').last().simulate('click')
+      expect(wrapper.find('ProficiencyRating').first().prop('pointsError')).toBe('Invalid points')
+    })
+  })
+
+  it('setting negative points sets error', () => {
+    const wrapper = mount(<ProficiencyTable {...defaultProps()}/>)
+    return promise.then(() => {
+      wrapper.instance().handlePointsChange(0)("-1")
+      wrapper.find('Button').last().simulate('click')
+      expect(wrapper.find('ProficiencyRating').first().prop('pointsError')).toBe('Negative points')
+    })
+  })
+
   it('sends POST on submit', () => {
     const postSpy = jest.spyOn(axios,'post').mockImplementation(() => Promise.resolve({status: 200}))
     const wrapper = mount(<ProficiencyTable {...defaultProps()}/>)
@@ -154,3 +190,9 @@ it('invalid rating points leaves state invalid', () => {
   wrapper.instance().handlePointsChange(0)("1.1.1")
   expect(wrapper.instance().isStateValid()).toBe(false)
 })
+
+it('negative rating points leaves state invalid', () => {
+  const wrapper = shallow(<ProficiencyTable {...defaultProps()}/>)
+  wrapper.instance().handlePointsChange(0)("-1")
+  expect(wrapper.instance().isStateValid()).toBe(false)
+})

app/models/outcome_proficiency_rating.rb

@@ -20,7 +20,7 @@ class OutcomeProficiencyRating < ApplicationRecord
   belongs_to :outcome_proficiency, inverse_of: :outcome_proficiency_ratings
 
   validates :description, presence: true
-  validates :points, presence: true
+  validates :points, presence: true, numericality: { greater_than_or_equal_to: 0 }
   validates :color, presence: true, format: /\A([A-Fa-f0-9]{6})\z/i
 
   def as_json(_options={})

spec/models/outcome_proficiency_rating_spec.rb

@@ -26,6 +26,7 @@ describe OutcomeProficiencyRating, type: :model do
   describe 'validations' do
     it { is_expected.to validate_presence_of :description }
     it { is_expected.to validate_presence_of :points }
+    it { is_expected.to validate_numericality_of(:points).is_greater_than_or_equal_to(0) }
     it { is_expected.to allow_value('0F160a').for(:color) }
     it { is_expected.not_to allow_value('#0F160a').for(:color) }
     it { is_expected.not_to allow_value('').for(:color) }