Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-11-06T01:38:58Z

Nurahman:

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
==Use boolean-like objects in conditionals==
Replace
<pre>
if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookkeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
Instructions to Manual Testing: 
1) Open Expertiza Website http://152.46.20.188:3000/ 
2) Login using Username: user2 and Password: password 
3) Click on Assignments on Top Bar 
4) Choose assignment Team Test 2 
5) Click on Your Team 
6) From this next page, 4 methods can be tested: 
-Make Advertisement
-Remove Advertisement
-Edit Teamname, I would request doing undo if you do try to change it
-Leave Team, I would request doing undo if you do test this feature

[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-11-04T03:24:05Z

Nurahman: /* RSpec Functional Testing */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
==Use boolean-like objects in conditionals==
Replace
<pre>
if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
Instructions to Manual Testing: 
1) Open Expertiza Website http://152.46.20.188:3000/ 
2) Login using Username: user2 and Password: password 
3) Click on Assignments on Top Bar 
4) Choose assignment Team Test 2 
5) Click on Your Team 
6) From this next page, 4 methods can be tested: 
-Make Advertisement
-Remove Advertisement
-Edit Teamname, I would request doing undo if you do try to change it
-Leave Team, I would request doing undo if you do test this feature

[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-11-04T03:23:04Z

Nurahman: /* RSpec Functional Testing */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
==Use boolean-like objects in conditionals==
Replace
<pre>
if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
Instructions to Manual Testing:
1) Open Expertiza Website http://152.46.20.188:3000/
2) Login using Username: user2 and Password: password
3) Click on Assignments on Top Bar
4) Choose assignment Team Test 2
5) Click on Your Team
6) From this next page, 4 methods can be tested:
-Make Advertisement
-Remove Advertisement
-Edit Teamname, I would request doing undo if you do try to change it
-Leave Team, I would request doing undo if you do test this feature

[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014

2014-10-29T20:01:06Z

Nurahman:

*[[CSC/ECE_517_Fall_2014/sample_page]]
*[[CSC/ECE_517_Fall_2014/ch1a 22 as]]
*[[CSC/ECE 517 Fall 2014/ch1a 19 mx]]
*[[CSC/ECE 517 Fall 2014/ch1a 3 zq]]
*[[CSC/ECE 517 Fall 2014/ch1a 4 lf]]
*[[CSC/ECE 517 Fall 2014/ch1a 4 wl]]
*[[CSC/ECE 517 Fall 2014/ch1a a7 ch]]
*[[CSC/ECE 517 Fall 2014/ch1a 25 rs]]
*[[CSC/ECE 517 Fall 2014/ch1a 25 jf]]
*[[CSC/ECE 517 Fall 2014/ch1a 8 os]]
*[[CSC/ECE 517 Fall 2014/ch1a 8 sn]]
*[[CSC/ECE 517 Fall 2014/ch1a 15 gs]]
*[[CSC/ECE 517 Fall 2014/ch1a 10 hu]]
*[[CSC/ECE 517 Fall 2014/ch1a 20 kv]]
*[[CSC/ECE 517 Fall 2014/ch1a 21 as]]
*[[CSC/ECE 517 Fall 2014/ch1a 24 sa]]
*[[CSC/ECE 517 Fall 2014/ch1a 26 sn]]
*[[CSC/ECE 517 Fall 2014/ch1a 6 rl]]
*[[CSC/ECE 517 Fall 2014/ch1a 2 ss]]
*[[CSC/ECE 517 Fall 2014/ch1a 16 av]]
*[[CSC/ECE 517 Fall 2014/ch1a 1 rm]]
*[[CSC/ECE 517 Fall 2014/ch1a 1 sj]]
*[[CSC/ECE 517 Fall 2014/ch1a 23 ss]]
*[[CSC/ECE 517 Fall 2014/ch1a 20 rn]]
*[[CSC/ECE 517 Fall 2014/ch1a 22 sp]]
*[[CSC/ECE 517 Fall 2014/oss M1454 rss]]

*[[CSC/ECE 517 Fall 2014/ch1a 26 gn]]

*[[CSC/ECE 517 Fall 2014/ch1a 13 va]]
*[[CSC/ECE 517 Fall 2014/ch1a 9 aa]]
*[[CSC/ECE 517 Fall 2014/ch1a 9 kn]]
*[[CSC/ECE 517 Fall 2014/ch1a 11 ap]]
*[[CSC/ECE 517 Fall 2014/ch1a 25 ks]]
*[[CSC/ECE 517 Fall 2014/ch1a 7 kz]]
*[[CSC/ECE_517_Fall_2014/ch1a_6_bn]]
*[[CSC/ECE_517_Fall_2014/ch1a 10 zz]]
*[[CSC/ECE_517_Fall_2014/ch1a 16 va]]
*[[CSC/ECE_517_Fall_2014/ch1a F1415 rv]]
*[[CSC/ECE_517_Fall_2014/ch1a_3_cp]]
*[[CSC/ECE 517 Fall 2014/ch1b 26 sa]]
*[[CSC/ECE_517_Fall_2014/ch1b_28_cg]]
*[[CSC/ECE_517_Fall_2014/ch1b 29 ry]]
*[[CSC/ECE_517_Fall_2014/ch1b 30 cs]]
*[[CSC/ECE_517_Fall_2014/ch1b_33_jy]]
*[[CSC/ECE_517_Fall_2014/ch1b_27_js]]
*[[CSC/ECE 517 Fall 2014/oss E1453 syy]]
*[[CSC/ECE 517 Fall 2014/oss E1463 vpd]]
*[[CSC/ECE 517 Fall 2014/oss E1465 oak]]
*[[CSC/ECE 517 Fall 2014/oss_M1456_kdv]]
*[[CSC/ECE 517 Fall 2014/oss_M1453_sst]]
*[[CSC/ECE 517 Fall 2014/oss_E1456_akk]]
*[[CSC/ECE_517_Fall_2014/oss_M1455_asa]]
*[[CSC/ECE_517_Fall_2014/oss_E1458_sst]]
*[[CSC/ECE_517_Fall_2014/oss_E1457_ags]]
*[[CSC/ECE_517_Fall_2014/OSS_E1466_gjf]]
*[[CSC/ECE_517_Fall_2014/oss_M1452_jns]]
*[[CSC/ECE_517_Fall_2014/oss_E1462_nms]]
*[[CSC/ECE_517_Fall_2014/OSS_S1455_ajp]]
*[[CSC/ECE_517_Fall_2014/OSS_S1454_ccc]]
*[[CSC/ECE_517_Fall_2014/OSS_E1450_cxm]]
*[[CSC/ECE_517_Fall_2014/OSS_E1455_skn]]
*[[CSC/ECE_517_Fall_2014/OSS_M1450_vda]]
*[[CSC/ECE_517_Fall_2014/OSS_E1467_rsv]]
*[[CSC/ECE_517_Fall_2014/OSS_E1464_vnn]]
*[[CSC/ECE 517 Fall 2014/OSS_M1451_ahs]]
*[[CSC/ECE 517 Fall 2014/OSS_E1451_las]]
*[[CSC/ECE 517 Fall 2014/OSS_E1461_knn]]

CSC/ECE 517 Fall 2014/OSS E1461 snn

2014-10-29T19:51:33Z

Nurahman: Created page with "'''Expertiza - Refactoring StudentTeamController''' __TOC__ =Introduction= Expertiza is an open source web application created by NCSU give a interface for team learning and pe..."

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
==Use boolean-like objects in conditionals==
Replace
<pre>
if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014

2014-10-29T19:49:54Z

Nurahman:

CSC/ECE 517 Fall 2014

2014-10-29T19:49:30Z

Nurahman:

CSC/ECE 517 Fall 2014

2014-10-29T19:49:08Z

Nurahman:

CSC/ECE 517 Fall 2014

2014-10-29T19:47:26Z

Nurahman:

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T02:39:30Z

Nurahman: /* Use boolean-like objects in conditionals */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
==Use boolean-like objects in conditionals==
Replace
<pre>
if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T02:39:18Z

Nurahman: /* Use good array checking */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
==Use boolean-like objects in conditionals==
Replace
<pre>
if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>

==Use boolean-like objects in conditionals==
Replace
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T02:37:49Z

Nurahman: /* Use good array checking */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?

if !first_waitlisted_user.nil?
...
end
</pre>

With
<pre>
if first_waitlisted_user
...
end
</pre>
==Use boolean-like objects in conditionals==
Replace
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T02:36:31Z

Nurahman: /* Use good array checking */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
if !first_waitlisted_user.nil?
...
end
<pre>

With
</pre>
if first_waitlisted_user
...
end
<pre>
==Use boolean-like objects in conditionals==
Replace
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T01:40:07Z

Nurahman: /* RSpec Functional Testing */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
if !first_waitlisted_user.nil?
...
end
<pre>

With
</pre>
if first_waitlisted_user
...
end
<pre>
==Use boolean-like objects in conditionals==
Replace
<pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T01:39:53Z

Nurahman: /* RSpec Functional Testing */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
if !first_waitlisted_user.nil?
...
end
<pre>

With
</pre>
if first_waitlisted_user
...
end
<pre>
==Use boolean-like objects in conditionals==
Replace
<pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|center|thumbnail|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T01:38:08Z

Nurahman: /* RSpec Functional Testing */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
if !first_waitlisted_user.nil?
...
end
<pre>

With
</pre>
if first_waitlisted_user
...
end
<pre>
==Use boolean-like objects in conditionals==
Replace
<pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:studentteamtest.jpg|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

File:Studentteamtest.jpg

2014-10-29T01:36:54Z

Nurahman: uploaded a new version of "File:Studentteamtest.jpg"

Verification of methods in Student_Teams_Controller

File:Studentteamtest.jpg

2014-10-29T01:36:28Z

Nurahman: uploaded a new version of "File:Studentteamtest.jpg": Verification of methods in Student_Teams_Controller

Verification of methods in Student_Teams_Controller

File:Studentteamtest.jpg

2014-10-29T01:35:43Z

Nurahman: uploaded a new version of "File:Studentteamtest.jpg": Verification of methods in Student_Teams_Controller

Verification of methods in Student_Teams_Controller

File:Studentteamtest.jpg

2014-10-29T01:34:29Z

Nurahman: uploaded a new version of "File:Studentteamtest.jpg": Verification of methods in Student_Teams_Controller

Verification of methods in Student_Teams_Controller

File:Studentteamtest.jpg

2014-10-29T01:33:44Z

Nurahman: Verification of methods in Student_Teams_Controller

Verification of methods in Student_Teams_Controller

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T01:31:23Z

Nurahman: /* RSpec Functional Testing */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==

All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:

===Non-SQL Style Where Method Calls===

Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
===Refactored Where Method Calls===
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
===Use good array checking===
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>
if !first_waitlisted_user.nil?
...
end
<pre>

With
</pre>
if first_waitlisted_user
...
end
<pre>
==Use boolean-like objects in conditionals==
Replace
<pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==RSpec Functional Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
[[File:http://i.imgur.com/c2izUU6.jpg|alt= TEST Verification of Methods.]]

==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces `action_allowed?` to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside `action_allowed?`. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id
@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

As you can see, the view method checks permissions again in this call (outside action_allowed?). In addition, this is not very dry, as the are defined here copies of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

==Bug in Create==

There exists a bug in the create call. Currently, Ruby throws:

<pre>comparison of Fixnum with nil failed</pre>
on
<pre>TeamNode.create parent_id: parent.id, node_object_id: team.id</pre>

This hapens with the unrefactored codebase (Git Rev d2144d13a6fd26203e464a90beaaaceb69506c6f) as well.

==Remove Dynamic Finders==
Dynamic Finders (find_by_xxx_yyyy) are depreciated in Rails 4. They should be replaced by `where(xxx: ,yyy:).first` or by `find_by` (non-dynamic find_by is not depreciated).

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

[https://docs.google.com/document/d/1qQD7fcypFk77nq7Jx7ZNyCNpLyt1oXKaq5G-W7zkV3k/edit Global Style Guide]

[https://github.com/expertiza/expertiza/ Expertiza Github Repository]

[https://github.com/expertiza/expertiza/pull/431 Pull Request for E1461]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T00:21:28Z

Nurahman: /* "Where" Method Calls and Change to More Descriptive Variable Names */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==
==="Where" Method Calls and Change to More Descriptive Variable Names===
All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:
Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

</pre>
Refactored Where Method Calls
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

</pre>
Some more name and styling changes:
<pre>
(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
They were changed to:
<pre>
matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces <pre>action_allowed?</pre> to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside <pre>action_allowed?</pre>. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

Permissions are checked AGAIN in this call (outside action_allowed?) and are defined here independent of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T00:19:39Z

Nurahman: /* "Where" Method Calls and Change to More Descriptive Variable Names */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

=Introduction=
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
==StudentTeamController==
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

=Refactoring of Old Code=
==Using Routing Helpers==
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
==Pluralize StudentTeamController Class Name==
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

==Style Changes Based on Current Conventions==
==="Where" Method Calls and Change to More Descriptive Variable Names===
All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:
Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])

(check.length == 0)
(check[0].name <=> @team.name) == 0)
</pre>
Refactored Where Method Calls
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id

matching_teams.length.zero?
(matching_teams[0].name <=> team.name).zero?
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
==Removed Comment out Code==
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
=Work left to be Done=
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
==Change View to Show==
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
==Do Bookeeping in Models not in the Controllers==
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
==Testing==
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
==Additional Changes to Name==
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
==Unessisary Member Variables==
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
==Clean up Permissions==
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces <pre>action_allowed?</pre> to happen before any other before actions. This causes any prerequisite or dependent conditions in action_allowed? to have to be set inside <pre>action_allowed?</pre>. However, this method is not called in the case of a Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

Permissions are checked AGAIN in this call (outside action_allowed?) and are defined here independent of the actual permissions set in the InitiationsController.

This could be addressed to clean up the beginning of the controller. However, this is a sweeping change across the whole codebase.

=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T00:13:01Z

Nurahman: /* Method Refactoring */

'''Expertiza - Refactoring StudentTeamController'''

__TOC__

==Introduction==
Expertiza is an open source web application created by NCSU give a interface for team learning and peer review. Expertiza allows students to create and communicate with teams as well as an easy platform for online assignment submission. Another feature of Expertiza is its framework for allowing a peer review system on assignments, creating an easy way to give and receive feedback on assignments, allowing for improvement and re-submission based on classmates constructive feedback.

One part of the OSS project for Fall 2014 was refactoring of different sections of Expertiza. Our team was tasked with refactoring of the StudentTeamController.
===StudentTeamController===
StudentTeamController is responsible for the methods creation and joining of teams for group projects on Expertiza. Specifically, this controller allows the user to:
*Create, edit or delete a team (name)
*Add a teamate to the team
*Leave a team if they want to move to a new team
*View received and sent invitations
This controller also provides the student and team instance variables to the corresponding student_team views.

We were tasked with refactoring this controller using DRY principles and more RESTful actions as well as updating our sections using the most current excepted ruby and rails style guidelines. In addition, 6 specific tasks were given:
*Change the list method to index (and others in the RESTful style)
*Use Ruby 1.9 style key: value pairs
*Remove commented-out code
*Use routing helpers (http://guides.rubyonrails.org/routing.html#path-and-url-helpers)
*Pluralize the class (StudentTeamsController)
*Use `.zero?` instead of `== 0`

----

==Setting Up Expertiza on Your Computer to Work on it==
Most people who are new to working on Linux and Github can have issues working, so these links and instructions can help you setting it up on
your computer for final project or other purposes.
Setup was done first on Windows and because of lot of gem conflicts with windows, Linux 14.04 was used instead.
-Update Linux package
-Install Git, Setup username , password
-Suggest Installing Ruby with Rbenv Manager
-Install ruby version 2+ and look for available versions using -l command
-Install rails 4+
-Install and setup Java Environment
-Check the Ruby, Rails and Java Environment Versions Before cloning Expertiza
-Setup for Nokogiri Gem so it doesn't give error
-Install Mysql and setup password and keep it safe as it will be required for importing database
-Fork and Clone Expertiza from https://github.com/expertiza/expertiza.git
-Import Database
-Bundle Install should work and GOOD LUCK!

<pre>
sudo apt-get update
sudo apt-get install git
git config --global user.name "Name Surname"
git config --global user.email aaa@ncsu.edu

git clone git://github.com/sstephenson/rbenv.git .rbenv
echo 'export PATH="$HOME/.rbenv/bin:$PATH"' >> ~/.bashrc
echo 'eval "$(rbenv init -)"' >> ~/.bashrc

git clone git://github.com/sstephenson/ruby-build.git ~/.rbenv/plugins/ruby-build
echo 'export PATH="$HOME/.rbenv/plugins/ruby-build/bin:$PATH"' >> ~/.bashrc
git clone https://github.com/sstephenson/rbenv-gem-rehash.git ~/.rbenv/plugins/rbenv-gem-rehash
git clone https://github.com/ianheggie/rbenv-binstubs.git ~/.rbenv/plugins/rbenv-binstubs
bundle install --binstubs .bundle/bin

rbenv install -l
rbenv install 2.1.2
rbenv global 2.1.2
gem install rails

sudo apt-get install java7-jdk
export JAVA_HOME=/usr/lib/jvm/java-7-openjdk-amd64/
sudo apt-get install mysql-server libmysqlclient-dev

ruby -v
rails -v
java -version

bundle config build.nokogiri --use-system-libraries
gem install mysql
mysql -u root -p Your_Database < Database-You-Importing.sql
gem install rjb
bundle install
</pre>

==Refactoring of Old Code==
===Using Routing Helpers===
Modifications were made to routes pointing to student_teams views by adding helpers as well as changing to new bracket initializers and removed unnecessary parens. Changes were made inside StudentTeamsController.rb as well as anywhere that used routes pointing to StudentTeamsController which consisted of:
*advertise_for_partner_controller.rb
*invitation_controller.rb
*join_team_requests_controller.rb
*reports_controller.rb
*response_controller.rb
*app\views\advertise_for_partner\show.html.erb
*app\views\student_task\view.html.erb
*app\views\student_teams\view.html.erb
The routes changed were as follows:

Before Refactoring:
<pre>
redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
redirect_to :controller =>'student_team', :action => 'edit', :team_id =>params[:team_id], :student_id => params[:student_id]
</pre>
After Refactoring:
<pre>
redirect_to view_student_teams_path student_id: student.id
redirect_to edit_student_teams_path team_id: params[:team_id], student_id: params[:student_id]
</pre>
===Pluralize StudentTeamController Class Name===
StudentTeamController was changed to StudentTeamsController in accordance to current Rails convention. This required the pluralizing of any references to the class such as in routes directing to StudentTeamsController and in the routes.rb file itself
<pre>
#Redirect Routes

redirect_to view_student_team_path student_id: student.id

#Routes.rb

resources :student_team do
</pre>
to
<pre>
#Redirect Routes

redirect_to view_student_teams_path student_id: student.id

#Routes.rb

resources :student_teams do
</pre>

===Style Changes Based on Current Conventions===
===="Where" Method Calls and Change to More Descriptive Variable Names====
All where method calls in the controller were refactored to reflect current style guidelines. These calls were changed from the older style more closely resembling a sql WHERE statement to the more conventional style. In addition, the check variables used in the create and update methods were changed to existing_assignment and matching_teams to better reflect what they represent. Finally, in places where ".where" was being used to find just one instance, it was instead changed to "find_by". The changes made are shown below:
Old Where Method Calls
<pre>
#view

@send_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user.id, @student.assignment.id])
@received_invs = Invitation.where( ['to_id = ? and assignment_id = ? and reply_status = "W"', @student.user.id, @student.assignment.id])

#create

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @student.parent_id])

#update

check = AssignmentTeam.where( ["name =? and parent_id =?", params[:team][:name], @team.parent_id])

#leave

user = TeamsUser.where(["team_id =? and user_id =?", params[:team_id], @student.user_id]).first
other_members = TeamsUser.where( ['team_id = ?', params[:team_id]])
old_team = AssignmentTeam.where( ['id = ?', params[:team_id]])
signups = SignedUpUser.where( {:creator_id => params[:team_id]})
non_waitlisted_users = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => false})
max_choosers = SignUpTopic.where( {:id => signup_topic_id}).first.max_choosers
first_waitlisted_user = SignedUpUser.where( {:topic_id => signup_topic_id, :is_waitlisted => true}).first
waitlisted_team_user = TeamsUser.where( {:team_id => first_waitlisted_user.creator_id}).first
old_invs = Invitation.where( ['from_id = ? and assignment_id = ?', @student.user_id, @student.parent_id])
</pre>
Refactored Where Method Calls
<pre>
#view

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'

#create (change of variable names)

existing_assignments = AssignmentTeam.where name: params[:team][:name], parent_id: student.parent_id

#update (change of variable names)

matching_teams = AssignmentTeam.where name: params[:team][:name], parent_id: team.parent_id

#remove_participant (renamed method - was #leave; .find and .find_by refactoring)

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id
old_team = AssignmentTeam.find params[:team_id]
sign_ups = SignedUpUser.where creator_id: params[:team_id]
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?
waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id
old_invites = Invitation.where from_id: student.user_id, assignment_id: student.parent_id
</pre>

==Method Refactoring==
Different method refactoring techniques were used to make clarity and understanding of code very easy and simple.
For example,
Rename method technique was used for "remove" method which didn't mean anything to more understandable name "remove_advertisement".
"Leave" was changed to "remove_participant" which is more helpful for reader.

<pre>
def remove

def remove_advertisement
</pre>
<pre>
def leave

def remove_participant
</pre>

Pull up method refactoring technique was used to DRY up the code and avoid duplication. For example, statement in code below was used at 3 different places in original code so pulling up that command in one method and calling "team_created_successfully" at all places made it easy to read code.

<pre>
def team_created_successfully
undo_link "Team \"#{team.name}\" has been updated successfully. "
end
</pre>

==Before Actions==
Two lines were repeated in multiple methods in the StudentTeamsController, one to set the @student instance variable and one to set the @team instance variable. The setting of the @student variable is used in the view, update, edit, create, and leave methods while @team is set in the edit and update methods. The setting of these variables was moved to methods which cache the values to set the local variables lazily. These methods had to be used as before_actions in order to be usable by the view and edit views. Finally, in order to standardize the setting of the instance variable, the name of the :id params value was changed to :student_id in all redirect calls.
Old Code
<pre>
Setting of @student and @team instance variable

#view, create

@student = AssignmentParticipant.find(params[:id])

#edit, leave

@student = AssignmentParticipant.find(params[:student_id])

#edit,update

@team = AssignmentTeam.find(params[:team_id])

Changing of Routes

redirect_to :controller => 'student_team', :action => 'view' , :id=> @student.id
</pre>
Refactored Code
<pre>
Before Actions

def team
@team ||= AssignmentTeam.find params[:team_id]
end
def team=(value)
@team = value
end

def student
@student ||= AssignmentParticipant.find(params[:student_id])
end

def student=(value)
@student = value
end

before_action :team, only: [:edit, :update]
before_action :student, only: [:view, :update, :edit, :create, :remove_participant]

Changing of Routes (Changed all instances of redirects to view to reflect student_id)

redirect_to :controller => 'student_team', :action => 'view' , :student_id=> @student.id

</pre>
=Removed Comment out Code=
Commented out code can be restored from the repository. It only serves to clutter the class. Blocks like the one shown below were removed
<pre>
#respond_to do |format|
# format.html # index.html.erb
#format.xml { render :xml => @log_entries }
#end
#redirect_to :controller => 'student_team', :action => 'advertise_for_partners' , :id => params[:team_id]
</pre>
==Work left to be Done==
There is much work left to be done with this controller. Some of the changes have wider scope than just this class.
=Change View to Show=
The correct name for the view method is show. This was noticed at the last minue, and because of the use of "view" throughout
rails simple refactoring tools were not enough to fix this.
=Do Bookeeping in Models not in the Controllers=
The largest function by far in this controller is the remove_participant (formally 'leave') function.
<pre>
def remove_participant
#remove the topic_id from participants
student.update_topic_id nil

#remove the entry from teams_users

team_user = TeamsUser.find_by team_id: params[:team_id], user_id: student.user_id

if team_user
team_user.destroy

undo_link "User \"#{team_user.name}\" has been removed from the team successfully. "
end

#if your old team does not have any members, delete the entry for the team
if TeamsUser.where(team_id: params[:team_id]).empty?
old_team = AssignmentTeam.find params[:team_id]
if old_team
old_team.destroy
#if assignment has signup sheet then the topic selected by the team has to go back to the pool
#or to the first team in the waitlist

sign_ups = SignedUpUser.where creator_id: params[:team_id]
sign_ups.each {|sign_up|
#get the topic_id
sign_up_topic_id = sign_up.topic_id
#destroy the sign_up
sign_up.destroy

#get the number of non-waitlisted users signed up for this topic
non_waitlisted_users = SignedUpUser.where topic_id: sign_up_topic_id, is_waitlisted: false
#get the number of max-choosers for the topic
max_choosers = SignUpTopic.find(sign_up_topic_id).max_choosers

#check if this number is less than the max choosers
if non_waitlisted_users.length < max_choosers
first_waitlisted_user = SignedUpUser.find_by topic_id: sign_up_topic_id, is_waitlisted: true#<order?

#moving the waitlisted user into the confirmed signed up users list
if first_waitlisted_user
first_waitlisted_user.is_waitlisted = false
first_waitlisted_user.save

waitlisted_team_user = TeamsUser.find_by team_id: first_waitlisted_user.creator_id #<this relationship is weird
#waitlisted_team_user could be nil since the team the student left could have been the one waitlisted on the topic
#and teams_users for the team has been deleted in one of the earlier lines of code

if waitlisted_team_user
user_id = waitlisted_team_user.user_id
if user_id
waitlisted_participant = Participant.find_by_user_id user_id
waitlisted_participant.update_topic_id nil

end
end
end
end

}
end
end
</pre>
Everything past the 14th line of code is bookeeping that should be handled by the model itself. In large part,
relationships could solve all of the checks if members exist or are empty and destroy them with dependent_destroy.
In addition, the current TeamsUser model should be removed entirely and replaced with the had_many_and_belongs_to relationship.
=Testing=
The system currently only has limited RSpec tests. The team was only able to implement rudementary tests. Additional tests
are required to ensure good code coverage.
=Additional Changes to Name=
There is no such construct as a "Student Team". In fact, "Student Teams" can currently have members who are not students (instructors),
though the permissions for the instructors actually doing anything in the controller are tied off. A more accurate name of the
class, and more accurate roles for the class, could be defined by "AssignmentTeamsController" which is the actual focus of this class.
=Unessisary Member Variables=
Member variables are exposed outside the scope of the specific action. They should be used in as few situations as possible. A minimal
set of target classes (Such as @team, and possibly @student) should be all that is used.
=Clean up Permissions=
The current permissions scheme forces the permissions to be checked at the beginning of the controller. This makes it so that
the controller has the dule role of interfaceing the views with the model AND managing its permissions.

The inheritance structure currently forces <pre>action_allowed?</pre> to happen before any other before actions. This causes any prerequisite or
dependent conditions in action_allowed? to have to be set inside <pre>action_allowed?</pre>. However, this method is not called in the case of a
Super User login, and therefore these types of members need to be set both in the action_allowed? and before the view is shown.

Finally, the current permissions scheme causes the odd structures we see in the "View" call.

def view
#View will check if send_invs and recieved_invs are set before showing
#only the owner should be able to see those.
return unless current_user_id? student.user_id

@send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
@received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
end

Permissions are checked AGAIN in this call (outside action_allowed?) and are defined here independent of the actual permissions set
in the InitiationsController.
=References=
[http://www.gotealeaf.com/blog/how-to-install-ruby-on-rails-development-environment-for-linux Setting up Ruby on Rails Environment on Linux]

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T00:11:55Z

Nurahman: /* Method Refactoring */

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T00:09:49Z

Nurahman: /* Method Refactoring */

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-29T00:01:40Z

Nurahman: /* Method Refactoring */

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-28T23:55:21Z

Nurahman:

CSC/ECE 517 Fall 2014/OSS E1461 knn

2014-10-28T23:48:03Z

Nurahman:

CSC/ECE 517 Fall 2014/ch1a 26 sn

2014-09-26T02:38:20Z

Nurahman: /* Comparison with other debugging tools */

= '''Debugging in Rails using Pry''' =

Debugging is a process of finding and reducing the bugs or defects in a computer program thereby making it behave as expected. Few of the debugging techniques are [http://en.wikipedia.org/wiki/Debug_code#Print_debugging Print debugging] (also known as printf debugging), Log debugging and Interactive debugging<ref>http://rubylearning.com/satishtalim/ruby_logging.html</ref><ref>http://object.io/site/2011/getting-to-know-ruby-debugger/</ref>.
[http://en.wikipedia.org/wiki/Ruby_on_Rails Rails] provides a range of options to make debugging<ref>http://en.wikipedia.org/wiki/Debugging</ref> easier which include debug helpers, loggers, graphical debuggers, Pry, and so on<ref>http://guides.rubyonrails.org/debugging_rails_applications.html</ref>. In the following sections, debugging in rails using PRY is being explained.

__TOC__

= Introduction to Pry debugger =

:Pry is an [http://en.wikipedia.org/wiki/Shell_(computing) interactive shell] for the Ruby programming language. Pry could be started at any point within a running program. Due to this, programmers can inspect the program, correct the source code, or change its current state without having to restart the process. Pry is thus notable for its ability to start a [http://en.wikipedia.org/wiki/Read%E2%80%93eval%E2%80%93print_loop REPL] within a running program.

:Pry has many features that make it extremely efficient to use. One of the example here could be Pry's ''hist --grep'' that makes it easy to search through your Pry history. Also, Pry uses the filesystem metaphor for exposing most of its introspective capabilities such as the ''ls'' command to list the methods and variables and the ''cd'' command to start interacting with a particular object.

:To start a debugging session, the ''require 'pry' '' and ''binding.pry'' lines need to be included in the program that needs to be debugged. The line ''binding.pry'' could be inserted at any point in your program. Whenever the Ruby interpreter executes this line, it opens a Pry REPL session. The code after the ''binding.pry'' statement is not executed unless instructed by the programmer. Changes could be made to fix the code and then return to running program from same point.

== Installation ==

:To use Pry for debugging a Rails application, you first need to download and install Pry-debugger. If you are using Ruby version 2.0 or higher, then download pry-debugger from [https://github.com/deivid-rodriguez/pry-byebug here]. If you are using Ruby version 1.9 or lower, then download pry-debugger from [https://rubygems.org/gems/pry-debugger here].

:To install the debugger use the following command:<ref>http://pryrepl.org/</ref>
<pre>
gem install pry-debugger.gem
</pre>
:Include the following line of code on the gem file:
<pre>
gem 'pry-rails', :group => :development
</pre>
:Once that is done, the necessary dependencies could be installed using the following command:
<pre>
$bundle install
</pre>

= Execution Commands =
:To use the execution command, invoke pry normally. There is no need to start your script or application differently. For the sake of clarity in explanation, we would refer to the program below:
<pre>
require 'pry'
def method1 (seller)
puts 'Inside method 1'
method2(seller)
end

def method2(seller)
binding.pry # Execution will stop here.
new_products = method3(seller)
puts 'After calling method3'
end

def method3(seller)
puts 'Inside method 3'
puts 'Continuing in method 3'
end

method1('xyz')

</pre>

=== The Step command: ===
:The ''step'' command continues execution by moving into the method on the next line. To put it in another way, the command takes us deeper into the method. Following our example, the ''step'' command would take us into ''method3'' at which point we would be asked what to do next. The following screenshot demonstrates our example:

[[File:Step.jpg]]

=== The Next command: ===
:The ''next'' command runs the current line and moves to the next line in the current context. If the next line in the code is a method call, then unlike the ''step'' method the execution does not go deeper into the method. Following our example, if we execute the above code, the execution stop at the ''binding.pry'' line and the debugger asks us for the further action. On using the ''next'' command, the execution moves to the next line that is '' new_products = method3(seller)'' without going deeper into the method. The debugger now waits for another instruction from the programmer and if the command is ''next'' again, the debugger simply goes to the next line of code after executing the method. Thus, all the code within the ''method3'' method (which would otherwise be many steps), has been concluded with a single use of the ''next'' command. The example could be seen in the following screenshot:

[[File:Next.jpg]]

=== The Finish command: ===
:Now once we have stepped into a method using the ''step'' command, the debugger would ask us what needs to be done next. Now if we want to jump to the end of the method without the debugger asking us our choice of navigation command at the end of each line, we could use the ''finish'' command. The ''finish'' command would execute the code in the method and return back to the point from where the method was called. In our example, while the debugger is in ''method3'', the ''finish'' command would complete the code execution in ''method3'' and return back to ''method2''.

=== The Continue command: ===
:With this command we can abandon the pry session and continue the normal execution of the program. If the interpreter encounters another ''binding.pry'', a pry session is again opened. This can be incredibly annoying, since our program will stop and we’ll have to navigate the debugger every single time, so we’ll probably want to type exit-program to cancel the effect of any following ''binding.pry'' statements, thereby exiting our rails application.

=== Breakpoints: ===
:You could set or change the breakpoints in a program directly using Pry. The ''break'' command is used for this purpose. You could set breakpoints in the current file, or in any other file, or a method. Conditional breakpoints could also be changed or set using optional expressions.

:Command to display the list of breakpoints and conditional breakpoints:
<pre>
break --condition 4 x > 2 Change condition on breakpoint #4 to 'x > 2'.
break --condition 6 Remove the condition on breakpoint #6.
break List all breakpoints. (Same as `breakpoints`)
break --show 2 Show details about breakpoint #2.
</pre>
:Setting breakpoints:
<pre>
break 5 Break at line 5 in the current file.
break <class_name>#run Break at the start of `<class_name>#run`.
break app/models/test.rb:4 Break at line 4 in test.rb.
</pre>
:Deleting breakpoints:
<pre>
break --delete 5 Delete breakpoint #5.
break --disable-all Disable all breakpoints.
</pre>

=== The Edit command: ===
:You could edit the code in a file by just switching to the editor rather than having to end the Pry session. This could be done using the ''edit'' command<ref>http://kgrz.io/pry/2014/02/07/Prys-edit-command.html</ref>. On using the ''edit'' command, a new editor window pops up thereby enabling the programmer to edit the code. However, you need to specify the type of editor in the ''.pryrc'' file. For example, in order to edit the code using a Notepad editor, you need to specify the following in the ''.pryrc'' file:
<pre>
Pry.config.editor = 'Notepad'
</pre>
[[File:SnOutput1.jpg]]

=== View the documentation or source for a method: ===
:You sometimes would want to know how a particular method works or what kind of arguments it takes. The ''show-doc'' method could help you determine that. Also, you would want to know the source of the method. This could be done using the ''show-method'' command.
<pre>
pry(main)> def foo
pry(main)* puts "Welcome to the foo!!"
pry(main)* end
=> nil
pry(main)> show-method foo

From: (pry) @ line 3:
Number of lines: 3
Owner: Object
Visibility: private

def foo
puts "Welcome to the foo!!"
end
</pre>

=== List of other commonly used commands: ===
:The other list of commands could be found by typing ''help''<ref>https://github.com/pry/pry/wiki/Command-system#Help_command</ref>. The following shows other list of commands:

[[File:Help1.jpg]]

:Interested people can learn more about using Pry as a debugger by watching the video [http://www.youtube.com/watch?v=o90CCPjcIKE tutorial].

= Remote Debugging<ref>http://wiki.freepascal.org/Remote_Debugging</ref> =
:So far, Pry has been a good tool to digging into the code and checking how the code is actually working and what is exactly going on inside the code. We used ''binding.pry'' inside the program where the execution stops and the user is asked to give further instructions. However, there are situations where the execution of the program would not stop and the would not allow you to inspect the code especially in situations when [https://github.com/yegrb/yeg-wiki/wiki/Managing-application-processes-with-the-Foreman-gem Foreman] is used to manage processes. In such situation, ''pry-remote'' could be of help.

:When a process that is being run by Foreman comes across ''binding.pry'', it would halt but a Pry session would not be opened. You would be forced to kill the process as the application actually freezes. Thus, in such cases ''pry-remote'' would be a great solution and in order to work around with this, you need to first install the gem:
<pre>
gem install pry-remote
</pre>
:Then run bundler:
<pre>
bundle install
</pre>
:Once you have installed ''pry-remote'', a breakpoint can be added using ''binding.remote_pry'' anywhere in the program from where the code needs to be debugged.
<pre>
def index
binding.remote_pry
...
end
end

</pre>
:''Pry-remote'' would block your program when it hits the ''binding.remote_pry'' and open a Drb endpoint that a client can connect to<ref>http://www.ruby-doc.org/stdlib-1.9.3/libdoc/drb/rdoc/DRb.html</ref>. Running pry-remote in a shell will then connect to the session. You would then be able to interact with Pry as you would normally do. You could unblock the program by using ''exit''.

= Comparison with other debugging tools =
Pry is a replacement for Ruby's default REPL,irb. It has many more additional features as compared to irb. Some of the features that provide Pry an edge over the other debugging tools are as follows:
:*Pry enables code or syntax highlighting, auto identation and a runtime debugging console.
:*It has the ability to load any portion of the file in your text editor directly into the REPL session.

Other Debuggers available in the market are:
:*[http://www.jetbrains.com/ruby/quickstart/ JetBrains RubyMine] Commercial but very easy to use windows based debugger. It supports Ruby versions from 1.8.6 to 2.1.
:*[http://komodoide.com/features/ Komodo] Commercial IDE used for Ruby and other languages like Perl and java. It has graphical debugger and supports multi-process debugging.
:*[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-chrome-devtools.html Chrome Dev-tools] The elements tab is used to debug and get rapid feedback about experimental design changes in HTML or CSS.
:*[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-memcached.html Memcached] In production environment this tool can be used in addition with Heroku to access detailed stats about Memcached usage to find if you have high hit rate or low eviction rate.
:*[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-operating-system-tools.html Operating System Tools] Mostly a mixture of commands used to debug only specific errors for example, Curl to debug URL's or G Debugger to debug infinite loops.

[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-pry-debugger.html This website has a lot of comparison between Pry Debugger with other debuggers available in market.]

= Reference =
<references />

CSC/ECE 517 Fall 2014/ch1a 26 sn

2014-09-26T02:37:14Z

Nurahman: /* Comparison with other debugging tools */

= '''Debugging in Rails using Pry''' =

Debugging is a process of finding and reducing the bugs or defects in a computer program thereby making it behave as expected. Few of the debugging techniques are [http://en.wikipedia.org/wiki/Debug_code#Print_debugging Print debugging] (also known as printf debugging), Log debugging and Interactive debugging<ref>http://rubylearning.com/satishtalim/ruby_logging.html</ref><ref>http://object.io/site/2011/getting-to-know-ruby-debugger/</ref>.
[http://en.wikipedia.org/wiki/Ruby_on_Rails Rails] provides a range of options to make debugging<ref>http://en.wikipedia.org/wiki/Debugging</ref> easier which include debug helpers, loggers, graphical debuggers, Pry, and so on<ref>http://guides.rubyonrails.org/debugging_rails_applications.html</ref>. In the following sections, debugging in rails using PRY is being explained.

__TOC__

= Introduction to Pry debugger =

:Pry is an [http://en.wikipedia.org/wiki/Shell_(computing) interactive shell] for the Ruby programming language. Pry could be started at any point within a running program. Due to this, programmers can inspect the program, correct the source code, or change its current state without having to restart the process. Pry is thus notable for its ability to start a [http://en.wikipedia.org/wiki/Read%E2%80%93eval%E2%80%93print_loop REPL] within a running program.

:Pry has many features that make it extremely efficient to use. One of the example here could be Pry's ''hist --grep'' that makes it easy to search through your Pry history. Also, Pry uses the filesystem metaphor for exposing most of its introspective capabilities such as the ''ls'' command to list the methods and variables and the ''cd'' command to start interacting with a particular object.

:To start a debugging session, the ''require 'pry' '' and ''binding.pry'' lines need to be included in the program that needs to be debugged. The line ''binding.pry'' could be inserted at any point in your program. Whenever the Ruby interpreter executes this line, it opens a Pry REPL session. The code after the ''binding.pry'' statement is not executed unless instructed by the programmer. Changes could be made to fix the code and then return to running program from same point.

== Installation ==

:To use Pry for debugging a Rails application, you first need to download and install Pry-debugger. If you are using Ruby version 2.0 or higher, then download pry-debugger from [https://github.com/deivid-rodriguez/pry-byebug here]. If you are using Ruby version 1.9 or lower, then download pry-debugger from [https://rubygems.org/gems/pry-debugger here].

:To install the debugger use the following command:<ref>http://pryrepl.org/</ref>
<pre>
gem install pry-debugger.gem
</pre>
:Include the following line of code on the gem file:
<pre>
gem 'pry-rails', :group => :development
</pre>
:Once that is done, the necessary dependencies could be installed using the following command:
<pre>
$bundle install
</pre>

= Execution Commands =
:To use the execution command, invoke pry normally. There is no need to start your script or application differently. For the sake of clarity in explanation, we would refer to the program below:
<pre>
require 'pry'
def method1 (seller)
puts 'Inside method 1'
method2(seller)
end

def method2(seller)
binding.pry # Execution will stop here.
new_products = method3(seller)
puts 'After calling method3'
end

def method3(seller)
puts 'Inside method 3'
puts 'Continuing in method 3'
end

method1('xyz')

</pre>

=== The Step command: ===
:The ''step'' command continues execution by moving into the method on the next line. To put it in another way, the command takes us deeper into the method. Following our example, the ''step'' command would take us into ''method3'' at which point we would be asked what to do next. The following screenshot demonstrates our example:

[[File:Step.jpg]]

=== The Next command: ===
:The ''next'' command runs the current line and moves to the next line in the current context. If the next line in the code is a method call, then unlike the ''step'' method the execution does not go deeper into the method. Following our example, if we execute the above code, the execution stop at the ''binding.pry'' line and the debugger asks us for the further action. On using the ''next'' command, the execution moves to the next line that is '' new_products = method3(seller)'' without going deeper into the method. The debugger now waits for another instruction from the programmer and if the command is ''next'' again, the debugger simply goes to the next line of code after executing the method. Thus, all the code within the ''method3'' method (which would otherwise be many steps), has been concluded with a single use of the ''next'' command. The example could be seen in the following screenshot:

[[File:Next.jpg]]

=== The Finish command: ===
:Now once we have stepped into a method using the ''step'' command, the debugger would ask us what needs to be done next. Now if we want to jump to the end of the method without the debugger asking us our choice of navigation command at the end of each line, we could use the ''finish'' command. The ''finish'' command would execute the code in the method and return back to the point from where the method was called. In our example, while the debugger is in ''method3'', the ''finish'' command would complete the code execution in ''method3'' and return back to ''method2''.

=== The Continue command: ===
:With this command we can abandon the pry session and continue the normal execution of the program. If the interpreter encounters another ''binding.pry'', a pry session is again opened. This can be incredibly annoying, since our program will stop and we’ll have to navigate the debugger every single time, so we’ll probably want to type exit-program to cancel the effect of any following ''binding.pry'' statements, thereby exiting our rails application.

=== Breakpoints: ===
:You could set or change the breakpoints in a program directly using Pry. The ''break'' command is used for this purpose. You could set breakpoints in the current file, or in any other file, or a method. Conditional breakpoints could also be changed or set using optional expressions.

:Command to display the list of breakpoints and conditional breakpoints:
<pre>
break --condition 4 x > 2 Change condition on breakpoint #4 to 'x > 2'.
break --condition 6 Remove the condition on breakpoint #6.
break List all breakpoints. (Same as `breakpoints`)
break --show 2 Show details about breakpoint #2.
</pre>
:Setting breakpoints:
<pre>
break 5 Break at line 5 in the current file.
break <class_name>#run Break at the start of `<class_name>#run`.
break app/models/test.rb:4 Break at line 4 in test.rb.
</pre>
:Deleting breakpoints:
<pre>
break --delete 5 Delete breakpoint #5.
break --disable-all Disable all breakpoints.
</pre>

=== The Edit command: ===
:You could edit the code in a file by just switching to the editor rather than having to end the Pry session. This could be done using the ''edit'' command<ref>http://kgrz.io/pry/2014/02/07/Prys-edit-command.html</ref>. On using the ''edit'' command, a new editor window pops up thereby enabling the programmer to edit the code. However, you need to specify the type of editor in the ''.pryrc'' file. For example, in order to edit the code using a Notepad editor, you need to specify the following in the ''.pryrc'' file:
<pre>
Pry.config.editor = 'Notepad'
</pre>
[[File:SnOutput1.jpg]]

=== View the documentation or source for a method: ===
:You sometimes would want to know how a particular method works or what kind of arguments it takes. The ''show-doc'' method could help you determine that. Also, you would want to know the source of the method. This could be done using the ''show-method'' command.
<pre>
pry(main)> def foo
pry(main)* puts "Welcome to the foo!!"
pry(main)* end
=> nil
pry(main)> show-method foo

From: (pry) @ line 3:
Number of lines: 3
Owner: Object
Visibility: private

def foo
puts "Welcome to the foo!!"
end
</pre>

=== List of other commonly used commands: ===
:The other list of commands could be found by typing ''help''<ref>https://github.com/pry/pry/wiki/Command-system#Help_command</ref>. The following shows other list of commands:

[[File:Help1.jpg]]

:Interested people can learn more about using Pry as a debugger by watching the video [http://www.youtube.com/watch?v=o90CCPjcIKE tutorial].

= Remote Debugging<ref>http://wiki.freepascal.org/Remote_Debugging</ref> =
:So far, Pry has been a good tool to digging into the code and checking how the code is actually working and what is exactly going on inside the code. We used ''binding.pry'' inside the program where the execution stops and the user is asked to give further instructions. However, there are situations where the execution of the program would not stop and the would not allow you to inspect the code especially in situations when [https://github.com/yegrb/yeg-wiki/wiki/Managing-application-processes-with-the-Foreman-gem Foreman] is used to manage processes. In such situation, ''pry-remote'' could be of help.

:When a process that is being run by Foreman comes across ''binding.pry'', it would halt but a Pry session would not be opened. You would be forced to kill the process as the application actually freezes. Thus, in such cases ''pry-remote'' would be a great solution and in order to work around with this, you need to first install the gem:
<pre>
gem install pry-remote
</pre>
:Then run bundler:
<pre>
bundle install
</pre>
:Once you have installed ''pry-remote'', a breakpoint can be added using ''binding.remote_pry'' anywhere in the program from where the code needs to be debugged.
<pre>
def index
binding.remote_pry
...
end
end

</pre>
:''Pry-remote'' would block your program when it hits the ''binding.remote_pry'' and open a Drb endpoint that a client can connect to<ref>http://www.ruby-doc.org/stdlib-1.9.3/libdoc/drb/rdoc/DRb.html</ref>. Running pry-remote in a shell will then connect to the session. You would then be able to interact with Pry as you would normally do. You could unblock the program by using ''exit''.

= Comparison with other debugging tools =
Pry is a replacement for Ruby's default REPL,irb. It has many more additional features as compared to irb. Some of the features that provide Pry an edge over the other debugging tools are as follows:
:*Pry enables code or syntax highlighting, auto identation and a runtime debugging console.
:*It has the ability to load any portion of the file in your text editor directly into the REPL session.

Other Debuggers available in the market are:
:*[http://www.jetbrains.com/ruby/quickstart/ JetBrains RubyMine] Commercial but very easy to use windows based debugger. It supports Ruby versions from 1.8.6 to 2.1.
:*[http://komodoide.com/features/ Komodo] Commercial IDE used for Ruby and other languages like Perl and java. It has graphical debugger and supports multi-process debugging.
:*[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-chrome-devtools.html Chrome Dev-tools] The elements tab is used to debug and get rapid feedback about experimental design changes in HTML or CSS.
:*[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-memcached.html Memcached] In production environment this tool can be used in addition with Heroku to access detailed stats about Memcached usage to find if you have hit rate or low eviction rate.
:*[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-operating-system-tools.html Operating System Tools] Mostly a mixture of commands used to debug only specific errors for example, Curl to debug URL's or G Debugger to debug infinite loops.

[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-pry-debugger.html This website has a lot of comparison between Pry Debugger with other debuggers available in market.]

= Reference =
<references />

CSC/ECE 517 Fall 2014/ch1a 26 sn

2014-09-24T03:01:54Z

Nurahman: /* List of other commonly used commands: */

= '''Debugging in Rails using Pry''' =

Debugging is a process of finding and reducing the bugs or defects in a computer program thereby making it behave as expected. Few of the debugging techniques are [http://en.wikipedia.org/wiki/Debug_code#Print_debugging Print debugging] (also known as printf debugging), Log debugging and Interactive debugging<ref>http://rubylearning.com/satishtalim/ruby_logging.html</ref><ref>http://object.io/site/2011/getting-to-know-ruby-debugger/</ref>.
[http://en.wikipedia.org/wiki/Ruby_on_Rails Rails] provides a range of options to make debugging<ref>http://en.wikipedia.org/wiki/Debugging</ref> easier which include debug helpers, loggers, graphical debuggers, Pry, and so on<ref>http://guides.rubyonrails.org/debugging_rails_applications.html</ref>. In the following sections, debugging in rails using PRY is being explained.

__TOC__

= Introduction to Pry debugger =

:Pry is an [http://en.wikipedia.org/wiki/Shell_(computing) interactive shell] for the Ruby programming language. Pry could be started at any point within a running program. Due to this, programmers can inspect the program, correct the source code, or change its current state without having to restart the process. Pry is thus notable for its ability to start a [http://en.wikipedia.org/wiki/Read%E2%80%93eval%E2%80%93print_loop REPL] within a running program.

:Pry has many features that make it extremely efficient to use. One of the example here could be Pry's ''hist --grep'' that makes it easy to search through your Pry history. Also, Pry uses the filesystem metaphor for exposing most of its introspective capabilities such as the ''ls'' command to list the methods and variables and the ''cd'' command to start interacting with a particular object.

:To start a debugging session, you need to include the ''require 'pry' '' and ''binding.pry'' lines in your program that needs to be debugged. You could insert the line ''binding.pry'' at any point in your program. Whenever the Ruby interpreter executes this line, it opens a Pry REPL session. The code after the ''binding.pry'' statement is not executed unless instructed by the programmer. We could make changes to fix the code and then return to running program from same point.

== Installation ==

:To use Pry for debugging a Rails application, you first need to download and install Pry-debugger. If you are using Ruby version 2.0 or higher, then download pry-debugger from [https://github.com/deivid-rodriguez/pry-byebug here]. If you are using Ruby version 1.9 or lower, then download pry-debugger from [https://rubygems.org/gems/pry-debugger here].

:To install the debugger use the following command:<ref>http://pryrepl.org/</ref>
<pre>
gem install pry-debugger.gem
</pre>
:Include the following line of code on the gem file:
<pre>
gem 'pry-rails', :group => :development
</pre>
:Once that is done, the necessary dependencies could be installed using the following command:
<pre>
$bundle install
</pre>

= Execution Commands =
:To use the execution command, invoke pry normally. There is no need to start your script or application differently. For the sake of clarity in explanation, we would refer to the program below:
<pre>
require 'pry'
def method1 (seller)
puts 'Inside method 1'
method2(seller)
end

def method2(seller)
binding.pry # Execution will stop here.
new_products = method3(seller)
puts 'After calling method3'
end

def method3(seller)
puts 'Inside method 3'
puts 'Continuing in method 3'
end

method1('xyz')

</pre>

=== The Step command: ===
:The ''step'' command continues execution by moving into the method on the next line. To put it in another way, the command takes us deeper into the method. Following our example, the ''step'' command would take us into ''method3'' at which point we would be asked what to do next. The following screenshot demonstrates our example:

[[File:Step.jpg]]

=== The Next command: ===
:The ''next'' command runs the current line and moves to the next line in the current context. If the next line in the code is a method call, then unlike the ''step'' method the execution does not go deeper into the method. Following our example, if we execute the above code, the execution stop at the ''binding.pry'' line and the debugger asks us for the further action. On using the ''next'' command, the execution moves to the next line that is '' new_products = method3(seller)'' without going deeper into the method. The debugger now waits for another instruction from the programmer and if the command is ''next'' again, the debugger simply goes to the next line of code after executing the method. Thus, all the code within the ''method3'' method (which would otherwise be many steps), has been concluded with a single use of the ''next'' command. The example could be seen in the following screenshot:

[[File:Next.jpg]]

=== The Finish command: ===
:Now once we have stepped into a method using the ''step'' command, the debugger would ask us what needs to be done next. Now if we want to jump to the end of the method without the debugger asking us our choice of navigation command at the end of each line, we could use the ''finish'' command. The ''finish'' command would execute the code in the method and return back to the point from where the method was called. In our example, while the debugger is in ''method3'', the ''finish'' command would complete the code execution in ''method3'' and return back to ''method2''.

=== The Continue command: ===
:With this command we can abandon the pry session and continue the normal execution of the program. If the interpreter encounters another ''binding.pry'', a pry session is again opened. This can be incredibly annoying, since our program will stop and we’ll have to navigate the debugger every single time, so we’ll probably want to type exit-program to cancel the effect of any following ''binding.pry'' statements, thereby exiting our rails application.

=== Breakpoints: ===
:You could set or change the breakpoints in a program directly using Pry. The ''break'' command is used for this purpose. You could set breakpoints in the current file, or in any other file, or a method. Conditional breakpoints could also be changed or set using optional expressions.

:Command to display the list of breakpoints and conditional breakpoints:
<pre>
break --condition 4 x > 2 Change condition on breakpoint #4 to 'x > 2'.
break --condition 6 Remove the condition on breakpoint #6.
break List all breakpoints. (Same as `breakpoints`)
break --show 2 Show details about breakpoint #2.
</pre>
:Setting breakpoints:
<pre>
break 5 Break at line 5 in the current file.
break <class_name>#run Break at the start of `<class_name>#run`.
break app/models/test.rb:4 Break at line 4 in test.rb.
</pre>
:Deleting breakpoints:
<pre>
break --delete 5 Delete breakpoint #5.
break --disable-all Disable all breakpoints.
</pre>

=== The Edit command: ===
:You could edit the code in a file by just switching to the editor rather than having to end the Pry session. This could be done using the ''edit'' command<ref>http://kgrz.io/pry/2014/02/07/Prys-edit-command.html</ref>. On using the ''edit'' command, a new editor window pops up thereby enabling the programmer to edit the code. However, you need to specify the type of editor in the ''.pryrc'' file. For example, in order to edit the code using a Notepad editor, you need to specify the following in the ''.pryrc'' file:
<pre>
Pry.config.editor = 'Notepad'
</pre>
[[File:SnOutput1.jpg]]

=== View the documentation or source for a method: ===
:You sometimes would want to know how a particular method works or what kind of arguments it takes. The ''show-doc'' method could help you determine that. Also, you would want to know the source of the method. This could be done using the ''show-method'' command.
<pre>
pry(main)> def foo
pry(main)* puts "Welcome to the foo!!"
pry(main)* end
=> nil
pry(main)> show-method foo

From: (pry) @ line 3:
Number of lines: 3
Owner: Object
Visibility: private

def foo
puts "Welcome to the foo!!"
end
</pre>

=== List of other commonly used commands: ===
:The other list of commands could be found by typing ''help''<ref>https://github.com/pry/pry/wiki/Command-system#Help_command</ref>. The following shows other list of commands:

[[File:Help1.jpg]]

:Interested people can learn more about using Pry as a debugger by watching this video [http://www.youtube.com/watch?v=o90CCPjcIKE tutorial].

= Remote Debugging<ref>http://wiki.freepascal.org/Remote_Debugging</ref> =
:So far, Pry has been a good tool to digging into the code and checking how the code is actually working and what is exactly going on inside the code. We used ''binding.pry'' inside the program where the execution stops and the user is asked to give further instructions. However, there are situations where the execution of the program would not stop and the would not allow you to inspect the code especially in situations when [https://github.com/yegrb/yeg-wiki/wiki/Managing-application-processes-with-the-Foreman-gem Foreman] is used to manage processes. In such situation, ''pry-remote'' could be of help.

:When a process that is being run by Foreman comes across ''binding.pry'', it would halt but a Pry session would not be opened. You would be forced to kill the process as the application actually freezes. Thus, in such cases ''pry-remote'' would be a great solution and in order to work around with this, you need to first install the gem:
<pre>
gem install pry-remote
</pre>
:Then run bundler:
<pre>
bundle install
</pre>
:Once you have installed ''pry-remote'', a breakpoint can be added using ''binding.remote_pry'' anywhere in the program from where the code needs to be debugged.
<pre>
def index
binding.remote_pry
...
end
end

</pre>
:''Pry-remote'' would block your program when it hits the ''binding.remote_pry'' and open a Drb endpoint that a client can connect to<ref>http://www.ruby-doc.org/stdlib-1.9.3/libdoc/drb/rdoc/DRb.html</ref>. Running pry-remote in a shell will then connect to the session. You would then be able to interact with Pry as you would normally do. You could unblock the program by using ''exit''.

= Comparison with other debugging tools =
Pry is a replacement for Ruby's default REPL,irb. It has many more additional features as compared to irb. Some of the features that provide Pry an edge over the other debugging tools are as follows:
:*Pry enables code or syntax highlighting, auto identation and a runtime debugging console.
:*It has the ability to load any portion of the file in your text editor directly into the REPL session.
[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-pry-debugger.html This website has a lot of comparison between Pry Debugger with other debuggers available in market.]

= Reference =
<references />

CSC/ECE 517 Fall 2014/ch1a 26 sn

2014-09-24T02:57:14Z

Nurahman: /* Comparison with other debugging tools */

= '''Debugging in Rails using Pry''' =

Debugging is a process of finding and reducing the bugs or defects in a computer program thereby making it behave as expected. Few of the debugging techniques are [http://en.wikipedia.org/wiki/Debug_code#Print_debugging Print debugging] (also known as printf debugging), Log debugging and Interactive debugging<ref>http://rubylearning.com/satishtalim/ruby_logging.html</ref><ref>http://object.io/site/2011/getting-to-know-ruby-debugger/</ref>.
[http://en.wikipedia.org/wiki/Ruby_on_Rails Rails] provides a range of options to make debugging<ref>http://en.wikipedia.org/wiki/Debugging</ref> easier which include debug helpers, loggers, graphical debuggers, Pry, and so on<ref>http://guides.rubyonrails.org/debugging_rails_applications.html</ref>. In the following sections, debugging in rails using PRY is being explained.

__TOC__

= Introduction to Pry debugger =

:Pry is an [http://en.wikipedia.org/wiki/Shell_(computing) interactive shell] for the Ruby programming language. Pry could be started at any point within a running program. Due to this, programmers can inspect the program, correct the source code, or change its current state without having to restart the process. Pry is thus notable for its ability to start a [http://en.wikipedia.org/wiki/Read%E2%80%93eval%E2%80%93print_loop REPL] within a running program.

:Pry has many features that make it extremely efficient to use. One of the example here could be Pry's ''hist --grep'' that makes it easy to search through your Pry history. Also, Pry uses the filesystem metaphor for exposing most of its introspective capabilities such as the ''ls'' command to list the methods and variables and the ''cd'' command to start interacting with a particular object.

:To start a debugging session, you need to include the ''require 'pry' '' and ''binding.pry'' lines in your program that needs to be debugged. You could insert the line ''binding.pry'' at any point in your program. Whenever the Ruby interpreter executes this line, it opens a Pry REPL session. The code after the ''binding.pry'' statement is not executed unless instructed by the programmer. We could make changes to fix the code and then return to running program from same point.

== Installation ==

:To use Pry for debugging a Rails application, you first need to download and install Pry-debugger. If you are using Ruby version 2.0 or higher, then download pry-debugger from [https://github.com/deivid-rodriguez/pry-byebug here]. If you are using Ruby version 1.9 or lower, then download pry-debugger from [https://rubygems.org/gems/pry-debugger here].

:To install the debugger use the following command:<ref>http://pryrepl.org/</ref>
<pre>
gem install pry-debugger.gem
</pre>
:Include the following line of code on the gem file:
<pre>
gem 'pry-rails', :group => :development
</pre>
:Once that is done, the necessary dependencies could be installed using the following command:
<pre>
$bundle install
</pre>

= Execution Commands =
:To use the execution command, invoke pry normally. There is no need to start your script or application differently. For the sake of clarity in explanation, we would refer to the program below:
<pre>
require 'pry'
def method1 (seller)
puts 'Inside method 1'
method2(seller)
end

def method2(seller)
binding.pry # Execution will stop here.
new_products = method3(seller)
puts 'After calling method3'
end

def method3(seller)
puts 'Inside method 3'
puts 'Continuing in method 3'
end

method1('xyz')

</pre>

=== The Step command: ===
:The ''step'' command continues execution by moving into the method on the next line. To put it in another way, the command takes us deeper into the method. Following our example, the ''step'' command would take us into ''method3'' at which point we would be asked what to do next. The following screenshot demonstrates our example:

[[File:Step.jpg]]

=== The Next command: ===
:The ''next'' command runs the current line and moves to the next line in the current context. If the next line in the code is a method call, then unlike the ''step'' method the execution does not go deeper into the method. Following our example, if we execute the above code, the execution stop at the ''binding.pry'' line and the debugger asks us for the further action. On using the ''next'' command, the execution moves to the next line that is '' new_products = method3(seller)'' without going deeper into the method. The debugger now waits for another instruction from the programmer and if the command is ''next'' again, the debugger simply goes to the next line of code after executing the method. Thus, all the code within the ''method3'' method (which would otherwise be many steps), has been concluded with a single use of the ''next'' command. The example could be seen in the following screenshot:

[[File:Next.jpg]]

=== The Finish command: ===
:Now once we have stepped into a method using the ''step'' command, the debugger would ask us what needs to be done next. Now if we want to jump to the end of the method without the debugger asking us our choice of navigation command at the end of each line, we could use the ''finish'' command. The ''finish'' command would execute the code in the method and return back to the point from where the method was called. In our example, while the debugger is in ''method3'', the ''finish'' command would complete the code execution in ''method3'' and return back to ''method2''.

=== The Continue command: ===
:With this command we can abandon the pry session and continue the normal execution of the program. If the interpreter encounters another ''binding.pry'', a pry session is again opened. This can be incredibly annoying, since our program will stop and we’ll have to navigate the debugger every single time, so we’ll probably want to type exit-program to cancel the effect of any following ''binding.pry'' statements, thereby exiting our rails application.

=== Breakpoints: ===
:You could set or change the breakpoints in a program directly using Pry. The ''break'' command is used for this purpose. You could set breakpoints in the current file, or in any other file, or a method. Conditional breakpoints could also be changed or set using optional expressions.

:Command to display the list of breakpoints and conditional breakpoints:
<pre>
break --condition 4 x > 2 Change condition on breakpoint #4 to 'x > 2'.
break --condition 6 Remove the condition on breakpoint #6.
break List all breakpoints. (Same as `breakpoints`)
break --show 2 Show details about breakpoint #2.
</pre>
:Setting breakpoints:
<pre>
break 5 Break at line 5 in the current file.
break <class_name>#run Break at the start of `<class_name>#run`.
break app/models/test.rb:4 Break at line 4 in test.rb.
</pre>
:Deleting breakpoints:
<pre>
break --delete 5 Delete breakpoint #5.
break --disable-all Disable all breakpoints.
</pre>

=== The Edit command: ===
:You could edit the code in a file by just switching to the editor rather than having to end the Pry session. This could be done using the ''edit'' command<ref>http://kgrz.io/pry/2014/02/07/Prys-edit-command.html</ref>. On using the ''edit'' command, a new editor window pops up thereby enabling the programmer to edit the code. However, you need to specify the type of editor in the ''.pryrc'' file. For example, in order to edit the code using a Notepad editor, you need to specify the following in the ''.pryrc'' file:
<pre>
Pry.config.editor = 'Notepad'
</pre>
[[File:SnOutput1.jpg]]

=== View the documentation or source for a method: ===
:You sometimes would want to know how a particular method works or what kind of arguments it takes. The ''show-doc'' method could help you determine that. Also, you would want to know the source of the method. This could be done using the ''show-method'' command.
<pre>
pry(main)> def foo
pry(main)* puts "Welcome to the foo!!"
pry(main)* end
=> nil
pry(main)> show-method foo

From: (pry) @ line 3:
Number of lines: 3
Owner: Object
Visibility: private

def foo
puts "Welcome to the foo!!"
end
</pre>

=== List of other commonly used commands: ===
:The other list of commands could be found by typing ''help''<ref>https://github.com/pry/pry/wiki/Command-system#Help_command</ref>. The following shows other list of commands:

[[File:Help1.jpg]]

= Remote Debugging<ref>http://wiki.freepascal.org/Remote_Debugging</ref> =
:So far, Pry has been a good tool to digging into the code and checking how the code is actually working and what is exactly going on inside the code. We used ''binding.pry'' inside the program where the execution stops and the user is asked to give further instructions. However, there are situations where the execution of the program would not stop and the would not allow you to inspect the code especially in situations when [https://github.com/yegrb/yeg-wiki/wiki/Managing-application-processes-with-the-Foreman-gem Foreman] is used to manage processes. In such situation, ''pry-remote'' could be of help.

:When a process that is being run by Foreman comes across ''binding.pry'', it would halt but a Pry session would not be opened. You would be forced to kill the process as the application actually freezes. Thus, in such cases ''pry-remote'' would be a great solution and in order to work around with this, you need to first install the gem:
<pre>
gem install pry-remote
</pre>
:Then run bundler:
<pre>
bundle install
</pre>
:Once you have installed ''pry-remote'', a breakpoint can be added using ''binding.remote_pry'' anywhere in the program from where the code needs to be debugged.
<pre>
def index
binding.remote_pry
...
end
end

</pre>
:''Pry-remote'' would block your program when it hits the ''binding.remote_pry'' and open a Drb endpoint that a client can connect to<ref>http://www.ruby-doc.org/stdlib-1.9.3/libdoc/drb/rdoc/DRb.html</ref>. Running pry-remote in a shell will then connect to the session. You would then be able to interact with Pry as you would normally do. You could unblock the program by using ''exit''.

= Comparison with other debugging tools =
Pry is a replacement for Ruby's default REPL,irb. It has many more additional features as compared to irb. Some of the features that provide Pry an edge over the other debugging tools are as follows:
:*Pry enables code or syntax highlighting, auto identation and a runtime debugging console.
:*It has the ability to load any portion of the file in your text editor directly into the REPL session.
[http://www.jackkinsella.ie/2014/06/06/debugging-rails-with-pry-debugger.html This website has a lot of comparison between Pry Debugger with other debuggers available in market.]

= Reference =
<references />

CSC/ECE 517 Fall 2014/ch1a 26 sn

2014-09-24T02:54:34Z

Nurahman: /* Debugging in Rails using Pry */

= '''Debugging in Rails using Pry''' =

Debugging is a process of finding and reducing the bugs or defects in a computer program thereby making it behave as expected. Few of the debugging techniques are [http://en.wikipedia.org/wiki/Debug_code#Print_debugging Print debugging] (also known as printf debugging), Log debugging and Interactive debugging<ref>http://rubylearning.com/satishtalim/ruby_logging.html</ref><ref>http://object.io/site/2011/getting-to-know-ruby-debugger/</ref>.
[http://en.wikipedia.org/wiki/Ruby_on_Rails Rails] provides a range of options to make debugging<ref>http://en.wikipedia.org/wiki/Debugging</ref> easier which include debug helpers, loggers, graphical debuggers, Pry, and so on<ref>http://guides.rubyonrails.org/debugging_rails_applications.html</ref>. In the following sections, debugging in rails using PRY is being explained.

__TOC__

= Introduction to Pry debugger =

:Pry is an [http://en.wikipedia.org/wiki/Shell_(computing) interactive shell] for the Ruby programming language. Pry could be started at any point within a running program. Due to this, programmers can inspect the program, correct the source code, or change its current state without having to restart the process. Pry is thus notable for its ability to start a [http://en.wikipedia.org/wiki/Read%E2%80%93eval%E2%80%93print_loop REPL] within a running program.

:Pry has many features that make it extremely efficient to use. One of the example here could be Pry's ''hist --grep'' that makes it easy to search through your Pry history. Also, Pry uses the filesystem metaphor for exposing most of its introspective capabilities such as the ''ls'' command to list the methods and variables and the ''cd'' command to start interacting with a particular object.

:To start a debugging session, you need to include the ''require 'pry' '' and ''binding.pry'' lines in your program that needs to be debugged. You could insert the line ''binding.pry'' at any point in your program. Whenever the Ruby interpreter executes this line, it opens a Pry REPL session. The code after the ''binding.pry'' statement is not executed unless instructed by the programmer. We could make changes to fix the code and then return to running program from same point.

== Installation ==

:To use Pry for debugging a Rails application, you first need to download and install Pry-debugger. If you are using Ruby version 2.0 or higher, then download pry-debugger from [https://github.com/deivid-rodriguez/pry-byebug here]. If you are using Ruby version 1.9 or lower, then download pry-debugger from [https://rubygems.org/gems/pry-debugger here].

:To install the debugger use the following command:<ref>http://pryrepl.org/</ref>
<pre>
gem install pry-debugger.gem
</pre>
:Include the following line of code on the gem file:
<pre>
gem 'pry-rails', :group => :development
</pre>
:Once that is done, the necessary dependencies could be installed using the following command:
<pre>
$bundle install
</pre>

= Execution Commands =
:To use the execution command, invoke pry normally. There is no need to start your script or application differently. For the sake of clarity in explanation, we would refer to the program below:
<pre>
require 'pry'
def method1 (seller)
puts 'Inside method 1'
method2(seller)
end

def method2(seller)
binding.pry # Execution will stop here.
new_products = method3(seller)
puts 'After calling method3'
end

def method3(seller)
puts 'Inside method 3'
puts 'Continuing in method 3'
end

method1('xyz')

</pre>

=== The Step command: ===
:The ''step'' command continues execution by moving into the method on the next line. To put it in another way, the command takes us deeper into the method. Following our example, the ''step'' command would take us into ''method3'' at which point we would be asked what to do next. The following screenshot demonstrates our example:

[[File:Step.jpg]]

=== The Next command: ===
:The ''next'' command runs the current line and moves to the next line in the current context. If the next line in the code is a method call, then unlike the ''step'' method the execution does not go deeper into the method. Following our example, if we execute the above code, the execution stop at the ''binding.pry'' line and the debugger asks us for the further action. On using the ''next'' command, the execution moves to the next line that is '' new_products = method3(seller)'' without going deeper into the method. The debugger now waits for another instruction from the programmer and if the command is ''next'' again, the debugger simply goes to the next line of code after executing the method. Thus, all the code within the ''method3'' method (which would otherwise be many steps), has been concluded with a single use of the ''next'' command. The example could be seen in the following screenshot:

[[File:Next.jpg]]

=== The Finish command: ===
:Now once we have stepped into a method using the ''step'' command, the debugger would ask us what needs to be done next. Now if we want to jump to the end of the method without the debugger asking us our choice of navigation command at the end of each line, we could use the ''finish'' command. The ''finish'' command would execute the code in the method and return back to the point from where the method was called. In our example, while the debugger is in ''method3'', the ''finish'' command would complete the code execution in ''method3'' and return back to ''method2''.

=== The Continue command: ===
:With this command we can abandon the pry session and continue the normal execution of the program. If the interpreter encounters another ''binding.pry'', a pry session is again opened. This can be incredibly annoying, since our program will stop and we’ll have to navigate the debugger every single time, so we’ll probably want to type exit-program to cancel the effect of any following ''binding.pry'' statements, thereby exiting our rails application.

=== Breakpoints: ===
:You could set or change the breakpoints in a program directly using Pry. The ''break'' command is used for this purpose. You could set breakpoints in the current file, or in any other file, or a method. Conditional breakpoints could also be changed or set using optional expressions.

:Command to display the list of breakpoints and conditional breakpoints:
<pre>
break --condition 4 x > 2 Change condition on breakpoint #4 to 'x > 2'.
break --condition 6 Remove the condition on breakpoint #6.
break List all breakpoints. (Same as `breakpoints`)
break --show 2 Show details about breakpoint #2.
</pre>
:Setting breakpoints:
<pre>
break 5 Break at line 5 in the current file.
break <class_name>#run Break at the start of `<class_name>#run`.
break app/models/test.rb:4 Break at line 4 in test.rb.
</pre>
:Deleting breakpoints:
<pre>
break --delete 5 Delete breakpoint #5.
break --disable-all Disable all breakpoints.
</pre>

=== The Edit command: ===
:You could edit the code in a file by just switching to the editor rather than having to end the Pry session. This could be done using the ''edit'' command<ref>http://kgrz.io/pry/2014/02/07/Prys-edit-command.html</ref>. On using the ''edit'' command, a new editor window pops up thereby enabling the programmer to edit the code. However, you need to specify the type of editor in the ''.pryrc'' file. For example, in order to edit the code using a Notepad editor, you need to specify the following in the ''.pryrc'' file:
<pre>
Pry.config.editor = 'Notepad'
</pre>
[[File:SnOutput1.jpg]]

=== View the documentation or source for a method: ===
:You sometimes would want to know how a particular method works or what kind of arguments it takes. The ''show-doc'' method could help you determine that. Also, you would want to know the source of the method. This could be done using the ''show-method'' command.
<pre>
pry(main)> def foo
pry(main)* puts "Welcome to the foo!!"
pry(main)* end
=> nil
pry(main)> show-method foo

From: (pry) @ line 3:
Number of lines: 3
Owner: Object
Visibility: private

def foo
puts "Welcome to the foo!!"
end
</pre>

=== List of other commonly used commands: ===
:The other list of commands could be found by typing ''help''<ref>https://github.com/pry/pry/wiki/Command-system#Help_command</ref>. The following shows other list of commands:

[[File:Help1.jpg]]

= Remote Debugging<ref>http://wiki.freepascal.org/Remote_Debugging</ref> =
:So far, Pry has been a good tool to digging into the code and checking how the code is actually working and what is exactly going on inside the code. We used ''binding.pry'' inside the program where the execution stops and the user is asked to give further instructions. However, there are situations where the execution of the program would not stop and the would not allow you to inspect the code especially in situations when [https://github.com/yegrb/yeg-wiki/wiki/Managing-application-processes-with-the-Foreman-gem Foreman] is used to manage processes. In such situation, ''pry-remote'' could be of help.

:When a process that is being run by Foreman comes across ''binding.pry'', it would halt but a Pry session would not be opened. You would be forced to kill the process as the application actually freezes. Thus, in such cases ''pry-remote'' would be a great solution and in order to work around with this, you need to first install the gem:
<pre>
gem install pry-remote
</pre>
:Then run bundler:
<pre>
bundle install
</pre>
:Once you have installed ''pry-remote'', a breakpoint can be added using ''binding.remote_pry'' anywhere in the program from where the code needs to be debugged.
<pre>
def index
binding.remote_pry
...
end
end

</pre>
:''Pry-remote'' would block your program when it hits the ''binding.remote_pry'' and open a Drb endpoint that a client can connect to<ref>http://www.ruby-doc.org/stdlib-1.9.3/libdoc/drb/rdoc/DRb.html</ref>. Running pry-remote in a shell will then connect to the session. You would then be able to interact with Pry as you would normally do. You could unblock the program by using ''exit''.

= Comparison with other debugging tools =
Pry is a replacement for Ruby's default REPL,irb. It has many more additional features as compared to irb. Some of the features that provide Pry an edge over the other debugging tools are as follows:
:*Pry enables code or syntax highlighting, auto identation and a runtime debugging console.
:*It has the ability to load any portion of the file in your text editor directly into the REPL session.

= Reference =
<references />