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
Before
def impersonate # Initial check to see if the username exists display_error_msg begin @original_user = session[:super_user] || session[:user] # Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method if params[:impersonate].nil? # Check if special chars /\?<>|&$# are used to avoid html tags or system command check_if_special_char # 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]) do_main_operation(user) else # Impersonate a new account if !params[:impersonate][:name].empty? #check if special chars /\?<>|&$# are used to avoid html tags or system command check_if_special_char # 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]) do_main_operation(user) # Revert to original account when currently in the impersonated session else if !session[:super_user].nil? AuthController.clear_user_info(session, nil) session[:user] = session[:super_user] user = session[:user] session[:super_user] = nil else display_error_msg end end end
After
def impersonate # Initial check to see if the username exists display_error_msg begin @original_user = session[:super_user] || session[:user] # Impersonate using form on /impersonate/start, based on the username provided, this method looks to see if that's possible by calling the do_main_operation method if params[:impersonate].nil? # Check if special chars /\?<>|&$# are used to avoid html tags or system command check_if_special_char # 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 do_main_operation(user) elsif !params[:impersonate][:name].empty? # Impersonate a new account #check if special chars /\?<>|&$# are used to avoid html tags or system command check_if_special_char # 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 do_main_operation(user) # Revert to original account when currently in the impersonated session elsif !session[:super_user].nil? AuthController.clear_user_info(session, nil) session[:user] = session[:super_user] user = session[:user] session[:super_user] = nil else display_error_msg end
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)