diff --git a/app/jsx/rubrics/ProficiencyTable.js b/app/jsx/rubrics/ProficiencyTable.js index 1398f0c7885..00a46467bc0 100644 --- a/app/jsx/rubrics/ProficiencyTable.js +++ b/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 }) diff --git a/app/jsx/rubrics/__tests__/ProficiencyTable.test.js b/app/jsx/rubrics/__tests__/ProficiencyTable.test.js index 1a7a2c1615e..f0d56189384 100644 --- a/app/jsx/rubrics/__tests__/ProficiencyTable.test.js +++ b/app/jsx/rubrics/__tests__/ProficiencyTable.test.js @@ -73,6 +73,42 @@ describe('default proficiency', () => { }) }) + it('setting blank description sets error', () => { + const wrapper = mount() + 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() + 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() + 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() + 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() @@ -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() + wrapper.instance().handlePointsChange(0)("-1") + expect(wrapper.instance().isStateValid()).toBe(false) +}) diff --git a/app/models/outcome_proficiency_rating.rb b/app/models/outcome_proficiency_rating.rb index 865cfd44cf9..ed10ad45715 100644 --- a/app/models/outcome_proficiency_rating.rb +++ b/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={}) diff --git a/spec/models/outcome_proficiency_rating_spec.rb b/spec/models/outcome_proficiency_rating_spec.rb index 339d1754deb..9fe3773a098 100644 --- a/spec/models/outcome_proficiency_rating_spec.rb +++ b/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) }