v0.3.5

Gemfile

@@ -72,7 +72,7 @@ gem 'formtastic'
 gem 'wkhtmltopdf-binary'
 gem 'thin'
 gem 'wicked_pdf'
-gem 'htmltoword'
+gem 'htmltoword', '>= 0.7'
 gem 'feedjira'
 gem 'yaml_db', :git => 'https://github.com/vyruss/yaml_db.git'
 

Gemfile.lock

@@ -142,7 +142,7 @@ GEM
       activesupport (>= 4.1.0)
     hashdiff (0.3.0)
     hashie (3.4.6)
-    htmltoword (0.5.1)
+    htmltoword (0.7.0)
       actionpack
       nokogiri
       rubyzip (>= 1.0)
@@ -282,7 +282,7 @@ GEM
       railties (>= 4.2.0, < 5.1)
     rolify (5.1.0)
     ruby-progressbar (1.8.1)
-    rubyzip (1.2.0)
+    rubyzip (1.2.1)
     safe_yaml (1.0.4)
     sass (3.4.22)
     sass-rails (5.0.6)
@@ -368,7 +368,7 @@ DEPENDENCIES
   gettext (>= 3.0.2)
   gettext_i18n_rails (~> 1.8)
   gettext_i18n_rails_js (~> 1.2.0)
-  htmltoword
+  htmltoword (>= 0.7)
   i18n-js (>= 3.0.0.rc11)
   jbuilder
   jquery-rails

ISSUE_TEMPLATE.md

@@ -0,0 +1,7 @@
+Please complete the following fields as applicable:
+
+**Expected behaviour:**
+
+**Actual behaviour:**
+
+**Steps to reproduce:**

PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,4 @@
+Fixes # .
+
+Changes proposed in this PR:
+-

app/controllers/answers_controller.rb

@@ -3,31 +3,33 @@ class AnswersController < ApplicationController
   respond_to :html
 
 	# PUT/PATCH /answers/[:id]
-	def update
+  def update
     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
+    Answer.transaction do
+      @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id]})
+      begin
+        if @answer.present?
+          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
+          # NOTE: save! and destroy! must be used for transactions as they raise errors instead of returning false
+          @answer.save!
         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)
+      rescue ActiveRecord::StaleObjectError
+        @stale_answer = @answer
+        @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id]})
       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: [ 
+      sections: {
+        questions: [
           :answers,
           :question_format
         ]
@@ -37,7 +39,7 @@ def update
     @section = @plan.get_section(@question.section_id)
 
     respond_to do |format|
-      format.js {} 
+      format.js {}
     end
   end # End update
 

app/controllers/notes_controller.rb

@@ -8,22 +8,25 @@ def create
     @note = Note.new
     user_id = params[:new_note][:user_id]
     @note.user_id = user_id
-    answer_id = params[:new_note][:answer_id]
     question_id = params[:new_note][:question_id]
     plan_id = params[:new_note][:plan_id]
 
     # create answer if we dont already have one
-    if answer_id.present?
-      answer = Answer.find(answer_id)
-    else
-      answer = Answer.new
-      answer.plan_id = plan_id
-      answer.question_id = question_id
-      answer.user_id = user_id
-      answer.save!
+    answer=nil  # if defined within transaction block, was not accessable after
+    # ensure user has access to plan BEFORE creating/finding an answer
+    raise Pundit::NotAuthorizedError unless Plan.find(plan_id).readable_by?(user_id)
+    Answer.transaction do
+      answer = Answer.find_by(question_id: question_id, plan_id: plan_id)
+      if answer.blank?
+        answer = Answer.new
+        answer.plan_id = plan_id
+        answer.question_id = question_id
+        answer.user_id = user_id
+        answer.save!
+      end
     end
 
-    @note.answer= answer
+    @note.answer = answer
     @note.text = params["#{question_id}new_note_text"]
 
     authorize @note

app/controllers/plans_controller.rb

@@ -66,7 +66,7 @@ def create
                                                      published: true)
         if !ggs.blank? then @plan.guidance_groups << ggs end
 
-        default = Template.find_by(is_default: true)
+        default = Template.default
 
         msg = "#{_('Plan was successfully created.')} "
 
@@ -404,7 +404,7 @@ def template_options(org_id, funder_id)
     # If no templates were available use the generic templates
     if @templates.empty?
       @msg = _("Using the generic Data Management Plan")
-      @templates << Template.where(is_default: true, published: true).first
+      @templates << Template.default
     end
 
     @templates = @templates.sort{|x,y| x.title <=> y.title } if @templates.count > 1

app/models/template.rb

@@ -17,7 +17,7 @@ class Template < ActiveRecord::Base
   ##
   # Possibly needed for active_admin
   #   -relies on protected_attributes gem as syntax depricated in rails 4.2
-  attr_accessible :id, :org_id, :description, :published, :title, :locale, :customization_of, 
+  attr_accessible :id, :org_id, :description, :published, :title, :locale, :customization_of,
                   :is_default, :guidance_group_ids, :org, :plans, :phases, :dmptemplate_id,
                   :migrated, :version, :visibility, :published, :as => [:default, :admin]
 
@@ -33,16 +33,20 @@ def self.dmptemplate_ids
     Template.all.valid.distinct.pluck(:dmptemplate_id)
   end
 
-  # Retrieves the most recent version of the template for the specified Org and dmptemplate_id 
+  # Retrieves the most recent version of the template for the specified Org and dmptemplate_id
   def self.current(dmptemplate_id)
     Template.where(dmptemplate_id: dmptemplate_id).order(version: :desc).valid.first
   end
-  
-  # Retrieves the current published version of the template for the specified Org and dmptemplate_id  
+
+  # Retrieves the current published version of the template for the specified Org and dmptemplate_id
   def self.live(dmptemplate_id)
     Template.where(dmptemplate_id: dmptemplate_id, published: true).valid.first
   end
 
+  def self.default
+    Template.valid.where(is_default: true, published: true).order(:version).last
+  end
+
   ##
   # Retrieves the most current customization of the template for the
   # specified org and dmptemplate_id
@@ -79,8 +83,8 @@ def self.deep_copy(template)
 
   ##
   # convert the given template to a hash and return with all it's associations
-  # to use, please pre-fetch org, phases, section, questions, annotations, 
-  #   question_options, question_formats, 
+  # to use, please pre-fetch org, phases, section, questions, annotations,
+  #   question_options, question_formats,
   # TODO: Themes & guidance?
   #
   # @return [hash] hash of template, phases, sections, questions, question_options, annotations
@@ -145,7 +149,7 @@ def set_creation_defaults
       self.visibility = 1
       self.is_default = false
       self.version = 0 if self.version.nil?
-    
+
       # Generate a unique identifier for the dmptemplate_id if necessary
       if self.dmptemplate_id.nil?
         self.dmptemplate_id = loop do

app/views/notes/_add.html.erb

@@ -12,7 +12,6 @@
              id: "new_note_form_#{questionid}") do |f| %>
 	<%= f.hidden_field :user_id, value: current_user.id %>
 	<%= f.hidden_field :question_id, value: questionid %>
-	<%= f.hidden_field :answer_id, value: answer.id %>
 	<%= f.hidden_field :plan_id, value: plan_id %>
 
 	<fieldset class="standard">

db/seeds.rb

@@ -368,10 +368,19 @@
    org: Org.find_by(abbreviation: 'GA'),
    is_default: false,
    version: 0,
-   migrated:false,
+   migrated: false,
    dmptemplate_id: 3}
 ]
-templates.map{ |t| Template.create!(t) if Template.find_by(title: t[:title]).nil? }
+# Template creation calls defaults handler which sets is_default and 
+# published to false automatically, so update them after creation
+templates.map do |t| 
+  if Template.find_by(title: t[:title]).nil? 
+    tmplt = Template.create!(t) 
+    tmplt.published = t[:published]
+    tmplt.is_default = t[:is_default]
+    tmplt.save!
+  end
+end
 
 # Create 2 phases for the funder's template and one for our generic template
 # -------------------------------------------------------