diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb index 51f76b2..7d48ddf 100644 --- a/app/controllers/answers_controller.rb +++ b/app/controllers/answers_controller.rb @@ -2,95 +2,54 @@ after_action :verify_authorized respond_to :html - # PUT/PATCH /[:locale]/answer/[:id] + # PUT/PATCH /answers/[:id] def update - 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 = nil - @timestamp = 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 - - if @answer.save - @timestamp = @answer.updated_at.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] - - authorize @answer - - @lock_version = @answer.lock_version - 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 - - 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 - @username = @answer.user.name - - @nquestions = 0 - @nanswers = 0 - @n_section_questions = 0 - @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| - @nquestions += 1 - if section.id == @section_id - @n_section_questions += 1 - end - question.answers = question.answers.to_a.select {|answer| answer.plan_id == plan.id} - if question.answers.present? && question.answers.first.text.present? - @nanswers += 1 - if section.id == @section_id - @n_section_answers += 1 - end - end + p_params = permitted_params() + @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id], }) + begin + if @answer + authorize @answer + @answer.update(p_params) + if p_params[:question_option_ids].present? + @answer.touch() # Saves the record with the updated_at set to the current time. Needed if only answer.question_options is updated end + else + @answer = Answer.new(p_params) + @answer.lock_version = 1 + authorize @answer + @answer.save() # NOTE, there is a chance to create multiple answer associated for a plan/question (IF any concurrent thread) INSERTS an answer after checking the existence of an answer (Line 8) + # In order to avoid that edge-case, it is recommended to create answers whenever a new plan is created (e.g. after_create callback) end + rescue ActiveRecord::StaleObjectError + @stale_answer = @answer + @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id]}) end + + @plan = Plan.includes({ + sections: { + questions: [ + :answers, + :question_format + ] + } + }).find(p_params[:plan_id]) + @question = @answer.question + @section = @plan.get_section(@question.section_id) respond_to do |format| format.js {} end - - rescue ActiveRecord::StaleObjectError - @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 - 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 = params.require(:answer).permit(:id, :text, :plan_id, :user_id, :question_id, :lock_version, :question_option_ids => []) + if !params[:answer][:question_option_ids].nil? && !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 + if !permitted[:id].present? + permitted.delete(:id) + end return permitted end # End permitted_params end diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 99406e9..b9d9685 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -15,24 +15,6 @@ @phase = @plan.template.phases.select {|p| p.id == phase_id}.first @readonly = !@plan.editable_by?(current_user.id) - # the eager_load pulls in ALL answers - # need to restrict to just ones for this plan - # at the same time count the questions and answers for the status - @nquestions = 0 - @nanswers = 0 - - @plan.template.phases.each do |phase| - phase.sections.each do |section| - section.questions.each do |question| - @nquestions += 1 - question.answers = question.answers.to_a.select {|answer| answer.plan_id == @plan.id} - if question.answers.present? && question.answers.first.text.present? - @nanswers += 1 - end - end - end - end - # Now we need to get all the themed guidance for the plan. # TODO: think this through again, there may be a better way to do this. # diff --git a/app/models/answer.rb b/app/models/answer.rb index 8267948..d46ad59 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -42,4 +42,18 @@ def has_question_option(option_id) self.question_option_ids.include?(option_id) end + + # Returns true if the answer is valid and false otherwise. If the answer's question is option_based, it is checked if exist + # any question_option selected. For non option_based (e.g. textarea or textfield), it is checked the presence of text + def is_valid? + if self.question.present? + if self.question.question_format.option_based? + return !self.question_options.empty? + else # (e.g. textarea or textfield question formats) + return self.text.present? + end + else + return false + end + end end diff --git a/app/models/plan.rb b/app/models/plan.rb index 549b4fe..5db4cde 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -949,8 +949,29 @@ end =end + # Returns the number of answered questions from the entire plan + def num_answered_questions + n = 0 + self.sections.each do |s| + n+= s.num_answered_questions(self.id) + end + return n + end + # Returns a section given its id or nil if does not exist for the current plan + def get_section(section_id) + self.sections.find { |s| s.id == section_id } + end + # Returns the number of questions for a plan. Note, this method becomes useful + # for when sections and their questions are eager loaded so that avoids SQL queries. + def num_questions + n = 0 + self.sections.each do |s| + n+= s.questions.size() + end + return n + end # the following two methods are for eager loading. One gets used for the plan/show # page and the oter for the plan/edit. The difference is just that one pulls in more than # the other. diff --git a/app/models/section.rb b/app/models/section.rb index abf1327..4d4562b 100644 --- a/app/models/section.rb +++ b/app/models/section.rb @@ -23,10 +23,11 @@ "#{title}" end + # Returns the number of answered questions for a given plan id def num_answered_questions(plan_id) n = 0 self.questions.each do |question| - n += question.plan_answers(plan_id).select{|answer| answer.text.present?}.count + n += question.plan_answers(plan_id).select{|answer| answer.is_valid?}.count end return n end diff --git a/app/views/answers/_form_fields.html.erb b/app/views/answers/_form_fields.html.erb new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/app/views/answers/_form_fields.html.erb diff --git a/app/views/answers/_locking.html.erb b/app/views/answers/_locking.html.erb new file mode 100644 index 0000000..0bc9cc2 --- /dev/null +++ b/app/views/answers/_locking.html.erb @@ -0,0 +1,5 @@ +
+

<%= _('The following answer cannot be persisted') %>

+ <%= render partial: '/answers/new_edit', locals: { question: question, answer: answer, readonly: true } %> +

<%= _('since %{name} saved the answer below while you were editing. Please, combine your changes and then save the answer again.') % { name: user.name} %>

+
diff --git a/app/views/answers/_new_edit.html.erb b/app/views/answers/_new_edit.html.erb new file mode 100644 index 0000000..54fa9b4 --- /dev/null +++ b/app/views/answers/_new_edit.html.erb @@ -0,0 +1,101 @@ + +<% q_format = question.question_format %> +<%= semantic_form_for answer, :url => {controller: :answers, action: :update }, html: {method: "put", class: "roadmap-form"}, remote: true do |f| %> +
+ <% if !readonly %> + <%= f.input :id, as: :hidden, input_html: { value: answer.id } %> + <%= f.input :plan_id, as: :hidden, input_html: { value: answer.plan_id } %> + <%= f.input :user_id, as: :hidden, input_html: { value: answer.user_id } %> + <%= f.input :question_id, as: :hidden, input_html: { value: answer.question_id } %> + <%= f.input :lock_version, as: :hidden, input_html: { value: answer.lock_version } %> + <% end %> + + +

+ <%= raw question.text %> +

+ + + <% if !readonly && question.annotations.where(type: Annotation.types[:example_answer]).any? %> + <% annotation = question.annotations.where(type: Annotation.types[:example_answer]).order(:created_at).first %> + <% if annotation.text.present? %> +
+ + <%="#{annotation.org.abbreviation} "%> <%=_('Example of answer')%> + + +
+

+ <%= raw annotation.text %> +

+
+
+ <% end %> + <% end %> + + <% if question.option_based? %> + <% options = question.question_options.by_number %> + <% if q_format.checkbox? %> +
    + <% options.each do |op| %> +
  1. + <%= f.check_box(:question_option_ids, { multiple: true, checked: answer.has_question_option(op.id), disabled: readonly }, op.id, nil) %> + <%= raw op.text %> +
  2. + <% end %> +
+ <% elsif q_format.radiobuttons? %> +
    + <% options.each do |op| %> +
  1. + <%= f.radio_button :question_option_ids, op.id, { checked: answer.has_question_option(op.id), id: "answer_option_ids_#{op.id}", disabled: readonly } %> + <%= raw op.text %> +
  2. + <% end %> +
+ <% elsif q_format.dropdown? || q_format.multiselectbox? %> + <% + options_html = "" + options.each do |op| + options_html += answer.has_question_option(op.id) ? + "" : + "" + end + %> + <%= select_tag('answer[question_option_ids]', raw(options_html), + {multiple: q_format.multiselectbox?, include_blank: q_format.dropdown?, disabled: readonly }) %> + <% end %> + + <% if question.option_comment_display == true %> + <%= label_tag('answer[text]', _('Comment')) %> + <% if readonly %> +

<%= raw(answer.text) %>

+ <% else %> + <%= text_area_tag('answer[text]', answer.text, id: "answer-text-#{question.id}") %> + <%= tinymce(selector: "#answer-text-#{question.id}", setup: "$.fn.change_answer") %> + <% end %> + <%end%> + <% end %> + + <% if q_format.textfield? %> + <% if readonly %> +

<%= strip_tags(answer.text) %>

+ <% else %> + <%= text_field_tag('answer[text]', strip_tags(answer.text)) %> + <% end %> + <% elsif q_format.textarea? %> + <% if readonly %> +

<%= raw(answer.text) %>

+ <% else %> + <%= text_area_tag('answer[text]', answer.text, id: "answer-text-#{question.id}") %> + <%= tinymce(selector: "#answer-text-#{question.id}", setup: "$.fn.change_answer") %> + <% end %> + <% end %> + + <% if !readonly %> + + <% end %> +
+ <% end %> \ No newline at end of file diff --git a/app/views/answers/_status.html.erb b/app/views/answers/_status.html.erb index e1c8d8b..ddf8dcf 100644 --- a/app/views/answers/_status.html.erb +++ b/app/views/answers/_status.html.erb @@ -3,7 +3,7 @@
<% if answer.updated_at.blank? %> - <%= _('Not answered yet') %> + <%= _('Not answered yet') %> <% else %> <%= _('Answered')%> <%= answer.updated_at.iso8601 %><%= _(' by')%> <%= answer.user.name %> <% end %> \ No newline at end of file diff --git a/app/views/answers/update.js.erb b/app/views/answers/update.js.erb index ec06441..2290a67 100644 --- a/app/views/answers/update.js.erb +++ b/app/views/answers/update.js.erb @@ -1,29 +1,26 @@ -// after saving the answer (and possibly getting a conflict) -// set the message div about the answer. -// On success this will be "" on error it will be the -// conflicting answer - -<% if @old_answer %> - $("#answer_notice_<%=@question.id%>").html("<%= escape_javascript(render partial: '/phases/answer', locals: { question: @question, answer: @old_answer}) %>"); +// partial /answers/locking +<% if @stale_answer %> + $("#answer-locking-<%= @question.id%>") + .html("<%= escape_javascript(render partial: '/answers/locking', locals: { question: @question, answer: @stale_answer, user: @answer.user }) %>"); <% else %> - $("#answer_notice_<%=@question.id%>").html(""); + $("#answer-locking-<%= @question.id%>").html(""); <% end %> -// have to update the lock_version on success or failure -// so that the next save can work. If you don't do -// this it will fail forever. Also update the answer id +// partial /answer/new_edit +if(tinymce) + tinymce.remove("#answer-text-<%= @question.id %>"); +$("#answer-form-<%= @question.id%>") + .html("<%= escape_javascript(render partial: '/answers/new_edit', locals: { question: @question, answer: @answer, readonly: false }) %>"); -$("#answer_lock_version-<%=@question.id%>").val(<%= @lock_version %>); -$("#question-form-<%= @question.id %> #answer_id").val(<%= @answer.id %>); - -// update the answer status +// partial /answer/status $("#answer-status-<%= @question.id %>") - .html("<%= escape_javascript(render partial: '/answers/status', locals: { question_id: @question.id, answer: @answer}) %>"); + .html("<%= escape_javascript(render partial: '/answers/status', locals: { answer: @answer}) %>"); if($.fn.init_answer_status) $.fn.init_answer_status(); -// update plan progress bar -$(".progress").html("<%= escape_javascript(render :partial => '/plans/progress', locals: {nquestions: @nquestions, nanswers: @nanswers}) %>"); +// partial /plans/progress +$(".progress").html("<%= escape_javascript(render :partial => '/plans/progress', locals: { plan: @plan }) %>"); -// update the section progress message -$("#<%=@section_id%>-status").html("(<%=@n_section_questions%> <%= _('questions') %>, <%=@n_section_answers%> <%= _('answered') %>)") +// partial /sections/progress +$("#section-progress-<%= @section.id %>") + .html("<%= escape_javascript(render partial: '/sections/progress', locals: { section: @section, plan: @plan }) %>"); \ No newline at end of file diff --git a/app/views/phases/_add_note.html.erb b/app/views/phases/_add_note.html.erb index 51e81e2..5a20c05 100644 --- a/app/views/phases/_add_note.html.erb +++ b/app/views/phases/_add_note.html.erb @@ -18,7 +18,7 @@
<%= label_tag "#{questionid}new_note_text", _('Share note with collaborators') %> <%= text_area_tag "#{questionid}new_note_text", nil, class: "tinymce" %> - <%= tinymce :content_css => asset_path("application.css"), :setup => "function(editor){editor.on('change', function(e){$.fn.check_textarea(editor)});}" %> + <%= tinymce %>
<% end %> diff --git a/app/views/phases/_answer.html.erb b/app/views/phases/_answer.html.erb deleted file mode 100644 index 1b3106c..0000000 --- a/app/views/phases/_answer.html.erb +++ /dev/null @@ -1,77 +0,0 @@ - - -<% qformat = question.question_format %> - - - -

<%= _("While you were editing ") + answer.user.name + _(" saved the following answer:") %>

- -<%= semantic_form_for answer, :url => {:controller => :answers, :action => :update }, :remote => true do |af| %> - <%= af.inputs do %> - - - <% if question.option_based? %> - <% options = question.options.order("number") %> - - - <% if qformat.checkbox? %> - <%= af.input :options, - :as => :check_boxes, - :collection => options, - :label => false, - :input_html => { :disabled => :true } - %> - <% elsif qformat.multiselectbox? %> - <%= af.input :options, - :as => :select, - :collection => options, - :label => false, - :input_html => { :multiple => true , :disabled => :true } %> - <% elsif qformat.radiobuttons?%> -
    - <% options.each do |op| %> -
  1. - <% checked = (answer.option_ids[0] == op.id) %> - <%= af.radio_button :option_ids, op.id, :checked => checked, id: "answer_option_ids_#{op.id}"%> - <%= raw op.text %> -
  2. - <% end %> -
- <% elsif qformat.dropdown? %> - <%= af.input :options, - :as => :select, - :collection => options, - :label => false, - :input_html => { :multiple => false, :disabled => :true } - %> - <% end %> - - - <% if question.option_comment_display %> - <%= label_tag("answer-text-Ans#{question.id}".to_sym, _('Comment')) %> - <%= text_area_tag("answer-text-Ans#{question.id}".to_sym, answer.text, class: "tinymce answer-notice") %> - <%end%> - <% end %> - - <% if qformat.textfield? %> - <%= text_field_tag("answer-text-Ans#{question.id}".to_sym, strip_tags(answer.text), class: "question_text_field", disabled: true) %> - <% elsif qformat.textarea? %> - <%= text_area_tag("answer-text-Ans#{question["id"]}".to_sym, strip_tags(answer.text), class: "tinymce answer-notice", disabled: true) %> - <% end %> - - <% end %> -<% end %> - -

<%= _('Combine their changes with your answer below and then save the answer again.') %>
<%= _('The edits in the box below will overwrite the existing answer from ') + answer.user.name + _(' once you click save!')%>

- - diff --git a/app/views/phases/_answer_form.html.erb b/app/views/phases/_answer_form.html.erb index 9d7dcd8..4a1028f 100644 --- a/app/views/phases/_answer_form.html.erb +++ b/app/views/phases/_answer_form.html.erb @@ -6,134 +6,34 @@ -->
- <% answers = question.plan_answers(plan.id) if answers.present? answer = answers.first else - answer = Answer.new + answer = Answer.new({ plan_id: plan.id, question_id: question.id, user_id: current_user.id }) end - question_id = question.id - q_format = question.question_format %> - -
- <%= semantic_form_for answer, :url => {controller: :answers, action: :update }, html: {method: "put", class: "roadmap-form"}, remote: true do |f| %> -
- <%= f.input :id, as: :hidden, input_html: { value: answer.id } %> - <%= f.input :plan_id, as: :hidden, input_html: { value: @plan.id } %> - <%= f.input :user_id, as: :hidden, input_html: { value: current_user.id } %> - <%= f.input :question_id, as: :hidden, input_html: { value: question_id, class: "question_id" } %> - <%= f.hidden_field :lock_version, id: "answer_lock_version-#{question_id}" %> - - - - <%= raw question.text %> - - - <% if question.annotations.where(type: Annotation.types[:example_answer]).any? %> - <% annotation = question.annotations.where(type: Annotation.types[:example_answer]).order(:created_at).first %> - <% if annotation.text.present? %> -
- - <%="#{annotation.org.abbreviation} "%> <%=_('example answer')%> - - -
-

- <%= raw annotation.text %> -

-
-
- <% end %> - <% end %> - - -
- - - <% if question.option_based? %> - <% options = question.question_options.by_number %> - - - <% if q_format.checkbox? %> -
    - <% options.each do |op| %> -
  1. - <%= f.check_box(:question_option_ids, { multiple: true, checked: answer.has_question_option(op.id), disabled: readonly }, op.id, nil) %> - <%= raw op.text %> -
  2. - <% end %> -
- <% elsif q_format.radiobuttons? %> -
    - <% options.each do |op| %> -
  1. - <%= f.radio_button :question_option_ids, op.id, { checked: answer.has_question_option(op.id), id: "answer_option_ids_#{op.id}", disabled: readonly } %> - <%= raw op.text %> -
  2. - <% end %> -
- <% elsif q_format.dropdown? || q_format.multiselectbox? %> - <% - options_html = "" - options.each do |op| - options_html += answer.has_question_option(op.id) ? - "" : - "" - end - %> - <%= select_tag('answer[question_option_ids]', raw(options_html), - {multiple: q_format.multiselectbox?, include_blank: q_format.dropdown?, disabled: readonly }) %> - <% end %> - - - - <% if question.option_comment_display == true %> - <%= label_tag("answer-text-#{question.id}".to_sym, _('Comment')) %> - <% if readonly %> -

<%= raw(answer.text) %>

- <% else %> - <%= text_area_tag("answer-text-#{question.id}".to_sym, answer.text, class: "tinymce") %> - <% end %> - <%end%> - <% end %> - - <% if q_format.textfield? %> - <% if readonly %> -

<%= strip_tags(answer.text) %>

- <% else %> - <%= text_field_tag("answer-text-#{question_id}".to_sym, strip_tags(answer.text), { class: "question_text_field" }) %> - <% end %> - <% elsif q_format.textarea? %> - <% if readonly %> -

<%= raw(answer.text) %>

- <% else %> - <%= text_area_tag("answer-text-#{question_id}".to_sym, answer.text, class: "tinymce") %> - <% end %> - <% end %> - - <% if !readonly %> - - <% end %> -
-
"> - <%= render(partial: 'answers/status', locals: { question_id: question_id, answer: answer }) %> -
- <% end %> +
+
">
+
"> + <%= render(partial: 'answers/new_edit', locals: { question: question, answer: answer, readonly: readonly }) %> +
+
" class="answer-status"> + <%= render(partial: 'answers/status', locals: { answer: answer }) %> +
-
+
<% comments = answer.notes.all %> - <%= hidden_field_tag :question_id, question_id, class: "question_id" %> + <%= hidden_field_tag :question_id, question.id, class: "question_id" %>
    <% annotations = question.annotations.where(type: Annotation.types[:guidance]) %> - <% if annotations.present? || question_guidances[question_id] %> + <% if annotations.present? || question_guidances[question.id] %> <% css_style_comment_div = "display: none;"%> <% css_style_guidance_div = ""%> @@ -143,9 +43,9 @@
  • <% if comments.count > 0%> <% comments_label_with_count = "#{_('Notes')} (#{comments.count})"%> - <%= link_to comments_label_with_count , "#", id: "notes_number_#{question_id}", class: "comments_accordion_button" %> + <%= link_to comments_label_with_count , "#", id: "notes_number_#{question.id}", class: "comments_accordion_button" %> <% else %> - <%= link_to _('Share note'), "#", id: "notes_number_#{question_id}", class: "comments_accordion_button" %> + <%= link_to _('Share note'), "#", id: "notes_number_#{question.id}", class: "comments_accordion_button" %> <% end %>
  • <% else %> @@ -155,9 +55,9 @@
  • <% if comments.count > 0 %> <% comments_label_with_count = "#{_('Notes')} (#{comments.count})"%> -

    <%= comments_label_with_count %>

    +

    <%= comments_label_with_count %>

    <% else %> -

    <%= _('Share note') %>

    +

    <%= _('Share note') %>

    <% end %>
  • <% end %> @@ -166,8 +66,8 @@ -
    -
    +
    +
    <% if annotations.present? %> @@ -177,7 +77,7 @@ -
    +
    <%= raw annotation.text %>
    @@ -199,13 +99,13 @@ <% group.each do |gobj| %>
    -
    +
    <%= raw gobj[:text] %>
    <% guidance_accordion_id += 1 %> @@ -216,13 +116,13 @@
    -
    +
    <%= render partial: "note", locals: {question: question, answer: answer, plan: plan, suffix: "" }%>
    -<% if last_question_id == question_id then %> +<% if last_question_id == question.id then %>
    <% else %>
    diff --git a/app/views/phases/admin_add.html.erb b/app/views/phases/admin_add.html.erb index a5141aa..449ef51 100644 --- a/app/views/phases/admin_add.html.erb +++ b/app/views/phases/admin_add.html.erb @@ -50,6 +50,7 @@
    <%= text_area_tag("phase-desc","" , class: "tinymce") %> + <%= tinymce %>
    <%= link_to( image_tag("help_button.png"), "#", class: "phase_desc_popover", rel: "popover", "data-html" => "true", "data-content" => _("Enter a basic description. This will be presented to users on the 'Admin Plan' tab, above the summary of the sections and questions which they will be asked to answer."))%> @@ -70,6 +71,4 @@
    -
    - -<%= tinymce content_css: asset_path("application.css") %> \ No newline at end of file +
    \ No newline at end of file diff --git a/app/views/phases/admin_show.html.erb b/app/views/phases/admin_show.html.erb index e27f41e..321ac60 100644 --- a/app/views/phases/admin_show.html.erb +++ b/app/views/phases/admin_show.html.erb @@ -2,7 +2,7 @@ <%= stylesheet_link_tag "admin" %> <% javascript 'admin.js' %> -<%= tinymce content_css: asset_path('application.css') %> +<%= tinymce %>

    <%= @phase.template.title %> diff --git a/app/views/phases/edit.html.erb b/app/views/phases/edit.html.erb index 9c73d91..8ddbcc2 100644 --- a/app/views/phases/edit.html.erb +++ b/app/views/phases/edit.html.erb @@ -11,7 +11,7 @@
    - <%= render :partial => "/plans/progress", locals: {nquestions: @nquestions, nanswers: @nanswers} %> + <%= render :partial => "/plans/progress", locals: { plan: @plan } %>
    @@ -34,26 +34,11 @@ <% end%>
    - - - <% num_section_questions = section.questions.to_a.count %> - <% num_section_answers = section.num_answered_questions(@plan.id) %> - <% question_word = "questions" %> - <% if num_section_questions == 1 then %> - <% question_word = "question" %> - <% end %> - <% section_status = "#{num_section_questions} #{question_word}, #{num_section_answers} answered" %> -
    <%= _('Export') %> diff --git a/app/views/sections/_progress.html.erb b/app/views/sections/_progress.html.erb new file mode 100644 index 0000000..7617def --- /dev/null +++ b/app/views/sections/_progress.html.erb @@ -0,0 +1,14 @@ + + <% num_section_questions = section.questions.size() %> + <% num_section_answers = section.num_answered_questions(plan.id) %> + <% question_word = "questions" %> + <% if num_section_questions == 1 then %> + <% question_word = "question" %> + <% end %> + <% section_status = "#{num_section_questions} #{question_word}, #{num_section_answers} answered" %> + <%= section.title %> + <% if num_section_questions.to_i > num_section_answers.to_i then %> + (<%= section_status %>) + <% else %> + (<%= section_status %>) + <% end %> \ No newline at end of file diff --git a/config/tinymce.yml b/config/tinymce.yml index bc00326..2b2bb03 100644 --- a/config/tinymce.yml +++ b/config/tinymce.yml @@ -1,3 +1,5 @@ +content_css: /assets/application.css +selector: 'textarea.tinymce' statusbar: false menubar: false toolbar: bold italic | bullist numlist | link | table diff --git a/lib/assets/javascripts/answers/status.js b/lib/assets/javascripts/answers/status.js index 3358bd9..ebf5165 100644 --- a/lib/assets/javascripts/answers/status.js +++ b/lib/assets/javascripts/answers/status.js @@ -2,14 +2,24 @@ $(document).ready(function(){ $("form.answer").submit(function(){ - var saving = $(this).find('.saving-message'); + var container = $(this).closest('.question-form'); + var saving = container.find('.saving-message'); saving.show(); }); $("form.answer fieldset input, form.answer fieldset select").change(function(){ - var unsaved = $(this).closest('form.answer').find('.answer-unsaved'); + var unsaved = $(this).closest('.question-form').find('.answer-unsaved'); unsaved.show(); + var notAnswered = $(this).closest('.question-form').find('.not-answered'); + notAnswered.hide(); }); - // TODO An adequate listener for textarea (e.g. tinymce) that triggers unsaved.show(). Temporary workaround defined at $.fn.toggle_dirty (plans.js) + $.fn.change_answer = function(editor){ + editor.on('change', function(event){ + var unsaved = $('#'+editor.id).closest('.question-form').find('.answer-unsaved'); + unsaved.show(); + var notAnswered = $('#'+editor.id).closest('.question-form').find('.not-answered'); + notAnswered.hide(); + }); + } $.fn.init_answer_status = function() { $('abbr.timeago').timeago(); } diff --git a/lib/assets/javascripts/plans.js b/lib/assets/javascripts/plans.js index 86acb77..5837312 100644 --- a/lib/assets/javascripts/plans.js +++ b/lib/assets/javascripts/plans.js @@ -353,6 +353,7 @@ }; $.fn.toggle_dirty = function(question_id, is_dirty) { + console.log($(this)); section_id = $(this).attr("id").split('-')[0]; if (dirty[section_id] == null) { dirty[section_id] = {}; @@ -360,17 +361,8 @@ dirty[section_id][question_id] = is_dirty; if (is_dirty) { $("#"+question_id+"-unsaved").show(); - $("#answer-status-"+question_id).find('.answer-unsaved').show(); // Temporary workaround ONLY triggered when textarea format type changes } else { $("#"+question_id+"-unsaved").hide(); } }; - - - - -// this is the function used to hanndle the interface between tinymce and the Dirty stuff -$.fn.check_textarea = function(editor) { - $("#"+editor.id).closest(".accordion-group").find(".section-status:first").toggle_dirty(editor.id.split('-')[2], editor.isDirty()); -}; diff --git a/lib/assets/stylesheets/bootstrap_and_overrides.css.less b/lib/assets/stylesheets/bootstrap_and_overrides.css.less index f083ce1..11fcd58 100644 --- a/lib/assets/stylesheets/bootstrap_and_overrides.css.less +++ b/lib/assets/stylesheets/bootstrap_and_overrides.css.less @@ -474,6 +474,7 @@ div.answer_notice { background-color: #fee; margin-bottom: 4px; + padding: 5px; } diff --git a/lib/assets/stylesheets/roadmap-form.scss b/lib/assets/stylesheets/roadmap-form.scss index 72345a1..5a860ae 100644 --- a/lib/assets/stylesheets/roadmap-form.scss +++ b/lib/assets/stylesheets/roadmap-form.scss @@ -45,6 +45,7 @@ /* Fieldset with labels over inputs */ fieldset.standard { + padding: 5px; background-color: $white; margin-bottom: 25px; diff --git a/test/functional/answers_controller_test.rb b/test/functional/answers_controller_test.rb index 1d5a3eb..a659295 100644 --- a/test/functional/answers_controller_test.rb +++ b/test/functional/answers_controller_test.rb @@ -27,30 +27,33 @@ plan.reload referrer = "/#{FastGettext.locale}/plans/#{plan.id}/phases/#{question.section.phase.id}/edit" - - answer = Answer.create(user: @user, plan: plan, question: question, - text: "#{format.title} Tester") if format.option_based else # Try creating one first - form_attributes = {"answer-text-#{question.id}": "#{format.title} Tester", - answer: {user_id: @user.id, plan_id: plan.id, - question_id: question.id}} + form_attributes = { + answer: {user_id: @user.id, + plan_id: plan.id, + question_id: question.id, + text: "#{format.title} Tester", + lock_version: 0} + } - put_answer(answer, form_attributes, referrer) + put_answer(Answer.new(), form_attributes, referrer) answer = Answer.find_by(user: @user, plan: plan, question: question) assert_not answer.id.nil?, "expected the answer to have been created and for an id to be present after creating a #{format.title} question!" # Try editing it - form_attributes = {"answer-text-#{question.id}": "Tested", + form_attributes = { answer: {id: answer.id, user_id: answer.user.id, plan_id: answer.plan.id, question_id: answer.question.id, - lock_version: answer.lock_version}} + text: "Tested", + lock_version: answer.lock_version} + } put_answer(answer, form_attributes, referrer) @@ -71,7 +74,6 @@ assert_response :success assert_equal "text/javascript", @response.content_type - # last line of JS updates section status with X questions, Y answered - assert_match /status"\).html\("\([0-9]+ questions, [0-9]+ answered/, @response.body + assert_match(/[^\$]*\$\("#answer-locking-[0-9]+"\).html\(""\);[^\$]*\$\("#answer-form-[0-9]+"\)[^\.]*.html\(".+"\);[^\$]*\$\("#answer-status-[0-9]+"\)[^.]*.html\(".+"\);[^\$]*\$.[^$]*\$.[^\$]*\$\(".progress"\).html\(".+"\);[^\$]*\$\("#section-progress-[0-9]+"\)[^.]*.html\(".+"\);/, @response.body) end end diff --git a/test/integration/answer_locking_test.rb b/test/integration/answer_locking_test.rb index e083dda..824e3f7 100644 --- a/test/integration/answer_locking_test.rb +++ b/test/integration/answer_locking_test.rb @@ -33,10 +33,10 @@ assert_equal "Initial answer - by UserA", updated.text assert_equal @plan.owner.id, updated.user_id - # Make sure the answer-notice is NOT displayed - assert_not @response.body.include?(_('Combine their changes with your answer below and then save the answer again.')), "expected there to be no lock error messaging" - assert @response.body.include?("#{_('by')} #{@plan.owner.name}"), "expected the messaging to say the plan was updated by the plan owner" - assert @response.body.include?(_('answered')), "expected the messaging to include the status" + # Make sure the answers/locking partial is NOT displayed + assert_not @response.body.include?(_('The following answer cannot be persisted')), "expected there to be no lock error messaging" + assert @response.body.include?(_('Answered')) + assert @response.body.include?("#{_(' by')} #{@plan.owner.name}"), "expected the messaging to say the plan was updated by the plan owner" # Signin as UserB and try to insert the new answer but fail sign_in @collaborator @@ -48,9 +48,9 @@ assert_equal @plan.owner.id, updated.user_id # Make sure the answer-notice IS displayed - assert @response.body.include?(_('Combine their changes with your answer below and then save the answer again.')), "expected there to be lock error messaging" - assert @response.body.include?("#{_('by')} #{@plan.owner.name}"), "expected the messaging to STILL say the plan was updated by the plan owner" - assert @response.body.include?(_('answered')), "expected the messaging to include the status" + assert @response.body.include?(_('The following answer cannot be persisted')), "expected there to be lock error messaging" + assert @response.body.include?(_('since %{name} saved the answer below while you were editing. Please, combine your changes and then save the answer again.') % { name: @plan.owner.name}), "expected the messaging to STILL say the plan was updated by the plan owner" + assert @response.body.include?(_('Answered')), "expected the messaging to include the status" end # ---------------------------------------------------------- @@ -70,10 +70,10 @@ assert_equal "Initial answer - by UserA - Updated by userA", updated.text assert_equal @plan.owner.id, updated.user_id - # Make sure the answer-notice is NOT displayed - assert_not @response.body.include?(_('Combine their changes with your answer below and then save the answer again.')), "expected there to be no lock error messaging" - assert @response.body.include?("#{_('by')} #{@plan.owner.name}"), "expected the messaging to say the plan was updated by the plan owner" - assert @response.body.include?(_('answered')), "expected the messaging to include the status" + # Make sure the answers/locking partial is NOT displayed + assert_not @response.body.include?(_('The following answer cannot be persisted')), "expected there to be no lock error messaging" + assert @response.body.include?(_('Answered')) + assert @response.body.include?("#{_(' by')} #{@plan.owner.name}"), "expected the messaging to say the plan was updated by the plan owner" # Signin as UserB and try to insert the new answer but fail sign_in @collaborator @@ -87,19 +87,20 @@ assert_equal @plan.owner.id, updated.user_id # Make sure the answer-notice IS displayed - assert @response.body.include?(_('Combine their changes with your answer below and then save the answer again.')), "expected there to be lock error messaging" - assert @response.body.include?("#{_('by')} #{@plan.owner.name}"), "expected the messaging to STILL say the plan was updated by the plan owner" - assert @response.body.include?(_('answered')), "expected the messaging to include the status" + assert @response.body.include?(_('The following answer cannot be persisted')), "expected there to be lock error messaging" + assert @response.body.include?(_('since %{name} saved the answer below while you were editing. Please, combine your changes and then save the answer again.') % { name: @plan.owner.name}), "expected the messaging to STILL say the plan was updated by the plan owner" + assert @response.body.include?(_('Answered')), "expected the messaging to include the status" end # ---------------------------------------------------------- private def obj_to_params(attributes) - {"answer-text-#{attributes['question_id']}": "#{attributes['text']}", + { answer: {id: attributes['id'], user_id: attributes['user_id'], plan_id: attributes['plan_id'], question_id: attributes['question_id'], + text: attributes['text'], lock_version: attributes['lock_version']} } end