CSC/ECE 517 Fall 2014/OSS E1461 knn
Expertiza - Refactoring StudentTeamController
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!
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
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:
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]
After Refactoring:
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]
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
#Redirect Routes redirect_to view_student_team_path student_id: student.id #Routes.rb resources :student_team do
to
#Redirect Routes redirect_to view_student_teams_path student_id: student.id #Routes.rb resources :student_teams do
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
#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])
Refactored Where Method Calls
#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
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
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
Refactored Code
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
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 #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]
References: Setting up Ruby on Rails Environment on Linux