CSC/ECE 517 Fall 2024 - E2454. Refactor student task.rb
Expertiza
Expertiza is a web application through which students can submit and peer-review learning objects (articles, code, web sites, etc). The National Science Foundation supports the Expertiza project. It is used in select courses at NC State and by professors at several other colleges and universities.
Expertiza is a Ruby on Rails based open source project.
Problem Statement
Background
This project focuses on refactoring the StudentTask class to improve the organization, functionality, and separation of concerns. Currently, the class contains a mix of utility and class methods, many of which are static and should be moved into helper modules or relevant model classes, or broken down into separate, more maintainable methods. The goal is to make the class more efficient and follow best practices in object-oriented programming.
The StudentTask model is a significant part of the Expertiza website which maintains and handles student data who are participants under an assignment. Each assignment has a team, individual user participant and submission deadlines. The motive here is to refactor static methods called in the student_task.rb file and move them to helper classes for better object-oriented code design.
Refactor
The StudentTask model do not strictly follow the SRP and DRY principles and needed a lot of refactoring to make them more manageable in following Object-Oriented design and development.
Files modified:
- student_task.rb
- student_task_controller.rb
- student_task_helper.rb
- student_task_spec.rb
- assessment360_controller.rb
- assessment360_controller_spec.rb
New files added:
- student_task_helper_spec.rb
Refactoring
self.from_participant(participant)
Before:
After:
self.from_participantid(id)
Before:
After:
This method is depreciated during the refactoring process. It no longer served its purpose and was removed from the codebase.
self.from_user(user)
Before:
After:
self.get_author_feedback_data(participant_id, timeline_list)
Before:
After:
self.get_due_date_data(assignment, timeline_list)
Before:
After:
self.peer_review_data(participant_id, timeline_list)
Before:
After:
self.get_submission_data(participant_id, team_id, timeline_list)
Before:
After:
This method, similar to from_participantid(), didn't showcase its usefulness during the refactoring process and was removed from the codebase.
self.get_timeline_data(assignment, participant, _team)
Before:
After:
self.teamed_student(user, ip_address=nil)
Before:
After:
Additional methods added:
Test Cases for StudentTask Model, Controller and helper classes
New Test Cases
1. To check each due date of assignment:
describe '#for_each_due_date_of_assignment' do
let(:due_date_modifier) do
lambda { |dd|
{ label: (dd.deadline_type.name + ' Deadline').humanize,
updated_at: dd.due_at.strftime('%a, %d %b %Y %H:%M') }
}
end
context 'when called with assignment having empty due dates' do
it 'return empty time_list array' do
timeline_list = []
student_task_helper.for_each_due_date_of_assignment(assignment) do |due_date|
timeline_list << due_date_modifier.call(due_date)
end
expect(timeline_list).to eq([])
end
end
context 'when called with assignment having due date' do
context 'and due_at value nil' do
it 'return empty time_list array' do
allow(due_date).to receive(:deadline_type).and_return(deadline_type)
timeline_list = []
due_date.due_at = nil
assignment.due_dates = [due_date]
student_task_helper.for_each_due_date_of_assignment(assignment) { |due_date| timeline_list << due_date_modifier.call(due_date) }
expect(timeline_list).to eq([])
end
end
context 'and due_at value not nil' do
it 'return time_list array' do
allow(due_date).to receive(:deadline_type).and_return(deadline_type)
timeline_list = []
assignment.due_dates = [due_date]
student_task_helper.for_each_due_date_of_assignment(assignment) do |due_date|
timeline_list << due_date_modifier.call(due_date)
end
expect(timeline_list).to eq([{
label: (due_date.deadline_type.name + ' Deadline').humanize,
updated_at: due_date.due_at.strftime('%a, %d %b %Y %H:%M')
}])
end
end
end
end
2. To check for peer reviews
describe '#for_each_peer_review' do
context 'when no review response mapped' do
it 'returns empty' do
timeline_list = []
for_each_peer_review(user2) do |response|
timeline_list << response_modifier.call(response, "Round #{response.round} Peer Review".humanize)
end
expect(timeline_list).to eq([])
end
end
context 'when mapped to review response map' do
it 'returns timeline array' do
timeline_list = []
allow(ReviewResponseMap).to receive_message_chain(:where, :find_each).with(reviewer_id: 1).with(no_args).and_yield(review_response_map)
allow(review_response_map).to receive(:id).and_return(1)
allow(Response).to receive_message_chain(:where, :last).with(map_id: 1).with(no_args).and_return(response)
allow(response).to receive(:round).and_return(1)
allow(response).to receive(:updated_at).and_return(Time.new(2019))
timevalue = Time.new(2019).strftime('%a, %d %b %Y %H:%M')
for_each_peer_review(1) do |resp|
timeline_list << response_modifier.call(resp, "Round #{resp.round} Peer Review".humanize)
end
expect(timeline_list).to eq([{ id: 1, label: 'Round 1 peer review', updated_at: timevalue }])
end
end
end
3. To check and validate author feedbacks
describe '#for_each_author_feedback' do
context 'when no feedback response mapped' do
it 'returns empty' do
timeline_list = []
for_each_author_feedback(user2) do |response|
timeline_list << response_modifier.call(response, 'Author feedback')
end
expect(timeline_list).to eq([])
end
end
context 'when mapped to feedback response map' do
it 'returns timeline array' do
timeline_list = []
allow(FeedbackResponseMap).to receive_message_chain(:where, :find_each).with(reviewer_id: 1).with(no_args).and_yield(review_response_map)
allow(review_response_map).to receive(:id).and_return(1)
allow(Response).to receive_message_chain(:where, :last).with(map_id: 1).with(no_args).and_return(response)
allow(response).to receive(:updated_at).and_return(Time.now)
timevalue = Time.now.strftime('%a, %d %b %Y %H:%M')
timeline_list = []
for_each_author_feedback(1) do |response|
timeline_list << response_modifier.call(response, 'Author feedback')
end
expect(timeline_list).to eq([{ id: 1, label: 'Author feedback', updated_at: timevalue }])
end
end
end
4. To create and validate generated timelines
describe '#generate_timeline' do
context 'when no timeline data mapped' do
it 'returns nil' do
allow(participant).to receive(:get_reviewer).and_return(participant)
expect(student_task_helper.generate_timeline(assignment, participant)).to eq([])
end
end
end
5. Check if StudentTask obeject is created properly
describe '#create_student_task_for_participant' do
it 'creates a StudentTask with the correct attributes' do
student_task = student_task_helper.create_student_task_for_participant(participant3)
expect(student_task).to be_an_instance_of(StudentTask)
expect(student_task.participant).to eq(participant3)
expect(student_task.assignment).to eq(assignment)
expect(student_task.topic).to eq(topic)
expect(student_task.current_stage).to eq('submission')
expect(student_task.stage_deadline).to eq(Time.parse('2024-12-31 12:00:00'))
end
end
6. To check the retrieved tasks for users
describe '#retrieve_tasks_for_user' do
before do
allow(user).to receive_message_chain(:assignment_participants, :includes).and_return([participant4, participant5])
end
it 'retrieves and sorts tasks by stage_deadline' do
tasks = student_task_helper.retrieve_tasks_for_user(user)
expect(tasks.size).to eq(2)
expect(tasks.first.stage_deadline).to eq(Time.parse('2024-11-01 12:00:00'))
expect(tasks.last.stage_deadline).to eq(Time.parse('2024-12-01 12:00:00'))
end
it 'creates StudentTask objects for each participant' do
tasks = student_task_helper.retrieve_tasks_for_user(user)
tasks.each do |task|
expect(task).to be_an_instance_of(StudentTask)
expect(task.participant).to be_in([participant4, participant5])
expect(task.assignment).to eq(assignment)
expect(task.topic).to eq(topic)
end
end
end
7. To check if deadlines are parsed properly
describe '#parse_stage_deadline' do
context 'If a valid time value is given' do
it 'parse the provided time correctly' do
given_time = '2024-12-31 12:00:00'
parsed_time = student_task_helper.parse_stage_deadline(given_time)
expect(parsed_time).to eq(Time.parse(given_time))
end
end
context 'If given time string is invalid' do
it 'return current time plus 1 year' do
given_time = 'invalid-time-string'
overhead_time = Time.now + 1.year
parsed_time = student_task_helper.parse_stage_deadline(given_time)
expect(parsed_time).to be_within(1.second).of(overhead_time)
end
end
end
8. To check various conditions of users tagged as teammates
describe '#group_teammates_by_course_for_user' do
context 'when not in any team' do
it 'returns empty' do
expect(student_task_helper.group_teammates_by_course_for_user(user3)).to eq({})
end
end
context 'when assigned in a course_team ' do
it 'returns empty' do
allow(user).to receive(:teams).and_return([course_team])
expect(student_task_helper.group_teammates_by_course_for_user(user)).to eq({})
end
end
context 'when assigned in a assignment_team ' do
it 'returns the students they are teamed with' do
allow(user).to receive(:teams).and_return([team])
allow(AssignmentParticipant).to receive(:find_by).with(user_id: 1, parent_id: assignment.id).and_return(participant)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: 5, parent_id: assignment.id).and_return(participant2)
allow(Assignment).to receive(:find_by).with(id: team.parent_id).and_return(assignment)
expect(student_task_helper.group_teammates_by_course_for_user(user)).to eq(assignment.course_id => [user2.fullname])
end
end
end
Next Steps
We are yet to discuss the scope of this project and whether it can be extended to other functionalities in Expertiza.
Team
Mentor
- Ammana, Sahithi <sammana@ncsu.edu>
Members
- Eathamukkala, Akarsh Reddy
- Koul, Anmol
- More, Harsh