Skip to content
This repository has been archived by the owner on May 10, 2022. It is now read-only.

lml/ost#346: Make feedback penalty configurable. #350

Merged
merged 3 commits into from
Jun 29, 2014
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion app/assets/javascripts/learning_conditions.js.coffee
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.

# Place all the behaviors and hooks related to the matching controller here.
# All this logic will automatically be available in application.js.
# You can use CoffeeScript in this file: http://jashkenas.github.com/coffee-script/
$ () ->
feedbackRequired = $ ".field.feedback-required input"
feedbackRequired.click () ->
feedbackPenalty = $ ".field.feedback-viewing-penalty"
feedbackPenaltyValue = feedbackPenalty.find "input"
if feedbackRequired.is ':checked'
feedbackPenaltyValue.val "100"
feedbackPenalty.show()
else
feedbackPenaltyValue.val "0"
feedbackPenalty.hide()
20 changes: 16 additions & 4 deletions app/models/feedback_condition.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.

class FeedbackCondition < ActiveRecord::Base
Expand All @@ -9,6 +9,7 @@ class FeedbackCondition < ActiveRecord::Base
store_accessor :settings, :label_regex
store_typed_accessor :settings, :integer, :exercise_correctness_option
store_typed_accessor :settings, :boolean, :is_feedback_required_for_credit
store_typed_accessor :settings, :integer, :feedback_viewing_penalty
store_typed_accessor :settings, :boolean, :can_automatically_show_feedback
store_typed_accessor :settings, :integer, :availability_opens_option
store_typed_accessor :settings, :float, :availability_opens_delay_days
Expand All @@ -30,11 +31,18 @@ class FeedbackCondition < ActiveRecord::Base
:credit_closes_option, :credit_closes_delay_days,
:can_automatically_show_feedback,
:show_correctness_feedback, :show_correct_answer_feedback,
:show_high_level_feedback, :show_detailed_feedback
:show_high_level_feedback, :show_detailed_feedback,
:feedback_viewing_penalty

after_initialize :supply_missing_values
before_validation :strip_regex, :nil_out_blank_regex

validates :feedback_viewing_penalty, allow_nil: true,
numericality: {
greater_than_or_equal_to: 0,
less_than_or_equal_to: 100
}

validates :availability_opens_delay_days, allow_nil: true,
numericality: { greater_than: 0 }

Expand All @@ -49,7 +57,7 @@ class FeedbackCondition < ActiveRecord::Base
validate :not_applicable_event_only_for_never
validate :feedback_cannot_close_if_never_opened
validate :feedback_must_open_if_needed_for_credit

class AvailabilityOpensOption < Enum
NEVER = 0
IMMEDIATELY_AFTER_EVENT = 1
Expand Down Expand Up @@ -83,6 +91,7 @@ def self.standard_practice_feedback_condition
FeedbackCondition.new(:label_regex => 'standard practice',
:exercise_correctness_option => ExerciseCorrectnessOption::ANY_CORRECTNESS.to_s,
:is_feedback_required_for_credit => false.to_s,
:feedback_viewing_penalty => 0.to_s,
:availability_opens_option => AvailabilityOpensOption::IMMEDIATELY_AFTER_EVENT.to_s,
:availability_closes_option => AvailabilityClosesOption::NEVER.to_s,
:availability_event => AvailabilityEvent::EXERCISE_COMPLETE.to_s,
Expand All @@ -97,6 +106,7 @@ def self.new_feedback_condition
FeedbackCondition.new(:label_regex => 'NewFeedback',
:exercise_correctness_option => ExerciseCorrectnessOption::ANY_CORRECTNESS.to_s,
:is_feedback_required_for_credit => false.to_s,
:feedback_viewing_penalty => 0.to_s,
:availability_opens_option => AvailabilityOpensOption::IMMEDIATELY_AFTER_EVENT.to_s,
:availability_closes_option => AvailabilityClosesOption::NEVER.to_s,
:availability_event => AvailabilityEvent::EXERCISE_COMPLETE.to_s,
Expand All @@ -111,6 +121,7 @@ def self.default_feedback_condition
FeedbackCondition.new(:label_regex => 'DefaultFeedback',
:exercise_correctness_option => ExerciseCorrectnessOption::ANY_CORRECTNESS.to_s,
:is_feedback_required_for_credit => false.to_s,
:feedback_viewing_penalty => 0.to_s,
:availability_opens_option => AvailabilityOpensOption::IMMEDIATELY_AFTER_EVENT.to_s,
:availability_closes_option => AvailabilityClosesOption::NEVER.to_s,
:availability_event => AvailabilityEvent::EXERCISE_COMPLETE.to_s,
Expand Down Expand Up @@ -261,6 +272,7 @@ def supply_missing_values
self.label_regex ||= '.*'
self.exercise_correctness_option ||= ExerciseCorrectnessOption::ANY_CORRECTNESS
self.is_feedback_required_for_credit ||= false
self.feedback_viewing_penalty ||= 0
self.availability_opens_option ||= AvailabilityOpensOption::NEVER
self.availability_closes_option ||= AvailabilityClosesOption::NEVER
self.availability_event ||= AvailabilityEvent::NOT_APPLICABLE
Expand Down Expand Up @@ -303,7 +315,7 @@ def schedule_feedback_availability_notification(student_exercise, event)

def adjust_credit(student_exercise, event)
if StudentExercise::Event::COMPLETE == event
student_exercise.update_feedback_credit_multiplier!(is_feedback_required_for_credit ? 0 : 1)
student_exercise.update_feedback_credit_multiplier!(is_feedback_required_for_credit ? feedback_viewing_penalty/100 : 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since feedback_viewing_penalty is an integer, dividing by another integer will do integer division, e.g. 12/100 == 0. To force the division we expect, divide the attribute by a float: feedback_viewing_penalty/100.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Its like I am learning to code again ;) Thanks. Will fix.

end

if StudentExercise::Event::FEEDBACK_VIEWED == event
Expand Down
13 changes: 11 additions & 2 deletions app/views/feedback_conditions/_form_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,19 @@
The assignment event to use when determining feedback availability:
<%= form.select :exercise_correctness_option, exercise_correctness_options %>
</div>
<div class="field">
<div class="field feedback-required">
<%= form.check_box :is_feedback_required_for_credit %>
Students must view the feedback to get credit for the problems.
</div>
<div class="field feedback-viewing-penalty"
<% if not feedback_condition.is_feedback_required_for_credit %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need some JS to display this when the checkbox is clicked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind... unobtrusive JS always gets me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha ha ha - yeah. Good thing is, code like this would soon be history :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The norm in the baseline is to use ! instead of not. Here you can also say

unless feedback_condition.is_feedback_required_for_credit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Will fix.

style="display:none"
<% end %> >
<%= link_to_help 'feedback_viewing_penalty',
text="Penalty",
{:title => 'Penalty'} %> if a student does not view feedback (0-100%):
<%= form.number_field :feedback_viewing_penalty, in:0..100, step:1, size:3 %>
</div>
<div class="field">
<%= form.check_box :can_automatically_show_feedback %>
The site can automatically present students with feedback if other conditions are met. <%#<%= link_to_help 'auto_feedback' %>
Expand Down Expand Up @@ -96,4 +105,4 @@
FeedbackCondition::AvailabilityClosesOption, :DELAY_AFTER_OPEN %>
of <%= form.text_field :availability_closes_delay_days, :size => 4 %> days</p>
</div>
</div>
</div>
3 changes: 2 additions & 1 deletion app/views/feedback_conditions/_show.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<% assignment_event = FeedbackCondition::AvailabilityEvent[feedback_condition.availability_event].to_s.humanize.downcase %>
Expand All @@ -7,6 +7,7 @@
<li>Matches labels against: <%= feedback_condition.label_regex || "[Not set]" %></li>
<li>Matches correctness against: <%= FeedbackCondition::ExerciseCorrectnessOption[feedback_condition.exercise_correctness_option].to_s.humanize %></li>
<li>Feedback viewing is required?: <%= tf_to_yn(feedback_condition.is_feedback_required_for_credit) %></li>
<li>Feedback viewing penalty: <%= feedback_condition.feedback_viewing_penalty || 0 %>%</li>
<li>Can automatically show feedback?: <%= tf_to_yn(feedback_condition.can_automatically_show_feedback) %></li>
<li>Feedback becomes available:
<%=
Expand Down
10 changes: 10 additions & 0 deletions app/views/help/blurbs/_feedback_viewing_penalty.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<%# Copyright 2011-2014 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<%# TODO: @Kim %>

<p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sticking this in.

This field allows you to configure the credit penalty in percentage. For example, if
you set a value of 25%, then students lose 25% of their credit if they fail to view
their feedback on time.
</p>