diff --git a/app/controllers/api/v0/statistics_controller.rb b/app/controllers/api/v0/statistics_controller.rb index 5567512..4036fe6 100644 --- a/app/controllers/api/v0/statistics_controller.rb +++ b/app/controllers/api/v0/statistics_controller.rb @@ -37,12 +37,33 @@ # GET # Returns the number of completed plans within the user's org for the data start_date and end_date specified def completed_plans - # raise Pundit::NotAuthorizedError unless Api::V0::StatisticsPolicy.new(@user, :statistics).completed_plans? - # users = User.unscoped.where(org_id: org) - # roles = Role.where(access: Role.access_values_for(:creator, :administrator, :editor, :commenter).min) - # plan_ids = Plan.select(:id).joins(:roles).merge(roles.joins(:user).merge(users)) - # Plan.joins(:questions).where("plans.id": plan_ids).group("plans.id").count("questions.id") plan_id followed by number of questions - # Plan.joins(:answers).where("plans.id": plan_ids).group("plans.id").count("answers.id") plan_id followed by numbers of answers + raise Pundit::NotAuthorizedError unless Api::V0::StatisticsPolicy.new(@user, :statistics).completed_plans? + + roles = Role.where("#{Role.creator_condition} OR #{Role.administrator_condition}") + + users = User.unscoped + if @user.can_super_admin? && params[:org_id].present? + users = users.where(org_id: params[:org_id]) + else + users = users.where(org_id: @user.org_id) + end + + plans = Plan.where(complete: true) + if params[:range_dates].present? + r = {} + params[:range_dates].each_pair do |k, v| + range_date_plans = plans + .where('plans.updated_at >=?', v['start_date']) + .where('plans.updated_at <=?', v['end_date']) + r[k] = roles.joins(:user, :plan).merge(users).merge(range_date_plans).select(:plan_id).distinct.count + end + render(json: r.to_json) + else + plans = plans.where('plans.updated_at >= ?', Date.parse(params[:start_date])) if params[:start_date].present? + plans = plans.where('plans.updated_at <= ?', Date.parse(params[:end_date])) if params[:end_date].present? + count = roles.joins(:user, :plan).merge(users).merge(plans).select(:plan_id).distinct.count + render(json: { completed_plans: count }) + end end ## diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index e9d4521..3d10ca9 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -6,26 +6,25 @@ # GET /plans/:plan_id/phases/:id/edit def edit - plan = Plan.load_for_phase(params[:plan_id], params[:id]) - - # authorization done on plan so found in plan_policy + plan = Plan.find(params[:plan_id]) authorize plan - - phase_id = params[:id].to_i - phase = plan.template.phases.select {|p| p.id == phase_id}.first - readonly = !plan.editable_by?(current_user.id) - guidance_groups_ids = plan.guidance_groups.collect(&:id) - guidance_groups = GuidanceGroup.where(published: true, id: guidance_groups_ids) - if !user_signed_in? then - respond_to do |format| - format.html { redirect_to edit_user_registration_path } - end - else - render('/phases/edit', locals: { plan: plan, phase: phase, readonly: readonly, - question_guidance: plan.guidance_by_question_as_hash, - guidance_groups: guidance_groups }) - end + plan, phase = Plan.load_for_phase(params[:plan_id], params[:id]) + + readonly = !plan.editable_by?(current_user.id) + + guidance_groups_ids = plan.guidance_groups.collect(&:id) + + guidance_groups = GuidanceGroup.where(published: true, id: guidance_groups_ids) + # Since the answers have been pre-fetched through plan (see Plan.load_for_phase) + # we create a hash whose keys are question id and value is the answer associated + answers = plan.answers.reduce({}){ |m, a| m[a.question_id] = a; m } + + render('/phases/edit', locals: { + plan: plan, phase: phase, readonly: readonly, + question_guidance: plan.guidance_by_question_as_hash, + guidance_groups: guidance_groups, + answers: answers }) end diff --git a/app/models/answer.rb b/app/models/answer.rb index d94cac1..c9e3d6c 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -1,9 +1,16 @@ class Answer < ActiveRecord::Base after_save do |answer| - # Updates the plan.updated_at attribute whenever an answer has been created/updated - # check first for a nil plan so that the controller can properly handle the ActiveRecord::NotFound exception - answer.plan.touch unless answer.plan.nil? + if answer.plan_id.present? + plan = answer.plan + complete = plan.no_questions_matches_no_answers? + if plan.complete != complete + plan.complete = complete + plan.save! + else + plan.touch # Force updated_at changes if nothing changed since save only saves if changes were made to the record + end + end end ## @@ -75,12 +82,8 @@ end return false end - # Returns all the notes for an instance answer whose archived is nil or false. The Array is ordered by updated_at (descending) + # Returns answer notes whose archived is blank sorted by updated_at in descending order def non_archived_notes - answer = Answer.includes(:notes).where({ id: self.id, notes: { archived: [nil, false] } }).order('notes.updated_at DESC').first - if !answer.nil? - return answer.notes.to_a - end - return [] + return notes.select{ |n| n.archived.blank? }.sort!{ |x,y| y.updated_at <=> x.updated_at } end end diff --git a/app/models/phase.rb b/app/models/phase.rb index 3b9ee24..61f9765 100644 --- a/app/models/phase.rb +++ b/app/models/phase.rb @@ -112,14 +112,12 @@ return phase_copy end - # Returns the number of answered question for the phase. It is assumed that the plan_id passed - # has this phase instance. - def num_answered_questions(plan_id) - n = 0 - self.sections.each do |s| - n+= s.num_answered_questions(plan_id) + # Returns the number of answered question for the phase. + def num_answered_questions(plan) + return 0 if plan.nil? + return sections.reduce(0) do |m, s| + m + s.num_answered_questions(plan) end - return n end # Returns the number of questions for a phase. Note, this method becomes useful diff --git a/app/models/plan.rb b/app/models/plan.rb index 9aa7c51..c5242f3 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -692,11 +692,12 @@ # 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 @@ -704,14 +705,9 @@ 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. + # Returns the number of questions for a plan. def num_questions - n = 0 - self.sections.each do |s| - n+= s.questions.size() - end - return n + return sections.includes(:questions).joins(:questions).reduce(0){ |m, s| m + s.questions.length } 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 @@ -729,17 +725,15 @@ ]).find(id) end + # Pre-fetched a plan phase together with its sections and questions associated. It also pre-fetches the answers and notes associated to the plan 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.joins(:phases).where('plans.id = ? AND phases.id = ?', id, phase_id).includes(:template, :sections, :questions, :answers, :notes).first + plan = Plan + .joins(template: { phases: { sections: :questions }}) + .where("plans.id = :id AND phases.id = :phase_id", { id: id, phase_id: phase_id }) + .includes(template: { phases: { sections: :questions }}) + .merge(Plan.includes(answers: :notes))[0] + phase = plan.template.phases.first + return plan, phase end # deep copy the given plan and all of it's associations @@ -786,6 +780,21 @@ Plan.joins(:questions).exists?(id: self.id, "questions.id": question_id) end + # Checks whether or not the number of questions matches the number of valid answers + def no_questions_matches_no_answers? + num_questions = question_ids.length + pre_fetched_answers = Answer + .includes({ question: :question_format }, :question_options) + .where(id: answer_ids) + num_answers = pre_fetched_answers.reduce(0) do |m, a| + if a.is_valid? + m+=1 + end + m + end + return num_questions == num_answers + end + private # Returns whether or not the user has the specified role for the plan diff --git a/app/models/section.rb b/app/models/section.rb index 4d4562b..1a145d1 100644 --- a/app/models/section.rb +++ b/app/models/section.rb @@ -23,13 +23,16 @@ "#{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.is_valid?}.count + # Returns the number of answered questions for a given plan + def num_answered_questions(plan) + return 0 if plan.nil? + 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 ## diff --git a/app/views/phases/_edit_plan_answers.html.erb b/app/views/phases/_edit_plan_answers.html.erb index 41445ac..aadfd72 100644 --- a/app/views/phases/_edit_plan_answers.html.erb +++ b/app/views/phases/_edit_plan_answers.html.erb @@ -52,10 +52,8 @@ <% section.questions.each_with_index do |question, i| %> <% # Load the answer or create a new one - answers = question.plan_answers(plan.id) if plan.present? - if answers.present? - answer = answers.first - else + answer = answers[question.id] if plan.present? + if answer.blank? answer = Answer.new({ plan: plan, question: question }) end %> diff --git a/app/views/plans/_navigation.html.erb b/app/views/plans/_navigation.html.erb index 003379e..d21eab0 100644 --- a/app/views/plans/_navigation.html.erb +++ b/app/views/plans/_navigation.html.erb @@ -6,7 +6,7 @@ - <% plan.template.phases.each do |phase| %> + <% phases.each do |phase| %>