CSC/ECE 517 Fall 2021 - E2122. Refactor impersonate controller.rb
This wiki page describes the changes made under E2122, in order to refactor impersonate_controller.rb.
Overview
About Expertiza
Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
Issues for this project
- Issue 1: Throughout the file, the user is being initialized but never used. This is bad coding practice as it slows down runtime.
- Issue 2: Convert nested if to elif around line 36.
- Issue 3: Refactor very long lines of code to make it more readable.
- Issue 4: Test functions and increase coverage:
- auto_complete_for_user_name
- overwrite_session test line 47-51
- test_
Project implementation
Submitted work and demonstration of project
Files involved
- app/controllers/impersonate_controller.rb
- spec/controllers/impersonate_controller_spec.rb
Code changes
This part shows the changes in ./app/controllers/impersonate_controller.rb
In function overwrite_session
Before
def overwrite_session
# If not impersonatable, then original user's session remains
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
session[:super_user] = session[:user] if session[:super_user].nil?
AuthController.clear_user_info(session, nil)
session[:original_user] = @original_user
session[:impersonate] = true
session[:user] = user
else
# If some user is to be impersonated, their session details are overwritten onto the current to impersonate
if !params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:impersonate][:name]) : user = user = User.find_by(name: params[:impersonate][:name])
AuthController.clear_user_info(session, nil)
session[:user] = user
session[:impersonate] = true
session[:original_user] = @original_user
else
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
AuthController.clear_user_info(session, nil)
session[:user] = session[:super_user]
user = session[:user]
session[:super_user] = nil
end
end
end
After
def overwrite_session
# If not impersonatable, then original user's session remains
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
if User.anonymized_view?(session[:ip])
user = User.real_user_from_anonymized_name(params[:user][:name])
else
user = User.find_by(name: params[:user][:name])
end
session[:super_user] = session[:user] if session[:super_user].nil?
AuthController.clear_user_info(session, nil)
session[:original_user] = @original_user
session[:impersonate] = true
session[:user] = user
# If some user is to be impersonated, their session details are overwritten onto the current to impersonate
elsif !params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
if User.anonymized_view?(session[:ip])
user = User.real_user_from_anonymized_name(params[:impersonate][:name])
else
user = User.find_by(name: params[:impersonate][:name])
end
AuthController.clear_user_info(session, nil)
session[:user] = user
session[:impersonate] = true
session[:original_user] = @original_user
else
# E1991 : check whether instructor is currently in anonymized view
AuthController.clear_user_info(session, nil)
session[:user] = session[:super_user]
session[:super_user] = nil
end
end
In function check_if_special_char
Before
def check_if_special_char
if params[:user]
if warn_for_special_chars(params[:user][:name], "Username")
redirect_back
return
end
end
if params[:impersonate]
if warn_for_special_chars(params[:impersonate][:name], "Username")
redirect_back
return
end
end
end
After
def check_if_special_char
if params[:user] && warn_for_special_chars(params[:user][:name], "Username")
redirect_back
return
end
if params[:impersonate] && warn_for_special_chars(params[:impersonate][:name], "Username")
redirect_back
return
end
end
In function check_if_user_impersonateable
Before
def check_if_user_impersonateable
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
if !@original_user.can_impersonate? user
@message = "You cannot impersonate '#{params[:user][:name]}'."
temp
@@ -85,7 +89,6 @@ def check_if_user_impersonateable
else
unless params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:impersonate][:name]) : user = user = User.find_by(name: params[:impersonate][:name])
overwrite_session
end
end
end
After
def check_if_user_impersonateable
if params[:impersonate].nil?
# E1991 : check whether instructor is currently in anonymized view
if User.anonymized_view?(session[:ip])
user = User.real_user_from_anonymized_name(params[:user][:name])
else
user = User.find_by(name: params[:user][:name])
end
if !@original_user.can_impersonate? user
@message = "You cannot impersonate '#{params[:user][:name]}'."
temp
@@ -85,7 +89,6 @@ def check_if_user_impersonateable
else
unless params[:impersonate][:name].empty?
# E1991 : check whether instructor is currently in anonymized view
overwrite_session
end
end
end
In function display_error_msg
Before
def display_error_msg
if params[:user]
@message = "No user exists with the name '#{params[:user][:name]}'."
elsif params[:impersonate]
@message = "No user exists with the name '#{params[:impersonate][:name]}'."
else
if params[:impersonate].nil?
@message = "You cannot impersonate '#{params[:user][:name]}'."
else
if !params[:impersonate][:name].empty?
@message = "You cannot impersonate '#{params[:impersonate][:name]}'."
else
@message = "No original account was found. Please close your browser and start a new session."
end
end
end
rescue Exception => e
flash[:error] = @message
end
After
def display_error_msg
if params[:user]
@message = "No user exists with the name '#{params[:user][:name]}'."
elsif params[:impersonate]
@message = "No user exists with the name '#{params[:impersonate][:name]}'."
elsif params[:impersonate].nil?
@message = "You cannot impersonate '#{params[:user][:name]}'."
elsif !params[:impersonate][:name].empty?
@message = "You cannot impersonate '#{params[:impersonate][:name]}'."
else
@message = "No original account was found. Please close your browser and start a new session."
end
rescue Exception => e
flash[:error] = @message
end
In function impersonate
Test Plan
Manual UI Testing
The following steps must be performed to test the project UI:
Step 1: Log in as an Instructor, with Username - instructor6, Password - password
Step 2: TODO
RSpec Testing
Team Information
Mentor: John Bumgardner (jwbumga2)
Robin Piao (lpiao)
Shengjie Guo (sguo25)
Haoze Du (hdu5)