CSC/ECE 517 Fall 2017/E1770 Refactor assignment participant.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
 
(34 intermediate revisions by the same user not shown)
Line 1: Line 1:
'''E1770. [Test First Development] Refactor assignment_participant.rb'''<ref>Project Description document https://docs.google.com/document/d/1rdolBAHxVGI9I0N-cT866AqnfORM2L1_m_bo2gRYRrI/edit#heading=h.s18ldps2kz5e</ref>
'''E1770. [Test First Development] Refactor assignment_participant.rb'''<ref>Project Description document https://docs.google.com/document/d/1rdolBAHxVGI9I0N-cT866AqnfORM2L1_m_bo2gRYRrI/edit#heading=h.s18ldps2kz5e</ref>


This page provides a brief description of the '''Expertiza''' project. The project is aimed at refactoring the AssignmentParticipant model which is subclass of Participant model. This model is used to maintain the list of students/users participating in a given assignment. For any new or existing assignments, this model manages the entire list of users assigned to that particular assignment. It primarily includes method for scores calculations, assignment submission paths, reviews, etc. As part of this project, some of the methods have been removed which were not being used anywhere, some have been moved to their appropriate helper module and some have been refactored into smaller ones.
This page provides a brief description of the '''Expertiza''' project. The project is aimed at using TFD(Test First Development) to refactor the AssignmentParticipant model which is subclass of Participant model. This model is used to maintain the list of students/users participating in a given assignment. For any new or existing assignments, this model manages the entire list of users assigned to that particular assignment. It primarily includes method for scores calculations, assignment submission paths, reviews, etc.
 
We used the TDF(Test First Development) method to refactor the AssignmentParticipant model. With the  RSpec tool, we wrote 38 test cases for 24 methods in '''assignment_participant_spec.rb''' first. After finishing these test cases, we executed them, and most of them failed. And then we refactored the methods, '''scores''', '''files''', '''self.import''', '''find_by''', '''where.first''', and executed the test cases again, finally they all passed.


==Introduction to Expertiza==
==Introduction to Expertiza==
Line 48: Line 50:
** Write failing tests first.
** Write failing tests first.


==Test Cases==
== Test Cases ==
Because the project is aimed at using TFD(Test First Development) to refactor the AssignmentParticipant model.
We wrote 38 test cases for 24 methods in '''assignment_participant_spec.rb''' first, with the  RSpec tool.
 
The introduction video to our project<ref>Introduction video to our project https://www.youtube.com/watch?v=sJdmN-S2Voo&feature=youtu.be</ref>
  describe '#dir_path' do
    it 'returns the directory path of current assignment' do
      expect(participant.dir_path).to eq "final_test"
    end
  end
 
  describe '#assign_quiz' do
    it 'creates a new QuizResponseMap record' do
      allow(QuizQuestionnaire).to receive(:find_by).with(instructor_id: 1).and_return(quiz_questionaire) # WHY CAN NOT DELETE THIS SENTENCE
      expect { participant.assign_quiz(participant, participant2, nil) }.to change { QuizResponseMap.count }.from(0).to(1)
      expect(participant.assign_quiz(participant, participant2, nil)).to be_an_instance_of(QuizResponseMap)
    end
  end
 
  describe '#reviewers' do
    it 'returns all the participants in this assignment who have reviewed the team where this participant belongs' do
      allow(ReviewResponseMap).to receive(:where).with(any_args).and_return([response_map]) # differences with .with(parameter)
      allow(AssignmentParticipant).to receive(:find).with(any_args).and_return(participant2)
      expect(participant.reviewers).to eq([participant2])
    end
  end
 
  describe '#review_score' do
    it 'returns the review score' do
      # diao yong de method tai duo le,er qie bu zhi dao sha yi si
      allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
      allow(review_questionnaire).to receive(:questions).and_return(question)
      allow(Answer).to receive(:compute_scores).with([response], question).and_return(avg: 100)
      allow(review_questionnaire).to receive(:max_possible_score).and_return(100)
      expect(participant.review_score).to eq(100)
    end
  end
 
  describe '#scores' do
    context 'when assignment is not varying rubric by round and not an microtask' do
      it 'calculates scores that this participant has been given' do
        expect(participant).to receive(:assignment_questionnaires)
        expect(assignment).to receive(:compute_total_score)
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
        allow(assignment).to receive(:is_microtask?).and_return(false)
        expect(assignment).to receive(:compute_total_score)
        expect(participant).to receive(:caculate_scores)
        participant.scores(question)
      end
    end
 
    context 'when assignment is varying rubric by round but not an microtask' do
      it 'calculates scores that this participant has been given' do
        expect(participant).to receive(:assignment_questionnaires)
        expect(assignment).to receive(:compute_total_score)
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(true)
        expect(participant).to receive(:merge_scores)
        allow(assignment).to receive(:is_microtask?).and_return(false)
        expect(assignment).to receive(:compute_total_score)
        expect(participant).to receive(:caculate_scores)
        participant.scores(question)
      end
    end
 
    context 'when assignment is not varying rubric by round but an microtask' do
      it 'calculates scores that this participant has been given' do
        expect(participant).to receive(:assignment_questionnaires)
        expect(assignment).to receive(:compute_total_score)
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
        allow(assignment).to receive(:is_microtask?).and_return(true)
        expect(participant).to receive(:topic_total_scores)
        expect(assignment).to receive(:compute_total_score)
        expect(participant).to receive(:caculate_scores)
        participant.scores(question)
      end
    end
  end
 
  # included method
  describe '#assignment_questionnaires' do
    context 'when the round of questionnaire is nil' do
      it 'record the result as review scores' do
        scores = {}
        question_hash = {review: question}
        score_map = {max: 100, min: 100, avg: 100}
        allow(AssignmentQuestionnaire).to receive(:find_by).with(any_args).and_return(assignment_questionnaire)
        allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
        allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
        participant.assignment_questionnaires(question_hash, scores)
        expect(scores[:review][:assessments]).to eq([response])
        expect(scores[:review][:scores]).to eq(score_map)
      end
    end
 
    context 'when the round of questionnaire is not nil' do
      it 'record the result as review#{n} scores' do
        scores = {}
        question_hash = {review1: question}
        score_map = {max: 100, min: 100, avg: 100}
        allow(AssignmentQuestionnaire).to receive(:find_by).and_return(assignment_questionnaire2)
        allow(review_questionnaire).to receive(:get_assessments_round_for).with(any_args).and_return([response])
        allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
        participant.assignment_questionnaires(question_hash, scores)
        expect(scores[:review1][:assessments]).to eq([response])
        expect(scores[:review1][:scores]).to eq(score_map)
      end
    end
  end
 
  describe '#merge_scores' do
    context 'when all of the review_n are nil' do
      it 'set max, min, avg of review score as 0' do
        scores = {}
        allow(assignment).to receive(:num_review_rounds).and_return(1)
        participant.merge_scores(scores)
        expect(scores[:review][:scores][:max]).to eq(0)
        expect(scores[:review][:scores][:min]).to eq(0)
        expect(scores[:review][:scores][:min]).to eq(0)
      end
    end
 
    context 'when the review_n is not nil' do
      it 'merge the score of review_n to the score of review' do
        score_map = {max: 100, min: 100, avg: 100}
        scores = {review1: {scores: score_map, assessments: [response]}}
        allow(assignment).to receive(:num_review_rounds).and_return(1)
        participant.merge_scores(scores)
        expect(scores[:review][:scores][:max]).to eq(100)
        expect(scores[:review][:scores][:min]).to eq(100)
        expect(scores[:review][:scores][:min]).to eq(100)
      end
    end
  end
 
  describe '#topic_total_scores' do
    it 'set total_score and max_pts_available of score when topic is not nil' do
      scores = {total_score: 100}
      allow(SignUpTopic).to receive(:find_by).with(any_args).and_return(topic)
      participant.topic_total_scores(scores)
      expect(scores[:total_score]).to eq(0)
      expect(scores[:max_pts_available]).to eq(0)
    end
  end
 
  describe '#caculate_scores' do
    context 'when the participant has the grade' do
      it 'his total scores equals his grade' do
        scores = {}
        expect(participant2.caculate_scores(scores)).to eq(100.0)
      end
    end
    context 'when the participant has the grade and the total score more than 100' do
      it 'return the score of a given participant with total score 100' do
        scores = {total_score: 110}
        expect(participant.caculate_scores(scores)).to eq(total_score: 100)
      end
    end
    context 'when the participant has the grade and the total score less than 100' do
      it 'return the score of a given participant with total score' do
        scores = {total_score: 90}
        expect(participant.caculate_scores(scores)).to eq(total_score: 90)
      end
    end
  end
 
  describe '#copy' do
    it 'copies assignment participants to a certain course' do
      expect(participant.copy(517)).to be_an_instance_of(CourseParticipant)
    end
  end
 
  describe '#feedback' do
    it 'returns corrsponding author feedback responses given by current participant' do
      expect(participant.feedback).to eq([response])
    end
  end
 
  describe '#reviews' do
    it 'returns corrsponding peer review responses given by current team' do
      expect(participant.reviews).to eq([response])
    end
  end
 
  describe '#reviews_by_reviewer' do
    it 'returns corrsponding peer review responses given by certain reviewer' do
      allow(ReviewResponseMap).to receive(:get_reviewer_assessments_for).with(team, participant2).and_return([response])
      expect(participant.reviews_by_reviewer(participant2)).to eq([response])
    end
  end
 
  describe '#quizzes_taken' do
    it 'returns corrsponding quiz responses given by current participant' do
      expect(participant.quizzes_taken).to eq([response])
    end
  end
 
  describe '#metareviews' do
    it 'returns corrsponding metareview responses given by current participant' do
      expect(participant.metareviews).to eq([response])
    end
  end
 
  describe '#teammate_reviews' do
    it 'returns corrsponding teammate review responses given by current participant' do
      expect(participant.teammate_reviews).to eq([response])
    end
  end
 
  describe '#bookmark_reviews' do
    it 'returns corrsponding bookmark review responses given by current participant' do
      expect(participant.bookmark_reviews).to eq([response])
    end
  end
 
  describe '#files' do
    context 'when there is not subdirectory in current directory' do
      it 'returns all files in current directory' do
        allow(Dir).to receive(:[]).with("a/*").and_return(["a/k.rb"])
        allow(File).to receive(:directory?).with("a/k.rb").and_return(false)
        expect(participant.files("a")).to eq(["a/k.rb"])
      end
    end
 
    context 'when there is subdirectory in current directory' do
      it 'recursively returns all files in current directory' do
        allow(Dir).to receive(:[]).with("a/*").and_return(["a/b"])
        allow(File).to receive(:directory?).with("a/b").and_return(true)
        allow(Dir).to receive(:[]).with("a/b/*").and_return(["a/b/k.rb"])
        allow(File).to receive(:directory?).with("a/b/k.rb").and_return(false)
        expect(participant.files("a")).to eq(["a/b/k.rb", "a/b"])
      end
    end
  end
 
  describe ".import" do
    context 'when record is empty' do
      it 'raises an ArgumentError' do
        row = []
        allow(AssignmentParticipant).to receive(:check_info_and_create).with(any_args).and_raise("No user id has been specified.")
        expect(AssignmentParticipant).to receive(:check_info_and_create)
        expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("No user id has been specified.")
      end
    end
 
    context 'when no user is found by offered username' do
      context 'when the record has less than 4 items' do
        it 'raises an ArgumentError' do
          row = ["user_name", "user_fullname", "name@email.com"]
          allow(AssignmentParticipant).
            to receive(:check_info_and_create).with(any_args).and_raise("The record containing #{row[0]} does not have enough items.")
          expect(AssignmentParticipant).to receive(:check_info_and_create)
          expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("The record containing #{row[0]} does not have enough items.")
        end
      end
 
      context 'when the record has more than 4 items' do
        context 'when certain assignment cannot be found' do
          it 'creates a new user based on import information and raises an ImportError' do
            row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
            session = {user: participant}
            allow(User).to receive(:find_by).with(any_args).and_return(nil)
            allow(Assignment).to receive(:find).with(2).and_return(nil)
            expect { AssignmentParticipant.import(row, nil, session, 2) }.
              to raise_error("The assignment with id \"2\" was not found.").and change { User.count }.by(1)
          end
        end
 
        context 'when certain assignment can be found and assignment participant does not exists' do
          it 'creates a new user, new participant and raises an ImportError' do
            row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
            session = {user: participant}
            allow(User).to receive(:find_by_name).with(any_args).and_return(nil)
            allow(Assignment).to receive(:find).with(2).and_return(assignment)
            allow(AssignmentParticipant).to receive(:exists?).and_return(false)
            expect { AssignmentParticipant.import(row, nil, session, 2) }.to change { User.count }.by(1).and change { AssignmentParticipant.count }.by(1)
          end
        end
      end
    end
  end
 
  describe '.export' do
    it 'exports all participants in current assignment' do
      csv = []
      expect(AssignmentParticipant).to receive_message_chain(:where, :find_each).with(any_args).and_yield(participant)
      expect(AssignmentParticipant.export(csv, 1, any_args)).
        to eq([["student2064", "2064, student", "expertiza@mailinator.com", "Student", "instructor6", true, true, true, "handle"]])
    end
  end
 
  describe '#set_handle' do
    context 'when the user of current participant does not have handle' do
      it 'sets the user name as the handle of current participant' do
        allow(student).to receive_message_chain(:handle, :nil?).and_return(true)
        allow(student).to receive(:handle).and_return("")
        participant.set_handle
        expect(participant.handle).to eq("student2064")
      end
    end
 
    context 'when current assignment exists participants with same handle as the one of current user' do
      it 'sets the user name as the name of current participant' do
        allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
        participant.set_handle
        expect(participant.handle).to eq("student2064")
      end
    end
 
    context 'when current assignment does not have participants with same handle as the one of current user' do
      it 'sets the user name as the handle of current participant' do
        participant.set_handle
        expect(participant.handle).to eq("handle")
      end
    end
  end
 
  describe '#review_file_path' do
    it 'returns the file path for reviewer to upload files during peer review' do
      allow(ResponseMap).to receive(:find).with(any_args).and_return(response_map)
      allow(TeamsUser).to receive_message_chain(:find_by, :user_id).with(any_args).and_return(1)
      allow(Participant).to receive(:find_by).with(any_args).and_return(participant)
      file_path = Rails.root.to_s + "/pg_data/instructor6/csc517/test/final_test/0_review/1"
      expect(participant.review_file_path(1)).to eq(file_path)
    end
  end
 
  describe '#current_stage' do
    it 'returns stage of current assignment' do
      allow(assignment).to receive(:get_current_stage).with(1).and_return("Finished")
      expect(participant.current_stage).to eq("Finished")
    end
  end
 
  describe '#stage_deadline' do
    context 'when stage of current assignment is not Finished' do
      it 'returns current stage' do
        allow(assignment).to receive(:stage_deadline).with(1).and_return("Unknow")
        expect(participant.stage_deadline).to eq("Unknow")
      end
    end
 
    context 'when stage of current assignment not Finished' do
      context 'current assignment is not a staggered deadline assignment' do
        it 'returns the due date of current assignment' do
          allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
          allow(assignment).to receive(:staggered_deadline?).and_return(false)
          allow(assignment).to receive_message_chain(:due_dates, :last, :due_at).and_return(1)
          expect(participant.stage_deadline).to eq("1")
        end
      end
 
      context 'current assignment is a staggered deadline assignment' do
        it 'returns the due date of current topic' do
          allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
          allow(assignment).to receive(:staggered_deadline?).and_return(true)
          allow(TopicDueDate).to receive_message_chain(:find_by, :last, :due_at).and_return(1)
          expect(participant.stage_deadline).to eq("1")
        end
      end
    end
  end


== Refactoring ==
== Refactoring ==
Line 58: Line 420:
!Before!!After
!Before!!After
|-
|-
|def scores(questions)
| def scores(questions)
     scores = {}
     scores = {}
     scores[:participant] = self
     scores[:participant] = self
     self.assignment.questionnaires.each do |questionnaire|
     self.assignment.questionnaires.each do |questionnaire|
       round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
       round = AssignmentQuestionnaire.find_by(assignment_id: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
       #create symbol for "varying rubrics" feature -Yang
       # create symbol for "varying rubrics" feature -Yang
       if(round!=nil)
       questionnaire_symbol = if round.nil?
        questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
                              questionnaire.symbol
      else
                            else
        questionnaire_symbol = questionnaire.symbol
                              (questionnaire.symbol.to_s + round.to_s).to_sym
      end
                            end


       scores[questionnaire_symbol] = {}
       scores[questionnaire_symbol] = {}


       if round==nil
       scores[questionnaire_symbol][:assessments] = if round.nil?
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
                                                    questionnaire.get_assessments_for(self)
      else
                                                  else
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
                                                    questionnaire.get_assessments_round_for(self, round)
      end
                                                  end
       scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
       scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
     end
     end
Line 82: Line 444:
     scores[:total_score] = self.assignment.compute_total_score(scores)
     scores[:total_score] = self.assignment.compute_total_score(scores)


     #merge scores[review#] (for each round) to score[review]  -Yang
     # merge scores[review#] (for each round) to score[review]  -Yang
     if self.assignment.varying_rubrics_by_round?
     if self.assignment.varying_rubrics_by_round?
       review_sym = "review".to_sym
       review_sym = "review".to_sym
       scores[review_sym] = Hash.new
       scores[review_sym] = {}
       scores[review_sym][:assessments] = Array.new
       scores[review_sym][:assessments] = []
       scores[review_sym][:scores] = Hash.new
       scores[review_sym][:scores] = {}
       scores[review_sym][:scores][:max] = -999999999
       scores[review_sym][:scores][:max] = -999_999_999
       scores[review_sym][:scores][:min] = 999999999
       scores[review_sym][:scores][:min] = 999_999_999
       scores[review_sym][:scores][:avg] = 0
       scores[review_sym][:scores][:avg] = 0
       total_score = 0
       total_score = 0
       for i in 1..self.assignment.get_review_rounds
       for i in 1..self.assignment.num_review_rounds
         round_sym = ("review"+i.to_s).to_sym
         round_sym = ("review" + i.to_s).to_sym
         if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
         if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
           next
           next
         end
         end
         length_of_assessments=scores[round_sym][:assessments].length.to_f
         length_of_assessments = scores[round_sym][:assessments].length.to_f
 
         scores[review_sym][:assessments] += scores[round_sym][:assessments]
         scores[review_sym][:assessments]+=scores[round_sym][:assessments]
         if !scores[round_sym][:scores][:max].nil? && scores[review_sym][:scores][:max] < scores[round_sym][:scores][:max]
 
           scores[review_sym][:scores][:max] = scores[round_sym][:scores][:max]
         if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
           scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
         end
         end
         if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
         if !scores[round_sym][:scores][:min].nil? && scores[review_sym][:scores][:min] > scores[round_sym][:scores][:min]
           scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
           scores[review_sym][:scores][:min] = scores[round_sym][:scores][:min]
         end
         end
         if(scores[round_sym][:scores][:avg]!=nil)
         unless scores[round_sym][:scores][:avg].nil?
           total_score += scores[round_sym][:scores][:avg]*length_of_assessments
           total_score += scores[round_sym][:scores][:avg] * length_of_assessments
         end
         end
       end
       end
 
       if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
       if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
        scores[review_sym][:scores][:max] = 0
              scores[review_sym][:scores][:max] = 0
        scores[review_sym][:scores][:min] = 0
              scores[review_sym][:scores][:min] = 0
       end
       end
 
       scores[review_sym][:scores][:avg] = total_score / scores[review_sym][:assessments].length.to_f
       scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
     end
     end
     # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
     # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
     # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
     # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
     if assignment.is_microtask?
     if assignment.is_microtask?
       topic = SignUpTopic.find_by_assignment_id(assignment.id)
       topic = SignUpTopic.find_by_assignment_id(assignment.id)
       if !topic.nil?
       unless topic.nil?
         scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
         scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
         scores[:max_pts_available] = topic.micropayment
         scores[:max_pts_available] = topic.micropayment
       end
       end
     end
     end
     # for all quiz questionnaires (quizzes) taken by the participant
     # for all quiz questionnaires (quizzes) taken by the participant
     quiz_responses = Array.new
     # quiz_responses = []
     quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
     # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
     quiz_response_mappings.each do |qmapping|
     # quiz_response_mappings.each do |qmapping|
      if (qmapping.response)
    #  quiz_responses << qmapping.response if qmapping.response
        quiz_responses << qmapping.response
     # end
      end
     # scores[:quiz] = Hash.new
     end
     # scores[:quiz][:assessments] = quiz_responses
     #scores[:quiz] = Hash.new
     # scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
     #scores[:quiz][:assessments] = quiz_responses
     #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
 
     scores[:total_score] = assignment.compute_total_score(scores)
     scores[:total_score] = assignment.compute_total_score(scores)
     #scores[:total_score] += compute_quiz_scores(scores)
     # scores[:total_score] += compute_quiz_scores(scores)
 
     # move lots of calculation from view(_participant.html.erb) to model
     # move lots of calculation from view(_participant.html.erb) to model
     if self.grade
     if self.grade
       scores[:total_score] = self.grade
       scores[:total_score] = self.grade
     else
     else
       total_score = scores[:total_score]
       scores[:total_score] = 100 if scores[:total_score] > 100
      if total_score > 100
      scores
        total_score = 100
      end
      scores[:total_score] = total_score
    scores
     end
     end
   end
   end
Line 163: Line 511:
     scores = {}
     scores = {}
     scores[:participant] = self
     scores[:participant] = self
     assignment_questionnaires(questions, scores)
     assignment_questionnaires(questions, scores)
     scores[:total_score] = self.assignment.compute_total_score(scores)
     scores[:total_score] = self.assignment.compute_total_score(scores)
 
    # merge scores[review#] (for each round) to score[review]  -Yang
     merge_scores(scores)
     merge_scores(scores) if self.assignment.varying_rubrics_by_round?
 
     # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
     # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
     # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
     # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
     if assignment.is_microtask?
     topic_total_scores(scores) if self.assignment.is_microtask?
      topic = SignUpTopic.find_by_assignment_id(assignment.id)
      if !topic.nil?
        scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
        scores[:max_pts_available] = topic.micropayment
      end
    end


     # for all quiz questionnaires (quizzes) taken by the participant
     # for all quiz questionnaires (quizzes) taken by the participant
     quiz_responses = Array.new
     # quiz_responses = []
     quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
     # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
     quiz_response_mappings.each do |qmapping|
     # quiz_response_mappings.each do |qmapping|
      if (qmapping.response)
    #  quiz_responses << qmapping.response if qmapping.response
        quiz_responses << qmapping.response
     # end
      end
     # scores[:quiz] = Hash.new
     end
     # scores[:quiz][:assessments] = quiz_responses
     #scores[:quiz] = Hash.new
     # scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
     #scores[:quiz][:assessments] = quiz_responses
     #scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
 
     scores[:total_score] = assignment.compute_total_score(scores)
     scores[:total_score] = assignment.compute_total_score(scores)
     #scores[:total_score] += compute_quiz_scores(scores)
     # scores[:total_score] += compute_quiz_scores(scores)
    # move lots of calculation from view(_participant.html.erb) to model
    caculate_scores(scores)
  end


    calculate_scores(scores)
  end
* assignment_questionnaires(questions, scores) method
* assignment_questionnaires(questions, scores) method


   def assignment_questionnaires(questions, scores)
   def assignment_questionnaires(questions, scores)
     self.assignment.questionnaires.each do |questionnaire|
     self.assignment.questionnaires.each do |questionnaire|
       round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
       round = AssignmentQuestionnaire.find_by(assignment_id: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
       #create symbol for "varying rubrics" feature -Yang
       # create symbol for "varying rubrics" feature -Yang
       if(round!=nil)
       questionnaire_symbol = if round.nil?
        questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
                              questionnaire.symbol
      else
                            else
        questionnaire_symbol = questionnaire.symbol
                              (questionnaire.symbol.to_s + round.to_s).to_sym
      end
                            end


       scores[questionnaire_symbol] = {}
       scores[questionnaire_symbol] = {}


       if round==nil
       scores[questionnaire_symbol][:assessments] = if round.nil?
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
                                                    questionnaire.get_assessments_for(self)
      else
                                                  else
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
                                                    questionnaire.get_assessments_round_for(self, round)
      end
                                                  end
       scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
       scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
     end
     end
Line 223: Line 560:


   def merge_scores(scores)
   def merge_scores(scores)
     #merge scores[review#] (for each round) to score[review]  -Yang
     review_sym = "review".to_sym
    if self.assignment.varying_rubrics_by_round?
    scores[review_sym] = {}
      review_sym = "review".to_sym
    scores[review_sym][:assessments] = []
      scores[review_sym] = Hash.new
    scores[review_sym][:scores] = {max: -999_999_999, min: 999_999_999, avg: 0}
      scores[review_sym][:assessments] = Array.new
    total_score = 0
      scores[review_sym][:scores] = Hash.new
    for i in 1..self.assignment.num_review_rounds
      scores[review_sym][:scores][:max] = -999999999
      round_sym = ("review" + i.to_s).to_sym
      scores[review_sym][:scores][:min] = 999999999
      if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
      scores[review_sym][:scores][:avg] = 0
        next
      total_score = 0
      end
      for i in 1..self.assignment.get_review_rounds
      length_of_assessments = scores[round_sym][:assessments].length.to_f
        round_sym = ("review"+i.to_s).to_sym
      scores[review_sym][:assessments] += scores[round_sym][:assessments]
        if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
      if !scores[round_sym][:scores][:max].nil? && scores[review_sym][:scores][:max] < scores[round_sym][:scores][:max]
          next
        scores[review_sym][:scores][:max] = scores[round_sym][:scores][:max]
        end
      end
        length_of_assessments=scores[round_sym][:assessments].length.to_f
      if !scores[round_sym][:scores][:min].nil? && scores[review_sym][:scores][:min] > scores[round_sym][:scores][:min]
 
        scores[review_sym][:scores][:min] = scores[round_sym][:scores][:min]
        scores[review_sym][:assessments]+=scores[round_sym][:assessments]
      end
 
      unless scores[round_sym][:scores][:avg].nil?
        if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
        total_score += scores[round_sym][:scores][:avg] * length_of_assessments
          scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
        end
        if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
          scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
        end
        if(scores[round_sym][:scores][:avg]!=nil)
          total_score += scores[round_sym][:scores][:avg]*length_of_assessments
        end
       end
       end
    end
    if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
      scores[review_sym][:scores][:max] = 0
      scores[review_sym][:scores][:min] = 0
    end
    scores[review_sym][:scores][:avg] = total_score / scores[review_sym][:assessments].length.to_f
  end


      if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
*  topic_total_scores(scores) method
        scores[review_sym][:scores][:max] = 0
        scores[review_sym][:scores][:min] = 0
      end


      scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
  def topic_total_scores(scores)
    topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
    unless topic.nil?
      scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
      scores[:max_pts_available] = topic.micropayment
     end
     end
   end
   end
Line 264: Line 601:
* calculate_scores(scores) method
* calculate_scores(scores) method


   def calculate_scores(scores)
   def caculate_scores(scores)
    # move lots of calculation from view(_participant.html.erb) to model
     if self.grade
     if self.grade
       scores[:total_score] = self.grade
       scores[:total_score] = self.grade
     else
     else
       total_score = scores[:total_score]
       scores[:total_score] = 100 if scores[:total_score] > 100
      if total_score > 100
        total_score = 100
      end
      scores[:total_score] = total_score
       scores
       scores
     end
     end
Line 281: Line 613:
|}
|}


===set_handle method===
===Files method===
The set_handle method had an unncessary ELSIF statement which was performing the same action '''self.handle = self.user.name'''. This has been clubbed as an OR condition to the IF statement and the first ELSIF statement has been removed.
The files method of AssignmentParticipant model is exactly the same as '''assignment_team.rb, L103''', so we move it to a module named Instance_method in '''lib/TFD1770_refactor.rb'''.
 
The below two lines are added to the AssignmentParticipant model and AssignmentTeam model.
{| class = "wikitable"
|-
!Before!!After
|-
|def set_handle
      if self.user.handle == nil or self.user.handle == ""
        self.handle = self.user.name
      elsif AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
        self.handle = self.user.name
      else
        self.handle = self.user.handle
      end
      self.save!
  end
 
||def set_handle
      if self.user.handle == nil or self.user.handle == "" or AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
        self.handle = self.user.name
      else
        self.handle = self.user.handle
      end
      self.save!
  end
 
|-
|}


===File and directory methods===
require 'TFD1770_refactor'
The files and directory methods from AssignmentParticipant model have been moved to the '''FileHelper''' module.
The methods '''files''', '''submitted_files''' and '''dir_path''' are appropriate for the '''FileHelper' module and hence been moved there.
The below line was added to the AssignmentParticipant model.
include FileHelper


The AssignmentParticipant class would look like
include Instance_method
  class AssignmentParticipant < Participant
    require 'wiki_helper'
    '''include FileHelper'''
Methods moved to FileHelper:
 
* submitted_files
 
  def submitted_files
    files(self.path) if self.directory_num
  end
 
* files(directory)


* module Instance_method
   def files(directory)
   def files(directory)
     files_list = Dir[directory + "/*"]
     files_list = Dir[directory + "/*"]
     files = Array.new
     files = []


     files_list.each do |file|
     files_list.each do |file|
       if File.directory?(file)
       if File.directory?(file)
         dir_files = files(file)
         dir_files = files(file)
         dir_files.each{|f| files << f}
         dir_files.each {|f| files << f }
       end
       end
       files << file
       files << file
Line 345: Line 636:
   end
   end


* submitted_files()
===Self.import method===
 
The self.import method of AssignmentParticipant model is exactly the same as '''course_participant.rb, L21''', so we move it to a module named Class_method in '''lib/TFD1770_refactor.rb'''.
  def submitted_files()
The below two lines are added to the AssignmentParticipant model and CourseParticipant model.
    files = Array.new
    if(self.directory_num)
      files = files(self.path)
    end
    return files
  end
 
* dir_path
 
  def dir_path
    assignment.try :directory_path
  end
 
===Remove unused methods===
The following methods have been removed as they were not being used by the application.
 
'''def average_score_per_assignment(assignment_id)'''
 
  def average_score_per_assignment(assignment_id)
    return 0 if self.response_maps.size == 0
 
    sum_of_scores = 0
 
    self.response_maps.metareview_response_maps.each do |metaresponse_map|
      if !metaresponse_map.response.empty? && response_map == assignment_id then
        sum_of_scores = sum_of_scores + response_map.response.last.average_score
      end
    end
 
    (sum_of_scores / self.response_maps.size).to_i
  end
 
 
'''def quiz_taken_by?(contributor, reviewer)'''
 
  def quiz_taken_by?(contributor, reviewer)
    quiz_id = QuizQuestionnaire.find_by_instructor_id(contributor.id)
    return QuizResponseMap.where(['reviewee_id = ? AND reviewer_id = ? AND reviewed_object_id = ?',
                                  self.id, reviewer.id, quiz_id]).count > 0
  end
 
 
'''def has_quiz?'''
 
  def has_quiz?
    return !QuizQuestionnaire.find_by_instructor_id(self.id).nil?
  end
 
 
'''def reviewees'''
 
  def reviewees
    reviewees = []
    rmaps = ResponseMap.all(conditions: ["reviewer_id = #{self.id} && type = 'ReviewResponseMap'"])
        rmaps.each { |rm| reviewees.concat(AssignmentTeam.find(rm.reviewee_id).participants) }
 
    reviewees
  end
 
 
'''def two_node_cycles'''
  def two_node_cycles
    cycles = []
    self.reviewers.each do |ap|
      if ap.reviewers.include?(self)
        self.reviews_by_reviewer(ap).nil? ? next : s01 = self.reviews_by_reviewer(ap).get_total_score
        ap.reviews_by_reviewer(self).nil? ? next : s10 = ap.reviews_by_reviewer(self).get_total_score
        cycles.push([[self, s01], [ap, s10]])
      end
    end
    cycles
  end
 
 
'''def three_node_cycles'''
 
  def three_node_cycles
    cycles = []
    self.reviewers.each do |ap1|
      ap1.reviewers.each do |ap2|
        if ap2.reviewers.include?(self)
          self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
          ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
          ap2.reviews_by_reviewer(self).nil? ? next : s20 = ap2.reviews_by_reviewer(self).get_total_score
          cycles.push([[self, s01], [ap1, s12], [ap2, s20]])
        end
      end
    end
    cycles
  end
 
 
'''def four_node_cycles'''
 
  def four_node_cycles
    cycles = []
    self.reviewers.each do |ap1|
      ap1.reviewers.each do |ap2|
        ap2.reviewers.each do |ap3|
          if ap3.reviewers.include?(self)
            self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
            ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
            ap2.reviews_by_reviewer(ap3).nil? ? next : s23 = ap2.reviews_by_reviewer(ap3).get_total_score
            ap3.reviews_by_reviewer(self).nil? ? next : s30 = ap3.reviews_by_reviewer(self).get_total_score
            cycles.push([[self, s01], [ap1, s12], [ap2, s23], [ap3, s30]])
          end
        end
      end
    end
    cycles
  end
 


'''def cycle_similarity_score(cycle)'''
require 'TFD1770_refactor'


  def cycle_similarity_score(cycle)
include Class_method
    similarity_score = 0.0
    count = 0.0


     0 ... cycle.size-1.each do |pivot|
  def self.import(row, _row_header = nil, session, id)
      pivot_score = cycle[pivot][1]
     user = AssignmentParticipant.check_info_and_create(row, _row_header = nil, session)
       similarity_score = similarity_score + (pivot_score - cycle[other][1]).abs
    raise ImportError, "The assignment with id \"" + id.to_s + "\" was not found." if Assignment.find(id).nil?
       count = count + 1.0
    unless AssignmentParticipant.exists?(user_id: user.id, parent_id: id)
       new_part = AssignmentParticipant.create(user_id: user.id, parent_id: id)
       new_part.set_handle
     end
     end
    similarity_score = similarity_score / count unless count == 0.0
    similarity_score
  end
'''def cycle_deviation_score(cycle)'''
  def cycle_deviation_score(cycle)
    deviation_score = 0.0
    count = 0.0
    0 ... cycle.size.each do |member|
      participant = AssignmentParticipant.find(cycle[member][0].id)
      total_score = participant.get_review_score
      deviation_score = deviation_score + (total_score - cycle[member][1]).abs
      count = count + 1.0
    end
    deviation_score = deviation_score / count unless count == 0.0
    deviation_score
   end
   end


 
* module Class_method
'''def compute_quiz_scores(scores)'''
  def check_info_and_create(row, _row_header = nil, session)
 
    raise ArgumentError, "No user id has been specified." if row.empty?
  def compute_quiz_scores(scores)
     user = User.find_by(name: row[0])
     total = 0
     if user.nil?
     if scores[:quiz][:scores][:avg]
       raise ArgumentError, "The record containing #{row[0]} does not have enough items." if row.length < 4
       return scores[:quiz][:scores][:avg] * 100  / 100.to_f
      attributes = ImportFileHelper.define_attributes(row)
    else
       user = ImportFileHelper.create_new_user(attributes, session)
       return 0
     end
     end
    return total
   end
   end


 
===Find_by method===
'''def review_response_maps'''
Use '''find_by''' instead of dynamic method
 
  def review_response_maps
      participant = Participant.find(id)
      team_id = TeamsUser.team_id(participant.parent_id, participant.user_id)
      ReviewResponseMap.where(reviewee_id: team_id, reviewed_object_id: assignment.id)
  end
 
 
'''def members'''
 
  def members
    team.try :participants
  end
 
 
'''def get_hash(time_stamp)'''
 
  def get_hash(time_stamp)
    # first generate a hash from the assignment name itself
    hash_data = Digest::SHA1.digest(self.assignment.name.to_s)
 
    # second generate a hash from the first hash plus the user name and time stamp
    sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S")
    Digest::SHA1.digest(sign)
  end
 
===Review methods===
* The bookmark_reviews() method from AssignmentParticipant was being called once in '''get_assessments_for''' method of '''BookmarkRatingQuestionnaire''' class. It has been replaced by the method statement directly since it only had a single line of code.


{| class = "wikitable"
{| class = "wikitable"
Line 544: Line 671:
!Before!!After
!Before!!After
|-
|-
|def bookmark_reviews
|
    BookmarkRatingResponseMap.get_assessments_for(self)
*  assignment_particpant.rb L32
  end
quiz = QuizQuestionnaire.find_by_instructor_id(contributor.id)
 
* assignment_particpant.rb L113
topic = SignUpTopic.find_by_assignment_id(assignment.id)
 
* assignment_particpant.rb L196
user = User.find_by_name(row[0])
 
|| 
*  assignment_particpant.rb L34
quiz = QuizQuestionnaire.find_by(instructor_id: contributor.id)


||def get_assessments_for(participant)
* assignment_particpant.rb L134
    BookmarkRatingResponseMap.get_assessments_for(participant) # participant.bookmark_reviews()
topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
  end


* TFD1770_refactor.rb L20
user = User.find_by(name:row[0])
|-
|-
|}
|}


* The teammate_reviews() method from AssignmentParticipant was being called once in '''get_assessments_for''' method of '''TeammateReviewQuestionnaire''' class. It has been replaced by the method statement directly since it only had a single line of code.
Use '''find_by''' instead of where.first


{| class = "wikitable"
{| class = "wikitable"
Line 561: Line 699:
!Before!!After
!Before!!After
|-
|-
|def teammate_reviews
|
    TeammateReviewResponseMap.get_assessments_for(self)
*  assignment_particpant.rb L267
  end
first_user_id = TeamsUser.where(team_id: response_map.reviewee_id).first.user_id
 
* assignment_particpant.rb L267
participant = Participant.where(parent_id: response_map.reviewed_object_id, user_id: first_user_id).first
 
||
*  assignment_particpant.rb L258
first_user_id = TeamsUser.find_by(team_id: response_map.reviewee_id).user_id


||def get_assessments_for(participant)
* assignment_particpant.rb L259
    TeammateReviewResponseMap.get_assessments_for(participant)  #participant.teammate_reviews()
participant = Participant.find_by(parent_id: response_map.reviewed_object_id, user_id: first_user_id)
  end


|-
|-
Line 574: Line 718:
==Resources==
==Resources==
* Expertiza Github repository<ref>Expertiza Github repository https://github.com/expertiza/expertiza</ref>
* Expertiza Github repository<ref>Expertiza Github repository https://github.com/expertiza/expertiza</ref>
* Our Github repository<ref>Our Github Repository https://github.com/terryliu1995/expertiza</ref>
* Our Github repository<ref>Our Github repository https://github.com/terryliu1995/expertiza</ref>
* Our pull request<ref>Our pull request https://github.com/expertiza/expertiza/pull/1049</ref>


== References ==
== References ==
<references/>
<references/>

Latest revision as of 12:52, 3 November 2017

E1770. [Test First Development] Refactor assignment_participant.rb<ref>Project Description document https://docs.google.com/document/d/1rdolBAHxVGI9I0N-cT866AqnfORM2L1_m_bo2gRYRrI/edit#heading=h.s18ldps2kz5e</ref>

This page provides a brief description of the Expertiza project. The project is aimed at using TFD(Test First Development) to refactor the AssignmentParticipant model which is subclass of Participant model. This model is used to maintain the list of students/users participating in a given assignment. For any new or existing assignments, this model manages the entire list of users assigned to that particular assignment. It primarily includes method for scores calculations, assignment submission paths, reviews, etc.

We used the TDF(Test First Development) method to refactor the AssignmentParticipant model. With the RSpec tool, we wrote 38 test cases for 24 methods in assignment_participant_spec.rb first. After finishing these test cases, we executed them, and most of them failed. And then we refactored the methods, scores, files, self.import, find_by, where.first, and executed the test cases again, finally they all passed.

Introduction to Expertiza

Expertiza is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using Ruby on Rails framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments. Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.

Why refactoring?

Code Refactoring<ref>Refactoring https://en.wikipedia.org/wiki/Code_refactoring</ref> is the process of restructuring existing computer code—changing the factoring—without changing its external behavior.

Refactoring improves nonfunctional attributes of the software. Advantages include improved code readability and reduced complexity; these can improve source-code maintainability and create a more expressive internal architecture or object model to improve extensibility. Typically, refactoring applies a series of standardised basic micro-refactorings, each of which is (usually) a tiny change in a computer program's source code that either preserves the behaviour of the software, or at least does not modify its conformance to functional requirements. Many development environments provide automated support for performing the mechanical aspects of these basic refactorings. If done extremely well, code refactoring may also resolve hidden, dormant, or undiscovered bugs or vulnerabilities in the system by simplifying the underlying logic and eliminating unnecessary levels of complexity. If done poorly it may fail the requirement that external functionality not be changed, introduce new bugs, or both.

Why Test-driven development?

Test-driven development (TDD)<ref>Test-driven development (TDD) https://en.wikipedia.org/wiki/Test-driven_development</ref> is a software development process that relies on the repetition of a very short development cycle: Requirements are turned into very specific test cases, then the software is improved to pass the new tests, only. This is opposed to software development that allows software to be added that is not proven to meet requirements. The followings are several benefits of Test-driven development (TDD).

  • Maintainable, Flexible, Easily Extensible.
  • Unparalleled Test Coverage & Streamlined Codebase.
  • Clean Interface.
  • Refactoring Encourages Improvements.
  • Executable Documentation.

Introduction to RSpec

RSpec<ref>RSpec https://en.wikipedia.org/wiki/RSpec</ref> is a 'Domain Specific Language' (DSL) testing tool written in Ruby to test Ruby code. It is a behavior-driven development (BDD) framework which is extensively used in the production applications. The basic idea behind this concept is that of Test Driven Development(TDD) where the tests are written first and the development is based on writing just enough code that will fulfill those tests followed by refactoring. It contains its own mocking framework that is fully integrated into the framework based upon JMock. The simplicity in the RSpec syntax makes it one of the popular testing tools for Ruby applications. The RSpec tool can be used by installing the rspec gem which consists of 3 other gems namely rspec-core , rspec-expectation and rspec-mock.

Project Description

Refactor AssignmentParticipant model which is a subclass of Participant model. The following tasks have been performed as per the requirements.

  • Refactor scores method.
    • Write failing tests first.
    • Split into several simpler methods and assign reasonable names.
    • Extract duplicated code into separate methods.
    • Replace the conditional with the relevant method calls.
  • Method files is exactly the same as assignment_team.rb L103.
    • Write failing tests first.
    • Solve the duplication, extract method to a new file or delete useless one.
  • Method self.import is exact the same as course_participant.rb L21.
    • Write failing tests first.
    • Solve the duplication, extract method to a new file or delete useless one.
  • Use find_by instead of dynamic method.
    • Write failing tests first.
  • Use find_by instead of where.first.
    • Write failing tests first.

Test Cases

Because the project is aimed at using TFD(Test First Development) to refactor the AssignmentParticipant model. We wrote 38 test cases for 24 methods in assignment_participant_spec.rb first, with the RSpec tool.

The introduction video to our project<ref>Introduction video to our project https://www.youtube.com/watch?v=sJdmN-S2Voo&feature=youtu.be</ref>

 describe '#dir_path' do
   it 'returns the directory path of current assignment' do
     expect(participant.dir_path).to eq "final_test"
   end
 end
 describe '#assign_quiz' do
   it 'creates a new QuizResponseMap record' do
     allow(QuizQuestionnaire).to receive(:find_by).with(instructor_id: 1).and_return(quiz_questionaire) # WHY CAN NOT DELETE THIS SENTENCE
     expect { participant.assign_quiz(participant, participant2, nil) }.to change { QuizResponseMap.count }.from(0).to(1)
     expect(participant.assign_quiz(participant, participant2, nil)).to be_an_instance_of(QuizResponseMap)
   end
 end
 describe '#reviewers' do
   it 'returns all the participants in this assignment who have reviewed the team where this participant belongs' do
     allow(ReviewResponseMap).to receive(:where).with(any_args).and_return([response_map]) # differences with .with(parameter)
     allow(AssignmentParticipant).to receive(:find).with(any_args).and_return(participant2)
     expect(participant.reviewers).to eq([participant2])
   end
 end
 describe '#review_score' do
   it 'returns the review score' do
     # diao yong de method tai duo le,er qie bu zhi dao sha yi si
     allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
     allow(review_questionnaire).to receive(:questions).and_return(question)
     allow(Answer).to receive(:compute_scores).with([response], question).and_return(avg: 100)
     allow(review_questionnaire).to receive(:max_possible_score).and_return(100)
     expect(participant.review_score).to eq(100)
   end
 end
 describe '#scores' do
   context 'when assignment is not varying rubric by round and not an microtask' do
     it 'calculates scores that this participant has been given' do
       expect(participant).to receive(:assignment_questionnaires)
       expect(assignment).to receive(:compute_total_score)
       allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
       allow(assignment).to receive(:is_microtask?).and_return(false)
       expect(assignment).to receive(:compute_total_score)
       expect(participant).to receive(:caculate_scores)
       participant.scores(question)
     end
   end
   context 'when assignment is varying rubric by round but not an microtask' do
     it 'calculates scores that this participant has been given' do
       expect(participant).to receive(:assignment_questionnaires)
       expect(assignment).to receive(:compute_total_score)
       allow(assignment).to receive(:varying_rubrics_by_round?).and_return(true)
       expect(participant).to receive(:merge_scores)
       allow(assignment).to receive(:is_microtask?).and_return(false)
       expect(assignment).to receive(:compute_total_score)
       expect(participant).to receive(:caculate_scores)
       participant.scores(question)
     end
   end
   context 'when assignment is not varying rubric by round but an microtask' do
     it 'calculates scores that this participant has been given' do
       expect(participant).to receive(:assignment_questionnaires)
       expect(assignment).to receive(:compute_total_score)
       allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
       allow(assignment).to receive(:is_microtask?).and_return(true)
       expect(participant).to receive(:topic_total_scores)
       expect(assignment).to receive(:compute_total_score)
       expect(participant).to receive(:caculate_scores)
       participant.scores(question)
     end
   end
 end
 # included method
 describe '#assignment_questionnaires' do
   context 'when the round of questionnaire is nil' do
     it 'record the result as review scores' do
       scores = {}
       question_hash = {review: question}
       score_map = {max: 100, min: 100, avg: 100}
       allow(AssignmentQuestionnaire).to receive(:find_by).with(any_args).and_return(assignment_questionnaire)
       allow(review_questionnaire).to receive(:get_assessments_for).with(any_args).and_return([response])
       allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
       participant.assignment_questionnaires(question_hash, scores)
       expect(scores[:review][:assessments]).to eq([response])
       expect(scores[:review][:scores]).to eq(score_map)
     end
   end
   context 'when the round of questionnaire is not nil' do
     it 'record the result as review#{n} scores' do
       scores = {}
       question_hash = {review1: question}
       score_map = {max: 100, min: 100, avg: 100}
       allow(AssignmentQuestionnaire).to receive(:find_by).and_return(assignment_questionnaire2)
       allow(review_questionnaire).to receive(:get_assessments_round_for).with(any_args).and_return([response])
       allow(Answer).to receive(:compute_scores).with(any_args).and_return(score_map)
       participant.assignment_questionnaires(question_hash, scores)
       expect(scores[:review1][:assessments]).to eq([response])
       expect(scores[:review1][:scores]).to eq(score_map)
     end
   end
 end
 describe '#merge_scores' do
   context 'when all of the review_n are nil' do
     it 'set max, min, avg of review score as 0' do
       scores = {}
       allow(assignment).to receive(:num_review_rounds).and_return(1)
       participant.merge_scores(scores)
       expect(scores[:review][:scores][:max]).to eq(0)
       expect(scores[:review][:scores][:min]).to eq(0)
       expect(scores[:review][:scores][:min]).to eq(0)
     end
   end
   context 'when the review_n is not nil' do
     it 'merge the score of review_n to the score of review' do
       score_map = {max: 100, min: 100, avg: 100}
       scores = {review1: {scores: score_map, assessments: [response]}}
       allow(assignment).to receive(:num_review_rounds).and_return(1)
       participant.merge_scores(scores)
       expect(scores[:review][:scores][:max]).to eq(100)
       expect(scores[:review][:scores][:min]).to eq(100)
       expect(scores[:review][:scores][:min]).to eq(100)
     end
   end
 end
 describe '#topic_total_scores' do
   it 'set total_score and max_pts_available of score when topic is not nil' do
     scores = {total_score: 100}
     allow(SignUpTopic).to receive(:find_by).with(any_args).and_return(topic)
     participant.topic_total_scores(scores)
     expect(scores[:total_score]).to eq(0)
     expect(scores[:max_pts_available]).to eq(0)
   end
 end
 describe '#caculate_scores' do
   context 'when the participant has the grade' do
     it 'his total scores equals his grade' do
       scores = {}
       expect(participant2.caculate_scores(scores)).to eq(100.0)
     end
   end
   context 'when the participant has the grade and the total score more than 100' do
     it 'return the score of a given participant with total score 100' do
       scores = {total_score: 110}
       expect(participant.caculate_scores(scores)).to eq(total_score: 100)
     end
   end
   context 'when the participant has the grade and the total score less than 100' do
     it 'return the score of a given participant with total score' do
       scores = {total_score: 90}
       expect(participant.caculate_scores(scores)).to eq(total_score: 90)
     end
   end
 end
 describe '#copy' do
   it 'copies assignment participants to a certain course' do
     expect(participant.copy(517)).to be_an_instance_of(CourseParticipant)
   end
 end
 describe '#feedback' do
   it 'returns corrsponding author feedback responses given by current participant' do
     expect(participant.feedback).to eq([response])
   end
 end
 describe '#reviews' do
   it 'returns corrsponding peer review responses given by current team' do
     expect(participant.reviews).to eq([response])
   end
 end
 describe '#reviews_by_reviewer' do
   it 'returns corrsponding peer review responses given by certain reviewer' do
     allow(ReviewResponseMap).to receive(:get_reviewer_assessments_for).with(team, participant2).and_return([response])
     expect(participant.reviews_by_reviewer(participant2)).to eq([response])
   end
 end
 describe '#quizzes_taken' do
   it 'returns corrsponding quiz responses given by current participant' do
     expect(participant.quizzes_taken).to eq([response])
   end
 end
 describe '#metareviews' do
   it 'returns corrsponding metareview responses given by current participant' do
     expect(participant.metareviews).to eq([response])
   end
 end
 describe '#teammate_reviews' do
   it 'returns corrsponding teammate review responses given by current participant' do
     expect(participant.teammate_reviews).to eq([response])
   end
 end
 describe '#bookmark_reviews' do
   it 'returns corrsponding bookmark review responses given by current participant' do
     expect(participant.bookmark_reviews).to eq([response])
   end
 end
 describe '#files' do
   context 'when there is not subdirectory in current directory' do
     it 'returns all files in current directory' do
       allow(Dir).to receive(:[]).with("a/*").and_return(["a/k.rb"])
       allow(File).to receive(:directory?).with("a/k.rb").and_return(false)
       expect(participant.files("a")).to eq(["a/k.rb"])
     end
   end
   context 'when there is subdirectory in current directory' do
     it 'recursively returns all files in current directory' do
       allow(Dir).to receive(:[]).with("a/*").and_return(["a/b"])
       allow(File).to receive(:directory?).with("a/b").and_return(true)
       allow(Dir).to receive(:[]).with("a/b/*").and_return(["a/b/k.rb"])
       allow(File).to receive(:directory?).with("a/b/k.rb").and_return(false)
       expect(participant.files("a")).to eq(["a/b/k.rb", "a/b"])
     end
   end
 end
 describe ".import" do
   context 'when record is empty' do
     it 'raises an ArgumentError' do
       row = []
       allow(AssignmentParticipant).to receive(:check_info_and_create).with(any_args).and_raise("No user id has been specified.")
       expect(AssignmentParticipant).to receive(:check_info_and_create)
       expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("No user id has been specified.")
     end
   end
   context 'when no user is found by offered username' do
     context 'when the record has less than 4 items' do
       it 'raises an ArgumentError' do
         row = ["user_name", "user_fullname", "name@email.com"]
         allow(AssignmentParticipant).
           to receive(:check_info_and_create).with(any_args).and_raise("The record containing #{row[0]} does not have enough items.")
         expect(AssignmentParticipant).to receive(:check_info_and_create)
         expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error("The record containing #{row[0]} does not have enough items.")
       end
     end
     context 'when the record has more than 4 items' do
       context 'when certain assignment cannot be found' do
         it 'creates a new user based on import information and raises an ImportError' do
           row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
           session = {user: participant}
           allow(User).to receive(:find_by).with(any_args).and_return(nil)
           allow(Assignment).to receive(:find).with(2).and_return(nil)
           expect { AssignmentParticipant.import(row, nil, session, 2) }.
             to raise_error("The assignment with id \"2\" was not found.").and change { User.count }.by(1)
         end
       end
       context 'when certain assignment can be found and assignment participant does not exists' do
         it 'creates a new user, new participant and raises an ImportError' do
           row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
           session = {user: participant}
           allow(User).to receive(:find_by_name).with(any_args).and_return(nil)
           allow(Assignment).to receive(:find).with(2).and_return(assignment)
           allow(AssignmentParticipant).to receive(:exists?).and_return(false)
           expect { AssignmentParticipant.import(row, nil, session, 2) }.to change { User.count }.by(1).and change { AssignmentParticipant.count }.by(1)
         end
       end
     end
   end
 end
 describe '.export' do
   it 'exports all participants in current assignment' do
     csv = []
     expect(AssignmentParticipant).to receive_message_chain(:where, :find_each).with(any_args).and_yield(participant)
     expect(AssignmentParticipant.export(csv, 1, any_args)).
       to eq("student2064", "2064, student", "expertiza@mailinator.com", "Student", "instructor6", true, true, true, "handle")
   end
 end
 describe '#set_handle' do
   context 'when the user of current participant does not have handle' do
     it 'sets the user name as the handle of current participant' do
       allow(student).to receive_message_chain(:handle, :nil?).and_return(true)
       allow(student).to receive(:handle).and_return("")
       participant.set_handle
       expect(participant.handle).to eq("student2064")
     end
   end
   context 'when current assignment exists participants with same handle as the one of current user' do
     it 'sets the user name as the name of current participant' do
       allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
       participant.set_handle
       expect(participant.handle).to eq("student2064")
     end
   end
   context 'when current assignment does not have participants with same handle as the one of current user' do
     it 'sets the user name as the handle of current participant' do
       participant.set_handle
       expect(participant.handle).to eq("handle")
     end
   end
 end
 describe '#review_file_path' do
   it 'returns the file path for reviewer to upload files during peer review' do
     allow(ResponseMap).to receive(:find).with(any_args).and_return(response_map)
     allow(TeamsUser).to receive_message_chain(:find_by, :user_id).with(any_args).and_return(1)
     allow(Participant).to receive(:find_by).with(any_args).and_return(participant)
     file_path = Rails.root.to_s + "/pg_data/instructor6/csc517/test/final_test/0_review/1"
     expect(participant.review_file_path(1)).to eq(file_path)
   end
 end
 describe '#current_stage' do
   it 'returns stage of current assignment' do
     allow(assignment).to receive(:get_current_stage).with(1).and_return("Finished")
     expect(participant.current_stage).to eq("Finished")
   end
 end
 describe '#stage_deadline' do
   context 'when stage of current assignment is not Finished' do
     it 'returns current stage' do
       allow(assignment).to receive(:stage_deadline).with(1).and_return("Unknow")
       expect(participant.stage_deadline).to eq("Unknow")
     end
   end
   context 'when stage of current assignment not Finished' do
     context 'current assignment is not a staggered deadline assignment' do
       it 'returns the due date of current assignment' do
         allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
         allow(assignment).to receive(:staggered_deadline?).and_return(false)
         allow(assignment).to receive_message_chain(:due_dates, :last, :due_at).and_return(1)
         expect(participant.stage_deadline).to eq("1")
       end
     end
     context 'current assignment is a staggered deadline assignment' do
       it 'returns the due date of current topic' do
         allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
         allow(assignment).to receive(:staggered_deadline?).and_return(true)
         allow(TopicDueDate).to receive_message_chain(:find_by, :last, :due_at).and_return(1)
         expect(participant.stage_deadline).to eq("1")
       end
     end
   end
 end

Refactoring

Scores method

The method scores has been converted to smaller methods scores, assignment_questionnaires, merge_scores, topic_total_scores, calculate_scores. It is a good practice to keep the methods not extremely long as they tend to complicate the functionality and the readability.

Before After
def scores(questions)
   scores = {}
   scores[:participant] = self
   self.assignment.questionnaires.each do |questionnaire|
     round = AssignmentQuestionnaire.find_by(assignment_id: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
     # create symbol for "varying rubrics" feature -Yang
     questionnaire_symbol = if round.nil?
                              questionnaire.symbol
                            else
                              (questionnaire.symbol.to_s + round.to_s).to_sym
                            end
     scores[questionnaire_symbol] = {}
     scores[questionnaire_symbol][:assessments] = if round.nil?
                                                    questionnaire.get_assessments_for(self)
                                                  else
                                                    questionnaire.get_assessments_round_for(self, round)
                                                  end
     scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
   end
   scores[:total_score] = self.assignment.compute_total_score(scores)
   # merge scores[review#] (for each round) to score[review]  -Yang
   if self.assignment.varying_rubrics_by_round?
     review_sym = "review".to_sym
     scores[review_sym] = {}
     scores[review_sym][:assessments] = []
     scores[review_sym][:scores] = {}
     scores[review_sym][:scores][:max] = -999_999_999
     scores[review_sym][:scores][:min] = 999_999_999
     scores[review_sym][:scores][:avg] = 0
     total_score = 0
     for i in 1..self.assignment.num_review_rounds
       round_sym = ("review" + i.to_s).to_sym
       if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
         next
       end
       length_of_assessments = scores[round_sym][:assessments].length.to_f
       scores[review_sym][:assessments] += scores[round_sym][:assessments]
       if !scores[round_sym][:scores][:max].nil? && scores[review_sym][:scores][:max] < scores[round_sym][:scores][:max]
         scores[review_sym][:scores][:max] = scores[round_sym][:scores][:max]
       end
       if !scores[round_sym][:scores][:min].nil? && scores[review_sym][:scores][:min] > scores[round_sym][:scores][:min]
         scores[review_sym][:scores][:min] = scores[round_sym][:scores][:min]
       end
       unless scores[round_sym][:scores][:avg].nil?
         total_score += scores[round_sym][:scores][:avg] * length_of_assessments
       end
     end
     if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
       scores[review_sym][:scores][:max] = 0
       scores[review_sym][:scores][:min] = 0
     end
     scores[review_sym][:scores][:avg] = total_score / scores[review_sym][:assessments].length.to_f
   end
   # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
   # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
   if assignment.is_microtask?
     topic = SignUpTopic.find_by_assignment_id(assignment.id)
     unless topic.nil?
       scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
       scores[:max_pts_available] = topic.micropayment
     end
   end
   # for all quiz questionnaires (quizzes) taken by the participant
   # quiz_responses = []
   # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
   # quiz_response_mappings.each do |qmapping|
   #   quiz_responses << qmapping.response if qmapping.response
   # end
   # scores[:quiz] = Hash.new
   # scores[:quiz][:assessments] = quiz_responses
   # scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
   scores[:total_score] = assignment.compute_total_score(scores)
   # scores[:total_score] += compute_quiz_scores(scores)
   # move lots of calculation from view(_participant.html.erb) to model
   if self.grade
     scores[:total_score] = self.grade
   else
     scores[:total_score] = 100 if scores[:total_score] > 100
     scores
   end
 end
  • scores(questions) method
 def scores(questions)
   scores = {}
   scores[:participant] = self
   assignment_questionnaires(questions, scores)
   scores[:total_score] = self.assignment.compute_total_score(scores)
   # merge scores[review#] (for each round) to score[review]  -Yang
   merge_scores(scores) if self.assignment.varying_rubrics_by_round?
   # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
   # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
   topic_total_scores(scores) if self.assignment.is_microtask?
   # for all quiz questionnaires (quizzes) taken by the participant
   # quiz_responses = []
   # quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
   # quiz_response_mappings.each do |qmapping|
   #   quiz_responses << qmapping.response if qmapping.response
   # end
   # scores[:quiz] = Hash.new
   # scores[:quiz][:assessments] = quiz_responses
   # scores[:quiz][:scores] = Answer.compute_quiz_scores(scores[:quiz][:assessments])
   scores[:total_score] = assignment.compute_total_score(scores)
   # scores[:total_score] += compute_quiz_scores(scores)
   # move lots of calculation from view(_participant.html.erb) to model
   caculate_scores(scores)
 end
  • assignment_questionnaires(questions, scores) method
 def assignment_questionnaires(questions, scores)
   self.assignment.questionnaires.each do |questionnaire|
     round = AssignmentQuestionnaire.find_by(assignment_id: self.assignment.id, questionnaire_id: questionnaire.id).used_in_round
     # create symbol for "varying rubrics" feature -Yang
     questionnaire_symbol = if round.nil?
                              questionnaire.symbol
                            else
                              (questionnaire.symbol.to_s + round.to_s).to_sym
                            end
     scores[questionnaire_symbol] = {}
     scores[questionnaire_symbol][:assessments] = if round.nil?
                                                    questionnaire.get_assessments_for(self)
                                                  else
                                                    questionnaire.get_assessments_round_for(self, round)
                                                  end
     scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
   end
 end
  • merge_scores(scores) method
 def merge_scores(scores)
   review_sym = "review".to_sym
   scores[review_sym] = {}
   scores[review_sym][:assessments] = []
   scores[review_sym][:scores] = {max: -999_999_999, min: 999_999_999, avg: 0}
   total_score = 0
   for i in 1..self.assignment.num_review_rounds
     round_sym = ("review" + i.to_s).to_sym
     if scores[round_sym].nil? || scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].empty?
       next
     end
     length_of_assessments = scores[round_sym][:assessments].length.to_f
     scores[review_sym][:assessments] += scores[round_sym][:assessments]
     if !scores[round_sym][:scores][:max].nil? && scores[review_sym][:scores][:max] < scores[round_sym][:scores][:max]
       scores[review_sym][:scores][:max] = scores[round_sym][:scores][:max]
     end
     if !scores[round_sym][:scores][:min].nil? && scores[review_sym][:scores][:min] > scores[round_sym][:scores][:min]
       scores[review_sym][:scores][:min] = scores[round_sym][:scores][:min]
     end
     unless scores[round_sym][:scores][:avg].nil?
       total_score += scores[round_sym][:scores][:avg] * length_of_assessments
     end
   end
   if scores[review_sym][:scores][:max] == -999_999_999 && scores[review_sym][:scores][:min] == 999_999_999
     scores[review_sym][:scores][:max] = 0
     scores[review_sym][:scores][:min] = 0
   end
   scores[review_sym][:scores][:avg] = total_score / scores[review_sym][:assessments].length.to_f
 end
  • topic_total_scores(scores) method
 def topic_total_scores(scores)
   topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
   unless topic.nil?
     scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
     scores[:max_pts_available] = topic.micropayment
   end
 end
  • calculate_scores(scores) method
 def caculate_scores(scores)
   if self.grade
     scores[:total_score] = self.grade
   else
     scores[:total_score] = 100 if scores[:total_score] > 100
     scores
   end
 end

Files method

The files method of AssignmentParticipant model is exactly the same as assignment_team.rb, L103, so we move it to a module named Instance_method in lib/TFD1770_refactor.rb. The below two lines are added to the AssignmentParticipant model and AssignmentTeam model.

require 'TFD1770_refactor'

include Instance_method

  • module Instance_method
 def files(directory)
   files_list = Dir[directory + "/*"]
   files = []
   files_list.each do |file|
     if File.directory?(file)
       dir_files = files(file)
       dir_files.each {|f| files << f }
     end
     files << file
   end
   files
 end

Self.import method

The self.import method of AssignmentParticipant model is exactly the same as course_participant.rb, L21, so we move it to a module named Class_method in lib/TFD1770_refactor.rb. The below two lines are added to the AssignmentParticipant model and CourseParticipant model.

require 'TFD1770_refactor'

include Class_method

 def self.import(row, _row_header = nil, session, id)
   user = AssignmentParticipant.check_info_and_create(row, _row_header = nil, session)
   raise ImportError, "The assignment with id \"" + id.to_s + "\" was not found." if Assignment.find(id).nil?
   unless AssignmentParticipant.exists?(user_id: user.id, parent_id: id)
     new_part = AssignmentParticipant.create(user_id: user.id, parent_id: id)
     new_part.set_handle
   end
 end
  • module Class_method
 def check_info_and_create(row, _row_header = nil, session)
   raise ArgumentError, "No user id has been specified." if row.empty?
   user = User.find_by(name: row[0])
   if user.nil?
     raise ArgumentError, "The record containing #{row[0]} does not have enough items." if row.length < 4
     attributes = ImportFileHelper.define_attributes(row)
     user = ImportFileHelper.create_new_user(attributes, session)
   end
 end

Find_by method

Use find_by instead of dynamic method

Before After
  • assignment_particpant.rb L32

quiz = QuizQuestionnaire.find_by_instructor_id(contributor.id)

  • assignment_particpant.rb L113

topic = SignUpTopic.find_by_assignment_id(assignment.id)

  • assignment_particpant.rb L196

user = User.find_by_name(row[0])

  • assignment_particpant.rb L34

quiz = QuizQuestionnaire.find_by(instructor_id: contributor.id)

  • assignment_particpant.rb L134

topic = SignUpTopic.find_by(assignment_id: self.assignment.id)

  • TFD1770_refactor.rb L20

user = User.find_by(name:row[0])

Use find_by instead of where.first

Before After
  • assignment_particpant.rb L267

first_user_id = TeamsUser.where(team_id: response_map.reviewee_id).first.user_id

  • assignment_particpant.rb L267

participant = Participant.where(parent_id: response_map.reviewed_object_id, user_id: first_user_id).first

  • assignment_particpant.rb L258

first_user_id = TeamsUser.find_by(team_id: response_map.reviewee_id).user_id

  • assignment_particpant.rb L259

participant = Participant.find_by(parent_id: response_map.reviewed_object_id, user_id: first_user_id)

Resources

References

<references/>