diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb
index f97d910..541bde0 100644
--- a/app/controllers/answers_controller.rb
+++ b/app/controllers/answers_controller.rb
@@ -2,68 +2,50 @@
after_action :verify_authorized
respond_to :html
- ##
# PUT/PATCH /[:locale]/answer/[:id]
def update
- # create a new answer based off the passed params
- ans_params = params[:answer]
- plan_id = ans_params[:plan_id]
- phase_id = ans_params[:phase_id]
- user_id = ans_params[:user_id]
- lock_version = ans_params[:lock_version]
- question_id = ans_params[:question_id]
+ question_id = params[:answer][:question_id]
+ plan_id = params[:answer][:plan_id]
@question = Question.find(question_id);
# If an answer id is present load that answer otherwise load by plan/question
@answer = Answer.find_by(plan_id: plan_id, question_id: question_id)
- @old_answer, race_on_creation = nil, false
+ @old_answer = nil
+ @timestamp = nil
- # This is the first answer for the question
- if @answer.nil?
+ if @answer.nil? # If there is no answer for plan/question
@answer = Answer.new(permitted_params)
@answer.text = params["answer-text-#{@answer.question_id}".to_sym]
authorize @answer
- @answer.save
-
+ if @answer.save
+ @timestamp = Time.now.iso8601
+ end
@lock_version = @answer.lock_version
+ elsif params[:answer][:id].blank? # Someone else already added an answer while the user was working
+ @old_answer = Marshal::load(Marshal.dump(@answer))
+ @answer.text = params["answer-text-#{@answer.question_id}".to_sym]
- # Someone else already added an answer while the user was working
- elsif ans_params[:id].blank?
- @old_answer = Marshal::load(Marshal.dump(@answer))
- @answer.text = params["answer-text-#{@answer.question_id}".to_sym]
authorize @answer
@lock_version = @answer.lock_version
-
- # We're updating an answer (let ActiveRecord check for a race condition)
- else
- # if you do the obvious clone here it will overwrite the old_answer text
- # in the next line
- #@old_answer = @answer.clone
+ else # We're about updating an answer (let ActiveRecord check for a race condition)
@old_answer = Marshal::load(Marshal.dump(@answer))
@answer.text = params["answer-text-#{@answer.question_id}".to_sym]
+
authorize @answer
- @answer.update(permitted_params)
-
- # The save was successful so get the lock version and nil the
- # old answer
+ if @answer.update(permitted_params)
+ @answer.touch # Saves the record with the updated_at set to the current time. Needed if only answer.question_options is updated
+ @timestamp = @answer.updated_at.iso8601
+ end
@lock_version = @answer.lock_version
@old_answer = nil
end
@section_id = @answer.question.section.id
-
- # these are used for updating the status line
@username = @answer.user.name
- @timestamp = ""
-
- if @answer.text.present?
- @timestamp = @answer.updated_at.iso8601
- end
-
@nquestions = 0
@nanswers = 0
@@ -71,6 +53,7 @@
@n_section_answers = 0
plan = Plan.find(plan_id)
+ # Problem of N+1 queries below
plan.template.phases.each do |phase|
phase.sections.each do |section|
section.questions.each do |question|
@@ -94,14 +77,20 @@
end
rescue ActiveRecord::StaleObjectError
- @username = @old_answer.user.name
- @lock_version = @old_answer.lock_version
- respond_to do |format|
- format.js {}
- end
- end
+ @username = @old_answer.user.name
+ @lock_version = @old_answer.lock_version
+ respond_to do |format|
+ format.js {}
+ end
+ end # End update
+
+ private
def permitted_params
- params.require(:answer).permit(:id, :plan_id, :user_id, :question_id, :lock_version, :question_option_ids)
- end
+ permitted = params.require(:answer).permit(:id, :plan_id, :user_id, :question_id, :lock_version, :question_option_ids => [])
+ if !permitted[:question_option_ids].present? #If question_option_ids has been filtered out because it was a scalar value (e.g. radiobutton answer)
+ permitted[:question_option_ids] = [params[:answer][:question_option_ids]] # then convert to an Array
+ end
+ return permitted
+ end # End permitted_params
end
diff --git a/app/models/answer.rb b/app/models/answer.rb
index 54ac937..8267948 100644
--- a/app/models/answer.rb
+++ b/app/models/answer.rb
@@ -36,4 +36,10 @@
#
# # Make sure the plan and question are associated with the same template!
# validates :plan, :question, answer_for_correct_template: true
+
+ # This method helps to decide if an answer option (:radiobuttons, :checkbox, etc ) in form views should be checked or not
+ # Returns true if the given option_id is present in question_options, otherwise returns false
+ def has_question_option(option_id)
+ self.question_option_ids.include?(option_id)
+ end
end
diff --git a/app/models/question_option.rb b/app/models/question_option.rb
index 84266ab..b0fec82 100644
--- a/app/models/question_option.rb
+++ b/app/models/question_option.rb
@@ -12,6 +12,7 @@
validates :text, :question, :number, presence: {message: _("can't be blank")}
+ scope :by_number, -> { order(:number) }
##
# deep copy the given question_option and all it's associations
#
diff --git a/app/views/answers/update.js.erb b/app/views/answers/update.js.erb
index 40907ae..8086893 100644
--- a/app/views/answers/update.js.erb
+++ b/app/views/answers/update.js.erb
@@ -23,7 +23,7 @@
// update the answer status
var q_status = $("#<%=@question.id%>-status");
-var timestamp = "<%=@timestamp%>";
+var timestamp = "<%= @timestamp.nil? ? "": @timestamp %>";
if( timestamp != "") {
q_status.text("");
diff --git a/app/views/phases/_answer_form.html.erb b/app/views/phases/_answer_form.html.erb
index 6c4eabf..e5408af 100644
--- a/app/views/phases/_answer_form.html.erb
+++ b/app/views/phases/_answer_form.html.erb
@@ -54,23 +54,27 @@
<% if question.option_based? %>
- <% options = question.question_options.order("number") %>
+ <% options = question.question_options.by_number %>
<% if q_format.checkbox? %>
- <%= f.input :options, as: :check_boxes, collection: options, label: false, input_html: { id: "options-#{question.id}" } %>
+
+ <% options.each do |op| %>
+ -
+ <%= f.check_box(:question_option_ids, { multiple: true, checked: answer.has_question_option(op.id) }, op.id, nil) %>
+ <%= raw op.text %>
+
+ <% end %>
+
<% elsif q_format.multiselectbox? %>
<%= f.input :options, as: :select, collection: options, label: false, input_html: { multiple: true , id: "options-#{question.id}" } %>
<% elsif q_format.radiobuttons? %>
<% options.each do |op| %>
- -
- <% if answer.question_option_ids[0] == op.id then%>
- <%= f.radio_button :question_option_ids, op.id, checked: true, id: "answer_option_ids_#{op.id}"%>
- <%else%>
- <%= f.radio_button :question_option_ids, op.id, checked: false, id: "answer_option_ids_#{op.id}"%>
- <% end %>
- <%= raw op.text %>
+ -
+ <%= f.radio_button :question_option_ids, op.id, { checked: answer.has_question_option(op.id), id: "answer_option_ids_#{op.id}" } %>
+ <%= raw op.text %>
+
<% end %>
<% elsif q_format.dropdown? %>