Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2017/E1793. Help students find teams to join

2017-12-02T02:27:15Z

Qwang32: /* Test Plan */

==Introduction==
==='''Problem Statement'''===
For team-based assignments, it always takes time to find suitable team members. We already have bidding, which could help you to join in a team with other team members hold similar bidding preferences. However, you may not be satisfied with automated team formation and want to switch to another team. In this project, we will build a new feature to help students find teams to join.

Currently, there are 2 ways to find other students to join your team:

1.If your team is not full, you could invite people by inputting his/her UnityID. It will send an invitation to certain user. If s/he accept your invitation,s/he will leave original team and join your team.
2.You could create an advisement by clicking “Your team” link and then clicking “Create” link under “Advertisement for teammates” section. Then your advertisement will appear the last column of the signup sheet page with a horn icon. In this way, all classmates could see your advisement. Someone could send a request to join your team. If you accept their request, s/he will leave original team and join in your team.

It would be better for students who do not have team yet or whose team is not full yet to be able to see a list of students who don’t already have teams. So too for instructors.

== Task Description ==

Fix the second way to find other students to join your team.

Currently, after you create an advertisement, the horn icon does not appear in the the last column of the signup sheet.

For student end:

Display a list of students who do not have a team with invitation links in student_teams#view page
You could invite students to your team by clicking invitation links. If s/he accept your invitation,s/he will leave original team and join in your team. It will be more straightforward than typing UnityID.

For instructor end:

Display a list of students who do not have team in teams#list page

Write feature tests to verify your modifications:

Create team_invitation_spec.rb file in spec/features folder

==Modified Files ==
1)app/views/student_teams/view.html.erb

2)app/views/join_team_requests/_list_received.erb

3)app/controllers/join_team_requests_controller.rb

4)app/models/join_team_request.rb

5)app/models/sign_up_sheet.rb

6)app/views/sign_up_sheet/_table_line.html.erb

7)app/views/sign_up_sheet/_all_actions.html.erb

8)app/views/teams/list.html.erb

9)spec/features/team_invitation_spec.rb

=='''Approach taken to resolve the issues'''==

===Issue1===
When someone creates an ad ,the horny icon does not appear in the the last column of the signup sheet.the code that show horny icon is below:

<% if SignUpSheet.has_teammate_ads?(topic.id) %>
<%= link_to image_tag('ad.png', :border => 0, :title => 'Ad', :align => 'middle', :style => 'width: 24px; height:24px'), :controller=>'sign_up_sheet',
:action=> 'show_team',assignment_id=>@assignment.id, :id=>topic.id%>
<% end %>

After debugging ,I found even if you create an ad before ,the if block of code will not be executed. Therefore something must be wrong with has_teammate_ads? method.Then I rewrite that method.Signupteam has an attribute:advertise_for_partner ,it will be set true after team members create an ad for their team.i return this attribute to the has_teammate_ads? method and the horny icon appears.This method code is below:
def self.has_teammate_ads?(topic_id)
@ads_exsit=false
@result=SignedUpTeam.where("topic_id = ?", topic_id.to_s)
@result.each do |result|
team=result.team
@ads_exsit=team.advertise_for_partner
end
@ads_exsit
end

===Issue2===
I found currently when you sent join team request in the ad,the team received that request would see invite and decline button on their student teams#view page.if they are willing to add the student who send request as member ,they have to send invitation and wait for reply. This is not reasonable and convenient. what should happen is that when the student send join team request,the team members could see accept and decline button ,After they click accept button ,the student would leave his/her original team and join the new team.
What I have done is that I add accept method in join team request controller.The method is below:

def accept
unless JoinTeamRequest.accept_invite(params[:team_id], @inviter_userid, @invited_userid, @assignment_id)
flash[:error] = 'The system failed to add you to the team that invited you.'
end
redirect_to view_student_teams_path student_id: params[:student_id]
end

Here @invited_userid is acquired from the student who sent join team request. inviter_userid is from the team members who received the request. team_id is invited student's team id.

===Issue3===

We build a new feature to help students find teams to join. Students who do not have teams or whose team is not full are able to see a list of students who do not haven a team.The code for this issue is showed below:
<% if @student.user_id!=nil %>
<%if @student.assignment.max_team_size > 1 %>
<% if @student.team==nil|| !@student.team.full? %>
Students without teams
<%@student.assignment.participants.each do |participant| %>
<% if participant.team==nil %>
<% if User.find(participant.user_id).role.name=="Student" %>
<% if participant.user.name!=@student.name %>
<td><%= participant.user.name %></td>
<%if @student.team==nil
@teamid="0"
else
@teamid=@student.team.id
end%>
<td><%= form_tag :controller => 'invitations', :action => 'create' do %>
<%= hidden_field_tag 'team_id', @teamid %>
<%= hidden_field_tag 'student_id', @student.id %>
<%= hidden_field_tag 'session[:dummy][:assignment_id]', @student.parent_id %>
<%= hidden_field_tag 'user[name]', participant.user.name%>
<%= submit_tag "invite"%>
<% end %>
</td>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
</table>
<% end %>
<% end %>

In the code above, if @student.assignment.max_team_size > 1 we then execute the below code ,this is because our new feature is only available when assignment team size greater than 1.we use if @student.team==nil|| !@student.team.full? to allow students who have no team or whose team is not full to see this list.Next we found all participants ,we use if statement to sample those who has no team , their role are students and they won't see themselves if they do not have teams.We then put these sampled participants on the list .these achieved by the code below
<%@student.assignment.participants.each do |participant| %>
<% if participant.team==nil %>
<% if User.find(participant.user_id).role.name=="Student" %>
<% if participant.user.name!=@student.name %>
<td><%= participant.user.name %></td>
We also need to add invitation links next to students without teams,before adding links , we need to know whether students who want to invite the students on the list have team or not .if they do not have their own teams. we will automatically create a team for them ,this mechanism is called team lazy initialization.the code for this part is below
<%if @student.team==nil
@teamid="0"
else
@teamid=@student.team.id
end%>
def check_inviterteam_before_invitation
@student = AssignmentParticipant.find(params[:student_id])
@team_id=params[:team_id]
if @team_id=="0"
team = AssignmentTeam.create_team_and_node(@student.parent_id)
user = User.find(@student.user_id)
# create TeamsUser and TeamUserNode
teamuser = ApplicationController.helpers.create_team_users(user, team.id)
@team_id=team.id
end
end
Finally we just use form_tag to create invite button which is related to invitation controller and create method in it .

===Issue4===
we also display a list of students who do not have team in teams#list page for instructor.This part of code is showed below.
<% @assignment.participants.each do |participant| %> 
<% if participant.team==nil %>
<% if User.find(participant.user_id).role.name=="Student" %>
<td><%= participant.user.name %></td>
<% end %>
<% end %>
<% end %>

==Screenshots of new feature==
1)The horny icons appear in the the last column of the signup sheet.
[[File:30.png]]
[[File:31.png]]

2)Received request from advertisement and the changed team members after clicking accept button.
[[File:32.png]]
[[File:33.png]]

3)For student end ,the list of students who do not have a team with invitation links
[[File:35.png]]

4)For instructor end ,the list of students who do not have a team.
[[File:36.png]]

==Test Plan==
The last part of this project is to test all the modifications. And in order to test the new features we created a new Rspec file, which is spec/features/team_invitation_spec.rb. The codes below are all tests we created.

Some edge cases:

1) The owner of a team can accept or decline the request sent by other students, if the team is already full, the team will remain the same even the request is accepted.
context 'when team owner declining the invitation' do
it 'makes team members remain the same as before' do
find("input[type=submit][value='Decline']").click
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
page.should have_no_content('student2064')
end
end

context 'when team owner accepting the invitation' do
context 'when the team is not full' do
it 'makes requester joins the team' do
allow_any_instance_of(TeamController).to receive(full?).and_return(false)
find("input[type=submit][value='Accept']").click
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
page.should have_content('student2064')
end
end

context 'when the team is already full' do
it 'makes team members remain the same as before' do
allow_any_instance_of(TeamController).to receive(full?).and_return(true)
find("input[type=submit][value='Accept']").click
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
page.should have_no_content('student2064')
end
end

2) In the student_team#view page, students should see a list of students who don't have teams and can be invited at present, if the invitation is declined, the team remains the same. And the invitee can only join the team when it is not full.

context 'on student_teams#view page (student end)' do
before(:each) do
stub_current_user(@owner, @owner.role.name, @owner.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
end
it 'shows a list of students who do not have a team' do
expect(page).to have_content('student2064')
expect(page).to have_selector("input[typ1e=submit][value='invite']")
end

context 'when invitee declining the invitation' do
it 'makes team members remain the same as before' do
find("input[type=submit][value='Decline']").click
end
end

context 'when invitee accepting the invitation' do
context 'when the team is not full' do
it 'makes invitee joins the team' do
allow_any_instance_of(TeamController).to receive(full?).and_return(false)
end
end

context 'when the team is already full' do
it 'makes team members remain the same as before' do
allow_any_instance_of(TeamController).to receive(full?).and_return(true)
end
end

3) Testing advertisement features.
context 'advertisement feature' do
before(:each) do
# team owner creates an advertisement from student_teams#view page
@owner = User.find_by(name: "student2064")
stub_current_user(@owner, @owner.role.name, @owner.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
click_link 'Create'
fill_in 'comments_for_advertisement', with: 'advertisement1'
click_button 'Create'
expect(page).to have_content 'advertisement1'
expect(page).to have_content 'Delete'
@user = User.find_by(name: "student2065")
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Signup sheet'
horn = find(:xpath, "//a[contains(@href,'show_team/2?')]")
horn.click
expect(page).to have_content 'Request invitation'
click_link 'Request invitation'
fill_in 'comments_', with: 'request_demo'
expect(page).to have_selector("input[type=submit][value='Create']")
create(:join_team_request)
# team owner is able to accept or decline the invitation
stub_current_user(@owner, @owner.role.name, @owner.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
# expect(page).to have_selector("input[typ1e=submit][value='Accept']")
# expect(page).to have_selector("input[type=submit][value='Decline']")
end

CSC/ECE 517 Fall 2017/E1793. Help students find teams to join

2017-12-02T02:24:49Z

Qwang32:

==Introduction==
==='''Problem Statement'''===
For team-based assignments, it always takes time to find suitable team members. We already have bidding, which could help you to join in a team with other team members hold similar bidding preferences. However, you may not be satisfied with automated team formation and want to switch to another team. In this project, we will build a new feature to help students find teams to join.

Currently, there are 2 ways to find other students to join your team:

1.If your team is not full, you could invite people by inputting his/her UnityID. It will send an invitation to certain user. If s/he accept your invitation,s/he will leave original team and join your team.
2.You could create an advisement by clicking “Your team” link and then clicking “Create” link under “Advertisement for teammates” section. Then your advertisement will appear the last column of the signup sheet page with a horn icon. In this way, all classmates could see your advisement. Someone could send a request to join your team. If you accept their request, s/he will leave original team and join in your team.

It would be better for students who do not have team yet or whose team is not full yet to be able to see a list of students who don’t already have teams. So too for instructors.

== Task Description ==

Fix the second way to find other students to join your team.

Currently, after you create an advertisement, the horn icon does not appear in the the last column of the signup sheet.

For student end:

Display a list of students who do not have a team with invitation links in student_teams#view page
You could invite students to your team by clicking invitation links. If s/he accept your invitation,s/he will leave original team and join in your team. It will be more straightforward than typing UnityID.

For instructor end:

Display a list of students who do not have team in teams#list page

Write feature tests to verify your modifications:

Create team_invitation_spec.rb file in spec/features folder

==Modified Files ==
1)app/views/student_teams/view.html.erb

2)app/views/join_team_requests/_list_received.erb

3)app/controllers/join_team_requests_controller.rb

4)app/models/join_team_request.rb

5)app/models/sign_up_sheet.rb

6)app/views/sign_up_sheet/_table_line.html.erb

7)app/views/sign_up_sheet/_all_actions.html.erb

8)app/views/teams/list.html.erb

9)spec/features/team_invitation_spec.rb

=='''Approach taken to resolve the issues'''==

===Issue1===
When someone creates an ad ,the horny icon does not appear in the the last column of the signup sheet.the code that show horny icon is below:

<% if SignUpSheet.has_teammate_ads?(topic.id) %>
<%= link_to image_tag('ad.png', :border => 0, :title => 'Ad', :align => 'middle', :style => 'width: 24px; height:24px'), :controller=>'sign_up_sheet',
:action=> 'show_team',assignment_id=>@assignment.id, :id=>topic.id%>
<% end %>

After debugging ,I found even if you create an ad before ,the if block of code will not be executed. Therefore something must be wrong with has_teammate_ads? method.Then I rewrite that method.Signupteam has an attribute:advertise_for_partner ,it will be set true after team members create an ad for their team.i return this attribute to the has_teammate_ads? method and the horny icon appears.This method code is below:
def self.has_teammate_ads?(topic_id)
@ads_exsit=false
@result=SignedUpTeam.where("topic_id = ?", topic_id.to_s)
@result.each do |result|
team=result.team
@ads_exsit=team.advertise_for_partner
end
@ads_exsit
end

===Issue2===
I found currently when you sent join team request in the ad,the team received that request would see invite and decline button on their student teams#view page.if they are willing to add the student who send request as member ,they have to send invitation and wait for reply. This is not reasonable and convenient. what should happen is that when the student send join team request,the team members could see accept and decline button ,After they click accept button ,the student would leave his/her original team and join the new team.
What I have done is that I add accept method in join team request controller.The method is below:

def accept
unless JoinTeamRequest.accept_invite(params[:team_id], @inviter_userid, @invited_userid, @assignment_id)
flash[:error] = 'The system failed to add you to the team that invited you.'
end
redirect_to view_student_teams_path student_id: params[:student_id]
end

Here @invited_userid is acquired from the student who sent join team request. inviter_userid is from the team members who received the request. team_id is invited student's team id.

===Issue3===

We build a new feature to help students find teams to join. Students who do not have teams or whose team is not full are able to see a list of students who do not haven a team.The code for this issue is showed below:
<% if @student.user_id!=nil %>
<%if @student.assignment.max_team_size > 1 %>
<% if @student.team==nil|| !@student.team.full? %>
Students without teams
<%@student.assignment.participants.each do |participant| %>
<% if participant.team==nil %>
<% if User.find(participant.user_id).role.name=="Student" %>
<% if participant.user.name!=@student.name %>
<td><%= participant.user.name %></td>
<%if @student.team==nil
@teamid="0"
else
@teamid=@student.team.id
end%>
<td><%= form_tag :controller => 'invitations', :action => 'create' do %>
<%= hidden_field_tag 'team_id', @teamid %>
<%= hidden_field_tag 'student_id', @student.id %>
<%= hidden_field_tag 'session[:dummy][:assignment_id]', @student.parent_id %>
<%= hidden_field_tag 'user[name]', participant.user.name%>
<%= submit_tag "invite"%>
<% end %>
</td>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
</table>
<% end %>
<% end %>

In the code above, if @student.assignment.max_team_size > 1 we then execute the below code ,this is because our new feature is only available when assignment team size greater than 1.we use if @student.team==nil|| !@student.team.full? to allow students who have no team or whose team is not full to see this list.Next we found all participants ,we use if statement to sample those who has no team , their role are students and they won't see themselves if they do not have teams.We then put these sampled participants on the list .these achieved by the code below
<%@student.assignment.participants.each do |participant| %>
<% if participant.team==nil %>
<% if User.find(participant.user_id).role.name=="Student" %>
<% if participant.user.name!=@student.name %>
<td><%= participant.user.name %></td>
We also need to add invitation links next to students without teams,before adding links , we need to know whether students who want to invite the students on the list have team or not .if they do not have their own teams. we will automatically create a team for them ,this mechanism is called team lazy initialization.the code for this part is below
<%if @student.team==nil
@teamid="0"
else
@teamid=@student.team.id
end%>
def check_inviterteam_before_invitation
@student = AssignmentParticipant.find(params[:student_id])
@team_id=params[:team_id]
if @team_id=="0"
team = AssignmentTeam.create_team_and_node(@student.parent_id)
user = User.find(@student.user_id)
# create TeamsUser and TeamUserNode
teamuser = ApplicationController.helpers.create_team_users(user, team.id)
@team_id=team.id
end
end
Finally we just use form_tag to create invite button which is related to invitation controller and create method in it .

===Issue4===
we also display a list of students who do not have team in teams#list page for instructor.This part of code is showed below.
<% @assignment.participants.each do |participant| %> 
<% if participant.team==nil %>
<% if User.find(participant.user_id).role.name=="Student" %>
<td><%= participant.user.name %></td>
<% end %>
<% end %>
<% end %>

==Screenshots of new feature==
1)The horny icons appear in the the last column of the signup sheet.
[[File:30.png]]
[[File:31.png]]

2)Received request from advertisement and the changed team members after clicking accept button.
[[File:32.png]]
[[File:33.png]]

3)For student end ,the list of students who do not have a team with invitation links
[[File:35.png]]

4)For instructor end ,the list of students who do not have a team.
[[File:36.png]]

==Test Plan==
The last part of this project is to test all the modifications. And in order to test the new features we created a new Rspec file, which is spec/features/team_invitation_spec.rb. The codes below are all tests we created.

1) The owner of a team can accept or decline the request sent by other students, if the team is already full, the team will remain the same even the request is accepted.
context 'when team owner declining the invitation' do
it 'makes team members remain the same as before' do
find("input[type=submit][value='Decline']").click
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
page.should have_no_content('student2064')
end
end

context 'when team owner accepting the invitation' do
context 'when the team is not full' do
it 'makes requester joins the team' do
allow_any_instance_of(TeamController).to receive(full?).and_return(false)
find("input[type=submit][value='Accept']").click
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
page.should have_content('student2064')
end
end

context 'when the team is already full' do
it 'makes team members remain the same as before' do
allow_any_instance_of(TeamController).to receive(full?).and_return(true)
find("input[type=submit][value='Accept']").click
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
page.should have_no_content('student2064')
end
end

2) In the student_team#view page, students should see a list of students who don't have teams and can be invited at present, if the invitation is declined, the team remains the same. And the invitee can only join the team when it is not full.

context 'on student_teams#view page (student end)' do
before(:each) do
stub_current_user(@owner, @owner.role.name, @owner.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
end
it 'shows a list of students who do not have a team' do
expect(page).to have_content('student2064')
expect(page).to have_selector("input[typ1e=submit][value='invite']")
end

context 'when invitee declining the invitation' do
it 'makes team members remain the same as before' do
find("input[type=submit][value='Decline']").click
end
end

context 'when invitee accepting the invitation' do
context 'when the team is not full' do
it 'makes invitee joins the team' do
allow_any_instance_of(TeamController).to receive(full?).and_return(false)
end
end

context 'when the team is already full' do
it 'makes team members remain the same as before' do
allow_any_instance_of(TeamController).to receive(full?).and_return(true)
end
end

3) Testing advertisement features.
context 'advertisement feature' do
before(:each) do
# team owner creates an advertisement from student_teams#view page
@owner = User.find_by(name: "student2064")
stub_current_user(@owner, @owner.role.name, @owner.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
click_link 'Create'
fill_in 'comments_for_advertisement', with: 'advertisement1'
click_button 'Create'
expect(page).to have_content 'advertisement1'
expect(page).to have_content 'Delete'
@user = User.find_by(name: "student2065")
stub_current_user(@user, @user.role.name, @user.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Signup sheet'
horn = find(:xpath, "//a[contains(@href,'show_team/2?')]")
horn.click
expect(page).to have_content 'Request invitation'
click_link 'Request invitation'
fill_in 'comments_', with: 'request_demo'
expect(page).to have_selector("input[type=submit][value='Create']")
create(:join_team_request)
# team owner is able to accept or decline the invitation
stub_current_user(@owner, @owner.role.name, @owner.role)
visit '/student_task/list'
click_link 'TestAssignment'
click_link 'Your team'
# expect(page).to have_selector("input[typ1e=submit][value='Accept']")
# expect(page).to have_selector("input[type=submit][value='Decline']")
end

CSC/ECE 517 Fall 2017/E1793. Help students find teams to join

2017-12-02T01:55:57Z

Qwang32:

CSC/ECE 517 Fall 2017/E1747 Refactor sign up sheet controller.rb

2017-11-02T14:40:19Z

Qwang32: /* Rspec test plan */

== '''Introduction of Expertiza''' ==
Expertiza is a web application where students can form their teams, submit their projects, peer-review other's work, view their grades and so on. Expertiza is open-source, maintained by students and faculty in NC State university.

== '''Why refactoring?''' ==
Code Refactoring 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.

== '''Test Last Development''' ==
Test last development is a traditional and most common approach of testing where testing is done only after implementation code has been written. But test driven development, test first coding or TDD is a relatively new concept where test cases are written before writing the implementation code.

== '''Introduction of RSpec''' ==
RSpec 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.

== '''Problem Statement''' ==
sign_up_sheet_controller.rb is the controller to handle topic assignment. sign_up_sheet_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These method need to be broken down into simpler and more specific methods that are easier to read or understand. Also, the few instances of code duplication that exist should be removed.

=='''Tasks'''==

* Refactor "list", "save_topic_deadlines" method
** 1.Split into several simpler methods and assign reasonable names
** 2.Extract duplicated code into separate methods
* Use find_by instead of dynamic method
* Complete the pending tests in sign_up_sheet_controller_spec.rb, and write integration tests for newly-created methods. Refactor corresponding methods, and only then finish pending tests.

== '''Refactor''' ==
'''Refactor "list" method'''

The 'list' method is a long method which is fairly complex and hard to read. It is not following "good Ruby and Rails coding practices". In "list" method we found that "signed_up_topics" occurred too many times, here we split it into three different methods, named as "find_signed_up_topics" ,"find_selected_topics" and "list". The changes are showed below.

Previous Code:
def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
if @assignment.is_intelligent
@bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
signed_up_topics = []
@bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
signed_up_topics &= @sign_up_topics
@sign_up_topics -= signed_up_topics
@bids = signed_up_topics
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(@assignment.id, session[:user].id)
@selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(@assignment.id, users_team[0].t_id)
end
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

Modified Code:
#Operating on the student bids, return signed up topics.
def find_signed_up_topics student_bids
signed_up_topics = []
student_bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
# signed_up_topics &= sign_up_topics
return signed_up_topics
end

#Operating on assignments, return selected topics.
def find_selected_topics assignment
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(assignment.id, session[:user].id)
selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(assignment.id, users_team.first.t_id)
end
return selected_topics
end

def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
if @assignment.is_intelligent
@bids = find_signed_up_topics @student_bids
@sign_up_topics = @bids
else
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
@selected_topics = find_selected_topics @assignment
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

'''Refactor "save_topic_deadlines" method'''

We found the "save_topic_deadlines" method is not hard to read, but it's a little bit long and some of the codes can be reused. Here The "save_topic_deadlines" method is modified and two new methods were created, named as "get_instance_variable_assignment" and "get_instance_variable_topic".

Previous Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if instance_variable_get('@topic_' + deadline_type + '_due_date') == instance_variable_get('@assignment_' + deadline_type + '_due_date')
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
round: i,
flag: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].flag,
threshold: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].threshold,
delayed_job_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].delayed_job_id,
deadline_name: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].deadline_name,
description_url: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].description_url,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

Modified Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if get_instance_varibale_topic == get_instance_variable_assignment
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: get_instance_varibale_topic,
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
round: i,
flag: get_instance_variable_assignment[i - 1].flag,
threshold: get_instance_variable_assignment[i - 1].threshold,
delayed_job_id: get_instance_variable_assignment[i - 1].delayed_job_id,
deadline_name: get_instance_variable_assignment[i - 1].deadline_name,
description_url: get_instance_variable_assignment[i - 1].description_url,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: get_instance_varibale_topic,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

#refactor duplicate code present in both create params
def get_instance_variable_assignment
instance_variable_get('@assignment_' + deadline_type + '_due_dates')
end

def get_instance_varibale_topic
instance_variable_get('@topic_' + deadline_type + '_due_date')
end

== '''Rspec test plan''' ==
1.Complete pending test in sign_up_sheet_controller_spec.rb.

2.Write integration test for newly created methods.

3.Write test for refactor methods.

== '''Rspec test cases''' ==
The most important two tests are about "list" method and "save_topic_deadlines" method. Here we wrote 27 examples of test and all of them are passed.
describe SignUpSheetController do
let(:assignment) { build(:assignment, id: 1, instructor_id: 6, due_dates: [due_date], microtask: true, staggered_deadline: true) }
let(:instructor) { build(:instructor, id: 6) }
let(:student) { build(:student, id: 8) }
let(:participant) { build(:participant, id: 1, user_id: 6, assignment: assignment) }
let(:topic) { build(:topic, id: 1) }
let(:signed_up_team) { build(:signed_up_team, team: team, topic: topic) }
let(:signed_up_team2) { build(:signed_up_team, team_id: 2, is_waitlisted: true) }
let(:team) { build(:assignment_team, id: 1, assignment: assignment) }
let(:due_date) { build(:assignment_due_date, deadline_type_id: 1) }
let(:due_date2) { build(:assignment_due_date, deadline_type_id: 2) }
let(:bid) { Bid.new(topic_id: 1, priority: 1) }
let(:team_user) { build(:team_user) }
before(:each) do
allow(Assignment).to receive(:find).with('1').and_return(assignment)
allow(Assignment).to receive(:find).with(1).and_return(assignment)
stub_current_user(instructor, instructor.role.name, instructor.role)
allow(SignUpTopic).to receive(:find).with('1').and_return(topic)
allow(Participant).to receive(:find_by).with(id: '1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with('1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with(1).and_return(participant)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: student.id, parent_id: 1).and_return(participant)
allow(Team).to receive(:find).with('1').and_return(team)
allow(TeamsUser).to receive(:find_by).with(team_id: 1).and_return(team_user)
allow(team_user).to receive(:user).and_return(student)
allow(participant).to receive(:team).and_return(team)
allow(participant.team).to receive(:submitted_files).and_return([])
allow(participant.team).to receive(:hyperlinks).and_return([])
end

describe '#new' do
it 'builds a new sign up topic and renders sign_up_sheet#new page' do
params = {id: 1}
get :new, params
expect(response).to render_template(:new)
end
end

describe '#create' do
context 'when topic cannot be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 0, category: 'test', id: 1}} }
context 'when new topic can be saved successfully' do
it 'sets up a new topic and redirects to assignment#edit page' do
session[:user] = participant
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(true)
post :create, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when new topic cannot be saved successfully' do
it 'sets up a new topic and renders sign_up_sheet#new page' do
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(false)
post :create, params
expect(response).to render_template(:new)
end
end
end

context 'when topic can be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 120, category: 'test', id: 1}} }
it 'updates the existing topic and redirects to sign_up_sheet#add_signup_topics_staggered page' do
new_topic = build(:topic, topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1)
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(new_topic)
post :create, params
expect(response).to redirect_to('/sign_up_sheet/add_signup_topics_staggered?id=' + params[:id].to_s)
end
end
end

describe '#destroy' do
let(:params) { {id: 1, assignment_id: 1} }
context 'when topic can be found' do
it 'redirects to assignment#edit page' do
session[:user] = participant
post :destroy, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic cannot be found' do
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :destroy, params
expect(flash[:error]).to eq("The topic could not be deleted.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#edit' do
let(:params) { {id: 1} }
it 'renders sign_up_sheet#edit page' do
get :edit, params
expect(response).to render_template(:edit)
end
end

describe '#update' do
context 'when topic cannot be found' do
let(:params) { {id: 1, assignment_id: 1} }
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :update, params
expect(flash[:error]).to eq("The topic could not be updated.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic can be found' do
let(:params) do
{
id: 1, assignment_id: 1,
topic: {topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1, micropayment: 0, description: 'test', link: 'test'}
}
end
it 'updates current topic and redirects to assignment#edit page' do
session[:user] = participant
post :update, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#list' do
let(:params) { {id: '1'} }
let(:bids) { [bid] }
let(:session) { {user: student} }
context 'when current assignment is intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#intelligent_topic_selection page' do
allow(SignUpTopic).to receive(:find_slots_filled).with(1).and_return(topic)
allow(SignUpTopic).to receive(:find_slots_waitlisted).with(1).and_return(topic)
allow(SignUpTopic).to receive(:where).with(assignment_id: assignment.id, private_to: nil).and_return([topic])
allow(assignment).to receive(:max_team_size).and_return(1)
allow(participant).to receive(:team).and_return(team)
allow(assignment).to receive(:is_intelligent).and_return(true)
allow(Bid).to receive(:where).with(team_id: team.try(:id)).and_return(bids)
allow(bids).to receive(:order).with(:priority).and_return(bids)
allow(SignUpTopic).to receive(:find_by).with(id: bid.topic_id).and_return(topic)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(7).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(6).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(1).and_return(due_date)
allow(assignment).to receive(:staggered_deadline).and_return(true)
allow(SignedUpTeam).to receive(:find_team_users).with(1, student.id).and_return([])
get :list, params, session
expect(response).to render_template(:intelligent_topic_selection)
end
end

context 'when current assignment is not intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#list page' do
allow(assignment).to receive(:is_intelligent).and_return(false)
get :list, params, session
expect(response).to render_template(:list)
end
end
end

describe '#sign_up' do
let(:params) { {id: '1'} }
let(:session) { {user: student} }
context 'when SignUpSheet.signup_team method return nil' do
it 'shows an error flash message and redirects to sign_up_sheet#list page' do
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :sign_up, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
expect(flash.now[:error]).to eq("You've already signed up for a topic!")
end
end
end

describe '#signup_as_instructor_action' do
let(:params) { {username: '1'} }
context 'when user cannot be found' do
it 'shows an flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(nil)
get :signup_as_instructor_action, params
expect(flash.now[:error]).to eq("That student does not exist!")
end
end

context 'when user can be found' do
context 'when an assignment_participant can be found' do
context 'when creating team related objects successfully' do
it 'shows a flash success message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(signed_up_team)
get :signup_as_instructor_action
expect(flash.now[:success]).to eq("You have successfully signed up the student for the topic!")
end
end

context 'when creating team related objects unsuccessfully' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student has already signed up for a topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

context 'when an assignment_participant cannot be found' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(false)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student is not registered for the assignment!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end
end

describe '#delete_signup' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup, params
expect(flash.now[:error]).to eq("You have already submitted your work, so you are not allowed to drop your topic.")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup, params
expect(flash.now[:error]).to eq("You cannot drop your topic after the drop topic deadline!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: student} }
it 'shows a flash success message and redirects to sign_up_sheet#list page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(SignedUpTeam).to receive_message_chain(:where, :first).and_return(signed_up_team2)
allow(signed_up_team).to receive(:t_id).and_return(1)
delete :delete_signup, params, session
expect(flash.now[:success]).to eq("You have successfully dropped your topic!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end
end

describe '#delete_signup_as_instructor' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("The student has already submitted their work, so you are not allowed to remove them.")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("You cannot drop a student after the drop topic deadline!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: instructor} }
it 'shows a flash success message and redirects to assignment#edit page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(signed_up_team).to receive(:t_id).and_return(1)
allow(SignedUpTeam).to receive_message_chain(:where, :first).with(any_args).with(no_args).and_return(signed_up_team2)
delete :delete_signup_as_instructor, params, session
expect(flash.now[:success]).to eq("You have successfully dropped the student from the topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end
end

describe '#set_priority' do
let(:params) { {participant_id: '1', id: 1, topic: [1], assignment_id: 1} }
let(:team_id) { participant.team.try(:id) }
let(:bids) { [bid] }
it 'sets priority of bidding topic and redirects to sign_up_sheet#list page' do
allow(AssignmentParticipant).to receive(:find_by).with(id: params[:participant_id]).and_return(participant)
allow(SignUpTopic).to receive_message_chain(:find, :assignment).with(params[:topic].first).with(no_args).and_return(assignment)
allow(Bid).to receive(:where).with(any_args).and_return(bids)
allow(bid).to receive(:topic_id).and_return(1)
allow(bids).to receive(:update_all).with(priority: Integer)
expect(bids).to receive(:update_all).with(priority: Integer)
get :set_priority, params
expect(response).to redirect_to action: 'list', assignment_id: params[:assignment_id]
end
end

describe '#save_topic_deadlines' do
let(:params) { {assignment_id: 1, due_date: {}} }
let(:topics) { [topic] }
context 'when topic_due_date cannot be found' do
it 'creates a new topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return nil
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(TopicDueDate).to receive(:create).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end

context 'when topic_due_date can be found' do
it 'updates the existing topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return([due_date])
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(due_date).to receive(:update_attributes).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

describe '#show_team' do
let(:params) { {id: '1', assignment_id: 1} }
it 'renders show_team page' do
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([signed_up_team])
get :show_team, params
expect(response).to render_template(:show_team)
end
end

describe '#switch_original_topic_to_approved_suggested_topic' do
let(:params) { {id: 1, topic_id: 1} }
let(:session) { {user: student} }
it 'redirects to sign_up_sheet#list page' do
allow(TeamsUser).to receive(:team_id).with(any_args).and_return(1)
allow(SignedUpTeam).to receive(:topic_id).with(any_args).and_return(1)
allow(SignUpTopic).to receive(:exists?).with(any_args).and_return(false)
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([])
get :switch_original_topic_to_approved_suggested_topic, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
end
end
end

== '''Reference''' ==
[https://www.dropbox.com/s/nr3fhk3wbxt4qsr/录音_480p.mov?dl=0]video about Refactor

[https://www.dropbox.com/s/l734csf16l4vghs/test_finish_480.MOV?dl=0]video about test

CSC/ECE 517 Fall 2017/E1747 Refactor sign up sheet controller.rb

2017-11-02T14:40:05Z

Qwang32:

== '''Introduction of Expertiza''' ==
Expertiza is a web application where students can form their teams, submit their projects, peer-review other's work, view their grades and so on. Expertiza is open-source, maintained by students and faculty in NC State university.

== '''Why refactoring?''' ==
Code Refactoring 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.

== '''Test Last Development''' ==
Test last development is a traditional and most common approach of testing where testing is done only after implementation code has been written. But test driven development, test first coding or TDD is a relatively new concept where test cases are written before writing the implementation code.

== '''Introduction of RSpec''' ==
RSpec 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.

== '''Problem Statement''' ==
sign_up_sheet_controller.rb is the controller to handle topic assignment. sign_up_sheet_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These method need to be broken down into simpler and more specific methods that are easier to read or understand. Also, the few instances of code duplication that exist should be removed.

=='''Tasks'''==

* Refactor "list", "save_topic_deadlines" method
** 1.Split into several simpler methods and assign reasonable names
** 2.Extract duplicated code into separate methods
* Use find_by instead of dynamic method
* Complete the pending tests in sign_up_sheet_controller_spec.rb, and write integration tests for newly-created methods. Refactor corresponding methods, and only then finish pending tests.

== '''Refactor''' ==
'''Refactor "list" method'''

The 'list' method is a long method which is fairly complex and hard to read. It is not following "good Ruby and Rails coding practices". In "list" method we found that "signed_up_topics" occurred too many times, here we split it into three different methods, named as "find_signed_up_topics" ,"find_selected_topics" and "list". The changes are showed below.

Previous Code:
def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
if @assignment.is_intelligent
@bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
signed_up_topics = []
@bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
signed_up_topics &= @sign_up_topics
@sign_up_topics -= signed_up_topics
@bids = signed_up_topics
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(@assignment.id, session[:user].id)
@selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(@assignment.id, users_team[0].t_id)
end
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

Modified Code:
#Operating on the student bids, return signed up topics.
def find_signed_up_topics student_bids
signed_up_topics = []
student_bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
# signed_up_topics &= sign_up_topics
return signed_up_topics
end

#Operating on assignments, return selected topics.
def find_selected_topics assignment
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(assignment.id, session[:user].id)
selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(assignment.id, users_team.first.t_id)
end
return selected_topics
end

def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
if @assignment.is_intelligent
@bids = find_signed_up_topics @student_bids
@sign_up_topics = @bids
else
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
@selected_topics = find_selected_topics @assignment
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

'''Refactor "save_topic_deadlines" method'''

We found the "save_topic_deadlines" method is not hard to read, but it's a little bit long and some of the codes can be reused. Here The "save_topic_deadlines" method is modified and two new methods were created, named as "get_instance_variable_assignment" and "get_instance_variable_topic".

Previous Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if instance_variable_get('@topic_' + deadline_type + '_due_date') == instance_variable_get('@assignment_' + deadline_type + '_due_date')
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
round: i,
flag: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].flag,
threshold: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].threshold,
delayed_job_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].delayed_job_id,
deadline_name: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].deadline_name,
description_url: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].description_url,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

Modified Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if get_instance_varibale_topic == get_instance_variable_assignment
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: get_instance_varibale_topic,
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
round: i,
flag: get_instance_variable_assignment[i - 1].flag,
threshold: get_instance_variable_assignment[i - 1].threshold,
delayed_job_id: get_instance_variable_assignment[i - 1].delayed_job_id,
deadline_name: get_instance_variable_assignment[i - 1].deadline_name,
description_url: get_instance_variable_assignment[i - 1].description_url,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: get_instance_varibale_topic,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

#refactor duplicate code present in both create params
def get_instance_variable_assignment
instance_variable_get('@assignment_' + deadline_type + '_due_dates')
end

def get_instance_varibale_topic
instance_variable_get('@topic_' + deadline_type + '_due_date')
end

== '''Rspec test plan''' ==
1.Complete pending test in sign_up_sheet_controller_spec.rb.
2.Write integration test for newly created methods.
3.Write test for refactor methods.

== '''Rspec test cases''' ==
The most important two tests are about "list" method and "save_topic_deadlines" method. Here we wrote 27 examples of test and all of them are passed.
describe SignUpSheetController do
let(:assignment) { build(:assignment, id: 1, instructor_id: 6, due_dates: [due_date], microtask: true, staggered_deadline: true) }
let(:instructor) { build(:instructor, id: 6) }
let(:student) { build(:student, id: 8) }
let(:participant) { build(:participant, id: 1, user_id: 6, assignment: assignment) }
let(:topic) { build(:topic, id: 1) }
let(:signed_up_team) { build(:signed_up_team, team: team, topic: topic) }
let(:signed_up_team2) { build(:signed_up_team, team_id: 2, is_waitlisted: true) }
let(:team) { build(:assignment_team, id: 1, assignment: assignment) }
let(:due_date) { build(:assignment_due_date, deadline_type_id: 1) }
let(:due_date2) { build(:assignment_due_date, deadline_type_id: 2) }
let(:bid) { Bid.new(topic_id: 1, priority: 1) }
let(:team_user) { build(:team_user) }
before(:each) do
allow(Assignment).to receive(:find).with('1').and_return(assignment)
allow(Assignment).to receive(:find).with(1).and_return(assignment)
stub_current_user(instructor, instructor.role.name, instructor.role)
allow(SignUpTopic).to receive(:find).with('1').and_return(topic)
allow(Participant).to receive(:find_by).with(id: '1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with('1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with(1).and_return(participant)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: student.id, parent_id: 1).and_return(participant)
allow(Team).to receive(:find).with('1').and_return(team)
allow(TeamsUser).to receive(:find_by).with(team_id: 1).and_return(team_user)
allow(team_user).to receive(:user).and_return(student)
allow(participant).to receive(:team).and_return(team)
allow(participant.team).to receive(:submitted_files).and_return([])
allow(participant.team).to receive(:hyperlinks).and_return([])
end

describe '#new' do
it 'builds a new sign up topic and renders sign_up_sheet#new page' do
params = {id: 1}
get :new, params
expect(response).to render_template(:new)
end
end

describe '#create' do
context 'when topic cannot be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 0, category: 'test', id: 1}} }
context 'when new topic can be saved successfully' do
it 'sets up a new topic and redirects to assignment#edit page' do
session[:user] = participant
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(true)
post :create, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when new topic cannot be saved successfully' do
it 'sets up a new topic and renders sign_up_sheet#new page' do
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(false)
post :create, params
expect(response).to render_template(:new)
end
end
end

context 'when topic can be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 120, category: 'test', id: 1}} }
it 'updates the existing topic and redirects to sign_up_sheet#add_signup_topics_staggered page' do
new_topic = build(:topic, topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1)
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(new_topic)
post :create, params
expect(response).to redirect_to('/sign_up_sheet/add_signup_topics_staggered?id=' + params[:id].to_s)
end
end
end

describe '#destroy' do
let(:params) { {id: 1, assignment_id: 1} }
context 'when topic can be found' do
it 'redirects to assignment#edit page' do
session[:user] = participant
post :destroy, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic cannot be found' do
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :destroy, params
expect(flash[:error]).to eq("The topic could not be deleted.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#edit' do
let(:params) { {id: 1} }
it 'renders sign_up_sheet#edit page' do
get :edit, params
expect(response).to render_template(:edit)
end
end

describe '#update' do
context 'when topic cannot be found' do
let(:params) { {id: 1, assignment_id: 1} }
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :update, params
expect(flash[:error]).to eq("The topic could not be updated.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic can be found' do
let(:params) do
{
id: 1, assignment_id: 1,
topic: {topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1, micropayment: 0, description: 'test', link: 'test'}
}
end
it 'updates current topic and redirects to assignment#edit page' do
session[:user] = participant
post :update, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#list' do
let(:params) { {id: '1'} }
let(:bids) { [bid] }
let(:session) { {user: student} }
context 'when current assignment is intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#intelligent_topic_selection page' do
allow(SignUpTopic).to receive(:find_slots_filled).with(1).and_return(topic)
allow(SignUpTopic).to receive(:find_slots_waitlisted).with(1).and_return(topic)
allow(SignUpTopic).to receive(:where).with(assignment_id: assignment.id, private_to: nil).and_return([topic])
allow(assignment).to receive(:max_team_size).and_return(1)
allow(participant).to receive(:team).and_return(team)
allow(assignment).to receive(:is_intelligent).and_return(true)
allow(Bid).to receive(:where).with(team_id: team.try(:id)).and_return(bids)
allow(bids).to receive(:order).with(:priority).and_return(bids)
allow(SignUpTopic).to receive(:find_by).with(id: bid.topic_id).and_return(topic)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(7).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(6).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(1).and_return(due_date)
allow(assignment).to receive(:staggered_deadline).and_return(true)
allow(SignedUpTeam).to receive(:find_team_users).with(1, student.id).and_return([])
get :list, params, session
expect(response).to render_template(:intelligent_topic_selection)
end
end

context 'when current assignment is not intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#list page' do
allow(assignment).to receive(:is_intelligent).and_return(false)
get :list, params, session
expect(response).to render_template(:list)
end
end
end

describe '#sign_up' do
let(:params) { {id: '1'} }
let(:session) { {user: student} }
context 'when SignUpSheet.signup_team method return nil' do
it 'shows an error flash message and redirects to sign_up_sheet#list page' do
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :sign_up, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
expect(flash.now[:error]).to eq("You've already signed up for a topic!")
end
end
end

describe '#signup_as_instructor_action' do
let(:params) { {username: '1'} }
context 'when user cannot be found' do
it 'shows an flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(nil)
get :signup_as_instructor_action, params
expect(flash.now[:error]).to eq("That student does not exist!")
end
end

context 'when user can be found' do
context 'when an assignment_participant can be found' do
context 'when creating team related objects successfully' do
it 'shows a flash success message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(signed_up_team)
get :signup_as_instructor_action
expect(flash.now[:success]).to eq("You have successfully signed up the student for the topic!")
end
end

context 'when creating team related objects unsuccessfully' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student has already signed up for a topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

context 'when an assignment_participant cannot be found' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(false)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student is not registered for the assignment!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end
end

describe '#delete_signup' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup, params
expect(flash.now[:error]).to eq("You have already submitted your work, so you are not allowed to drop your topic.")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup, params
expect(flash.now[:error]).to eq("You cannot drop your topic after the drop topic deadline!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: student} }
it 'shows a flash success message and redirects to sign_up_sheet#list page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(SignedUpTeam).to receive_message_chain(:where, :first).and_return(signed_up_team2)
allow(signed_up_team).to receive(:t_id).and_return(1)
delete :delete_signup, params, session
expect(flash.now[:success]).to eq("You have successfully dropped your topic!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end
end

describe '#delete_signup_as_instructor' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("The student has already submitted their work, so you are not allowed to remove them.")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("You cannot drop a student after the drop topic deadline!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: instructor} }
it 'shows a flash success message and redirects to assignment#edit page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(signed_up_team).to receive(:t_id).and_return(1)
allow(SignedUpTeam).to receive_message_chain(:where, :first).with(any_args).with(no_args).and_return(signed_up_team2)
delete :delete_signup_as_instructor, params, session
expect(flash.now[:success]).to eq("You have successfully dropped the student from the topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end
end

describe '#set_priority' do
let(:params) { {participant_id: '1', id: 1, topic: [1], assignment_id: 1} }
let(:team_id) { participant.team.try(:id) }
let(:bids) { [bid] }
it 'sets priority of bidding topic and redirects to sign_up_sheet#list page' do
allow(AssignmentParticipant).to receive(:find_by).with(id: params[:participant_id]).and_return(participant)
allow(SignUpTopic).to receive_message_chain(:find, :assignment).with(params[:topic].first).with(no_args).and_return(assignment)
allow(Bid).to receive(:where).with(any_args).and_return(bids)
allow(bid).to receive(:topic_id).and_return(1)
allow(bids).to receive(:update_all).with(priority: Integer)
expect(bids).to receive(:update_all).with(priority: Integer)
get :set_priority, params
expect(response).to redirect_to action: 'list', assignment_id: params[:assignment_id]
end
end

describe '#save_topic_deadlines' do
let(:params) { {assignment_id: 1, due_date: {}} }
let(:topics) { [topic] }
context 'when topic_due_date cannot be found' do
it 'creates a new topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return nil
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(TopicDueDate).to receive(:create).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end

context 'when topic_due_date can be found' do
it 'updates the existing topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return([due_date])
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(due_date).to receive(:update_attributes).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

describe '#show_team' do
let(:params) { {id: '1', assignment_id: 1} }
it 'renders show_team page' do
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([signed_up_team])
get :show_team, params
expect(response).to render_template(:show_team)
end
end

describe '#switch_original_topic_to_approved_suggested_topic' do
let(:params) { {id: 1, topic_id: 1} }
let(:session) { {user: student} }
it 'redirects to sign_up_sheet#list page' do
allow(TeamsUser).to receive(:team_id).with(any_args).and_return(1)
allow(SignedUpTeam).to receive(:topic_id).with(any_args).and_return(1)
allow(SignUpTopic).to receive(:exists?).with(any_args).and_return(false)
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([])
get :switch_original_topic_to_approved_suggested_topic, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
end
end
end

== '''Reference''' ==
[https://www.dropbox.com/s/nr3fhk3wbxt4qsr/录音_480p.mov?dl=0]video about Refactor

[https://www.dropbox.com/s/l734csf16l4vghs/test_finish_480.MOV?dl=0]video about test

CSC/ECE 517 Fall 2017/E1747 Refactor sign up sheet controller.rb

2017-11-02T14:36:48Z

Qwang32:

== '''Introduction of Expertiza''' ==
Expertiza is a web application where students can form their teams, submit their projects, peer-review other's work, view their grades and so on. Expertiza is open-source, maintained by students and faculty in NC State university.

== '''Why refactoring?''' ==
Code Refactoring 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.

== '''Test Last Development''' ==
Test last development is a traditional and most common approach of testing where testing is done only after implementation code has been written. But test driven development, test first coding or TDD is a relatively new concept where test cases are written before writing the implementation code.

== '''Introduction of RSpec''' ==
RSpec 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.

== '''Problem Statement''' ==
sign_up_sheet_controller.rb is the controller to handle topic assignment. sign_up_sheet_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These method need to be broken down into simpler and more specific methods that are easier to read or understand. Also, the few instances of code duplication that exist should be removed.

=='''Tasks'''==

* Refactor "list", "save_topic_deadlines" method
** 1.Split into several simpler methods and assign reasonable names
** 2.Extract duplicated code into separate methods
* Use find_by instead of dynamic method
* Complete the pending tests in sign_up_sheet_controller_spec.rb, and write integration tests for newly-created methods. Refactor corresponding methods, and only then finish pending tests.

== '''Refactor''' ==
'''Refactor "list" method'''

The 'list' method is a long method which is fairly complex and hard to read. It is not following "good Ruby and Rails coding practices". In "list" method we found that "signed_up_topics" occurred too many times, here we split it into three different methods, named as "find_signed_up_topics" ,"find_selected_topics" and "list". The changes are showed below.

Previous Code:
def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
if @assignment.is_intelligent
@bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
signed_up_topics = []
@bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
signed_up_topics &= @sign_up_topics
@sign_up_topics -= signed_up_topics
@bids = signed_up_topics
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(@assignment.id, session[:user].id)
@selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(@assignment.id, users_team[0].t_id)
end
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

Modified Code:
#Operating on the student bids, return signed up topics.
def find_signed_up_topics student_bids
signed_up_topics = []
student_bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
# signed_up_topics &= sign_up_topics
return signed_up_topics
end

#Operating on assignments, return selected topics.
def find_selected_topics assignment
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(assignment.id, session[:user].id)
selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(assignment.id, users_team.first.t_id)
end
return selected_topics
end

def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
if @assignment.is_intelligent
@bids = find_signed_up_topics @student_bids
@sign_up_topics = @bids
else
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
@selected_topics = find_selected_topics @assignment
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

'''Refactor "save_topic_deadlines" method'''

We found the "save_topic_deadlines" method is not hard to read, but it's a little bit long and some of the codes can be reused. Here The "save_topic_deadlines" method is modified and two new methods were created, named as "get_instance_variable_assignment" and "get_instance_variable_topic".

Previous Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if instance_variable_get('@topic_' + deadline_type + '_due_date') == instance_variable_get('@assignment_' + deadline_type + '_due_date')
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
round: i,
flag: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].flag,
threshold: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].threshold,
delayed_job_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].delayed_job_id,
deadline_name: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].deadline_name,
description_url: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].description_url,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

Modified Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if get_instance_varibale_topic == get_instance_variable_assignment
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: get_instance_varibale_topic,
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
round: i,
flag: get_instance_variable_assignment[i - 1].flag,
threshold: get_instance_variable_assignment[i - 1].threshold,
delayed_job_id: get_instance_variable_assignment[i - 1].delayed_job_id,
deadline_name: get_instance_variable_assignment[i - 1].deadline_name,
description_url: get_instance_variable_assignment[i - 1].description_url,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: get_instance_varibale_topic,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

#refactor duplicate code present in both create params
def get_instance_variable_assignment
instance_variable_get('@assignment_' + deadline_type + '_due_dates')
end

def get_instance_varibale_topic
instance_variable_get('@topic_' + deadline_type + '_due_date')
end

== '''Rspec test''' ==
The most important two tests are about "list" method and "save_topic_deadlines" method. Here we wrote 27 examples of test and all of them are passed.
describe SignUpSheetController do
let(:assignment) { build(:assignment, id: 1, instructor_id: 6, due_dates: [due_date], microtask: true, staggered_deadline: true) }
let(:instructor) { build(:instructor, id: 6) }
let(:student) { build(:student, id: 8) }
let(:participant) { build(:participant, id: 1, user_id: 6, assignment: assignment) }
let(:topic) { build(:topic, id: 1) }
let(:signed_up_team) { build(:signed_up_team, team: team, topic: topic) }
let(:signed_up_team2) { build(:signed_up_team, team_id: 2, is_waitlisted: true) }
let(:team) { build(:assignment_team, id: 1, assignment: assignment) }
let(:due_date) { build(:assignment_due_date, deadline_type_id: 1) }
let(:due_date2) { build(:assignment_due_date, deadline_type_id: 2) }
let(:bid) { Bid.new(topic_id: 1, priority: 1) }
let(:team_user) { build(:team_user) }
before(:each) do
allow(Assignment).to receive(:find).with('1').and_return(assignment)
allow(Assignment).to receive(:find).with(1).and_return(assignment)
stub_current_user(instructor, instructor.role.name, instructor.role)
allow(SignUpTopic).to receive(:find).with('1').and_return(topic)
allow(Participant).to receive(:find_by).with(id: '1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with('1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with(1).and_return(participant)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: student.id, parent_id: 1).and_return(participant)
allow(Team).to receive(:find).with('1').and_return(team)
allow(TeamsUser).to receive(:find_by).with(team_id: 1).and_return(team_user)
allow(team_user).to receive(:user).and_return(student)
allow(participant).to receive(:team).and_return(team)
allow(participant.team).to receive(:submitted_files).and_return([])
allow(participant.team).to receive(:hyperlinks).and_return([])
end

describe '#new' do
it 'builds a new sign up topic and renders sign_up_sheet#new page' do
params = {id: 1}
get :new, params
expect(response).to render_template(:new)
end
end

describe '#create' do
context 'when topic cannot be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 0, category: 'test', id: 1}} }
context 'when new topic can be saved successfully' do
it 'sets up a new topic and redirects to assignment#edit page' do
session[:user] = participant
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(true)
post :create, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when new topic cannot be saved successfully' do
it 'sets up a new topic and renders sign_up_sheet#new page' do
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(false)
post :create, params
expect(response).to render_template(:new)
end
end
end

context 'when topic can be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 120, category: 'test', id: 1}} }
it 'updates the existing topic and redirects to sign_up_sheet#add_signup_topics_staggered page' do
new_topic = build(:topic, topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1)
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(new_topic)
post :create, params
expect(response).to redirect_to('/sign_up_sheet/add_signup_topics_staggered?id=' + params[:id].to_s)
end
end
end

describe '#destroy' do
let(:params) { {id: 1, assignment_id: 1} }
context 'when topic can be found' do
it 'redirects to assignment#edit page' do
session[:user] = participant
post :destroy, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic cannot be found' do
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :destroy, params
expect(flash[:error]).to eq("The topic could not be deleted.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#edit' do
let(:params) { {id: 1} }
it 'renders sign_up_sheet#edit page' do
get :edit, params
expect(response).to render_template(:edit)
end
end

describe '#update' do
context 'when topic cannot be found' do
let(:params) { {id: 1, assignment_id: 1} }
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :update, params
expect(flash[:error]).to eq("The topic could not be updated.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic can be found' do
let(:params) do
{
id: 1, assignment_id: 1,
topic: {topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1, micropayment: 0, description: 'test', link: 'test'}
}
end
it 'updates current topic and redirects to assignment#edit page' do
session[:user] = participant
post :update, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#list' do
let(:params) { {id: '1'} }
let(:bids) { [bid] }
let(:session) { {user: student} }
context 'when current assignment is intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#intelligent_topic_selection page' do
allow(SignUpTopic).to receive(:find_slots_filled).with(1).and_return(topic)
allow(SignUpTopic).to receive(:find_slots_waitlisted).with(1).and_return(topic)
allow(SignUpTopic).to receive(:where).with(assignment_id: assignment.id, private_to: nil).and_return([topic])
allow(assignment).to receive(:max_team_size).and_return(1)
allow(participant).to receive(:team).and_return(team)
allow(assignment).to receive(:is_intelligent).and_return(true)
allow(Bid).to receive(:where).with(team_id: team.try(:id)).and_return(bids)
allow(bids).to receive(:order).with(:priority).and_return(bids)
allow(SignUpTopic).to receive(:find_by).with(id: bid.topic_id).and_return(topic)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(7).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(6).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(1).and_return(due_date)
allow(assignment).to receive(:staggered_deadline).and_return(true)
allow(SignedUpTeam).to receive(:find_team_users).with(1, student.id).and_return([])
get :list, params, session
expect(response).to render_template(:intelligent_topic_selection)
end
end

context 'when current assignment is not intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#list page' do
allow(assignment).to receive(:is_intelligent).and_return(false)
get :list, params, session
expect(response).to render_template(:list)
end
end
end

describe '#sign_up' do
let(:params) { {id: '1'} }
let(:session) { {user: student} }
context 'when SignUpSheet.signup_team method return nil' do
it 'shows an error flash message and redirects to sign_up_sheet#list page' do
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :sign_up, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
expect(flash.now[:error]).to eq("You've already signed up for a topic!")
end
end
end

describe '#signup_as_instructor_action' do
let(:params) { {username: '1'} }
context 'when user cannot be found' do
it 'shows an flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(nil)
get :signup_as_instructor_action, params
expect(flash.now[:error]).to eq("That student does not exist!")
end
end

context 'when user can be found' do
context 'when an assignment_participant can be found' do
context 'when creating team related objects successfully' do
it 'shows a flash success message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(signed_up_team)
get :signup_as_instructor_action
expect(flash.now[:success]).to eq("You have successfully signed up the student for the topic!")
end
end

context 'when creating team related objects unsuccessfully' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student has already signed up for a topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

context 'when an assignment_participant cannot be found' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(false)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student is not registered for the assignment!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end
end

describe '#delete_signup' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup, params
expect(flash.now[:error]).to eq("You have already submitted your work, so you are not allowed to drop your topic.")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup, params
expect(flash.now[:error]).to eq("You cannot drop your topic after the drop topic deadline!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: student} }
it 'shows a flash success message and redirects to sign_up_sheet#list page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(SignedUpTeam).to receive_message_chain(:where, :first).and_return(signed_up_team2)
allow(signed_up_team).to receive(:t_id).and_return(1)
delete :delete_signup, params, session
expect(flash.now[:success]).to eq("You have successfully dropped your topic!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end
end

describe '#delete_signup_as_instructor' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("The student has already submitted their work, so you are not allowed to remove them.")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("You cannot drop a student after the drop topic deadline!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: instructor} }
it 'shows a flash success message and redirects to assignment#edit page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(signed_up_team).to receive(:t_id).and_return(1)
allow(SignedUpTeam).to receive_message_chain(:where, :first).with(any_args).with(no_args).and_return(signed_up_team2)
delete :delete_signup_as_instructor, params, session
expect(flash.now[:success]).to eq("You have successfully dropped the student from the topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end
end

describe '#set_priority' do
let(:params) { {participant_id: '1', id: 1, topic: [1], assignment_id: 1} }
let(:team_id) { participant.team.try(:id) }
let(:bids) { [bid] }
it 'sets priority of bidding topic and redirects to sign_up_sheet#list page' do
allow(AssignmentParticipant).to receive(:find_by).with(id: params[:participant_id]).and_return(participant)
allow(SignUpTopic).to receive_message_chain(:find, :assignment).with(params[:topic].first).with(no_args).and_return(assignment)
allow(Bid).to receive(:where).with(any_args).and_return(bids)
allow(bid).to receive(:topic_id).and_return(1)
allow(bids).to receive(:update_all).with(priority: Integer)
expect(bids).to receive(:update_all).with(priority: Integer)
get :set_priority, params
expect(response).to redirect_to action: 'list', assignment_id: params[:assignment_id]
end
end

describe '#save_topic_deadlines' do
let(:params) { {assignment_id: 1, due_date: {}} }
let(:topics) { [topic] }
context 'when topic_due_date cannot be found' do
it 'creates a new topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return nil
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(TopicDueDate).to receive(:create).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end

context 'when topic_due_date can be found' do
it 'updates the existing topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return([due_date])
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(due_date).to receive(:update_attributes).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

describe '#show_team' do
let(:params) { {id: '1', assignment_id: 1} }
it 'renders show_team page' do
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([signed_up_team])
get :show_team, params
expect(response).to render_template(:show_team)
end
end

describe '#switch_original_topic_to_approved_suggested_topic' do
let(:params) { {id: 1, topic_id: 1} }
let(:session) { {user: student} }
it 'redirects to sign_up_sheet#list page' do
allow(TeamsUser).to receive(:team_id).with(any_args).and_return(1)
allow(SignedUpTeam).to receive(:topic_id).with(any_args).and_return(1)
allow(SignUpTopic).to receive(:exists?).with(any_args).and_return(false)
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([])
get :switch_original_topic_to_approved_suggested_topic, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
end
end
end

== '''Reference''' ==
[https://www.dropbox.com/s/nr3fhk3wbxt4qsr/录音_480p.mov?dl=0]video about Refactor

[https://www.dropbox.com/s/l734csf16l4vghs/test_finish_480.MOV?dl=0]video about test

CSC/ECE 517 Fall 2017/E1747 Refactor sign up sheet controller.rb

2017-11-02T14:31:59Z

Qwang32: /* About sign_up_sheet_controller.rb */

== '''Introduction of Expertiza''' ==
Expertiza is a web application where students can form their teams, submit their projects, peer-review other's work, view their grades and so on. Expertiza is open-source, maintained by students and faculty in NC State university.

== '''Why refactoring?''' ==
Code Refactoring 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.

== '''Problem Statement''' ==
sign_up_sheet_controller.rb is the controller to handle topic assignment. sign_up_sheet_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These method need to be broken down into simpler and more specific methods that are easier to read or understand. Also, the few instances of code duplication that exist should be removed.

=='''Tasks'''==

* Refactor "list", "save_topic_deadlines" method
** 1.Split into several simpler methods and assign reasonable names
** 2.Extract duplicated code into separate methods
* Use find_by instead of dynamic method
* Complete the pending tests in sign_up_sheet_controller_spec.rb, and write integration tests for newly-created methods. Refactor corresponding methods, and only then finish pending tests.

== '''Refactor''' ==
'''Refactor "list" method'''

The 'list' method is a long method which is fairly complex and hard to read. It is not following "good Ruby and Rails coding practices". In "list" method we found that "signed_up_topics" occurred too many times, here we split it into three different methods, named as "find_signed_up_topics" ,"find_selected_topics" and "list". The changes are showed below.

Previous Code:
def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
if @assignment.is_intelligent
@bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
signed_up_topics = []
@bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
signed_up_topics &= @sign_up_topics
@sign_up_topics -= signed_up_topics
@bids = signed_up_topics
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(@assignment.id, session[:user].id)
@selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(@assignment.id, users_team[0].t_id)
end
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

Modified Code:
#Operating on the student bids, return signed up topics.
def find_signed_up_topics student_bids
signed_up_topics = []
student_bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
# signed_up_topics &= sign_up_topics
return signed_up_topics
end

#Operating on assignments, return selected topics.
def find_selected_topics assignment
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(assignment.id, session[:user].id)
selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(assignment.id, users_team.first.t_id)
end
return selected_topics
end

def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
if @assignment.is_intelligent
@bids = find_signed_up_topics @student_bids
@sign_up_topics = @bids
else
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
@selected_topics = find_selected_topics @assignment
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

'''Refactor "save_topic_deadlines" method'''

We found the "save_topic_deadlines" method is not hard to read, but it's a little bit long and some of the codes can be reused. Here The "save_topic_deadlines" method is modified and two new methods were created, named as "get_instance_variable_assignment" and "get_instance_variable_topic".

Previous Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if instance_variable_get('@topic_' + deadline_type + '_due_date') == instance_variable_get('@assignment_' + deadline_type + '_due_date')
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
round: i,
flag: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].flag,
threshold: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].threshold,
delayed_job_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].delayed_job_id,
deadline_name: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].deadline_name,
description_url: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].description_url,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

Modified Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if get_instance_varibale_topic == get_instance_variable_assignment
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: get_instance_varibale_topic,
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
round: i,
flag: get_instance_variable_assignment[i - 1].flag,
threshold: get_instance_variable_assignment[i - 1].threshold,
delayed_job_id: get_instance_variable_assignment[i - 1].delayed_job_id,
deadline_name: get_instance_variable_assignment[i - 1].deadline_name,
description_url: get_instance_variable_assignment[i - 1].description_url,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: get_instance_varibale_topic,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

#refactor duplicate code present in both create params
def get_instance_variable_assignment
instance_variable_get('@assignment_' + deadline_type + '_due_dates')
end

def get_instance_varibale_topic
instance_variable_get('@topic_' + deadline_type + '_due_date')
end

== '''Rspec test''' ==
The most important two tests are about "list" method and "save_topic_deadlines" method. Here we wrote 27 examples of test and all of them are passed.
describe SignUpSheetController do
let(:assignment) { build(:assignment, id: 1, instructor_id: 6, due_dates: [due_date], microtask: true, staggered_deadline: true) }
let(:instructor) { build(:instructor, id: 6) }
let(:student) { build(:student, id: 8) }
let(:participant) { build(:participant, id: 1, user_id: 6, assignment: assignment) }
let(:topic) { build(:topic, id: 1) }
let(:signed_up_team) { build(:signed_up_team, team: team, topic: topic) }
let(:signed_up_team2) { build(:signed_up_team, team_id: 2, is_waitlisted: true) }
let(:team) { build(:assignment_team, id: 1, assignment: assignment) }
let(:due_date) { build(:assignment_due_date, deadline_type_id: 1) }
let(:due_date2) { build(:assignment_due_date, deadline_type_id: 2) }
let(:bid) { Bid.new(topic_id: 1, priority: 1) }
let(:team_user) { build(:team_user) }
before(:each) do
allow(Assignment).to receive(:find).with('1').and_return(assignment)
allow(Assignment).to receive(:find).with(1).and_return(assignment)
stub_current_user(instructor, instructor.role.name, instructor.role)
allow(SignUpTopic).to receive(:find).with('1').and_return(topic)
allow(Participant).to receive(:find_by).with(id: '1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with('1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with(1).and_return(participant)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: student.id, parent_id: 1).and_return(participant)
allow(Team).to receive(:find).with('1').and_return(team)
allow(TeamsUser).to receive(:find_by).with(team_id: 1).and_return(team_user)
allow(team_user).to receive(:user).and_return(student)
allow(participant).to receive(:team).and_return(team)
allow(participant.team).to receive(:submitted_files).and_return([])
allow(participant.team).to receive(:hyperlinks).and_return([])
end

describe '#new' do
it 'builds a new sign up topic and renders sign_up_sheet#new page' do
params = {id: 1}
get :new, params
expect(response).to render_template(:new)
end
end

describe '#create' do
context 'when topic cannot be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 0, category: 'test', id: 1}} }
context 'when new topic can be saved successfully' do
it 'sets up a new topic and redirects to assignment#edit page' do
session[:user] = participant
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(true)
post :create, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when new topic cannot be saved successfully' do
it 'sets up a new topic and renders sign_up_sheet#new page' do
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(false)
post :create, params
expect(response).to render_template(:new)
end
end
end

context 'when topic can be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 120, category: 'test', id: 1}} }
it 'updates the existing topic and redirects to sign_up_sheet#add_signup_topics_staggered page' do
new_topic = build(:topic, topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1)
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(new_topic)
post :create, params
expect(response).to redirect_to('/sign_up_sheet/add_signup_topics_staggered?id=' + params[:id].to_s)
end
end
end

describe '#destroy' do
let(:params) { {id: 1, assignment_id: 1} }
context 'when topic can be found' do
it 'redirects to assignment#edit page' do
session[:user] = participant
post :destroy, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic cannot be found' do
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :destroy, params
expect(flash[:error]).to eq("The topic could not be deleted.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#edit' do
let(:params) { {id: 1} }
it 'renders sign_up_sheet#edit page' do
get :edit, params
expect(response).to render_template(:edit)
end
end

describe '#update' do
context 'when topic cannot be found' do
let(:params) { {id: 1, assignment_id: 1} }
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :update, params
expect(flash[:error]).to eq("The topic could not be updated.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic can be found' do
let(:params) do
{
id: 1, assignment_id: 1,
topic: {topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1, micropayment: 0, description: 'test', link: 'test'}
}
end
it 'updates current topic and redirects to assignment#edit page' do
session[:user] = participant
post :update, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#list' do
let(:params) { {id: '1'} }
let(:bids) { [bid] }
let(:session) { {user: student} }
context 'when current assignment is intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#intelligent_topic_selection page' do
allow(SignUpTopic).to receive(:find_slots_filled).with(1).and_return(topic)
allow(SignUpTopic).to receive(:find_slots_waitlisted).with(1).and_return(topic)
allow(SignUpTopic).to receive(:where).with(assignment_id: assignment.id, private_to: nil).and_return([topic])
allow(assignment).to receive(:max_team_size).and_return(1)
allow(participant).to receive(:team).and_return(team)
allow(assignment).to receive(:is_intelligent).and_return(true)
allow(Bid).to receive(:where).with(team_id: team.try(:id)).and_return(bids)
allow(bids).to receive(:order).with(:priority).and_return(bids)
allow(SignUpTopic).to receive(:find_by).with(id: bid.topic_id).and_return(topic)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(7).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(6).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(1).and_return(due_date)
allow(assignment).to receive(:staggered_deadline).and_return(true)
allow(SignedUpTeam).to receive(:find_team_users).with(1, student.id).and_return([])
get :list, params, session
expect(response).to render_template(:intelligent_topic_selection)
end
end

context 'when current assignment is not intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#list page' do
allow(assignment).to receive(:is_intelligent).and_return(false)
get :list, params, session
expect(response).to render_template(:list)
end
end
end

describe '#sign_up' do
let(:params) { {id: '1'} }
let(:session) { {user: student} }
context 'when SignUpSheet.signup_team method return nil' do
it 'shows an error flash message and redirects to sign_up_sheet#list page' do
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :sign_up, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
expect(flash.now[:error]).to eq("You've already signed up for a topic!")
end
end
end

describe '#signup_as_instructor_action' do
let(:params) { {username: '1'} }
context 'when user cannot be found' do
it 'shows an flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(nil)
get :signup_as_instructor_action, params
expect(flash.now[:error]).to eq("That student does not exist!")
end
end

context 'when user can be found' do
context 'when an assignment_participant can be found' do
context 'when creating team related objects successfully' do
it 'shows a flash success message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(signed_up_team)
get :signup_as_instructor_action
expect(flash.now[:success]).to eq("You have successfully signed up the student for the topic!")
end
end

context 'when creating team related objects unsuccessfully' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student has already signed up for a topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

context 'when an assignment_participant cannot be found' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(false)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student is not registered for the assignment!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end
end

describe '#delete_signup' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup, params
expect(flash.now[:error]).to eq("You have already submitted your work, so you are not allowed to drop your topic.")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup, params
expect(flash.now[:error]).to eq("You cannot drop your topic after the drop topic deadline!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: student} }
it 'shows a flash success message and redirects to sign_up_sheet#list page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(SignedUpTeam).to receive_message_chain(:where, :first).and_return(signed_up_team2)
allow(signed_up_team).to receive(:t_id).and_return(1)
delete :delete_signup, params, session
expect(flash.now[:success]).to eq("You have successfully dropped your topic!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end
end

describe '#delete_signup_as_instructor' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("The student has already submitted their work, so you are not allowed to remove them.")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("You cannot drop a student after the drop topic deadline!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: instructor} }
it 'shows a flash success message and redirects to assignment#edit page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(signed_up_team).to receive(:t_id).and_return(1)
allow(SignedUpTeam).to receive_message_chain(:where, :first).with(any_args).with(no_args).and_return(signed_up_team2)
delete :delete_signup_as_instructor, params, session
expect(flash.now[:success]).to eq("You have successfully dropped the student from the topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end
end

describe '#set_priority' do
let(:params) { {participant_id: '1', id: 1, topic: [1], assignment_id: 1} }
let(:team_id) { participant.team.try(:id) }
let(:bids) { [bid] }
it 'sets priority of bidding topic and redirects to sign_up_sheet#list page' do
allow(AssignmentParticipant).to receive(:find_by).with(id: params[:participant_id]).and_return(participant)
allow(SignUpTopic).to receive_message_chain(:find, :assignment).with(params[:topic].first).with(no_args).and_return(assignment)
allow(Bid).to receive(:where).with(any_args).and_return(bids)
allow(bid).to receive(:topic_id).and_return(1)
allow(bids).to receive(:update_all).with(priority: Integer)
expect(bids).to receive(:update_all).with(priority: Integer)
get :set_priority, params
expect(response).to redirect_to action: 'list', assignment_id: params[:assignment_id]
end
end

describe '#save_topic_deadlines' do
let(:params) { {assignment_id: 1, due_date: {}} }
let(:topics) { [topic] }
context 'when topic_due_date cannot be found' do
it 'creates a new topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return nil
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(TopicDueDate).to receive(:create).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end

context 'when topic_due_date can be found' do
it 'updates the existing topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return([due_date])
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(due_date).to receive(:update_attributes).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

describe '#show_team' do
let(:params) { {id: '1', assignment_id: 1} }
it 'renders show_team page' do
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([signed_up_team])
get :show_team, params
expect(response).to render_template(:show_team)
end
end

describe '#switch_original_topic_to_approved_suggested_topic' do
let(:params) { {id: 1, topic_id: 1} }
let(:session) { {user: student} }
it 'redirects to sign_up_sheet#list page' do
allow(TeamsUser).to receive(:team_id).with(any_args).and_return(1)
allow(SignedUpTeam).to receive(:topic_id).with(any_args).and_return(1)
allow(SignUpTopic).to receive(:exists?).with(any_args).and_return(false)
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([])
get :switch_original_topic_to_approved_suggested_topic, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
end
end
end

== '''Reference''' ==
[https://www.dropbox.com/s/nr3fhk3wbxt4qsr/录音_480p.mov?dl=0]video about Refactor

[https://www.dropbox.com/s/l734csf16l4vghs/test_finish_480.MOV?dl=0]video about test

CSC/ECE 517 Fall 2017/E1747 Refactor sign up sheet controller.rb

2017-11-02T14:31:28Z

Qwang32: /* Problem Statement */

== '''Introduction of Expertiza''' ==
Expertiza is a web application where students can form their teams, submit their projects, peer-review other's work, view their grades and so on. Expertiza is open-source, maintained by students and faculty in NC State university.

== '''About sign_up_sheet_controller.rb''' ==
sign_up_sheet_controller.rb is the controller to handle topic assignment.

== '''Problem Statement''' ==
sign_up_sheet_controller.rb is the controller to handle topic assignment. sign_up_sheet_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These method need to be broken down into simpler and more specific methods that are easier to read or understand. Also, the few instances of code duplication that exist should be removed.

=='''Tasks'''==

* Refactor "list", "save_topic_deadlines" method
** 1.Split into several simpler methods and assign reasonable names
** 2.Extract duplicated code into separate methods
* Use find_by instead of dynamic method
* Complete the pending tests in sign_up_sheet_controller_spec.rb, and write integration tests for newly-created methods. Refactor corresponding methods, and only then finish pending tests.

== '''Refactor''' ==
'''Refactor "list" method'''

The 'list' method is a long method which is fairly complex and hard to read. It is not following "good Ruby and Rails coding practices". In "list" method we found that "signed_up_topics" occurred too many times, here we split it into three different methods, named as "find_signed_up_topics" ,"find_selected_topics" and "list". The changes are showed below.

Previous Code:
def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
if @assignment.is_intelligent
@bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
signed_up_topics = []
@bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
signed_up_topics &= @sign_up_topics
@sign_up_topics -= signed_up_topics
@bids = signed_up_topics
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(@assignment.id, session[:user].id)
@selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(@assignment.id, users_team[0].t_id)
end
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

Modified Code:
#Operating on the student bids, return signed up topics.
def find_signed_up_topics student_bids
signed_up_topics = []
student_bids.each do |bid|
sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
signed_up_topics << sign_up_topic if sign_up_topic
end
# signed_up_topics &= sign_up_topics
return signed_up_topics
end

#Operating on assignments, return selected topics.
def find_selected_topics assignment
# Find whether the user has signed up for any topics; if so the user won't be able to
# sign up again unless the former was a waitlisted topic
# if team assignment, then team id needs to be passed as parameter else the user's id
users_team = SignedUpTeam.find_team_users(assignment.id, session[:user].id)
selected_topics = if users_team.empty?
nil
else
# TODO: fix this; cant use 0
SignedUpTeam.find_user_signup_topics(assignment.id, users_team.first.t_id)
end
return selected_topics
end

def list
@participant = AssignmentParticipant.find(params[:id].to_i)
@assignment = @participant.assignment
@slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)
@show_actions = true
@priority = 0
@max_team_size = @assignment.max_team_size
team_id = @participant.team.try(:id)
@student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
if @assignment.is_intelligent
@bids = find_signed_up_topics @student_bids
@sign_up_topics = @bids
else
@sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
end
@num_of_topics = @sign_up_topics.size
@signup_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(7)
@drop_topic_deadline = @assignment.due_dates.find_by_deadline_type_id(6)
unless @assignment.due_dates.find_by_deadline_type_id(1).nil?
if !@assignment.staggered_deadline? and @assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now
@show_actions = false
end
@selected_topics = find_selected_topics @assignment
end
if @assignment.is_intelligent
render 'sign_up_sheet/intelligent_topic_selection' and return
end
end

'''Refactor "save_topic_deadlines" method'''

We found the "save_topic_deadlines" method is not hard to read, but it's a little bit long and some of the codes can be reused. Here The "save_topic_deadlines" method is modified and two new methods were created, named as "get_instance_variable_assignment" and "get_instance_variable_topic".

Previous Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if instance_variable_get('@topic_' + deadline_type + '_due_date') == instance_variable_get('@assignment_' + deadline_type + '_due_date')
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
round: i,
flag: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].flag,
threshold: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].threshold,
delayed_job_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].delayed_job_id,
deadline_name: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].deadline_name,
description_url: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].description_url,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: instance_variable_get('@topic_' + deadline_type + '_due_date'),
submission_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].submission_allowed_id,
review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_allowed_id,
review_of_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].review_of_review_allowed_id,
quiz_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].quiz_allowed_id,
teammate_review_allowed_id: instance_variable_get('@assignment_' + deadline_type + '_due_dates')[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

Modified Code:
def save_topic_deadlines
assignment = Assignment.find(params[:assignment_id])
@assignment_submission_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 1 }
@assignment_review_due_dates = assignment.due_dates.select {|due_date| due_date.deadline_type_id == 2 }
due_dates = params[:due_date]
topics = SignUpTopic.where(assignment_id: params[:assignment_id])
review_rounds = assignment.num_review_rounds
topics.each_with_index do |topic, index|
for i in 1..review_rounds
@topic_submission_due_date = due_dates[topics[index].id.to_s + '_submission_' + i.to_s + '_due_date']
@topic_review_due_date = due_dates[topics[index].id.to_s + '_review_' + i.to_s + '_due_date']
@assignment_submission_due_date = DateTime.parse(@assignment_submission_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
@assignment_review_due_date = DateTime.parse(@assignment_review_due_dates[i - 1].due_at.to_s).strftime("%Y-%m-%d %H:%M")
%w(submission review).each do |deadline_type|
deadline_type_id = DeadlineType.find_by_name(deadline_type).id
next if get_instance_varibale_topic == get_instance_variable_assignment
topic_due_date = TopicDueDate.where(parent_id: topic.id, deadline_type_id: deadline_type_id, round: i).first rescue nil
if topic_due_date.nil? # create a new record
TopicDueDate.create(
due_at: get_instance_varibale_topic,
deadline_type_id: deadline_type_id,
parent_id: topic.id,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
round: i,
flag: get_instance_variable_assignment[i - 1].flag,
threshold: get_instance_variable_assignment[i - 1].threshold,
delayed_job_id: get_instance_variable_assignment[i - 1].delayed_job_id,
deadline_name: get_instance_variable_assignment[i - 1].deadline_name,
description_url: get_instance_variable_assignment[i - 1].description_url,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id,
type: 'TopicDueDate'
)
else # update an existed record
topic_due_date.update_attributes(
due_at: get_instance_varibale_topic,
submission_allowed_id: get_instance_variable_assignment[i - 1].submission_allowed_id,
review_allowed_id: get_instance_variable_assignment[i - 1].review_allowed_id,
review_of_review_allowed_id: get_instance_variable_assignment[i - 1].review_of_review_allowed_id,
quiz_allowed_id: get_instance_variable_assignment[i - 1].quiz_allowed_id,
teammate_review_allowed_id: get_instance_variable_assignment[i - 1].teammate_review_allowed_id
)
end
end
end
end
redirect_to_assignment_edit(params[:assignment_id])
end

#refactor duplicate code present in both create params
def get_instance_variable_assignment
instance_variable_get('@assignment_' + deadline_type + '_due_dates')
end

def get_instance_varibale_topic
instance_variable_get('@topic_' + deadline_type + '_due_date')
end

== '''Rspec test''' ==
The most important two tests are about "list" method and "save_topic_deadlines" method. Here we wrote 27 examples of test and all of them are passed.
describe SignUpSheetController do
let(:assignment) { build(:assignment, id: 1, instructor_id: 6, due_dates: [due_date], microtask: true, staggered_deadline: true) }
let(:instructor) { build(:instructor, id: 6) }
let(:student) { build(:student, id: 8) }
let(:participant) { build(:participant, id: 1, user_id: 6, assignment: assignment) }
let(:topic) { build(:topic, id: 1) }
let(:signed_up_team) { build(:signed_up_team, team: team, topic: topic) }
let(:signed_up_team2) { build(:signed_up_team, team_id: 2, is_waitlisted: true) }
let(:team) { build(:assignment_team, id: 1, assignment: assignment) }
let(:due_date) { build(:assignment_due_date, deadline_type_id: 1) }
let(:due_date2) { build(:assignment_due_date, deadline_type_id: 2) }
let(:bid) { Bid.new(topic_id: 1, priority: 1) }
let(:team_user) { build(:team_user) }
before(:each) do
allow(Assignment).to receive(:find).with('1').and_return(assignment)
allow(Assignment).to receive(:find).with(1).and_return(assignment)
stub_current_user(instructor, instructor.role.name, instructor.role)
allow(SignUpTopic).to receive(:find).with('1').and_return(topic)
allow(Participant).to receive(:find_by).with(id: '1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with('1').and_return(participant)
allow(AssignmentParticipant).to receive(:find).with(1).and_return(participant)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: student.id, parent_id: 1).and_return(participant)
allow(Team).to receive(:find).with('1').and_return(team)
allow(TeamsUser).to receive(:find_by).with(team_id: 1).and_return(team_user)
allow(team_user).to receive(:user).and_return(student)
allow(participant).to receive(:team).and_return(team)
allow(participant.team).to receive(:submitted_files).and_return([])
allow(participant.team).to receive(:hyperlinks).and_return([])
end

describe '#new' do
it 'builds a new sign up topic and renders sign_up_sheet#new page' do
params = {id: 1}
get :new, params
expect(response).to render_template(:new)
end
end

describe '#create' do
context 'when topic cannot be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 0, category: 'test', id: 1}} }
context 'when new topic can be saved successfully' do
it 'sets up a new topic and redirects to assignment#edit page' do
session[:user] = participant
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(true)
post :create, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when new topic cannot be saved successfully' do
it 'sets up a new topic and renders sign_up_sheet#new page' do
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(nil)
allow_any_instance_of(SignUpTopic).to receive(:save).and_return(false)
post :create, params
expect(response).to render_template(:new)
end
end
end

context 'when topic can be found' do
let(:params) { {id: 1, topic: {topic_name: 'new topic', micropayment: 120, category: 'test', id: 1}} }
it 'updates the existing topic and redirects to sign_up_sheet#add_signup_topics_staggered page' do
new_topic = build(:topic, topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1)
allow(SignUpTopic).to receive_message_chain(:where, :first).and_return(new_topic)
post :create, params
expect(response).to redirect_to('/sign_up_sheet/add_signup_topics_staggered?id=' + params[:id].to_s)
end
end
end

describe '#destroy' do
let(:params) { {id: 1, assignment_id: 1} }
context 'when topic can be found' do
it 'redirects to assignment#edit page' do
session[:user] = participant
post :destroy, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic cannot be found' do
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :destroy, params
expect(flash[:error]).to eq("The topic could not be deleted.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#edit' do
let(:params) { {id: 1} }
it 'renders sign_up_sheet#edit page' do
get :edit, params
expect(response).to render_template(:edit)
end
end

describe '#update' do
context 'when topic cannot be found' do
let(:params) { {id: 1, assignment_id: 1} }
it 'shows an error flash message and redirects to assignment#edit page' do
allow(SignUpTopic).to receive(:find).with('1').and_return(nil)
get :update, params
expect(flash[:error]).to eq("The topic could not be updated.")
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end

context 'when topic can be found' do
let(:params) do
{
id: 1, assignment_id: 1,
topic: {topic_name: 'new topic', topic_identifier: '120', category: 'test', id: 1, micropayment: 0, description: 'test', link: 'test'}
}
end
it 'updates current topic and redirects to assignment#edit page' do
session[:user] = participant
post :update, params
expect(response).to redirect_to('/assignments/' + assignment.id.to_s + '/edit#tabs-5')
end
end
end

describe '#list' do
let(:params) { {id: '1'} }
let(:bids) { [bid] }
let(:session) { {user: student} }
context 'when current assignment is intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#intelligent_topic_selection page' do
allow(SignUpTopic).to receive(:find_slots_filled).with(1).and_return(topic)
allow(SignUpTopic).to receive(:find_slots_waitlisted).with(1).and_return(topic)
allow(SignUpTopic).to receive(:where).with(assignment_id: assignment.id, private_to: nil).and_return([topic])
allow(assignment).to receive(:max_team_size).and_return(1)
allow(participant).to receive(:team).and_return(team)
allow(assignment).to receive(:is_intelligent).and_return(true)
allow(Bid).to receive(:where).with(team_id: team.try(:id)).and_return(bids)
allow(bids).to receive(:order).with(:priority).and_return(bids)
allow(SignUpTopic).to receive(:find_by).with(id: bid.topic_id).and_return(topic)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(7).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(6).and_return(nil)
allow(assignment.due_dates).to receive(:find_by_deadline_type_id).with(1).and_return(due_date)
allow(assignment).to receive(:staggered_deadline).and_return(true)
allow(SignedUpTeam).to receive(:find_team_users).with(1, student.id).and_return([])
get :list, params, session
expect(response).to render_template(:intelligent_topic_selection)
end
end

context 'when current assignment is not intelligent assignment and has submission duedate (deadline_type_id 1)' do
it 'renders sign_up_sheet#list page' do
allow(assignment).to receive(:is_intelligent).and_return(false)
get :list, params, session
expect(response).to render_template(:list)
end
end
end

describe '#sign_up' do
let(:params) { {id: '1'} }
let(:session) { {user: student} }
context 'when SignUpSheet.signup_team method return nil' do
it 'shows an error flash message and redirects to sign_up_sheet#list page' do
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :sign_up, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
expect(flash.now[:error]).to eq("You've already signed up for a topic!")
end
end
end

describe '#signup_as_instructor_action' do
let(:params) { {username: '1'} }
context 'when user cannot be found' do
it 'shows an flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(nil)
get :signup_as_instructor_action, params
expect(flash.now[:error]).to eq("That student does not exist!")
end
end

context 'when user can be found' do
context 'when an assignment_participant can be found' do
context 'when creating team related objects successfully' do
it 'shows a flash success message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(signed_up_team)
get :signup_as_instructor_action
expect(flash.now[:success]).to eq("You have successfully signed up the student for the topic!")
end
end

context 'when creating team related objects unsuccessfully' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(true)
allow(SignUpSheet).to receive(:signup_team).with(any_args).and_return(nil)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student has already signed up for a topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

context 'when an assignment_participant cannot be found' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(User).to receive(:find_by).with(any_args).and_return(student)
allow(AssignmentParticipant).to receive(:exists?).with(any_args).and_return(false)
get :signup_as_instructor_action
expect(flash.now[:error]).to eq("The student is not registered for the assignment!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end
end

describe '#delete_signup' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup, params
expect(flash.now[:error]).to eq("You have already submitted your work, so you are not allowed to drop your topic.")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to sign_up_sheet#list page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup, params
expect(flash.now[:error]).to eq("You cannot drop your topic after the drop topic deadline!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: student} }
it 'shows a flash success message and redirects to sign_up_sheet#list page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(SignedUpTeam).to receive_message_chain(:where, :first).and_return(signed_up_team2)
allow(signed_up_team).to receive(:t_id).and_return(1)
delete :delete_signup, params, session
expect(flash.now[:success]).to eq("You have successfully dropped your topic!")
expect(response).to redirect_to(action: 'list', id: params[:id])
end
end
end

describe '#delete_signup_as_instructor' do
let(:params) { {id: 1, topic_id: 1} }
context 'when either submitted files or hyperlinks of current team are not empty' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(participant.team).to receive(:submitted_files).and_return(['file'])
expect = proc do
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("The student has already submitted their work, so you are not allowed to remove them.")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
expect.call
allow(participant.team).to receive(:hyperlinks).and_return(['link'])
expect.call
allow(participant.team).to receive(:submitted_files).and_return([])
expect.call
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is not nil and its due date has already passed' do
it 'shows a flash error message and redirects to assignment#edit page' do
allow(due_date).to receive(:due_at).and_return(Time.current - 1.day)
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return(due_date)
delete :delete_signup_as_instructor, params
expect(flash.now[:error]).to eq("You cannot drop a student after the drop topic deadline!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end

context 'when both submitted files and hyperlinks of current team are empty and drop topic deadline is nil' do
let(:session) { {user: instructor} }
it 'shows a flash success message and redirects to assignment#edit page' do
allow(assignment).to receive_message_chain(:due_dates, :find_by_deadline_type_id).with(no_args).with(6).and_return nil
allow(SignedUpTeam).to receive(:find_team_users).with(participant.assignment.id, session[:user].id).and_return([signed_up_team])
allow(signed_up_team).to receive(:t_id).and_return(1)
allow(SignedUpTeam).to receive_message_chain(:where, :first).with(any_args).with(no_args).and_return(signed_up_team2)
delete :delete_signup_as_instructor, params, session
expect(flash.now[:success]).to eq("You have successfully dropped the student from the topic!")
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
end
end

describe '#set_priority' do
let(:params) { {participant_id: '1', id: 1, topic: [1], assignment_id: 1} }
let(:team_id) { participant.team.try(:id) }
let(:bids) { [bid] }
it 'sets priority of bidding topic and redirects to sign_up_sheet#list page' do
allow(AssignmentParticipant).to receive(:find_by).with(id: params[:participant_id]).and_return(participant)
allow(SignUpTopic).to receive_message_chain(:find, :assignment).with(params[:topic].first).with(no_args).and_return(assignment)
allow(Bid).to receive(:where).with(any_args).and_return(bids)
allow(bid).to receive(:topic_id).and_return(1)
allow(bids).to receive(:update_all).with(priority: Integer)
expect(bids).to receive(:update_all).with(priority: Integer)
get :set_priority, params
expect(response).to redirect_to action: 'list', assignment_id: params[:assignment_id]
end
end

describe '#save_topic_deadlines' do
let(:params) { {assignment_id: 1, due_date: {}} }
let(:topics) { [topic] }
context 'when topic_due_date cannot be found' do
it 'creates a new topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return nil
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(TopicDueDate).to receive(:create).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end

context 'when topic_due_date can be found' do
it 'updates the existing topic_due_date record and redirects to assignment#edit page' do
allow(TopicDueDate).to receive(:where).with(any_args).and_return([due_date])
allow(SignUpTopic).to receive(:where).with(any_args).and_return(topics)
allow(assignment).to receive(:num_review_rounds).and_return(1)
assignment.due_dates = assignment.due_dates.push(due_date2)
allow(DeadlineType).to receive_message_chain(:find_by_name, :id).with(String).with(no_args).and_return(1)
expect(due_date).to receive(:update_attributes).exactly(2).times.with(any_args)
get :save_topic_deadlines, params
expect(response).to redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
end
end

describe '#show_team' do
let(:params) { {id: '1', assignment_id: 1} }
it 'renders show_team page' do
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([signed_up_team])
get :show_team, params
expect(response).to render_template(:show_team)
end
end

describe '#switch_original_topic_to_approved_suggested_topic' do
let(:params) { {id: 1, topic_id: 1} }
let(:session) { {user: student} }
it 'redirects to sign_up_sheet#list page' do
allow(TeamsUser).to receive(:team_id).with(any_args).and_return(1)
allow(SignedUpTeam).to receive(:topic_id).with(any_args).and_return(1)
allow(SignUpTopic).to receive(:exists?).with(any_args).and_return(false)
allow(SignedUpTeam).to receive(:where).with(any_args).and_return([])
get :switch_original_topic_to_approved_suggested_topic, params, session
expect(response).to redirect_to action: 'list', id: params[:id]
end
end
end

== '''Reference''' ==
[https://www.dropbox.com/s/nr3fhk3wbxt4qsr/录音_480p.mov?dl=0]video about Refactor

[https://www.dropbox.com/s/l734csf16l4vghs/test_finish_480.MOV?dl=0]video about test