diff --git a/Gemfile b/Gemfile index 1786836d0..d448e49ff 100644 --- a/Gemfile +++ b/Gemfile @@ -44,6 +44,7 @@ group :development, :test do gem 'rswag-specs' gem 'rubocop' gem 'simplecov', require: false, group: :test + gem 'rails-controller-testing' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index f5506b4dc..807f4defa 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -132,6 +132,8 @@ GEM racc (~> 1.4) nokogiri (1.15.2-x64-mingw-ucrt) racc (~> 1.4) + nokogiri (1.15.2-x86_64-linux) + racc (~> 1.4) parallel (1.23.0) parser (3.2.2.3) ast (~> 2.4.1) @@ -242,6 +244,7 @@ GEM PLATFORMS aarch64-linux x64-mingw-ucrt + x86_64-linux DEPENDENCIES bcrypt (~> 3.1.7) diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb new file mode 100644 index 000000000..fcf4dabb8 --- /dev/null +++ b/app/controllers/advice_controller.rb @@ -0,0 +1,105 @@ +class AdviceController < ApplicationController + before_action :set_questionnaire, only: %i[ edit_advice save_advice ] + # Advice_controller first checks whether current user has TA privileges or not by implementing action_allowed? method. Secondly it sets the number of advices based on score and sort it in descending order. Then it checks four conditions for the advices. + # 1. If number of advices is not equal to given advices + # 2. If the sorted advices is empty + # 3. If first advice score of sorted advices is NOT equal to max score + # 4. If last advice score of sorted advices is NOT equal to min score + # If any of the above condition are True, the edit_advice method calls adjust_advice_size of the QuestionnaireHelper class which adjust the advice sizes accordingly. + # In the end, save_advice method is called which updates and saves the changes in the advices and displays the success/failure message. + + include AuthorizationHelper + # If current user is TA then only current user can edit and update the advice + def action_allowed? + current_user_has_ta_privileges? + end + + ## This method will return true if the advice and its scores is invalid. + # Validates by utilizing the private methods invalid_advice_length? and invalid_advice_scores? + def invalid_advice?(sorted_advice, num_advices, question) + invalid_advice_length?(num_advices, question, sorted_advice) || + invalid_advice_scores?(sorted_advice) + end + + # Modify the advice associated with a questionnaire + # Separate methods were introduced to calculate the number of advices and sort the advices related to the current question attribute + # This is done to adhere to Single Responsibility Principle + def edit_advice + # For each question in a questionnaire, this method adjusts the advice size if the advice size is <,> number of advices or + # the max or min score of the advices does not correspond to the max or min score of questionnaire respectively. + @questionnaire.questions.each do |question| + # if the question is a scored question, store the number of advices corresponding to that question (max_score - min_score), else 0 + + num_advices = calculate_num_advices(question) + + # sorting question advices in descending order by score + + sorted_advice = sort_question_advices(question) + + # Checks the condition for adjusting the advice size + if invalid_advice?(sorted_advice, num_advices, question) + # The number of advices for this question has changed. + QuestionnaireHelper.adjust_advice_size(@questionnaire, question) + end + end + end + + + + # save the advice for a questionnaire + def save_advice + begin + # checks if advice is present or not + unless params[:advice].nil? + params[:advice].each do |advice_key, advice_params| + # get existing advice to update by key with the passed in advice param + QuestionAdvice.update(advice_key, advice: advice_params.slice(:advice)[:advice]) + end + # we made it here so it was saved + flash[:notice] = 'The advice was successfully saved!' + end + rescue ActiveRecord::RecordNotFound + # If record not found, redirects to edit_advice and sends flash + flash[:notice] = 'The advice record was not found and saved!' + end + # regardless of action above redirect to edit and show flash message if one exists + redirect_to action: 'edit_advice', id: params[:id] + end + + private + + # Common code for set questionnaire + def set_questionnaire + # Stores the questionnaire with given id in URL + @questionnaire = Questionnaire.find(params[:id]) + end + + ## Checks to see if the advice is the correct length. + # Checks to see if the number of advices is different than the question_advices or advice is empty + def invalid_advice_length?(num_advices, question, sorted_advice) + question.question_advices.length != num_advices || + sorted_advice.empty? + end + + ## Checks to see if the scores are valid + # Checks to see if the first and last index of the sorted_advice array are different than expected. + def invalid_advice_scores?(sorted_advice) + sorted_advice[0].score != @questionnaire.max_question_score || + sorted_advice[sorted_advice.length - 1].score != @questionnaire.min_question_score + end + + # Function to calculate number of advices for the current question attribute based on max and min question score. + # Method name is consistent with the functionality + # Refactored the 'if' statement into a ternary operator. This accomplishes the same logic in a more concise manner. + def calculate_num_advices(question) + question.is_a?(ScoredQuestion) ? @questionnaire.max_question_score - @questionnaire.min_question_score + 1 : 0 + end + + + # Function to sort question advices related to the current question attribute + # While sorting questions, sort_by(&:score) is used instead of using a block. It is a shorthand notation and avoids creating a new Proc object for every element in the collection of the questions. + def sort_question_advices(question) + question.question_advices.sort_by(&:score).reverse + end + +end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4c8c36ece..881e25f56 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,3 +1,14 @@ -class ApplicationController < ActionController::API - include JwtToken +class ApplicationController < ActionController::Base + #include JwtToken + def current_user + @current_user ||= session[:user] + end + + def current_role_name + current_role.try :name + end + + def current_role + current_user.try :role + end end diff --git a/app/helpers/authorization_helper.rb b/app/helpers/authorization_helper.rb new file mode 100644 index 000000000..5b9e0fb93 --- /dev/null +++ b/app/helpers/authorization_helper.rb @@ -0,0 +1,36 @@ +module AuthorizationHelper + # Notes: + # We use session directly instead of current_role_name and the like + # Because helpers do not seem to have access to the methods defined in app/controllers/application_controller.rb + + # PUBLIC METHODS + + # Determine if the currently logged-in user has the privileges of a TA (or higher) + def current_user_has_ta_privileges? + current_user_has_privileges_of?('Teaching Assistant') + end + + # Determine if there is a current user + # The application controller method session[:user] + # will return a user even if session[:user] has been explicitly cleared out + # because it is "sticky" in that it uses "@session[:user] ||= session[:user]" + # So, this method can be used to answer a controller's question + # "is anyone CURRENTLY logged in" + def user_logged_in? + !session[:user].nil? + end + + # PRIVATE METHODS + private + + # Determine if the currently logged-in user has the privileges of the given role name (or higher privileges) + # Let the Role model define this logic for the sake of DRY + # If there is no currently logged-in user simply return false + def current_user_has_privileges_of?(role_name) + current_user_and_role_exist? && session[:user].role.all_privileges_of?(Role.find_by(name: role_name)) + end + + def current_user_and_role_exist? + user_logged_in? && !session[:user].role.nil? + end +end \ No newline at end of file diff --git a/app/models/criterion.rb b/app/models/criterion.rb new file mode 100644 index 000000000..e69fba9d3 --- /dev/null +++ b/app/models/criterion.rb @@ -0,0 +1,189 @@ +# Initial commit +class Criterion < ScoredQuestion + include ActionView::Helpers + validates :size, presence: true + + # This method returns what to display if an instructor (etc.) is creating or editing a questionnaire (questionnaires_controller.rb) + def edit(_count) + html = 'Remove' + + html += '' + + html += '' + + html += '' + + html += '' + + html += 'text area size ' + + html += ' min_label max_label ' + + safe_join([''.html_safe, ''.html_safe], html.html_safe) + end + + # This method returns what to display if an instructor (etc.) is viewing a questionnaire + def view_question_text + html = ' ' + txt + ' ' + html += '' + type + '' + html += '' + weight.to_s + '' + questionnaire = self.questionnaire + if !max_label.nil? && !min_label.nil? + html += ' (' + min_label + ') ' + questionnaire.min_question_score.to_s + html += ' to ' + questionnaire.max_question_score.to_s + ' (' + max_label + ')' + else + html += '' + questionnaire.min_question_score.to_s + ' to ' + questionnaire.max_question_score.to_s + '' + end + safe_join([''.html_safe, ''.html_safe], html.html_safe) + end + + # Reduced the number of lines. Removed some redundant if-else statements, and combined some HTML concatenations. + # Display for the students when they are filling the questionnaire + def complete(count, answer = nil, questionnaire_min, questionnaire_max, dropdown_or_scale) + html = '
' + question_advices = QuestionAdvice.where(question_id: id).sort_by(&:id) + advice_total_length = 0 + question_advices.each do |question_advice| + advice_total_length += question_advice.advice.length if question_advice.advice && question_advice.advice != '' + end + # show advice given for different questions + html += advices_criterion_question(count, question_advices) if !question_advices.empty? && (advice_total_length > 0) + # dropdown options to rate a project based on the question + html += dropdown_criterion_question(count, answer, questionnaire_min, questionnaire_max) if dropdown_or_scale == 'dropdown' + # scale options to rate a project based on the question + html += scale_criterion_question(count, answer, questionnaire_min, questionnaire_max) if dropdown_or_scale == 'scale' + safe_join([''.html_safe, ''.html_safe], html.html_safe) + end + + # show advice for each criterion question + def advices_criterion_question(count, question_advices) + html = 'Show advice' + html += '' + end + + # dropdown options to rate a project based on the question + def dropdown_criterion_question(count, answer = nil, questionnaire_min, questionnaire_max) + current_value = '' + current_value += 'data-current-rating =' + answer.answer.to_s unless answer.nil? + html = '
' + end + + html += '' + html += min_label unless min_label.nil? + html += '' + + (questionnaire_min..questionnaire_max).each do |j| + html += '' + html += answer.comments if !answer.nil? && !answer.comments.nil? + html += '' + end + + # This method returns what to display if a student is viewing a filled-out questionnaire + def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil) + html = '' + count.to_s + '. ' + txt + ' [Max points: ' + questionnaire_max.to_s + ']' + score = answer && !answer.answer.nil? ? answer.answer.to_s : '-' + score_percent = score != '-' ? answer.answer * 1.0 / questionnaire_max : 0 + score_color = if score_percent > 0.8 + 'c5' + elsif score_percent > 0.6 + 'c4' + elsif score_percent > 0.4 + 'c3' + elsif score_percent > 0.2 + 'c2' + else + 'c1' + end + + html += '' + + if answer && !answer.comments.nil? + html += '' + #### start code to show tag prompts #### + if !tag_prompt_deployments.nil? && tag_prompt_deployments.count > 0 + # show check boxes for answer tagging + question = Question.find(answer.question_id) + html += '' + end + #### end code to show tag prompts #### + end + html += '
' + html += '
' + html += score + '

' + answer.comments.html_safe + '
' + tag_prompt_deployments.each do |tag_dep| + tag_prompt = TagPrompt.find(tag_dep.tag_prompt_id) + if tag_dep.question_type == question.type && answer.comments.length > tag_dep.answer_length_threshold.to_i + html += tag_prompt.html_control(tag_dep, answer, current_user) + end + end + html += '
' + safe_join([''.html_safe, ''.html_safe], html.html_safe) + end +end \ No newline at end of file diff --git a/app/models/question.rb b/app/models/question.rb index 4dc76ae10..7dae765b2 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -2,6 +2,7 @@ class Question < ApplicationRecord before_create :set_seq belongs_to :questionnaire # each question belongs to a specific questionnaire has_many :answers, dependent: :destroy + has_many :question_advices, dependent: :destroy # for each question, there is separate advice about each possible score validates :seq, presence: true, numericality: true # sequence must be numeric validates :txt, length: { minimum: 0, allow_nil: false, message: "can't be nil" } # user must define text content for a question diff --git a/app/models/question_advice.rb b/app/models/question_advice.rb new file mode 100644 index 000000000..a84fd39b6 --- /dev/null +++ b/app/models/question_advice.rb @@ -0,0 +1,29 @@ +class QuestionAdvice < ApplicationRecord + # attr_accessible :score, :advice + belongs_to :question + + # This method returns an array of fields present in question advice model + def self.export_fields(_options) + fields = [] + QuestionAdvice.columns.each do |column| + fields.push(column.name) + end + fields + end + + # This method adds the question advice data to CSV for the respective questionnaire + def self.export(csv, parent_id, _options) + questionnaire = Questionnaire.find(parent_id) + questions = questionnaire.questions + questions.each do |question| + question_advices = QuestionAdvice.where('question_id = ?', question.id) + question_advices.each do |advice| + tcsv = [] + advice.attributes.each_pair do |_name, value| + tcsv.push(value) + end + csv << tcsv + end + end + end +end \ No newline at end of file diff --git a/app/models/review_questionnaire.rb b/app/models/review_questionnaire.rb new file mode 100644 index 000000000..5a3bf356a --- /dev/null +++ b/app/models/review_questionnaire.rb @@ -0,0 +1,42 @@ +class ReviewQuestionnaire < Questionnaire + after_initialize :post_initialization + @print_name = 'Review Rubric' + + class << self + attr_reader :print_name + end + + def post_initialization + self.display_type = 'Review' + end + + def symbol + 'review'.to_sym + end + + def get_assessments_for(participant) + participant.reviews + end + + # return the responses for specified round, for varying rubric feature -Yang + def get_assessments_round_for(participant, round) + team = AssignmentTeam.team(participant) + return nil unless team + + team_id = team.id + responses = [] + if participant + maps = ResponseMap.where(reviewee_id: team_id, type: 'ReviewResponseMap') + maps.each do |map| + next if map.response.empty? + + map.response.each do |response| + responses << response if response.round == round && response.is_submitted + end + end + # responses = Response.find(:all, :include => :map, :conditions => ['reviewee_id = ? and type = ?',participant.id, self.to_s]) + responses.sort! { |a, b| a.map.reviewer.fullname <=> b.map.reviewer.fullname } + end + responses + end +end \ No newline at end of file diff --git a/app/views/advice/edit_advice.erb b/app/views/advice/edit_advice.erb new file mode 100644 index 000000000..e69de29bb diff --git a/config/routes.rb b/config/routes.rb index 1ea817f03..f8a76eb29 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -73,4 +73,11 @@ end end end + resources :advice, only: [] do + collection do + post :save_advice + put :edit_advice + get :edit_advice + end + end end diff --git a/db/migrate/20240408145535_create_question_advices.rb b/db/migrate/20240408145535_create_question_advices.rb new file mode 100644 index 000000000..c221c7c42 --- /dev/null +++ b/db/migrate/20240408145535_create_question_advices.rb @@ -0,0 +1,16 @@ +class CreateQuestionAdvices < ActiveRecord::Migration[7.0] + def self.up + create_table 'question_advices', force: true do |t| + t.column 'question_id', :integer # the question to which this advice belongs + t.column 'score', :integer # the score associated with this advice + t.column 'advice', :text # the advice to be given to the user + end + + add_index 'question_advices', ['question_id'], name: 'fk_question_question_advices' + + end + + def self.down + drop_table 'question_advices' + end +end diff --git a/spec/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb new file mode 100644 index 000000000..a20f7c259 --- /dev/null +++ b/spec/controllers/advice_controller_spec.rb @@ -0,0 +1,428 @@ +require 'rails_helper' +require 'questionnaire_helper' + +describe AdviceController, type: :controller do + let(:super_admin) { build(:superadmin, id: 1, role_id: 1) } + let(:instructor1) { build(:instructor, id: 10, role_id: 3, parent_id: 3, name: 'Instructor1') } + let(:student1) { build(:student, id: 21, role_id: 5) } + + describe '#action_allowed?' do + context 'when the role of current user is Super-Admin' do + # Checking for Super-Admin + it 'allows certain action' do + stub_current_user(super_admin, super_admin.role.name, super_admin.role) + expect(controller.send(:action_allowed?)).to be_truthy + end + end + context 'when the role of current user is Instructor' do + # Checking for Instructor + it 'allows certain action' do + stub_current_user(instructor1, instructor1.role.name, instructor1.role) + expect(controller.send(:action_allowed?)).to be_truthy + end + end + context 'when the role of current user is Student' do + # Checking for Student + it 'refuses certain action' do + stub_current_user(student1, student1.role.name, student1.role) + expect(controller.send(:action_allowed?)).to be_falsey + end + end + end + + describe "#invalid_advice?" do + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1) } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 3, question_id: 1) } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + context "when the sorted advice is empty" do + it "returns true" do + ## Set sorted advice to empty. + sorted_advice = [] + num_advices = 5 + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to be_truthy + end + end + + context "when the number of advices does not match the expected number" do + it "returns true" do + ## Set sorted advice to be different length the num_advices. + sorted_advice = [questionAdvice1, questionAdvice2] + num_advices = 1 + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to be_truthy + end + end + + context "when the highest scoring advice does not have the maximum question score" do + it "returns true" do + # Create a question advice with a score lower than the maximum question score + questionAdvice1 = build(:question_advice, id: 1, score: 3, question_id: 1) + questionnaire = build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1])], max_question_score: 5) + + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + sorted_advice = questionnaire.questions[0].question_advices.sort_by(&:score).reverse + + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to be_truthy + end + end + + + context "when the lowest scoring advice does not have the minimum question score" do + it "returns true" do + # Create a question advice with a score higher than the minimum question score + questionAdvice1 = build(:question_advice, id: 1, score: 1, question_id: 1) # Assuming minimum question score is 2 + questionnaire = build(:questionnaire, id: 1, min_question_score: 2, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1])], max_question_score: 5) + + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + sorted_advice = questionnaire.questions[0].question_advices.sort_by(&:score).reverse + + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to be_truthy + end + end + + context 'when invalid_advice? is called with all conditions satisfied' do + # Question Advices passing all conditions + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + it 'invalid_advice? returns false when called with all correct pre-conditions ' do + sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + controller.instance_variable_set(:@questionnaire, questionnaire) + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to eq(false) + end + end + + + + end + + ######################################################################################## + ### These are the old invalid_advice tests. Waiting for Anuj to give feedback on what to do with these. + describe '#invalid_advice?' do + context 'when invalid_advice? is called with question advice score > max score of questionnaire' do + # max score of advice = 3 (!=2) + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 3, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + it 'invalid_advice? returns true when called with incorrect maximum score for a question advice' do + sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + controller.instance_variable_set(:@questionnaire, questionnaire) + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to eq(true) + end + end + + context 'when invalid_advice? is called with question advice score < min score of questionnaire' do + # min score of advice = 0 (!=1) + let(:questionAdvice1) { build(:question_advice, id: 1, score: 0, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + it 'invalid_advice? returns true when called with incorrect minimum score for a question advice' do + sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + controller.instance_variable_set(:@questionnaire, questionnaire) + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to eq(true) + end + end + + context 'when invalid_advice? is called with number of advices > (max-min) score of questionnaire' do + # number of advices > 2 + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionAdvice3) { build(:question_advice, id: 3, score: 2, question_id: 1, advice: 'Advice3') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2, questionAdvice3])], max_question_score: 2) + end + + it 'invalid_advice? returns true when called with incorrect number of question advices' do + sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + controller.instance_variable_set(:@questionnaire, questionnaire) + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to eq(true) + end + end + + context 'when invalid_advice? is called with no advices for a question in questionnaire' do + # 0 advices - empty list scenario + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [])], max_question_score: 2) + end + + it 'invalid_advice? returns true when called with an empty advice list ' do + sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + controller.instance_variable_set(:@questionnaire, questionnaire) + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to eq(true) + end + end + + context 'when invalid_advice? is called with all conditions satisfied' do + # Question Advices passing all conditions + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + it 'invalid_advice? returns false when called with all correct pre-conditions ' do + sorted_advice = questionnaire.questions[0].question_advices.sort_by { |x| x.score }.reverse + num_advices = questionnaire.max_question_score - questionnaire.min_question_score + 1 + controller.instance_variable_set(:@questionnaire, questionnaire) + expect(controller.invalid_advice?(sorted_advice, num_advices, questionnaire.questions[0])).to eq(false) + end + end + end + + ######################################################################################## + + + describe '#edit_advice' do + + context 'when edit_advice is called and invalid_advice? evaluates to true' do + # edit advice called + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + it 'edit advice redirects correctly when called' do + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + params = { id: 1 } + session = { user: instructor1 } + result = get(:edit_advice, params:, session:) + expect(result.status).to eq 200 + expect(result).to render_template(:edit_advice) + end + end + + context 'when advice adjustment is not necessary' do + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + it 'does not adjust advice size when called' do + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(controller).to receive(:invalid_advice?).and_return(false) + expect(QuestionnaireHelper).not_to receive(:adjust_advice_size) + get :edit_advice, params: { id: 1 } + end + end + context "when the advice size needs adjustment" do + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + before do + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(controller).to receive(:invalid_advice?).and_return(true) + end + + it "calculates the number of advices for each question" do + expect(controller).to receive(:calculate_num_advices).once # Assuming there is one question in the questionnaire + get :edit_advice, params: { id: 1 } + end + + it "sorts question advices in descending order by score" do + expect(controller).to receive(:sort_question_advices).once # Assuming there is one question in the questionnaire + get :edit_advice, params: { id: 1 } + end + + it "adjusts the advice size if the number of advices is less than the max score of the questionnaire" do + allow(controller).to receive(:calculate_num_advices).and_return(1) # Assuming only one advice calculated + expect(QuestionnaireHelper).to receive(:adjust_advice_size).with(questionnaire, questionnaire.questions.first) + get :edit_advice, params: { id: 1 } + end + + it "adjusts the advice size if the number of advices is greater than the max score of the questionnaire" do + allow(controller).to receive(:calculate_num_advices).and_return(3) # Assuming three advices calculated + expect(QuestionnaireHelper).to receive(:adjust_advice_size).with(questionnaire, questionnaire.questions.first) + get :edit_advice, params: { id: 1 } + end + + it "adjusts the advice size if the max score of the advices does not correspond to the max score of the questionnaire" do + allow(controller).to receive(:sort_question_advices).and_return([questionAdvice2, questionAdvice1]) # Assuming advices not sorted correctly + expect(QuestionnaireHelper).to receive(:adjust_advice_size).with(questionnaire, questionnaire.questions.first) + get :edit_advice, params: { id: 1 } + end + + it "adjusts the advice size if the min score of the advices does not correspond to the min score of the questionnaire" do + allow(questionnaire).to receive(:min_question_score).and_return(0) # Assuming min score not matching + expect(QuestionnaireHelper).to receive(:adjust_advice_size).with(questionnaire, questionnaire.questions.first) + get :edit_advice, params: { id: 1 } + end + end + + context "when the advice size does not need adjustment" do + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + + before do + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(controller).to receive(:invalid_advice?).and_return(false) + end + + it "does not adjust the advice size if the number of advices is equal to the max score of the questionnaire" do + allow(controller).to receive(:calculate_num_advices).and_return(2) # Assuming two advices calculated + expect(QuestionnaireHelper).not_to receive(:adjust_advice_size) + get :edit_advice, params: { id: 1 } + end + + it "does not adjust the advice size if the max score of the advices corresponds to the max score of the questionnaire" do + expect(QuestionnaireHelper).not_to receive(:adjust_advice_size) + get :edit_advice, params: { id: 1 } + end + + it "does not adjust the advice size if the min score of the advices corresponds to the min score of the questionnaire" do + expect(QuestionnaireHelper).not_to receive(:adjust_advice_size) + get :edit_advice, params: { id: 1 } + end + end + end + + describe '#save_advice' do + let(:questionAdvice1) { build(:question_advice, id: 1, score: 1, question_id: 1, advice: 'Advice1') } + let(:questionAdvice2) { build(:question_advice, id: 2, score: 2, question_id: 1, advice: 'Advice2') } + let(:questionnaire) do + build(:questionnaire, id: 1, min_question_score: 1, + questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2) + end + context 'when the advice is present' do + it 'updates the advice for each key' do + # Arrange + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(QuestionAdvice).to receive(:update).with('1', { advice: 'Hello' }).and_return('Ok') + allow(QuestionAdvice).to receive(:update).with('2', { advice: 'Goodbye' }).and_return('Ok') + # Add some advice parameters that will allow for update success + advice_params = { + '1' => { advice: 'Hello' }, + '2' => { advice: 'Goodbye' } + } + params = { advice: advice_params, id: 1 } + session = { user: instructor1 } + + # Act + result = get(:save_advice, params:, session:) + + # Assert + # check each key to see if it received update + # Always expect redirect + advice_params.keys.each do |advice_key| + expect(QuestionAdvice).to have_received(:update).with(advice_key, advice: advice_params[advice_key][:advice]) + end + expect(result.status).to eq 302 + expect(result).to redirect_to('/advice/edit_advice?id=1') + end + + it 'sets a success flash notice' do + # Arrange + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(QuestionAdvice).to receive(:update).with('1', { advice: 'Hello' }).and_return('Ok') + allow(QuestionAdvice).to receive(:update).with('2', { advice: 'Goodbye' }).and_return('Ok') + # Add some advice parameters that will allow for update success + params = { advice: { + '1' => { advice: 'Hello' }, + '2' => { advice: 'Goodbye' } + }, id: 1 } + session = { user: instructor1 } + + # Act + get(:save_advice, params:, session:) + + # Assert + expect(flash[:notice]).to eq('The advice was successfully saved!') + end + end + + context 'when the advice is not present' do + it 'does not update any advice' do + # Arrange + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(QuestionAdvice).to receive(:update).with(any_args).and_return('Ok') + # no advice parameter + params = { id: 1 } + session = { user: instructor1 } + + # Act + result = get(:save_advice, params:, session:) + + # Assert + # Expect no update to be called with nil params for advice + # Expect no flash + # Always expect redirect + expect(QuestionAdvice).not_to have_received(:update) + expect(flash[:notice]).not_to be_present + expect(result.status).to eq 302 + expect(result).to redirect_to('/advice/edit_advice?id=1') + end + end + + context 'when the questionnaire is not found' do + it 'renders the edit_advice view' do + # Arrange + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(QuestionAdvice).to receive(:update).with(any_args).and_raise(ActiveRecord::RecordNotFound) + + #Act + # call get on edit_advice with params that will hit the record not found path + get :edit_advice, params: { advice: { + '1' => { advice: 'Hello' }, + '2' => { advice: 'Goodbye' } + }, id: 1 } + + # Assert: Verify that the controller renders the correct view + expect(response).to render_template('edit_advice') + end + end + + it 'redirects to the edit_advice action' do + # Arrange + allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) + allow(QuestionAdvice).to receive(:update).with(any_args).and_return('Ok') + params = { advice: { + '1' => { advice: 'Hello' }, + '2' => { advice: 'Goodbye' } + }, id: 1 } + session = { user: instructor1 } + + # Act + result = get(:save_advice, params:, session:) + + # Assert + # expect 302 redirect and for it to redirect to edit_advice + expect(result.status).to eq 302 + expect(result).to redirect_to('/advice/edit_advice?id=1') + end + end +end \ No newline at end of file diff --git a/spec/factories.rb b/spec/factories.rb index 2d3055e99..caea7eaf2 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -37,7 +37,6 @@ factory :instructor, class: Instructor do name {'instructor6'} role {Role.where(name: 'Instructor').first || association(:role_of_instructor)} - fullname {'6, instructor'} parent_id {1} email {'instructor6@gmail.com'} end diff --git a/spec/factories/factories.rb b/spec/factories/factories.rb index eb06d7682..9b5ca2ebd 100644 --- a/spec/factories/factories.rb +++ b/spec/factories/factories.rb @@ -1,6 +1,72 @@ FactoryBot.define do - + factory :superadmin, class: User do + sequence(:name) { |n| "superadmin#{n}" } + role { Role.where(name: 'Super-Administrator').first || association(:role_of_superadministrator) } + password { 'password' } + password_confirmation { 'password' } + email { 'expertiza@mailinator.com' } + parent_id { 1 } + mru_directory_path { nil } + email_on_review { true } + email_on_submission { true } + email_on_review_of_review { true } + is_new_user { false } + master_permission_granted { 0 } + handle { 'handle' } + # public_key { nil } + copy_of_emails { false } + end + + factory :student, class: User do + # Zhewei: In order to keep students the same names (2065, 2066, 2064) before each example. + sequence(:name) { |n| n = n % 3; "student206#{n + 4}" } + role { Role.where(name: 'Student').first || association(:role_of_student) } + password { 'password' } + password_confirmation { 'password' } + email { 'expertiza@mailinator.com' } + parent_id { 1 } + mru_directory_path { nil } + email_on_review { true } + email_on_submission { true } + email_on_review_of_review { true } + is_new_user { false } + master_permission_granted { 0 } + handle { 'handle' } + # public_key { nil } + copy_of_emails { false } + end + + factory :questionnaire, class: ReviewQuestionnaire do + name { 'Test questionnaire' } + # Beware: it is fragile to assume that role_id of instructor is 1 (or any other unchanging value) + instructor { Instructor.first || association(:instructor) } + private { 0 } + min_question_score { 0 } + max_question_score { 5 } + # type { 'ReviewQuestionnaire' } + display_type { 'Review' } + instruction_loc { nil } + end + + factory :question, class: Criterion do + txt { 'Test question:' } + weight { 1 } + questionnaire { Questionnaire.first || association(:questionnaire) } + seq { 1.00 } + # type { 'Criterion' } + size { '70,1' } + alternatives { nil } + break_before { 1 } + max_label { nil } + min_label { nil } + end + + factory :question_advice, class: QuestionAdvice do + question { Question.first || association(:question) } + score { 5 } + advice { 'LGTM' } + end end diff --git a/spec/questionnaire_helper.rb b/spec/questionnaire_helper.rb new file mode 100644 index 000000000..22b709f28 --- /dev/null +++ b/spec/questionnaire_helper.rb @@ -0,0 +1,57 @@ +# OSS808 Change 28/10/2013 +# FasterCSV replaced now by CSV which is present by default in Ruby +# require 'fastercsv' +# require 'csv' + +module QuestionnaireHelper + CSV_QUESTION = 0 + CSV_TYPE = 1 + CSV_PARAM = 2 + CSV_WEIGHT = 3 + + def self.adjust_advice_size(questionnaire, question) + # now we only support question advices for scored questions + if question.is_a?(ScoredQuestion) + + max = questionnaire.max_question_score + min = questionnaire.min_question_score + + QuestionAdvice.delete(['question_id = ? AND (score > ? OR score < ?)', question.id, max, min]) if !max.nil? && !min.nil? + + (questionnaire.min_question_score..questionnaire.max_question_score).each do |i| + qas = QuestionAdvice.where('question_id = ? AND score = ?', question.id, i) + question.question_advices << QuestionAdvice.new(score: i) if qas.first.nil? + QuestionAdvice.delete(['question_id = ? AND score = ?', question.id, i]) if qas.size > 1 + end + end + end + +# Updates the attributes of questionnaire questions based on form data, without modifying unchanged attributes. + def update_questionnaire_questions + return if params[:question].nil? + + params[:question].each_pair do |k, v| + question = Question.find(k) + v.each_pair do |key, value| + question.send(key + '=', value) unless question.send(key) == value + end + question.save + end + end + + #Map type to questionnaire + QUESTIONNAIRE_MAP = { + 'ReviewQuestionnaire' => ReviewQuestionnaire + }.freeze + + # factory method to create the appropriate questionnaire based on the type + def questionnaire_factory(type) + questionnaire = QUESTIONNAIRE_MAP[type] + if questionnaire.nil? + flash[:error] = 'Error: Undefined Questionnaire' + else + questionnaire.new + end + end + +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 922f64586..af1dfa2d0 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -56,6 +56,19 @@ # https://relishapp.com/rspec/rspec-rails/docs config.infer_spec_type_from_file_location! + def stub_current_user(current_user, current_role_name = 'Student', current_role) + allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(current_user) if defined?(session) + allow_any_instance_of(ApplicationController).to receive(:current_role_name).and_return(current_role_name) + allow_any_instance_of(ApplicationController).to receive(:current_role).and_return(current_role) + # Also pop this stub user into the session to support the authorization helper + + # Check if session is defined to differentiate between controller and non-controller tests. + # This is required as the session variable is only defined for controller specs. + # Other kinds of specs(feature specs,etc) use an internal rack.session that cannot be interacted with. + session[:user] = current_user if defined?(session) + end + + # Filter lines from Rails gems in backtraces. config.filter_rails_from_backtrace! # arbitrary gems may also be filtered via: