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

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
 
(54 intermediate revisions by the same user not shown)
Line 1: Line 1:
'''E1562. Refactor AssignmentParticipant model'''<ref>Project Description document https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus</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 8: Line 10:


==Why refactoring?==
==Why refactoring?==
'''Refactoring'''<ref>Refactoring https://en.wikipedia.org/wiki/Code_refactoring</ref> is restructuring of code without the need of changing any external behavior. It reduces complexity and improves readability. It also becomes easy to extend the application with respect to different modules and their functionalities.
'''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.  
Some common techniques to refactor are:


* Moving methods to appropriate modules
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.
* Breaking methods into more meaningful functionality
 
* Creating more generalized code.
==Why Test-driven development?==
* Renaming methods and variable.
'''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).
* Inheritance
 
* 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 ==
== Project Description ==
Line 21: Line 30:
The following tasks have been performed as per the requirements.
The following tasks have been performed as per the requirements.


* Refactor scores method to smaller methods.
* Refactor scores method.
* In set_handle method, there is no need for a separate ELSEIF statement. Club the ELSEIF statement into IF statement as an another OR condition.
** Write failing tests first.
* Remove methods like compute_quiz_scores(scores) , average_score_per_assignment(assignment_id) which are not being used.
** Split into several simpler methods and assign reasonable names.
* Method cycle_deviation_score(cycle) is also present in CollusionCycle, remove it from this class.
** Extract duplicated code into separate methods.
* Methods related to reviews like teammate_reviews, bookmark_reviews do not belong to AssignmentParticipant model. Move them to the appropriate class.
** Replace the conditional with the relevant method calls.
* Methods related to files and directories like files(directory) should not be present in AssignmentParticipant model, move them to FileHelper module.


== Refactoring ==
* Method files is exactly the same as assignment_team.rb L103.
===Scores method===
** Write failing tests first.
The method '''scores''' has been converted to smaller methods '''scores''', '''assignment_questionnaires''',  '''merge_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.
** Solve the duplication, extract method to a new file or delete useless one.


{| class = "wikitable"
* Method self.import is exact the same as course_participant.rb L21.
|-
** Write failing tests first.
!Before!!After
** Solve the duplication, extract method to a new file or delete useless one.
|-
|def scores(questions)
    scores = {}
    scores[:participant] = self
    self.assignment.questionnaires.each do |questionnaire|
      round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
      #create symbol for "varying rubrics" feature -Yang
      if(round!=nil)
        questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
      else
        questionnaire_symbol = questionnaire.symbol
      end


      scores[questionnaire_symbol] = {}
* Use find_by instead of dynamic method.
** Write failing tests first.


      if round==nil
* Use find_by instead of where.first.
        scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
** Write failing tests first.
      else
        scores[questionnaire_symbol][:assessments] = 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)
== 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.


    #merge scores[review#] (for each round) to score[review]  -Yang
The introduction video to our project<ref>Introduction video to our project https://www.youtube.com/watch?v=sJdmN-S2Voo&feature=youtu.be</ref>
    if self.assignment.varying_rubrics_by_round?
  describe '#dir_path' do
      review_sym = "review".to_sym
    it 'returns the directory path of current assignment' do
      scores[review_sym] = Hash.new
       expect(participant.dir_path).to eq "final_test"
      scores[review_sym][:assessments] = Array.new
    end
      scores[review_sym][:scores] = Hash.new
  end
       scores[review_sym][:scores][:max] = -999999999
      scores[review_sym][:scores][:min] = 999999999
      scores[review_sym][:scores][:avg] = 0
      total_score = 0
      for i in 1..self.assignment.get_review_rounds
        round_sym = ("review"+i.to_s).to_sym
        if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
          next
        end
        length_of_assessments=scores[round_sym][:assessments].length.to_f


        scores[review_sym][:assessments]+=scores[round_sym][:assessments]
  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


        if(scores[round_sym][:scores][:max]!=nil && scores[review_sym][:scores][:max]<scores[round_sym][:scores][:max])
  describe '#reviewers' do
          scores[review_sym][:scores][:max]= scores[round_sym][:scores][:max]
    it 'returns all the participants in this assignment who have reviewed the team where this participant belongs' do
        end
      allow(ReviewResponseMap).to receive(:where).with(any_args).and_return([response_map]) # differences with .with(parameter)
        if(scores[round_sym][:scores][:min]!= nil && scores[review_sym][:scores][:min]>scores[round_sym][:scores][:min])
      allow(AssignmentParticipant).to receive(:find).with(any_args).and_return(participant2)
          scores[review_sym][:scores][:min]= scores[round_sym][:scores][:min]
      expect(participant.reviewers).to eq([participant2])
        end
    end
        if(scores[round_sym][:scores][:avg]!=nil)
  end
          total_score += scores[round_sym][:scores][:avg]*length_of_assessments
        end
      end
 
      if scores[review_sym][:scores][:max] == -999999999 && scores[review_sym][:scores][:min] == 999999999
              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
  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
  end


     # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
  describe '#scores' do
    # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
     context 'when assignment is not varying rubric by round and not an microtask' do
    if assignment.is_microtask?
      it 'calculates scores that this participant has been given' do
      topic = SignUpTopic.find_by_assignment_id(assignment.id)
        expect(participant).to receive(:assignment_questionnaires)
      if !topic.nil?
        expect(assignment).to receive(:compute_total_score)
         scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
         scores[:max_pts_available] = topic.micropayment
        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
     end
     end


     # for all quiz questionnaires (quizzes) taken by the participant
     context 'when assignment is varying rubric by round but not an microtask' do
    quiz_responses = Array.new
      it 'calculates scores that this participant has been given' do
    quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
        expect(participant).to receive(:assignment_questionnaires)
    quiz_response_mappings.each do |qmapping|
        expect(assignment).to receive(:compute_total_score)
      if (qmapping.response)
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(true)
         quiz_responses << qmapping.response
        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
     end
     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)
     context 'when assignment is not varying rubric by round but an microtask' do
    #scores[:total_score] += compute_quiz_scores(scores)
      it 'calculates scores that this participant has been given' do
 
        expect(participant).to receive(:assignment_questionnaires)
    # move lots of calculation from view(_participant.html.erb) to model
        expect(assignment).to receive(:compute_total_score)
    if self.grade
        allow(assignment).to receive(:varying_rubrics_by_round?).and_return(false)
      scores[:total_score] = self.grade
        allow(assignment).to receive(:is_microtask?).and_return(true)
    else
        expect(participant).to receive(:topic_total_scores)
      total_score = scores[:total_score]
        expect(assignment).to receive(:compute_total_score)
      if total_score > 100
        expect(participant).to receive(:caculate_scores)
         total_score = 100
         participant.scores(question)
       end
       end
      scores[:total_score] = total_score
    scores
     end
     end
   end
   end
||
* scores(questions) method


   def scores(questions)
   # included method
    scores = {}
  describe '#assignment_questionnaires' do
    scores[:participant] = self
    context 'when the round of questionnaire is nil' do
 
      it 'record the result as review scores' do
    assignment_questionnaires(questions, scores)
        scores = {}
 
        question_hash = {review: question}
    scores[:total_score] = self.assignment.compute_total_score(scores)
        score_map = {max: 100, min: 100, avg: 100}
 
        allow(AssignmentQuestionnaire).to receive(:find_by).with(any_args).and_return(assignment_questionnaire)
    merge_scores(scores)
        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)
    # In the event that this is a microtask, we need to scale the score accordingly and record the total possible points
        participant.assignment_questionnaires(question_hash, scores)
    # PS: I don't like the fact that we are doing this here but it is difficult to make it work anywhere else
         expect(scores[:review][:assessments]).to eq([response])
    if assignment.is_microtask?
         expect(scores[:review][:scores]).to eq(score_map)
      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
     end
     end


     # for all quiz questionnaires (quizzes) taken by the participant
     context 'when the round of questionnaire is not nil' do
    quiz_responses = Array.new
      it 'record the result as review#{n} scores' do
    quiz_response_mappings = QuizResponseMap.where(reviewer_id: self.id)
        scores = {}
    quiz_response_mappings.each do |qmapping|
        question_hash = {review1: question}
      if (qmapping.response)
        score_map = {max: 100, min: 100, avg: 100}
         quiz_responses << qmapping.response
        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
     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)
    calculate_scores(scores)
   end
   end
* assignment_questionnaires(questions, scores) method


   def assignment_questionnaires(questions, scores)
   describe '#merge_scores' do
     self.assignment.questionnaires.each do |questionnaire|
     context 'when all of the review_n are nil' do
       round = AssignmentQuestionnaire.find_by_assignment_id_and_questionnaire_id(self.assignment.id, questionnaire.id).used_in_round
       it 'set max, min, avg of review score as 0' do
      #create symbol for "varying rubrics" feature -Yang
        scores = {}
      if(round!=nil)
        allow(assignment).to receive(:num_review_rounds).and_return(1)
         questionnaire_symbol = (questionnaire.symbol.to_s+round.to_s).to_sym
        participant.merge_scores(scores)
      else
        expect(scores[:review][:scores][:max]).to eq(0)
         questionnaire_symbol = questionnaire.symbol
         expect(scores[:review][:scores][:min]).to eq(0)
         expect(scores[:review][:scores][:min]).to eq(0)
       end
       end
    end


       scores[questionnaire_symbol] = {}
    context 'when the review_n is not nil' do
 
       it 'merge the score of review_n to the score of review' do
      if round==nil
        score_map = {max: 100, min: 100, avg: 100}
         scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_for(self)
        scores = {review1: {scores: score_map, assessments: [response]}}
      else
        allow(assignment).to receive(:num_review_rounds).and_return(1)
         scores[questionnaire_symbol][:assessments] = questionnaire.get_assessments_round_for(self,round)
        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
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
     end
     end
   end
   end


* merge_scores(scores) method
  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


   def merge_scores(scores)
   describe '#caculate_scores' do
    #merge scores[review#] (for each round) to score[review]  -Yang
     context 'when the participant has the grade' do
     if self.assignment.varying_rubrics_by_round?
       it 'his total scores equals his grade' do
      review_sym = "review".to_sym
        scores = {}
       scores[review_sym] = Hash.new
         expect(participant2.caculate_scores(scores)).to eq(100.0)
      scores[review_sym][:assessments] = Array.new
      end
      scores[review_sym][:scores] = Hash.new
    end
      scores[review_sym][:scores][:max] = -999999999
    context 'when the participant has the grade and the total score more than 100' do
      scores[review_sym][:scores][:min] = 999999999
      it 'return the score of a given participant with total score 100' do
      scores[review_sym][:scores][:avg] = 0
         scores = {total_score: 110}
      total_score = 0
         expect(participant.caculate_scores(scores)).to eq(total_score: 100)
      for i in 1..self.assignment.get_review_rounds
         round_sym = ("review"+i.to_s).to_sym
        if scores[round_sym][:assessments].nil? || scores[round_sym][:assessments].length==0
          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
        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] == -999999999 && scores[review_sym][:scores][:min] == 999999999
    context 'when the participant has the grade and the total score less than 100' do
         scores[review_sym][:scores][:max] = 0
       it 'return the score of a given participant with total score' do
         scores[review_sym][:scores][:min] = 0
         scores = {total_score: 90}
         expect(participant.caculate_scores(scores)).to eq(total_score: 90)
       end
       end
    end
  end


       scores[review_sym][:scores][:avg] = total_score/scores[review_sym][:assessments].length.to_f
  describe '#copy' do
    it 'copies assignment participants to a certain course' do
       expect(participant.copy(517)).to be_an_instance_of(CourseParticipant)
     end
     end
   end
   end


* calculate_scores(scores) method
  describe '#feedback' do
    it 'returns corrsponding author feedback responses given by current participant' do
      expect(participant.feedback).to eq([response])
    end
  end


   def calculate_scores(scores)
   describe '#reviews' do
     # move lots of calculation from view(_participant.html.erb) to model
     it 'returns corrsponding peer review responses given by current team' do
    if self.grade
      expect(participant.reviews).to eq([response])
      scores[:total_score] = self.grade
    else
      total_score = scores[:total_score]
      if total_score > 100
        total_score = 100
      end
      scores[:total_score] = total_score
      scores
     end
     end
   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])
===set_handle method===
       expect(participant.reviews_by_reviewer(participant2)).to eq([response])
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.
    end
 
{| 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
   end


||def set_handle
  describe '#quizzes_taken' do
       if self.user.handle == nil or self.user.handle == "" or AssignmentParticipant.where(parent_id: self.assignment.id, handle: self.user.handle).length > 0
    it 'returns corrsponding quiz responses given by current participant' do
        self.handle = self.user.name
       expect(participant.quizzes_taken).to eq([response])
      else
    end
        self.handle = self.user.handle
      end
      self.save!
   end
   end


|-
   describe '#metareviews' do
|}
     it 'returns corrsponding metareview responses given by current participant' do
 
      expect(participant.metareviews).to eq([response])
===File and directory methods===
    end
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
   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
   end


* files(directory)
   describe '#teammate_reviews' do
 
     it 'returns corrsponding teammate review responses given by current participant' do
   def files(directory)
       expect(participant.teammate_reviews).to eq([response])
    files_list = Dir[directory + "/*"]
    files = Array.new
 
     files_list.each do |file|
       if File.directory?(file)
        dir_files = files(file)
        dir_files.each{|f| files << f}
      end
      files << file
     end
     end
    files
   end
   end


* submitted_files()
   describe '#bookmark_reviews' do
 
     it 'returns corrsponding bookmark review responses given by current participant' do
   def submitted_files()
      expect(participant.bookmark_reviews).to eq([response])
     files = Array.new
    if(self.directory_num)
      files = files(self.path)
     end
     end
    return files
   end
   end


* dir_path
  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


  def dir_path
    context 'when there is subdirectory in current directory' do
    assignment.try :directory_path
      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
   end


===Remove unused methods===
  describe ".import" do
The following methods have been removed as they were not being used by the application.
    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


'''def average_score_per_assignment(assignment_id)'''
    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


def average_score_per_assignment(assignment_id)
      context 'when the record has more than 4 items' do
    return 0 if self.response_maps.size == 0
        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


    sum_of_scores = 0
        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
    self.response_maps.metareview_response_maps.each do |metaresponse_map|
            row = ["user_name", "user_fullname", "name@email.com", "user_role_name", "user_parent_name"]
      if !metaresponse_map.response.empty? && response_map == assignment_id then
            session = {user: participant}
        sum_of_scores = sum_of_scores + response_map.response.last.average_score
            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
     end
  end


     (sum_of_scores / self.response_maps.size).to_i
  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
   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


'''def quiz_taken_by?(contributor, reviewer)'''
    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


  def quiz_taken_by?(contributor, reviewer)
     context 'when current assignment does not have participants with same handle as the one of current user' do
     quiz_id = QuizQuestionnaire.find_by_instructor_id(contributor.id)
      it 'sets the user name as the handle of current participant' do
    return QuizResponseMap.where(['reviewee_id = ? AND reviewer_id = ? AND reviewed_object_id = ?',
        participant.set_handle
                                  self.id, reviewer.id, quiz_id]).count > 0
        expect(participant.handle).to eq("handle")
      end
    end
   end
   end


 
  describe '#review_file_path' do
'''def has_quiz?'''
    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)
  def has_quiz?
      allow(TeamsUser).to receive_message_chain(:find_by, :user_id).with(any_args).and_return(1)
    return !QuizQuestionnaire.find_by_instructor_id(self.id).nil?
      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
   end


 
  describe '#current_stage' do
'''def reviewees'''
    it 'returns stage of current assignment' do
 
      allow(assignment).to receive(:get_current_stage).with(1).and_return("Finished")
  def reviewees
      expect(participant.current_stage).to eq("Finished")
    reviewees = []
     end
    rmaps = ResponseMap.all(conditions: ["reviewer_id = #{self.id} && type = 'ReviewResponseMap'"])
        rmaps.each { |rm| reviewees.concat(AssignmentTeam.find(rm.reviewee_id).participants) }
 
     reviewees
   end
   end


 
  describe '#stage_deadline' do
'''def two_node_cycles'''
    context 'when stage of current assignment is not Finished' do
      it 'returns current stage' do
  def two_node_cycles
        allow(assignment).to receive(:stage_deadline).with(1).and_return("Unknow")
    cycles = []
         expect(participant.stage_deadline).to eq("Unknow")
    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
     end
     end
    cycles
  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


'''def three_node_cycles'''
      context 'current assignment is a staggered deadline assignment' do
 
        it 'returns the due date of current topic' do
  def three_node_cycles
          allow(assignment).to receive(:stage_deadline).with(1).and_return("Finished")
    cycles = []
           allow(assignment).to receive(:staggered_deadline?).and_return(true)
    self.reviewers.each do |ap1|
           allow(TopicDueDate).to receive_message_chain(:find_by, :last, :due_at).and_return(1)
      ap1.reviewers.each do |ap2|
           expect(participant.stage_deadline).to eq("1")
        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
       end
     end
     end
    cycles
   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.


'''def four_node_cycles'''
{| class = "wikitable"
|-
!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


  def four_node_cycles
      scores[questionnaire_symbol] = {}
    cycles = []
 
    self.reviewers.each do |ap1|
       scores[questionnaire_symbol][:assessments] = if round.nil?
       ap1.reviewers.each do |ap2|
                                                    questionnaire.get_assessments_for(self)
        ap2.reviewers.each do |ap3|
                                                  else
          if ap3.reviewers.include?(self)
                                                    questionnaire.get_assessments_round_for(self, round)
            self.reviews_by_reviewer(ap1).nil? ? next : s01 = self.reviews_by_reviewer(ap1).get_total_score
                                                  end
            ap1.reviews_by_reviewer(ap2).nil? ? next : s12 = ap1.reviews_by_reviewer(ap2).get_total_score
      scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
            ap2.reviews_by_reviewer(ap3).nil? ? next : s23 = ap2.reviews_by_reviewer(ap3).get_total_score
    end
            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]])
    scores[:total_score] = self.assignment.compute_total_score(scores)
           end
 
    # 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
       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
     cycles
  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
   end


* assignment_questionnaires(questions, scores) method


'''def cycle_similarity_score(cycle)'''
  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


  def cycle_similarity_score(cycle)
      scores[questionnaire_symbol] = {}
    similarity_score = 0.0
    count = 0.0


    0 ... cycle.size-1.each do |pivot|
      scores[questionnaire_symbol][:assessments] = if round.nil?
       pivot_score = cycle[pivot][1]
                                                    questionnaire.get_assessments_for(self)
      similarity_score = similarity_score + (pivot_score - cycle[other][1]).abs
                                                  else
      count = count + 1.0
                                                    questionnaire.get_assessments_round_for(self, round)
                                                  end
       scores[questionnaire_symbol][:scores] = Answer.compute_scores(scores[questionnaire_symbol][:assessments], questions[questionnaire_symbol])
     end
     end
    similarity_score = similarity_score / count unless count == 0.0
    similarity_score
   end
   end


* merge_scores(scores) method


'''def cycle_deviation_score(cycle)'''
  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


  def cycle_deviation_score(cycle)
*  topic_total_scores(scores) method
    deviation_score = 0.0
    count = 0.0


     0 ... cycle.size.each do |member|
  def topic_total_scores(scores)
      participant = AssignmentParticipant.find(cycle[member][0].id)
     topic = SignUpTopic.find_by(assignment_id: self.assignment.id)
       total_score = participant.get_review_score
    unless topic.nil?
       deviation_score = deviation_score + (total_score - cycle[member][1]).abs
       scores[:total_score] *= (topic.micropayment.to_f / 100.to_f)
      count = count + 1.0
       scores[:max_pts_available] = topic.micropayment
     end
     end
    deviation_score = deviation_score / count unless count == 0.0
    deviation_score
   end
   end


* calculate_scores(scores) method


'''def compute_quiz_scores(scores)'''
   def caculate_scores(scores)
 
     if self.grade
   def compute_quiz_scores(scores)
       scores[:total_score] = self.grade
    total = 0
     if scores[:quiz][:scores][:avg]
       return scores[:quiz][:scores][:avg] * 100  / 100.to_f
     else
     else
       return 0
       scores[:total_score] = 100 if scores[:total_score] > 100
      scores
     end
     end
    return total
   end
   end


|-
|}


'''def review_response_maps'''
===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.


  def review_response_maps
require 'TFD1770_refactor'
      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


include Instance_method


'''def members'''
* module Instance_method
  def files(directory)
    files_list = Dir[directory + "/*"]
    files = []


  def members
    files_list.each do |file|
     team.try :participants
      if File.directory?(file)
        dir_files = files(file)
        dir_files.each {|f| files << f }
      end
      files << file
    end
     files
   end
   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.


'''def get_hash(time_stamp)'''
require 'TFD1770_refactor'


  def get_hash(time_stamp)
include Class_method
    # 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
  def self.import(row, _row_header = nil, session, id)
     sign = hash_data + self.user.name.to_s + time_stamp.strftime("%Y-%m-%d %H:%M:%S")
     user = AssignmentParticipant.check_info_and_create(row, _row_header = nil, session)
     Digest::SHA1.digest(sign)
     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
   end


===Review methods===
* module Class_method
* 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.
  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


{| class = "wikitable"
{| class = "wikitable"
Line 522: 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 539: 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


|-
|-
|}
|}
== Testing ==
Please follow the below instruction to test UI:
* Login to Expertiza on http://152.46.20.199:3000/.
* Enter username as 'student13' and password as 'password'.
* You can select a previous assignment 'shivam test' or 'Pankti test'.
* You should be able to access the pages further
** Your team- The page content should be visible without any errors.
** Your work - The upload button for submission should be visible.
** Your scores- The page should give result 0 and not any error.
When the user runs the application, the behavior of the application remains the same.
The existing [https://en.wikipedia.org/wiki/RSpec RSpec] test cases remain successful. The same can be checked by running
Rspec spec
The participant is able to log in to the system, view the assignments, submit links and files in assignment submission, view the scores, review peers, etc. Below are some of the execution screenshots after the refactoring of the model.
The Expertiza Login Screen
[[File:ExpertizaLoginScreen.jpg]]
User can see all the assignments as below.
[[File:ExpertizaAPS2.jpg]]
View team members for the assignment selected. In this case, we have taken a dummy assignment we created.
[[File:ExpertizaAPS3.jpg]]
View the submission page to upload the assignment.
[[File:ExpertizaAPS4.jpg]]
View the scores of an assignment.
[[File:ExpertizaAPS5.jpg]]


==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/>