diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 82633c7..1fce879 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -6,13 +6,17 @@ # GET /plans/:plan_id/phases/:id/edit def edit - - @plan = Plan.load_for_phase(params[:plan_id], params[:id]) + @plan, @phase = Plan.load_for_phase(params[:plan_id], params[:id]) + # check if plan exists first + if @plan.nil? + raise Pundit::NotAuthorizedError, "Must have access to plan" + end + if @phase.nil? + raise Pundit::NotAuthorizedError, "Phase must belong to plan" + end # authorization done on plan so found in plan_policy authorize @plan - - phase_id = params[:id].to_i - @phase = @plan.template.phases.first + @answers = @plan.answers.reduce({}){ |m, a| m[a.question_id] = a; m } @readonly = !@plan.editable_by?(current_user.id) # Now we need to get all the themed guidance for the plan. @@ -57,9 +61,9 @@ # Puts in question_guidance (key/value) entries where key is the question id and value is a hash. # Each question id hash has (key/value) entries where key is a theme and value is an Array of {text, org} objects # Example hash - # question_guidance = { question.id => + # question_guidance = { question.id => # { theme => [ {text: "......", org: "....."} ] } - # } + # } questions.each do |question| qg = {} question.themes.each do |t| diff --git a/app/models/plan.rb b/app/models/plan.rb index b32f3f6..a71bcff 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -28,7 +28,7 @@ ## # Possibly needed for active_admin # -relies on protected_attributes gem as syntax depricated in rails 4.2 - attr_accessible :locked, :project_id, :version_id, :version, :plan_sections, + attr_accessible :locked, :project_id, :version_id, :version, :plan_sections, :exported_plans, :project, :title, :template, :grant_number, :identifier, :principal_investigator, :principal_investigator_identifier, :description, :data_contact, :funder_name, :visibility, :exported_plans, @@ -38,7 +38,7 @@ # public is a Ruby keyword so using publicly enum visibility: [:organisationally_visible, :publicly_visible, :is_test, :privately_visible] - #TODO: work out why this messes up plan creation : + #TODO: work out why this messes up plan creation : # briley: Removed reliance on :users, its really on :roles (shouldn't have a plan without at least a creator right?) It should be ok like this though now # validates :template, :title, presence: true @@ -210,7 +210,7 @@ end end end - + # Get guidance by theme from any guidance groups currently selected self.guidance_groups.each do |group| group.guidances.each do |guidance| @@ -369,7 +369,7 @@ format = rec.qformat answer = nil - if qa_map.has_key?(qid) + if qa_map.has_key?(qid) answer = qa_map[qid] end @@ -423,7 +423,7 @@ opt_hash[aid] << optid end - status["questions"].each_key do |questionid| + status["questions"].each_key do |questionid| answerid = status["questions"][questionid]["answer_id"] status["questions"][questionid]["answer_option_ids"] = opt_hash[answerid] end @@ -636,10 +636,10 @@ user_id = user_id.id if user_id.is_a?(User) add_user(user_id, true, true, true) end - -# TODO: commenting these out because they are overriden by private methods below, so this + +# TODO: commenting these out because they are overriden by private methods below, so this # is unreachable =begin ## @@ -696,7 +696,7 @@ end =end -# TODO: What are these used for? Should just be using self.org and self.org.funder? +# TODO: What are these used for? Should just be using self.org and self.org.funder? =begin ## # sets a new funder for the project @@ -950,14 +950,15 @@ self.dmptemplate.try(:organisation).try(:abbreviation) 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) + return Answer.where(id: answers.map(&:id)).includes({question: :question_format}, :question_options).reduce(0) do |m, a| + if a.is_valid? + m+=1 + end + m end - return n end # Returns a section given its id or nil if does not exist for the current plan @@ -969,8 +970,8 @@ # 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() + sections.includes(:questions).joins(:questions).each do |s| + n+= s.questions.length end return n end @@ -991,14 +992,9 @@ end def self.load_for_phase(id, phase_id) - Plan.includes( - [template: [ - {phases: {sections: {questions: [{answers: :notes}, :annotations, :question_format, :themes]}}}, - {customizations: :org}, - :org - ], - plans_guidance_groups: {guidance_group: {guidances: :themes}} - ]).where(id: id, phases: { id: phase_id }).first + plan = Plan.joins(template: {phases: {sections: :questions}}).where("plans.id = #{id} AND phases.id = #{phase_id} ").includes(template: {phases: {sections: :questions}}).merge(Plan.includes(answers: :notes))[0] + phase = plan.template.phases.first + return plan, phase end @@ -1048,12 +1044,12 @@ end role.save - - # This is necessary because we're creating the associated record but not assigning it + + # This is necessary because we're creating the associated record but not assigning it # to roles. Auto-saving like this may be confusing when coding upstream in a controller, - # view or api. Should probably change this to: + # view or api. Should probably change this to: # self.roles << role - # and then let the save be called manually via: + # and then let the save be called manually via: # plan.save! #self.reload end @@ -1133,7 +1129,7 @@ # -------------------------------------------------------- def set_creation_defaults # Only run this before_validation because rails fires this before save/create - if self.id.nil? + if self.id.nil? self.title = "My plan (#{self.template.title})" if self.title.nil? && !self.template.nil? self.visibility = 3 end diff --git a/app/models/section.rb b/app/models/section.rb index 4d4562b..18a3420 100644 --- a/app/models/section.rb +++ b/app/models/section.rb @@ -24,14 +24,15 @@ 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.is_valid?}.count + def num_answered_questions(plan) + questions_hash = questions.reduce({}){ |m, q| m[q.id] = q; m } + return plan.answers.includes({question: :question_format}, :question_options).reduce(0) do |m, a| + if questions_hash[a.question_id].present? && a.is_valid? + m+=1 + end + m end - return n end - ## # deep copy of the given section and all it's associations # diff --git a/app/views/phases/_answer_form.html.erb b/app/views/phases/_answer_form.html.erb index c95732b..566b757 100644 --- a/app/views/phases/_answer_form.html.erb +++ b/app/views/phases/_answer_form.html.erb @@ -1,4 +1,4 @@ - - +
- <% comments = answer.notes.all %> + <% comments = answer.notes.all %> <%= hidden_field_tag :question_id, question.id, class: "question_id" %> <% active_tab = nil %>