diff --git a/app/models/course/condition/achievement.rb b/app/models/course/condition/achievement.rb index 369cefefb0a..ee28a0783fc 100644 --- a/app/models/course/condition/achievement.rb +++ b/app/models/course/condition/achievement.rb @@ -22,8 +22,8 @@ def satisfied_by?(course_user) # Given a conditional object, returns all achievements that it requires. # - # @param [Object] conditional The object that is declared as acts_as_conditional and for which - # returned achievements are required. + # @param [#conditions] conditional The object that is declared as acts_as_conditional and for + # which returned achievements are required. # @return [Array] def required_achievements_for(conditional) # Course::Condition::Achievement. @@ -33,13 +33,16 @@ def required_achievements_for(conditional) # Workaround, pending the squeel bugfix (activerecord-hackery/squeel#390) that will allow # allow the above query to work without #reload - Course::Achievement.joins( - "INNER JOIN + Course::Achievement.joins(<<-SQL) + INNER JOIN (SELECT cca.achievement_id FROM course_condition_achievements cca INNER JOIN course_conditions cc ON cc.actable_type = 'Course::Condition::Achievement' AND cc.actable_id = cca.id - WHERE cc.conditional_id = #{conditional.id}) ids - ON ids.achievement_id = course_achievements.id") + WHERE cc.conditional_id = #{conditional.id} + AND cc.conditional_type = #{ActiveRecord::Base.sanitize(conditional.class.name)} + ) ids + ON ids.achievement_id = course_achievements.id + SQL end def validate_achievement_condition diff --git a/spec/factories/course_condition_achievements.rb b/spec/factories/course_condition_achievements.rb index f7d8ba1b57e..8e86a452cef 100644 --- a/spec/factories/course_condition_achievements.rb +++ b/spec/factories/course_condition_achievements.rb @@ -4,28 +4,5 @@ course achievement association :conditional, factory: :course_achievement - - after(:stub) do |achievement_condition, evaluator| - achievement_condition.achievement = build_stubbed(:course_achievement, - course: evaluator.course) - achievement_condition.conditional = build_stubbed(:course_achievement, - course: evaluator.course) - end - - trait :self_referential do - after(:stub) do |achievement_condition, _| - achievement_condition.achievement = achievement_condition.conditional - end - end - - trait :duplicate_child do - after(:build) do |achievement_condition, evaluator| - condition = build(:course_condition_achievement, - course: evaluator.course, - achievement: evaluator.achievement) - achievement_condition.conditional.conditions << condition.condition - end - to_create { |instance| instance.save(validate: false) } - end end end diff --git a/spec/models/course/condition/achievement_spec.rb b/spec/models/course/condition/achievement_spec.rb index 708aec8de5b..3c4047ecb17 100644 --- a/spec/models/course/condition/achievement_spec.rb +++ b/spec/models/course/condition/achievement_spec.rb @@ -10,9 +10,11 @@ describe 'validations' do context 'when an achievement is its own condition' do subject do - build_stubbed(:course_condition_achievement, :self_referential, - course: course).tap do |cca| - allow(cca).to receive(:achievement_id_changed?).and_return(true) + achievement = create(:achievement, course: course) + build_stubbed(:achievement_condition, + course: course, achievement: achievement, conditional: achievement). + tap do |achievement_condition| + allow(achievement_condition).to receive(:achievement_id_changed?).and_return(true) end end it { is_expected.to_not be_valid } @@ -20,11 +22,31 @@ context "when an achievement is already included in its conditional's conditions" do subject do - create(:course_condition_achievement, :duplicate_child, course: course).tap do |cca| - allow(cca).to receive(:achievement_id_changed?).and_return(true) + existing_achievement_condition = create(:achievement_condition, course: course) + build(:achievement_condition, + course: course, conditional: existing_achievement_condition.conditional, + achievement: existing_achievement_condition.achievement) end end - it { is_expected.to_not be_valid } + + it 'is not valid' do + expect(subject).to_not be_valid + expect(subject.errors[:achievement]).to_not be_blank + end + end + + context 'when an achievement is required by another conditional with the same id' do + subject do + id = Time.now.to_i + assessment = create(:assessment, course: course, id: id) + achievement = create(:achievement, course: course, id: id) + required_achievement = create(:achievement, course: course) + create(:achievement_condition, + course: course, achievement: required_achievement, conditional: assessment) + build_stubbed(:achievement_condition, + course: course, achievement: required_achievement, conditional: achievement) + end + it { is_expected.to be_valid } end end