From 49a15b06dfc0774838504849247f3b44e7686faf Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Thu, 4 Apr 2024 04:29:25 -0400
Subject: [PATCH 01/16] Adding a lot of initial files and factories needed

---
 app/controllers/advice_controller.rb       |  70 +++++++++
 app/helpers/authorization_helper.rb        |  36 +++++
 spec/controllers/advice_controller_spec.rb | 173 +++++++++++++++++++++
 spec/factories/factories.rb                |  76 ++++++++-
 spec/rails_helper.rb                       |  13 ++
 5 files changed, 367 insertions(+), 1 deletion(-)
 create mode 100644 app/controllers/advice_controller.rb
 create mode 100644 app/helpers/authorization_helper.rb
 create mode 100644 spec/controllers/advice_controller_spec.rb

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
new file mode 100644
index 000000000..00d2c8e7b
--- /dev/null
+++ b/app/controllers/advice_controller.rb
@@ -0,0 +1,70 @@
+class AdviceController < ApplicationController
+  # 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
+
+  # checks whether the advices for a question in questionnaire have valid attributes
+  # return true if the number of advices and their scores are invalid, else returns false
+  def invalid_advice?(sorted_advice, num_advices, question)
+    return ((question.question_advices.length != num_advices) ||
+      sorted_advice.empty? ||
+      (sorted_advice[0].score != @questionnaire.max_question_score) ||
+      (sorted_advice[sorted_advice.length - 1].score != @questionnaire.min_question_score))
+  end
+
+  # Modify the advice associated with a questionnaire
+  def edit_advice
+    # Stores the questionnaire with given id in URL
+    @questionnaire = Questionnaire.find(params[:id])
+
+    # 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 = if question.is_a?(ScoredQuestion)
+                      @questionnaire.max_question_score - @questionnaire.min_question_score + 1
+                    else
+                      0
+                    end
+
+      # sorting question advices in descending order by score
+      sorted_advice = question.question_advices.sort_by { |x| x.score }.reverse
+
+      # 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
+    # Stores the questionnaire with given id in URL
+    @questionnaire = Questionnaire.find(params[:id])
+    begin
+      # checks if advice is present or not
+      unless params[:advice].nil?
+        params[:advice].keys.each do |advice_key|
+          # Updates the advice corresponding to the key
+          QuestionAdvice.update(advice_key, advice: params[:advice][advice_key.to_sym][:advice])
+        end
+        flash[:notice] = 'The advice was successfully saved!'
+      end
+    rescue ActiveRecord::RecordNotFound
+      # If record not found, redirects to edit_advice
+      render action: 'edit_advice', id: params[:id]
+    end
+    redirect_to action: 'edit_advice', id: params[:id]
+  end
+end
\ No newline at end of file
diff --git a/app/helpers/authorization_helper.rb b/app/helpers/authorization_helper.rb
new file mode 100644
index 000000000..ca01890d0
--- /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.has_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/spec/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb
new file mode 100644
index 000000000..92929975d
--- /dev/null
+++ b/spec/controllers/advice_controller_spec.rb
@@ -0,0 +1,173 @@
+describe AdviceController do
+  let(:super_admin) { build(:superadmin, id: 1, role_id: 5) }
+  let(:instructor1) { build(:instructor, id: 10, role_id: 3, parent_id: 3, name: 'Instructor1') }
+  let(:student1) { build(:student, id: 21, role_id: 1) }
+
+  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
+    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: params, session: session
+        expect(result.status).to eq 200
+        expect(result).to render_template(:edit_advice)
+      end
+    end
+  end
+
+  describe '#save_advice' do
+    context "when save_advice is called" do
+      # When ad advice is saved
+      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 "saves advice successfully" do
+        # When an advice is saved successfully
+        allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire)
+        allow(QuestionAdvice).to receive(:update).with('1',{:advice => "Hello"}).and_return("Ok")
+        params = {advice: {"1" => {:advice => "Hello"}}, id: 1}
+        session = {user: instructor1}
+        result = get :save_advice, params: params, session: session
+        expect(flash[:notice]).to eq('The advice was successfully saved!')
+        expect(result.status).to eq 302
+        expect(result).to redirect_to('/advice/edit_advice?id=1')
+      end
+
+      it "does not save the advice" do
+        # When an advice is not saved
+        allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire)
+        allow(QuestionAdvice).to receive(:update).with(any_args).and_return("Ok")
+        params = {id: 1}
+        session = {user: instructor1}
+        result = get :save_advice, params: params, session: session
+        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
+  end
+end
\ No newline at end of file
diff --git a/spec/factories/factories.rb b/spec/factories/factories.rb
index eb06d7682..7945e2963 100644
--- a/spec/factories/factories.rb
+++ b/spec/factories/factories.rb
@@ -1,6 +1,80 @@
 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'
+    sequence(:fullname) { |n| "#{n}, superadministrator" }
+    email 'expertiza@mailinator.com'
+    parent_id 1
+    private_by_default  false
+    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'
+    digital_certificate nil
+    timezonepref nil
+    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'
+    sequence(:fullname) { |n| n = n % 3; "206#{n + 4}, student" }
+    email 'expertiza@mailinator.com'
+    parent_id 1
+    private_by_default  false
+    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'
+    digital_certificate nil
+    timezonepref 'Eastern Time (US & Canada)'
+    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/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:

From 217445a3fbfd6b7704e60875075977862599270f Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Wed, 10 Apr 2024 18:24:48 -0400
Subject: [PATCH 02/16] These are all the changes to get rspec running. there
 is one migrate

---
 Gemfile.lock                                  |   3 +
 app/controllers/application_controller.rb     |  13 +-
 app/helpers/authorization_helper.rb           |   2 +-
 app/models/criterion.rb                       | 189 ++++++++++++++++++
 app/models/question.rb                        |   1 +
 app/models/question_advice.rb                 |  29 +++
 app/models/review_questionnaire.rb            |  42 ++++
 config/routes.rb                              |   7 +
 .../20240408145535_create_question_advices.rb |  16 ++
 spec/controllers/advice_controller_spec.rb    |  12 +-
 spec/factories.rb                             |   1 -
 spec/factories/factories.rb                   | 100 +++++----
 12 files changed, 353 insertions(+), 62 deletions(-)
 create mode 100644 app/models/criterion.rb
 create mode 100644 app/models/question_advice.rb
 create mode 100644 app/models/review_questionnaire.rb
 create mode 100644 db/migrate/20240408145535_create_question_advices.rb

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/application_controller.rb b/app/controllers/application_controller.rb
index 4c8c36ece..39923752c 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,3 +1,14 @@
 class ApplicationController < ActionController::API
-  include JwtToken
+  #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
index ca01890d0..5b9e0fb93 100644
--- a/app/helpers/authorization_helper.rb
+++ b/app/helpers/authorization_helper.rb
@@ -27,7 +27,7 @@ def user_logged_in?
   # 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.has_all_privileges_of?(Role.find_by(name: 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?
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 = '<td align="center"><a rel="nofollow" data-method="delete" href="/questions/' + id.to_s + '">Remove</a></td>'
+
+    html += '<td><input size="6" value="' + seq.to_s + '" name="question[' + id.to_s + '][seq]"'
+    html += ' id="question_' + id.to_s + '_seq" type="text"></td>'
+
+    html += '<td><textarea cols="50" rows="1" name="question[' + id.to_s + '][txt]"'
+    html += ' id="question_' + id.to_s + '_txt" placeholder="Edit question content here">' + txt + '</textarea></td>'
+
+    html += '<td><input size="10" disabled="disabled" value="' + type + '" name="question[' + id.to_s + '][type]"'
+    html += ' id="question_' + id.to_s + '_type" type="text"></td>'
+
+    html += '<td><input size="2" value="' + weight.to_s
+    html += '" name="question[' + id.to_s + '][weight]" id="question_' + id.to_s + '_weight" type="text"></td>'
+
+    html += '<td>text area size <input size="3" value="' + size.to_s
+    html += '" name="question[' + id.to_s + '][size]" id="question_' + id.to_s + '_size" type="text"></td>'
+
+    html += '<td> min_label <input size="12" value="' + min_label.to_s
+    html += '" name="question[' + id.to_s + '][min_label]" id="question_' + id.to_s + '_min_label" type="text">  max_label <input size="10" value="' + max_label.to_s + '" name="question[' + id.to_s
+    html += '][max_label]" id="question_' + id.to_s + '_max_label" type="text"></td>'
+
+    safe_join(['<tr>'.html_safe, '</tr>'.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 = '<TD align="left"> ' + txt + ' </TD>'
+    html += '<TD align="left">' + type + '</TD>'
+    html += '<td align="center">' + weight.to_s + '</TD>'
+    questionnaire = self.questionnaire
+    if !max_label.nil? && !min_label.nil?
+      html += '<TD align="center"> (' + min_label + ') ' + questionnaire.min_question_score.to_s
+      html += ' to ' + questionnaire.max_question_score.to_s + ' (' + max_label + ')</TD>'
+    else
+      html += '<TD align="center">' + questionnaire.min_question_score.to_s + ' to ' + questionnaire.max_question_score.to_s + '</TD>'
+    end
+    safe_join(['<TR>'.html_safe, '</TR>'.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 = '<div><label for="responses_' + count.to_s + '">' + txt + '</label></div>'
+    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 = '<a id="showAdvice_' + id.to_s + '" onclick="showAdvice(' + id.to_s + ')">Show advice</a><script>'
+    html += 'function showAdvice(i){var element = document.getElementById("showAdvice_" + i.toString());'
+    html += 'var show = element.innerHTML == "Hide advice";'
+    html += 'if (show){element.innerHTML="Show advice";} else{element.innerHTML="Hide advice";}toggleAdvice(i);}'
+    html += 'function toggleAdvice(i) {var elem = document.getElementById(i.toString() + "_myDiv");'
+    html += 'if (elem.style.display == "none") {elem.style.display = "";} else {elem.style.display = "none";}}</script>'
+    html += '<div id="' + id.to_s + '_myDiv" style="display: none;">'
+    # [2015-10-26] Zhewei:
+    # best to order advices high to low, e.g., 5 to 1
+    # each level used to be a link;
+    # clicking on the link caused the dropbox to be filled in with the corresponding number
+    question_advices.reverse.each_with_index do |question_advice, index|
+      html += '<a id="changeScore_>' + id.to_s + '" onclick="changeScore(' + count.to_s + ',' + index.to_s + ')">'
+      html += (questionnaire.max_question_score - index).to_s + ' - ' + question_advice.advice + '</a><br/><script>'
+      html += 'function changeScore(i, j) {var elem = jQuery("#responses_" + i.toString() + "_score");'
+      html += 'var opts = elem.children("option").length;'
+      html += 'elem.val((' + questionnaire.max_question_score.to_s + ' - j).toString());}</script>'
+    end
+    html += '</div>'
+  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 = '<div><select id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" class="review-rating" ' + current_value + '>'
+    html += "<option value = ''>--</option>"
+    questionnaire_min.upto(questionnaire_max).each do |j|
+      html += '<option value=' + j.to_s
+      html += ' selected="selected"' if !answer.nil? && j == answer.answer
+      html += '>' + j.to_s
+      html += '-' + min_label if min_label.present? && j == questionnaire_min
+      html += '-' + max_label if max_label.present? && j == questionnaire_max
+      html += '</option>'
+    end
+
+    html += '</select></div><br><br><textarea' + ' id="responses_' + count.to_s + '_comments"'
+    html += ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
+    html += answer.comments if !answer.nil? && !answer.comments.nil?
+    html += '</textarea></td>'
+  end
+
+  # scale options to rate a project based on the question
+  def scale_criterion_question(count, answer = nil, questionnaire_min, questionnaire_max)
+    if size.nil? || size.blank?
+      cols = '70'
+      rows = '1'
+    else
+      cols = size.split(',')[0]
+      rows = size.split(',')[1]
+    end
+    html = '<input id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" type="hidden"'
+    html += 'value="' + answer.answer.to_s + '"' unless answer.nil?
+    html += '><table><tr><td width="10%"></td>'
+
+    (questionnaire_min..questionnaire_max).each do |j|
+      html += '<td width="10%"><label>' + j.to_s + '</label></td>'
+    end
+
+    html += '<td width="10%"></td></tr><tr><td width="10%">'
+    html += min_label unless min_label.nil?
+    html += '</td>'
+
+    (questionnaire_min..questionnaire_max).each do |j|
+      html += '<td width="10%"><input type="radio" id="' + j.to_s + '" value="' + j.to_s + '" name="Radio_' + id.to_s + '"'
+      html += 'checked="checked"' if (!answer.nil? && answer.answer == j) || (answer.nil? && questionnaire_min == j)
+      html += '></td>'
+    end
+    html += '<script>jQuery("input[name=Radio_' + id.to_s + ']:radio").change(function() {'
+    html += 'var response_score = jQuery("#responses_' + count.to_s + '_score");'
+    html += 'var checked_value = jQuery("input[name=Radio_' + id.to_s + ']:checked").val();'
+    html += 'response_score.val(checked_value);});</script><td width="10%">'
+    html += max_label unless max_label.nil?
+    html += '</td><td width="10%"></td></tr></table>'
+    html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
+      ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
+    html += answer.comments if !answer.nil? && !answer.comments.nil?
+    html += '</textarea>'
+  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 = '<b>' + count.to_s + '. ' + txt + ' [Max points: ' + questionnaire_max.to_s + ']</b>'
+    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 += '<table cellpadding="5"><tr><td>'
+    html += '<div class="' + score_color + '" style="width:30px; height:30px;' \
+      ' border-radius:50%; font-size:15px; color:black; line-height:30px; text-align:center;">'
+    html += score + '</div></td>'
+
+    if answer && !answer.comments.nil?
+      html += '<td style="padding-left:10px"><br>' + answer.comments.html_safe + '</td>'
+      #### 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 += '<tr><td colspan="2">'
+        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 += '</td></tr>'
+      end
+      #### end code to show tag prompts ####
+    end
+    html += '</tr></table>'
+    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/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
index 92929975d..0941f675c 100644
--- a/spec/controllers/advice_controller_spec.rb
+++ b/spec/controllers/advice_controller_spec.rb
@@ -1,7 +1,9 @@
-describe AdviceController do
-  let(:super_admin) { build(:superadmin, id: 1, role_id: 5) }
+require 'rails_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: 1) }
+  let(:student1) { build(:student, id: 21, role_id: 5) }
 
   describe '#action_allowed?' do
     context 'when the role of current user is Super-Admin' do
@@ -126,8 +128,8 @@
 
       it "edit advice redirects correctly when called" do
         allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire)
-        params = {id: 1}
-        session = {user: instructor1}
+        params = { id: 1 }
+        session = { user: instructor1 }
         result = get :edit_advice, params: params, session: session
         expect(result.status).to eq 200
         expect(result).to render_template(:edit_advice)
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 7945e2963..9b5ca2ebd 100644
--- a/spec/factories/factories.rb
+++ b/spec/factories/factories.rb
@@ -1,80 +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'
-    sequence(:fullname) { |n| "#{n}, superadministrator" }
-    email 'expertiza@mailinator.com'
-    parent_id 1
-    private_by_default  false
-    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'
-    digital_certificate nil
-    timezonepref nil
-    public_key nil
-    copy_of_emails false
+    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'
-    sequence(:fullname) { |n| n = n % 3; "206#{n + 4}, student" }
-    email 'expertiza@mailinator.com'
-    parent_id 1
-    private_by_default  false
-    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'
-    digital_certificate nil
-    timezonepref 'Eastern Time (US & Canada)'
-    public_key nil
-    copy_of_emails false
+    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'
+    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
+    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
+    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
+    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'
+   question { Question.first || association(:question) }
+   score { 5 }
+   advice { 'LGTM' }
   end
+
 end
 

From a1e26c0679f71df65b18961ba7e26da1deb04fe1 Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Sun, 14 Apr 2024 17:57:37 -0400
Subject: [PATCH 03/16] Reimplemented edit advice method

---
 app/controllers/advice_controller.rb | 154 +++++++++++++++------------
 1 file changed, 85 insertions(+), 69 deletions(-)

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index 00d2c8e7b..0396ad72b 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -1,70 +1,86 @@
-class AdviceController < ApplicationController
-  # 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
-
-  # checks whether the advices for a question in questionnaire have valid attributes
-  # return true if the number of advices and their scores are invalid, else returns false
-  def invalid_advice?(sorted_advice, num_advices, question)
-    return ((question.question_advices.length != num_advices) ||
-      sorted_advice.empty? ||
-      (sorted_advice[0].score != @questionnaire.max_question_score) ||
-      (sorted_advice[sorted_advice.length - 1].score != @questionnaire.min_question_score))
-  end
-
-  # Modify the advice associated with a questionnaire
-  def edit_advice
-    # Stores the questionnaire with given id in URL
-    @questionnaire = Questionnaire.find(params[:id])
-
-    # 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 = if question.is_a?(ScoredQuestion)
-                      @questionnaire.max_question_score - @questionnaire.min_question_score + 1
-                    else
-                      0
-                    end
-
-      # sorting question advices in descending order by score
-      sorted_advice = question.question_advices.sort_by { |x| x.score }.reverse
-
-      # 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
-    # Stores the questionnaire with given id in URL
-    @questionnaire = Questionnaire.find(params[:id])
-    begin
-      # checks if advice is present or not
-      unless params[:advice].nil?
-        params[:advice].keys.each do |advice_key|
-          # Updates the advice corresponding to the key
-          QuestionAdvice.update(advice_key, advice: params[:advice][advice_key.to_sym][:advice])
-        end
-        flash[:notice] = 'The advice was successfully saved!'
-      end
-    rescue ActiveRecord::RecordNotFound
-      # If record not found, redirects to edit_advice
-      render action: 'edit_advice', id: params[:id]
-    end
-    redirect_to action: 'edit_advice', id: params[:id]
-  end
+class AdviceController < ApplicationController
+  # 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
+
+  # checks whether the advices for a question in questionnaire have valid attributes
+  # return true if the number of advices and their scores are invalid, else returns false
+  def invalid_advice?(sorted_advice, num_advices, question)
+    return ((question.question_advices.length != num_advices) ||
+      sorted_advice.empty? ||
+      (sorted_advice[0].score != @questionnaire.max_question_score) ||
+      (sorted_advice[sorted_advice.length - 1].score != @questionnaire.min_question_score))
+  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
+    # Stores the questionnaire with given id in URL
+    @questionnaire = Questionnaire.find(params[:id])
+
+    # 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
+      # # Call to a separate method to adhere to Single Responsibility Principle
+      num_advices = calculate_num_advices(question)
+
+      # sorting question advices in descending order by score
+      # Call to a separate method to adhere to Single Responsibility Principle
+      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
+
+  # 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
+  def calculate_num_advices(question)
+    if question.is_a?(ScoredQuestion)
+      @questionnaire.max_question_score - @questionnaire.min_question_score + 1
+    else
+      0
+    end
+  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
+
+  # save the advice for a questionnaire
+  def save_advice
+    # Stores the questionnaire with given id in URL
+    @questionnaire = Questionnaire.find(params[:id])
+    begin
+      # checks if advice is present or not
+      unless params[:advice].nil?
+        params[:advice].keys.each do |advice_key|
+          # Updates the advice corresponding to the key
+          QuestionAdvice.update(advice_key, advice: params[:advice][advice_key.to_sym][:advice])
+        end
+        flash[:notice] = 'The advice was successfully saved!'
+      end
+    rescue ActiveRecord::RecordNotFound
+      # If record not found, redirects to edit_advice
+      render action: 'edit_advice', id: params[:id]
+    end
+    redirect_to action: 'edit_advice', id: params[:id]
+  end
 end
\ No newline at end of file

From 538e7c366214347a986955a519568a61558ade29 Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Mon, 15 Apr 2024 09:57:57 -0400
Subject: [PATCH 04/16] Updated edit_advice method

---
 app/controllers/advice_controller.rb | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index 0396ad72b..a6f4d45e6 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -50,14 +50,12 @@ def edit_advice
 
   # 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)
-    if question.is_a?(ScoredQuestion)
-      @questionnaire.max_question_score - @questionnaire.min_question_score + 1
-    else
-      0
-    end
+    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)

From 8b5db0cd60a23c3b032eda025bec863aa059efaa Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Mon, 15 Apr 2024 18:59:24 -0400
Subject: [PATCH 05/16] Updated controller save method

---
 app/controllers/advice_controller.rb | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index a6f4d45e6..6d8d15bf6 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -69,16 +69,18 @@ def save_advice
     begin
       # checks if advice is present or not
       unless params[:advice].nil?
-        params[:advice].keys.each do |advice_key|
-          # Updates the advice corresponding to the key
-          QuestionAdvice.update(advice_key, advice: params[:advice][advice_key.to_sym][:advice])
+        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
-      render action: 'edit_advice', id: params[:id]
+      # 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
-end
\ No newline at end of file
+end

From 3be59d5a6edd020e00bf5bba3c97fffae4538aa0 Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Mon, 15 Apr 2024 19:00:06 -0400
Subject: [PATCH 06/16] Update save method rspec tests based on test skeleton
 provided

---
 spec/controllers/advice_controller_spec.rb | 201 ++++++++++++++-------
 1 file changed, 140 insertions(+), 61 deletions(-)

diff --git a/spec/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb
index 0941f675c..718cfe8e6 100644
--- a/spec/controllers/advice_controller_spec.rb
+++ b/spec/controllers/advice_controller_spec.rb
@@ -30,107 +30,107 @@
   end
 
   describe '#invalid_advice?' do
-    context "when invalid_advice? is called with question advice score > max score of questionnaire" 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(: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)
+                              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
+      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)
+        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
+    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(: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)
+                              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
+      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)
+        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
+    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(: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)
+                              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
+      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)
+        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
+    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)
+                              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
+      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)
+        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
+    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(: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)
+                              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
+      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)
+        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
+    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(: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)
+                              questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2)
       end
 
-      it "edit advice redirects correctly when called" do
+      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: params, session: session
+        result = get(:edit_advice, params:, session:)
         expect(result.status).to eq 200
         expect(result).to render_template(:edit_advice)
       end
@@ -138,38 +138,117 @@
   end
 
   describe '#save_advice' do
-    context "when save_advice is called" do
-      # When ad advice is saved
-      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)
+    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 "saves advice successfully" do
-        # When an advice is saved successfully
+      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")
-        params = {advice: {"1" => {:advice => "Hello"}}, id: 1}
-        session = {user: instructor1}
-        result = get :save_advice, params: params, session: session
+        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!')
-        expect(result.status).to eq 302
-        expect(result).to redirect_to('/advice/edit_advice?id=1')
       end
+    end
 
-      it "does not save the advice" do
-        # When an advice is not saved
+    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")
-        params = {id: 1}
-        session = {user: instructor1}
-        result = get :save_advice, params: params, session: session
+        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

From 8b6de5f8c9a37db81c2a61feae3a84cb9e9e056b Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Mon, 15 Apr 2024 19:00:36 -0400
Subject: [PATCH 07/16] Gem update for rspec testing

---
 Gemfile | 1 +
 1 file changed, 1 insertion(+)

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

From bebf4a15bbb9d8b5d1d0c6f1ac77c664c261be1b Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Mon, 15 Apr 2024 19:01:14 -0400
Subject: [PATCH 08/16] Final commit with superclass change for controllers and
 missing erb for testing

---
 app/controllers/application_controller.rb | 2 +-
 app/views/advice/edit_advice.erb          | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 create mode 100644 app/views/advice/edit_advice.erb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 39923752c..881e25f56 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,4 +1,4 @@
-class ApplicationController < ActionController::API
+class ApplicationController < ActionController::Base
   #include JwtToken
   def current_user
     @current_user ||= session[:user]
diff --git a/app/views/advice/edit_advice.erb b/app/views/advice/edit_advice.erb
new file mode 100644
index 000000000..e69de29bb

From e882f7b49c9edad41705d5c6b7582936069fa279 Mon Sep 17 00:00:00 2001
From: bdevine2 <bdevine2@ncsu.edu>
Date: Tue, 16 Apr 2024 13:32:20 -0400
Subject: [PATCH 09/16] Moved the setting of questionnaire into more modern
 before action with private method. To make DRY

---
 app/controllers/advice_controller.rb | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index 6d8d15bf6..8d6fd215b 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -1,4 +1,5 @@
 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
@@ -26,9 +27,6 @@ def invalid_advice?(sorted_advice, num_advices, question)
   # 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
-    # Stores the questionnaire with given id in URL
-    @questionnaire = Questionnaire.find(params[:id])
-
     # 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|
@@ -64,8 +62,6 @@ def sort_question_advices(question)
 
   # save the advice for a questionnaire
   def save_advice
-    # Stores the questionnaire with given id in URL
-    @questionnaire = Questionnaire.find(params[:id])
     begin
       # checks if advice is present or not
       unless params[:advice].nil?
@@ -83,4 +79,12 @@ def save_advice
     # 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
 end

From 9a910d46e7511f48c26b9e4e4d6f0d637cc2b2e8 Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Tue, 16 Apr 2024 15:00:02 -0400
Subject: [PATCH 10/16] Added tests for edit_advice method

---
 app/helpers/questionnaire_helper.rb        | 57 ++++++++++++++++++++++
 spec/controllers/advice_controller_spec.rb | 41 ++++++++++++++++
 spec/questionnaire_helper.rb               | 57 ++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 app/helpers/questionnaire_helper.rb
 create mode 100644 spec/questionnaire_helper.rb

diff --git a/app/helpers/questionnaire_helper.rb b/app/helpers/questionnaire_helper.rb
new file mode 100644
index 000000000..22b709f28
--- /dev/null
+++ b/app/helpers/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/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb
index 718cfe8e6..549d3d1c1 100644
--- a/spec/controllers/advice_controller_spec.rb
+++ b/spec/controllers/advice_controller_spec.rb
@@ -1,4 +1,5 @@
 require 'rails_helper'
+require 'questionnaire_helper'
 
 describe AdviceController, type: :controller do
   let(:super_admin) { build(:superadmin, id: 1, role_id: 1) }
@@ -135,6 +136,46 @@
         expect(result).to render_template(:edit_advice)
       end
     end
+
+    context 'when advice adjustment is 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
+
+      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 # Expecting it to be called once for the question
+        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 # Expecting it to be called once for the question
+        get :edit_advice, params: { id: 1 }
+      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
   end
 
   describe '#save_advice' do
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

From ba4ce13bf98dd43f858e52c15037b173013d65b8 Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Wed, 17 Apr 2024 18:43:21 -0400
Subject: [PATCH 11/16] Deleted duplicate questionnaire helper file

---
 app/helpers/questionnaire_helper.rb | 57 -----------------------------
 1 file changed, 57 deletions(-)
 delete mode 100644 app/helpers/questionnaire_helper.rb

diff --git a/app/helpers/questionnaire_helper.rb b/app/helpers/questionnaire_helper.rb
deleted file mode 100644
index 22b709f28..000000000
--- a/app/helpers/questionnaire_helper.rb
+++ /dev/null
@@ -1,57 +0,0 @@
-# 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

From ed4e1410b1c24883db455fa87721a58cda96cd12 Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Wed, 17 Apr 2024 18:53:01 -0400
Subject: [PATCH 12/16] updated edit advice method

---
 spec/controllers/advice_controller_spec.rb | 69 +++++++++++++++++++---
 1 file changed, 61 insertions(+), 8 deletions(-)

diff --git a/spec/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb
index 549d3d1c1..c7243e979 100644
--- a/spec/controllers/advice_controller_spec.rb
+++ b/spec/controllers/advice_controller_spec.rb
@@ -124,7 +124,7 @@
       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)
+              questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1, questionAdvice2])], max_question_score: 2)
       end
 
       it 'edit advice redirects correctly when called' do
@@ -137,7 +137,22 @@
       end
     end
 
-    context 'when advice adjustment is necessary' do
+    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
@@ -150,18 +165,42 @@
         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 # Expecting it to be called once for the question
+      it "calculates the number of advices for each question" do
+        expect(controller).to receive(:calculate_num_advices).once # Assuming there are two questions 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 are two questions 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 'sorts question advices in descending order by score' do
-        expect(controller).to receive(:sort_question_advices).once # Expecting it to be called once for the question
+      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 advice adjustment is not necessary' do
+    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
@@ -169,9 +208,23 @@
               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
+      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

From 5db9a88ef93d47806084179e54317c5b93661523 Mon Sep 17 00:00:00 2001
From: Aiden Bartlett <aebartl3@ncsu.edu>
Date: Sun, 21 Apr 2024 14:12:31 -0400
Subject: [PATCH 13/16] refactoring invalid_advice and updating tests

---
 app/controllers/advice_controller.rb       | 15 ++++
 spec/controllers/advice_controller_spec.rb | 80 ++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index 8d6fd215b..a8dbeff3b 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -87,4 +87,19 @@ 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
+
 end
diff --git a/spec/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb
index c7243e979..539a65ad4 100644
--- a/spec/controllers/advice_controller_spec.rb
+++ b/spec/controllers/advice_controller_spec.rb
@@ -30,6 +30,83 @@
     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)
@@ -116,6 +193,9 @@
     end
   end
 
+  ########################################################################################
+
+
   describe '#edit_advice' do
 
     context 'when edit_advice is called and invalid_advice? evaluates to true' do

From e21d15857cda139ab002609631b494f5be54fbdd Mon Sep 17 00:00:00 2001
From: Aiden Bartlett <aebartl3@ncsu.edu>
Date: Sun, 21 Apr 2024 16:20:21 -0400
Subject: [PATCH 14/16] implementing new invalid_advice change

---
 app/controllers/advice_controller.rb | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index a8dbeff3b..7fe946a0b 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -14,13 +14,11 @@ def action_allowed?
     current_user_has_ta_privileges?
   end
 
-  # checks whether the advices for a question in questionnaire have valid attributes
-  # return true if the number of advices and their scores are invalid, else returns false
+  ## 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)
-    return ((question.question_advices.length != num_advices) ||
-      sorted_advice.empty? ||
-      (sorted_advice[0].score != @questionnaire.max_question_score) ||
-      (sorted_advice[sorted_advice.length - 1].score != @questionnaire.min_question_score))
+    invalid_advice_length?(num_advices, question, sorted_advice) ||
+      invalid_advice_scores?(sorted_advice)
   end
 
   # Modify the advice associated with a questionnaire

From 2956063755f53a088d7c06bd4a7a0f0ca74bc3df Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Tue, 23 Apr 2024 21:23:46 -0400
Subject: [PATCH 15/16] Final updates for edit_advice method

---
 app/controllers/advice_controller.rb       | 30 ++++++------
 config/database.yml.example                | 55 ----------------------
 spec/controllers/advice_controller_spec.rb |  4 +-
 3 files changed, 18 insertions(+), 71 deletions(-)
 delete mode 100644 config/database.yml.example

diff --git a/app/controllers/advice_controller.rb b/app/controllers/advice_controller.rb
index 7fe946a0b..fcf4dabb8 100644
--- a/app/controllers/advice_controller.rb
+++ b/app/controllers/advice_controller.rb
@@ -29,11 +29,11 @@ def edit_advice
     # 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
-      # # Call to a separate method to adhere to Single Responsibility Principle
+
       num_advices = calculate_num_advices(question)
 
       # sorting question advices in descending order by score
-      # Call to a separate method to adhere to Single Responsibility Principle
+
       sorted_advice = sort_question_advices(question)
 
       # Checks the condition for adjusting the advice size
@@ -44,19 +44,7 @@ def edit_advice
     end
   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
 
   # save the advice for a questionnaire
   def save_advice
@@ -100,4 +88,18 @@ def invalid_advice_scores?(sorted_advice)
       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/config/database.yml.example b/config/database.yml.example
deleted file mode 100644
index b460620e1..000000000
--- a/config/database.yml.example
+++ /dev/null
@@ -1,55 +0,0 @@
-# MySQL. Versions 5.5.8 and up are supported.
-#
-# Install the MySQL driver
-#   gem install mysql2
-#
-# Ensure the MySQL gem is defined in your Gemfile
-#   gem "mysql2"
-#
-# And be sure to use new-style password hashing:
-#   https://dev.mysql.com/doc/refman/5.7/en/password-hashing.html
-#
-default: &default
-  adapter: mysql2
-  encoding: utf8mb4
-  pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
-  username: dev
-  password: Root@123
-
-
-development:
-  <<: *default
-  database: reimplementation_development
-
-# Warning: The database defined as "test" will be erased and
-# re-generated from your development database when you run "rake".
-# Do not set this db to the same as development or production.
-test:
-  <<: *default
-  database: reimplementation_test
-
-# As with config/credentials.yml, you never want to store sensitive information,
-# like your database password, in your source code. If your source code is
-# ever seen by anyone, they now have access to your database.
-#
-# Instead, provide the password or a full connection URL as an environment
-# variable when you boot the app. For example:
-#
-#   DATABASE_URL="mysql2://myuser:mypass@localhost/somedatabase"
-#
-# If the connection URL is provided in the special DATABASE_URL environment
-# variable, Rails will automatically merge its configuration values on top of
-# the values provided in this file. Alternatively, you can specify a connection
-# URL environment variable explicitly:
-#
-#   production:
-#     url: <%= ENV["MY_APP_DATABASE_URL"] %>
-#
-# Read https://guides.rubyonrails.org/configuring.html#configuring-a-database
-# for a full overview on how database connection configuration can be specified.
-#
-production:
-  <<: *default
-  database: reimplementation_production
-  username: reimplementation
-  password: <%= ENV["REIMPLEMENTATION_DATABASE_PASSWORD"] %>
\ No newline at end of file
diff --git a/spec/controllers/advice_controller_spec.rb b/spec/controllers/advice_controller_spec.rb
index 539a65ad4..a20f7c259 100644
--- a/spec/controllers/advice_controller_spec.rb
+++ b/spec/controllers/advice_controller_spec.rb
@@ -246,12 +246,12 @@
       end
 
       it "calculates the number of advices for each question" do
-        expect(controller).to receive(:calculate_num_advices).once # Assuming there are two questions in the questionnaire
+        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 are two questions in the questionnaire
+        expect(controller).to receive(:sort_question_advices).once # Assuming there is one question in the questionnaire
         get :edit_advice, params: { id: 1 }
       end
 

From b9eaa2b0bb05c9e77332c339f0b4d0118ac7b656 Mon Sep 17 00:00:00 2001
From: Aditya Karthikeyan <adityakofficial01@gmail.com>
Date: Tue, 23 Apr 2024 21:36:41 -0400
Subject: [PATCH 16/16] Added database.yml.example

---
 config/database.yml.example | 55 +++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 config/database.yml.example

diff --git a/config/database.yml.example b/config/database.yml.example
new file mode 100644
index 000000000..b460620e1
--- /dev/null
+++ b/config/database.yml.example
@@ -0,0 +1,55 @@
+# MySQL. Versions 5.5.8 and up are supported.
+#
+# Install the MySQL driver
+#   gem install mysql2
+#
+# Ensure the MySQL gem is defined in your Gemfile
+#   gem "mysql2"
+#
+# And be sure to use new-style password hashing:
+#   https://dev.mysql.com/doc/refman/5.7/en/password-hashing.html
+#
+default: &default
+  adapter: mysql2
+  encoding: utf8mb4
+  pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
+  username: dev
+  password: Root@123
+
+
+development:
+  <<: *default
+  database: reimplementation_development
+
+# Warning: The database defined as "test" will be erased and
+# re-generated from your development database when you run "rake".
+# Do not set this db to the same as development or production.
+test:
+  <<: *default
+  database: reimplementation_test
+
+# As with config/credentials.yml, you never want to store sensitive information,
+# like your database password, in your source code. If your source code is
+# ever seen by anyone, they now have access to your database.
+#
+# Instead, provide the password or a full connection URL as an environment
+# variable when you boot the app. For example:
+#
+#   DATABASE_URL="mysql2://myuser:mypass@localhost/somedatabase"
+#
+# If the connection URL is provided in the special DATABASE_URL environment
+# variable, Rails will automatically merge its configuration values on top of
+# the values provided in this file. Alternatively, you can specify a connection
+# URL environment variable explicitly:
+#
+#   production:
+#     url: <%= ENV["MY_APP_DATABASE_URL"] %>
+#
+# Read https://guides.rubyonrails.org/configuring.html#configuring-a-database
+# for a full overview on how database connection configuration can be specified.
+#
+production:
+  <<: *default
+  database: reimplementation_production
+  username: reimplementation
+  password: <%= ENV["REIMPLEMENTATION_DATABASE_PASSWORD"] %>
\ No newline at end of file