CSC/ECE 517 Fall 2014/oss E1465 oak: Difference between revisions
No edit summary |
|||
Line 26: | Line 26: | ||
* Modify methods to conform to RESTful style : The purpose of the index method in Users Controller is to list all registered users. The list method was called from the index method to list all users. This implementation did not conform to RESTful guidelines. Hence, we refactored the list method in users controller to index. Further, we refactored UserControllers references, Users view references, Users tests references and routes.rb. | * Modify methods to conform to RESTful style : The purpose of the index method in Users Controller is to list all registered users. The list method was called from the index method to list all users. This implementation did not conform to RESTful guidelines. Hence, we refactored the list method in users controller to index. Further, we refactored UserControllers references, Users view references, Users tests references and routes.rb. | ||
Before Refactoring : | |||
<code> | |||
def index | |||
if (current_user_role? == "Student") | |||
redirect_to(:action => AuthHelper::get_home_action(session[:user]), :controller => AuthHelper::get_home_controller(session[:user])) | |||
else | |||
list | |||
render :action => 'list' | |||
end | |||
end | |||
#for displaying the list of users | |||
def list | |||
user = session[:user] | |||
role = Role.find(user.role_id) | |||
all_users = User.order('name').where( ['role_id in (?) or id = ?', role.get_available_roles, user.id]) | |||
letter = params[:letter] | |||
session[:letter] = letter | |||
if letter == nil | |||
letter = all_users.first.name[0,1].downcase | |||
end | |||
logger.info "#{letter}" | |||
@letters = Array.new | |||
@per_page = 1 | |||
# Check if the "Show" button for pagination is clicked | |||
# If yes, set @per_page to the value of the selection dropdown | |||
# Else, if the request is from one of the letter links on the top | |||
# set @per_page to 1 (25 names per page). | |||
# Else, set @per_page to the :num_users param passed in from | |||
# the will_paginate method from the 'pagination' partial. | |||
if params[:paginate_show] | |||
@per_page = params[:num_users] | |||
elsif params[:from_letter] | |||
@per_page = 1 | |||
else | |||
@per_page = params[:num_users] | |||
end | |||
# Get the users list to show on current page | |||
@users = paginate_list(role, user.id, letter) | |||
@letters = ('A'..'Z').to_a | |||
end | |||
</code> | |||
After Refactoring : | |||
<code> | |||
#for displaying the list of users | |||
def index | |||
if (current_user_role.eql? "Student") | |||
redirect_to(action: AuthHelper::get_home_action(session[:user]), controller: AuthHelper::get_home_controller(session[:user])) | |||
else | |||
user = session[:user] | |||
role = Role.find(user.role_id) | |||
all_users = User.order('name').where( ['role_id in (?) or id = ?', role.get_available_roles, user.id]) | |||
letter = params[:letter] | |||
session[:letter] = letter | |||
if letter.eql? nil | |||
letter = all_users.first.name[0,1].downcase | |||
end | |||
logger.info "#{letter}" | |||
@letters =[] | |||
# Get the users list to show on current page | |||
@users = paginate_list(role, user.id, letter) | |||
@letters = ('A'..'Z').to_a | |||
end | |||
end | |||
</code> | |||
===Modifications to index.html.erb=== | ===Modifications to index.html.erb=== |
Revision as of 03:13, 25 October 2014
Expertiza - Refactoring UsersController
Introduction
Expertiza<ref name="expertiza>Expertiza http://wikis.lib.ncsu.edu/index.php/Expertiza</ref> is a web application available to both students and professors. The Expertiza project is a system to create reusable learning objects through peer review. Expertiza supports team projects, and the submission of almost any document type, including URLs and wiki pages. Students can keep a track of their assignments, teammates and can conduct peer reviews on diverse topics and projects. It is an open source project developed on Ruby on Rails platform. More information on Expertiza can be found here. The source code can be forked and cloned for making modifications.
As a part of the coursework of Object Oriented Design and Development, we were expected to refactor the funtionality of some modules of Expertiza. This wiki provides an insight into our contributions to the Open Source Software project Expertiza with main focus of Refactoring the Users Controller.
Project Description
The Users Controller deals with managing activities peripheral to the User registered with Expertiza. Users are mapped to different roles like super-admin, admin, instructor, teaching assistant and student with each role having different access permissions. The Manage Users module can be accessed by roles other than the student. It provides users with the functionality to search other users based on keywords like username, email, etc. New user can be created and existing user information and role can be updated.
Classes : users_controller.rb
What it does : Manage Users i.e.search and list users, create new user, edit/update existing users, delete users.
What has to be changed :
- Modify methods to conform to RESTful style
- Refactor code such that it follows Ruby Conventions for Collection Objects like Arrays and Hashes
- Reducing the number of instance variables per action to one
- Code cleansing by removing commented out code, Using string interpolation instead of concatenation, Replacing '==' with eql? and :key =>'value' with key: 'value'
- Use of routing helpers instead of hardcoded URLs
- Replace find_by with where to follow Rails 4.0 conventions
Modification to Existing Code
Modifications to users_controller.rb
- Modify methods to conform to RESTful style : The purpose of the index method in Users Controller is to list all registered users. The list method was called from the index method to list all users. This implementation did not conform to RESTful guidelines. Hence, we refactored the list method in users controller to index. Further, we refactored UserControllers references, Users view references, Users tests references and routes.rb.
Before Refactoring :
def index
if (current_user_role? == "Student")
redirect_to(:action => AuthHelper::get_home_action(session[:user]), :controller => AuthHelper::get_home_controller(session[:user]))
else
list
render :action => 'list'
end
end
#for displaying the list of users
def list
user = session[:user]
role = Role.find(user.role_id)
all_users = User.order('name').where( ['role_id in (?) or id = ?', role.get_available_roles, user.id])
letter = params[:letter]
session[:letter] = letter
if letter == nil
letter = all_users.first.name[0,1].downcase
end
logger.info "#{letter}"
@letters = Array.new
@per_page = 1
# Check if the "Show" button for pagination is clicked
# If yes, set @per_page to the value of the selection dropdown
# Else, if the request is from one of the letter links on the top
# set @per_page to 1 (25 names per page).
# Else, set @per_page to the :num_users param passed in from
# the will_paginate method from the 'pagination' partial.
if params[:paginate_show]
@per_page = params[:num_users]
elsif params[:from_letter]
@per_page = 1
else
@per_page = params[:num_users]
end
# Get the users list to show on current page
@users = paginate_list(role, user.id, letter)
@letters = ('A'..'Z').to_a
end
After Refactoring :
#for displaying the list of users
def index
if (current_user_role.eql? "Student")
redirect_to(action: AuthHelper::get_home_action(session[:user]), controller: AuthHelper::get_home_controller(session[:user]))
else
user = session[:user]
role = Role.find(user.role_id)
all_users = User.order('name').where( ['role_id in (?) or id = ?', role.get_available_roles, user.id])
letter = params[:letter]
session[:letter] = letter
if letter.eql? nil
letter = all_users.first.name[0,1].downcase
end
logger.info "#{letter}"
@letters =[]
# Get the users list to show on current page
@users = paginate_list(role, user.id, letter)
@letters = ('A'..'Z').to_a
end
end
Modifications to index.html.erb
References
<references/>