CSC/ECE 517 Spring 2020 - E2003. Refactor and improve assessment360 controller
About Expertiza
Expertiza is an open-source project based on Ruby on Rails framework. The software allows the instructor and students with the following privileges,
Instructors
- Create new assignments
- Customize new or existing assignments
- Create a list of topics that students can sign up for
- Give grades/reviews for the students' submissions
Students
- Form teams with other students to work on various projects and assignments
- Peer review other students' submissions
Note: Expertiza supports submissions across various document types, including URL and wiki pages
Project Description
About assessment_360 Controller
The assessment_360 controller is designed to gather data from all stakeholders about the performance of a student in a course. Thus, its purpose is to provide course-level statistics. At present, the controller provides only two top-level functions where each of them gives a list of students in the class.
- all_students_all_reviews contains teammate and meta-review scores received by students for all assignments in the course
- course_student_grade_summary gives a listing of students and what topic they worked on and what grade they received on each assignment during the semester
Problem Statement
The assessment360 controller had three major issues. This project is aimed to address some of them, including:
Issue 1: Methods all_students_all_reviews and course_student_grade_summary were too long. Common initialization and looping had to factored out by moving them into smaller methods with mnemonic names
Issue 2: Method populate_hash_for_all_students_all_reviews is not easy to understand. It uses a special data structure to do a very simple task. That had to be refactored
Issue 3: Method course_student_grade_summary stopped working and has to be fixed
Motivation
This project aims at encouraging students to collaborate and contribute to open-source project in turn helping them understand what goes on in a full-fledged software deployment process. It also paves a way to understand how one can use Rails, RSpec to develop a system that does justice to the DRY code practicing principles.
Approach
Problem 1: Refactoring all_students_all_reviews and course_student_grade_summary
(a) The common elements in these files i.e., checking if the course participant list is empty or not is moved to a different function called inspect_course_participants
(b) Adding on to this, all_student_all_review was broken down to two more functions, avg_review_calc_per_student and overall_review_count
(c) The method course_student_grade_summary is broken down to one more function assignment_grade_summary
Problem 2: Refactoring populate_hash_for_all_students_all_reviews
The approach in this function is pretty straight forward when we try to break it down. It calculates,
(a) The average score for each assignment for each course
(b) The overall review grade for each assignment for all the students who have taken the course
Passing the average score calculated from (a) checks if the student has teammates which could result in not having an average teammate_review_score. If we calculate (b) in our main function (this can be done) we would be repeating the checks that we are already doing for (a). Hence, this function is not changed
Problem 3: Fix course_student_grade_summary
The current implementation ran into an issue when it was comparing the Key-Value pair passed in peer_review_score. It was hard to check if each of the keys were present or not. The proposed solution makes use of a basic hash value presence check that retrieves a value from the array/hash, given a set of keys. If no value is found, it'll return nil.
Proposed Implementation
Fixing the bug on course_student_grade_summary
The current implementation of the method throws a nil error at the moment on the following line,
unless (peer_review_score.nil? || peer_review_score[:review][:scores][:avg].nil?)
This is rectified by introducing,
next if (peer_review_score[:review] && peer_review_score[:review][:scores] && peer_review_score[:review][:scores][:avg]).nil?
Common refactoring of all_students_all_reviews and course_student_grade_summary method
The functionality that the methods shared was that, both of them checked if there were course participants who had taken a particular course. This occupied 4 lines of code in both the functions. So, this is defined in a separate function called insure_existence_of and called from the two functions.
def insure_existence_of(course_participants)
if course_participants.empty?
flash[:error] = "There is no course participant in course #{course.name}"
redirect_to(:back)
end
end
Refactor all_students_all_reviews method
The all_students_all_reviews method is one of the biggest methods in assessment360_controller.rb file. It finds the list of all students and assignments pertaining to the course. This data is then used to compute the metareview and teammate review scores for each student.
The code before refactoring had 64 lines of code as shown below,
def all_students_all_reviews
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
if @course_participants.empty?
flash[:error] = "There is no course participant in course #{course.name}"
redirect_to(:back)
end
# hashes for view
@meta_review = {}
@teammate_review = {}
@teamed_count = {}
# for course
# eg. @overall_teammate_review_grades = {assgt_id1: 100, assgt_id2: 178, ...}
# @overall_teammate_review_count = {assgt_id1: 1, assgt_id2: 2, ...}
%w[teammate meta].each do |type|
instance_variable_set("@overall_#{type}_review_grades", {})
instance_variable_set("@overall_#{type}_review_count", {})
end
@course_participants.each do |cp|
# for each assignment
# [aggregrate_review_grades_per_stu, review_count_per_stu] --> [0, 0]
%w[teammate meta].each {|type| instance_variable_set("@#{type}_review_info_per_stu", [0, 0]) }
students_teamed = StudentTask.teamed_students(cp.user)
@teamed_count[cp.id] = students_teamed[course.id].try(:size).to_i
@assignments.each do |assignment|
@meta_review[cp.id] = {} unless @meta_review.key?(cp.id)
@teammate_review[cp.id] = {} unless @teammate_review.key?(cp.id)
assignment_participant = assignment.participants.find_by(user_id: cp.user_id)
next if assignment_participant.nil?
teammate_reviews = assignment_participant.teammate_reviews
meta_reviews = assignment_participant.metareviews
populate_hash_for_all_students_all_reviews(assignment,
cp,
teammate_reviews,
@teammate_review,
@overall_teammate_review_grades,
@overall_teammate_review_count,
@teammate_review_info_per_stu)
populate_hash_for_all_students_all_reviews(assignment,
cp,
meta_reviews,
@meta_review,
@overall_meta_review_grades,
@overall_meta_review_count,
@meta_review_info_per_stu)
end
# calculate average grade for each student on all assignments in this course
if @teammate_review_info_per_stu[1] > 0
temp_avg_grade = @teammate_review_info_per_stu[0] * 1.0 / @teammate_review_info_per_stu[1]
@teammate_review[cp.id][:avg_grade_for_assgt] = temp_avg_grade.round.to_s + '%'
end
if @meta_review_info_per_stu[1] > 0
temp_avg_grade = @meta_review_info_per_stu[0] * 1.0 / @meta_review_info_per_stu[1]
@meta_review[cp.id][:avg_grade_for_assgt] = temp_avg_grade.round.to_s + '%'
end
end
# avoid divide by zero error
@assignments.each do |assignment|
temp_count = @overall_teammate_review_count[assignment.id]
@overall_teammate_review_count[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
temp_count = @overall_meta_review_count[assignment.id]
@overall_meta_review_count[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
end
end
To calculate the average grade for each student on all assignments in this course that a student has taken in this course is refactoring in the base code by adding the avg_review_calc_per_student method.
def avg_review_calc_per_student(cp, review_info_per_stu, review)
# check to see if the student has been given a review
if review_info_per_stu[1] > 0
temp_avg_grade = review_info_per_stu[0] * 1.0 / review_info_per_stu[1]
review[cp.id][:avg_grade_for_assgt] = temp_avg_grade.round.to_s + '%'
end
end
To avoid a divide by zero error that is thrown when we try to calculate the average grade when a student has no teammate is handled by the overall_review_count method.
def overall_review_count(assignments, overall_teammate_review_count, overall_meta_review_count)
assignments.each do |assignment|
temp_count = overall_teammate_review_count[assignment.id]
overall_review_count_hash[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
temp_count = overall_meta_review_count[assignment.id]
overall_meta_review_count[assignment.id] = 1 if temp_count.nil? or temp_count.zero?
end
end
After refactoring
def all_students_all_reviews
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
insure_existence_of(@course_participants)
# hashes for view
@meta_review = {}
@teammate_review = {}
@teamed_count = {}
# for course
# eg. @overall_teammate_review_grades = {assgt_id1: 100, assgt_id2: 178, ...}
# @overall_teammate_review_count = {assgt_id1: 1, assgt_id2: 2, ...}
%w[teammate meta].each do |type|
instance_variable_set("@overall_#{type}_review_grades", {})
instance_variable_set("@overall_#{type}_review_count", {})
end
@course_participants.each do |cp|
# for each assignment
# [aggregrate_review_grades_per_stu, review_count_per_stu] --> [0, 0]
%w[teammate meta].each {|type| instance_variable_set("@#{type}_review_info_per_stu", [0, 0]) }
students_teamed = StudentTask.teamed_students(cp.user)
@teamed_count[cp.id] = students_teamed[course.id].try(:size).to_i
@assignments.each do |assignment|
@meta_review[cp.id] = {} unless @meta_review.key?(cp.id)
@teammate_review[cp.id] = {} unless @teammate_review.key?(cp.id)
assignment_participant = assignment.participants.find_by(user_id: cp.user_id)
next if assignment_participant.nil?
teammate_reviews = assignment_participant.teammate_reviews
meta_reviews = assignment_participant.metareviews
calc_overall_review_info(assignment,
cp,
teammate_reviews,
@teammate_review,
@overall_teammate_review_grades,
@overall_teammate_review_count,
@teammate_review_info_per_stu)
calc_overall_review_info(assignment,
cp,
meta_reviews,
@meta_review,
@overall_meta_review_grades,
@overall_meta_review_count,
@meta_review_info_per_stu)
end
# calculate average grade for each student on all assignments in this course
avg_review_calc_per_student(cp, @teammate_review_info_per_stu, @teammate_review)
avg_review_calc_per_student(cp, @meta_review_info_per_stu, @meta_review)
end
# avoid divide by zero error
overall_review_count(@assignments, @overall_teammate_review_count, @overall_meta_review_count)
end
Refactor course_student_grade_summary method
This method finds the list of all students and assignments pertaining to the course. The data obtained is then used to compute the instructor assigned grade and peer review scores. It was the secong biggest method in the assessment360_controller.rb controller file. The code before refactoring is shown below,
def course_student_grade_summary
@topics = {}
@assignment_grades = {}
@peer_review_scores = {}
@final_grades = {}
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
if @course_participants.empty?
flash[:error] = "There is no course participant in course #{course.name}"
redirect_to(:back)
end
@course_participants.each do |cp|
@topics[cp.id] = {}
@assignment_grades[cp.id] = {}
@peer_review_scores[cp.id] = {}
@final_grades[cp.id] = 0
@assignments.each do |assignment|
user_id = cp.user_id
assignment_id = assignment.id
assignment_participant = assignment.participants.find_by(user_id: user_id)
next if assignment.participants.find_by(user_id: user_id).nil?
# A topic exists if a team signed up for a topic, which can be found via the user and the assignment
topic_id = SignedUpTeam.topic_id(assignment_id, user_id)
@topics[cp.id][assignment_id] = SignUpTopic.find_by(id: topic_id)
# Instructor grade is stored in the team model, which is found by finding the user's team for the assignment
team_id = TeamsUser.team_id(assignment_id, user_id)
next if team_id.nil?
team = Team.find(team_id)
peer_review_score = find_peer_review_score(user_id, assignment_id)
# Set the assignment grade, peer review score, and sum for the final student summary
@assignment_grades[cp.id][assignment_id] = team[:grade_for_submission]
unless @assignment_grades[cp.id][assignment_id].nil?
@final_grades[cp.id] += @assignment_grades[cp.id][assignment_id]
end
unless (peer_review_score.nil? || peer_review_score[:review][:scores][:avg].nil?)
@peer_review_scores[cp.id][assignment_id] = peer_review_score[:review][:scores][:avg].round(2)
end
end
end
end
In this function, all the functionalities pertaining to a course participant - student in this case, in a given assignment is invoked by introducing a new method called assignment_grade_summary. This method checks if a topic exists and if it does, it passes the final grade that is calculated for that student in that course.
def assignment_grade_summary(cp, assignment_id)
user_id = cp.user_id
# topic exists if a team signed up for a topic, which can be found via the user and the assignment
topic_id = SignedUpTeam.topic_id(assignment_id, user_id)
@topics[cp.id][assignment_id] = SignUpTopic.find_by(id: topic_id)
# instructor grade is stored in the team model, which is found by finding the user's team for the assignment
team_id = TeamsUser.team_id(assignment_id, user_id)
team = Team.find(team_id)
@assignment_grades[cp.id][assignment_id] = team[:grade_for_submission]
return if @assignment_grades[cp.id][assignment_id].nil?
@final_grades[cp.id] += @assignment_grades[cp.id][assignment_id]
end
After refactoring
def course_student_grade_summary
@topics = {}
@assignment_grades = {}
@peer_review_scores = {}
@final_grades = {}
course = Course.find(params[:course_id])
@assignments = course.assignments.reject(&:is_calibrated).reject {|a| a.participants.empty? }
@course_participants = course.get_participants
insure_existence_of(@course_participants)
@course_participants.each do |cp|
@topics[cp.id] = {}
@assignment_grades[cp.id] = {}
@peer_review_scores[cp.id] = {}
@final_grades[cp.id] = 0
@assignments.each do |assignment|
user_id = cp.user_id
assignment_id = assignment.id
assignment_participant = assignment.participants.find_by(user_id: user_id)
# break out of the loop if there are no participants in the assignment
next if assignment.participants.find_by(user_id: user_id).nil?
# break out of the loop if the participant has no team
next if TeamsUser.team_id(assignment_id, user_id).nil?
# pull information about the student's grades for particular assignment
assignment_grade_summary(cp, assignment_id)
peer_review_score = find_peer_review_score(user_id, assignment_id)
#Skip if there are no peers
next if peer_review_score.nil?
#Skip if there are no reviews done by peers
next if peer_review_score[:review].nil?
#Skip if there are no reviews scores assigned by peers
next if peer_review_score[:review][:scores].nil?
#Skip if there are is no peer review average score
next if peer_review_score[:review][:scores][:avg].nil?
@peer_review_scores[cp.id][assignment_id] = peer_review_score[:review][:scores][:avg].round(2)
end
end
end
Method name explanations
One of the methods devised in this project is use nouns instead of verbs. Ideally the name of a variable or method should be enough to identify it's function in the class. This is because assessment_360 controller, of course is an exception and does explain what each of the functions do on their function names,
- all_students_all_reviews - Find the list of all students and assignments pertaining to the course. This data is used to compute the metareview and teammate review scores.
- avg_review_calc_per_student - Calculate the overall average review grade that a student has gotten from their teammate(s) and instructor(s)
- overall_review_count - To avoid divide by zero error
- course_student_grade_summary - Find the list of all students and assignments pertaining to the course. This data is used to compute the instructor assigned grade and peer review scores.
- assignment_grade_summary - To pull information about the student's grades for particular assignment
- insure_existence_of - To make sure there are participants for the course
- calc_overall_review_info - The function populates the hash value for all students for all the reviews that they have gotten. I.e., Teammate and Meta for each of the assignments that they have taken. This value is then used to display the overall teammate_review and meta_review grade in the view
- find_peer_review_score - The peer review score is taken from the questions for the assignment
Testing
Test Plan
The main objective of improving the current Assessment 360 Controller testing is to make sure each of the functionalities are being looked into regardless of the test coverage because of the number of calculations that are actually being taken place inside the controller. In order to achieve this, tests were written for two main newly added methods in the controller. The first few tests have assumed to expect a normal condition i.e., a person exists and all other information do exist.
Test Cases
The following test cases were made for the major methods added to the Assessment 360 Controller:
- insure_existence_of
- Checking if the course participants are empty
- Check if function redirects to back
- Check if function flashes an error if it's not present
- When method is called, it should check if there are course participants and redirects to back and flashes error if there are no participants
- assignment_grade_summary
- Checking if the course participants are empty
- Check if function redirects to back
- Check if function flashes an error if it's not present
- When method is called, it should check if there are course participants. When this passes, and when there are participants with team id, the total value returned for final grade should be valid
UI Testing
By following the below steps, a reviewer can manually test the changes made to the Assessment 360 Controller.
1. Visit http://152.46.16.125:8080/ and enter the following credentials,
Username: instructor6
Password: password
2. From the menu bar, select Manage > Courses
3. Scroll down to find the respective course, and select the blue globe icon. The page should load.
4. Scroll down to find the respective course, and select the brown menu icon. The page should load.
Submission
Github Link
The changes made to the code is present on this link. Multiple comments and refactoring have been addressed in detail here.
Pull Request
The initial build made out of pull request 1691, kept failing continuously since it was made to the master branch of expertiza. It changed the Gemfile.lock and also the database schema. This push was actually made because the beta branch and master branch of expertiza were not different at all. The new pull request 1713 was made to the beta branch from our beta branch and it worked completely fine.
Team Information
Sri Harsha Varma Uppalapati - LinkedIn
Mentor: Dr. Edward F Gehringer - Website