Main Page/CSC/CSC 517 Spring 2020 Refactor impersonate controller: Difference between revisions
(added intial code pics and usage guide images) |
No edit summary |
||
Line 1: | Line 1: | ||
This wiki page describes the work done under E2002 OSS Program for Spring 2020, in the CSC/ECE 517 course. | This wiki page describes the work done under E2002 OSS Program for Spring 2020, in the CSC/ECE 517 course. | ||
===About Expertiza=== | |||
[http://expertiza.ncsu.edu/ Expertiza] is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza is a complete instructor-student usage website where the instructor can assign assignments, deadlines, grades, etc that is required for the course. Similarly, the students can use this website to perform the tasks required as part of the course like project or assignments submission, forming groups and collaborating with them, as well as reviewing projects and teammates. | |||
[[File:Impersonate guide.jpg|figure 1|frame|center]] | [[File:Impersonate guide.jpg|figure 1|frame|center]] | ||
Line 13: | Line 16: | ||
[[File:Impersonated view.jpg|figure 3|frame|center]] | [[File:Impersonated view.jpg|figure 3|frame|center]] | ||
===Problem Statement=== | ===Problem Statement=== | ||
The aim of the project is to refactor the impersonate controller. The pre-existing code had two major issues. | The aim of the project is to refactor the impersonate controller. The pre-existing code had two major issues. | ||
*All functions related to impersonate controller were present in a single method | *All functions related to impersonate controller were present in a single method ( from figure 4a and 4b) | ||
*Presence of repetitive code | *Presence of repetitive code ( from figure 4a and 4b) | ||
*3 levels of block nesting (from figure 5) | |||
* Too many return statements (from figure 6) | |||
[[File:initial code1.jpg| figure 4a| frame|center]] | [[File:initial code1.jpg| figure 4a| frame|center]] | ||
[[File:initial code2.jpg| figure 4b| frame|center]] | [[File:initial code2.jpg| figure 4b| frame|center]] | ||
[[File:code climate3.jpg| figure 5| frame|center]] | |||
[[File:code climate2.jpg| figure 6| frame|center]] | |||
Line 31: | Line 31: | ||
Expertiza allows the administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. For example, an instructor impersonating a student’s account can view their assignments, stage deadlines, peer reviews and anything else that the student can view. One thing to be noted is that most of these users con only impersonate users for whom they are a parent. For example, instructor6 is a parent of student3841 and not student3836; as a result, instructor6 can impersonate only 3841. | Expertiza allows the administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. For example, an instructor impersonating a student’s account can view their assignments, stage deadlines, peer reviews and anything else that the student can view. One thing to be noted is that most of these users con only impersonate users for whom they are a parent. For example, instructor6 is a parent of student3841 and not student3836; as a result, instructor6 can impersonate only 3841. | ||
===Impersonate functionality navigation=== | |||
1. Instructor login: username -> instructor6, password -> password | |||
when logged in as an instructor, under the manage option in the ribbon as in Figure 1, select impersonate user. Upon redirected to impersonate page, enter the account which needs to be impersonated. If possible it impersonates that user. Now a new button called revert appears on the ribbon as in figure 3, this can be used to revert the impersonation and return to the instructor profile. | |||
===Problem Solution=== | ===Problem Solution=== | ||
The above-mentioned issues have been tackled by refactoring the impersonate controller by splitting into many smaller methods which are later called by the main impersonate controller. | |||
* | The following are the refactored new methods that hep in tackling the issue: | ||
*check_if_user_impersonateable | |||
*display_error_msg | *display_error_msg | ||
* | *overwrite_session | ||
*check_if_special_char | |||
*do_main_operartion | |||
=====check_if_user_impersonateable===== | |||
This method plays a main role in tackling issue3 - 3 levels of block nesting apart from issue1 which is the recurrent idea behind all of these methods. | |||
<pre> | |||
def check_if_user_impersonateable | |||
if params[:impersonate].nil? | |||
user = User.find_by(name: params[:user][:name]) | |||
if !@original_user.can_impersonate? user | |||
@message = "You cannot impersonate '#{params[:user][:name]}'." | |||
temp | |||
AuthController.clear_user_info(session, nil) | |||
else | |||
overwrite_session | |||
end | |||
else | |||
if !params[:impersonate][:name].empty? | |||
user = User.find_by(name: params[:impersonate][:name]) | |||
overwrite_session | |||
end | |||
end | |||
end | |||
</pre> | |||
=====display_error_msg===== | |||
<pre> | |||
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 | |||
redirect_to :back | |||
end | |||
</pre> | |||
=====overwrite_session===== | |||
<pre> | |||
def overwrite_session | |||
#if not impersonatable, then original user's session remains | |||
if params[:impersonate].nil? | |||
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? | |||
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 | |||
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 | |||
</pre> | |||
=====check_if_special_char===== | |||
<pre> | |||
def check_if_special_char | |||
if warn_for_special_chars(params[:user][:name], "Username") | |||
redirect_back | |||
return | |||
end | |||
end | |||
</pre> | |||
=====do_main_operation===== | |||
<pre> | |||
def do_main_operation(user) | |||
if user | |||
check_if_user_impersonateable | |||
else | |||
display_error_msg | |||
end | |||
end | |||
</pre> | |||
=== Test Plan === |
Revision as of 19:39, 31 March 2020
This wiki page describes the work done under E2002 OSS Program for Spring 2020, in the CSC/ECE 517 course.
About Expertiza
Expertiza is an open-source project based on Ruby on Rails framework. Expertiza is a complete instructor-student usage website where the instructor can assign assignments, deadlines, grades, etc that is required for the course. Similarly, the students can use this website to perform the tasks required as part of the course like project or assignments submission, forming groups and collaborating with them, as well as reviewing projects and teammates.
Problem Statement
The aim of the project is to refactor the impersonate controller. The pre-existing code had two major issues.
- All functions related to impersonate controller were present in a single method ( from figure 4a and 4b)
- Presence of repetitive code ( from figure 4a and 4b)
- 3 levels of block nesting (from figure 5)
- Too many return statements (from figure 6)
About Impersonate Controller
Expertiza allows the administrators, instructors or teaching assistants to impersonate another user (like a student) and access their account. For example, an instructor impersonating a student’s account can view their assignments, stage deadlines, peer reviews and anything else that the student can view. One thing to be noted is that most of these users con only impersonate users for whom they are a parent. For example, instructor6 is a parent of student3841 and not student3836; as a result, instructor6 can impersonate only 3841.
1. Instructor login: username -> instructor6, password -> password
when logged in as an instructor, under the manage option in the ribbon as in Figure 1, select impersonate user. Upon redirected to impersonate page, enter the account which needs to be impersonated. If possible it impersonates that user. Now a new button called revert appears on the ribbon as in figure 3, this can be used to revert the impersonation and return to the instructor profile.
Problem Solution
The above-mentioned issues have been tackled by refactoring the impersonate controller by splitting into many smaller methods which are later called by the main impersonate controller.
The following are the refactored new methods that hep in tackling the issue:
- check_if_user_impersonateable
- display_error_msg
- overwrite_session
- check_if_special_char
- do_main_operartion
check_if_user_impersonateable
This method plays a main role in tackling issue3 - 3 levels of block nesting apart from issue1 which is the recurrent idea behind all of these methods.
def check_if_user_impersonateable if params[:impersonate].nil? user = User.find_by(name: params[:user][:name]) if !@original_user.can_impersonate? user @message = "You cannot impersonate '#{params[:user][:name]}'." temp AuthController.clear_user_info(session, nil) else overwrite_session end else if !params[:impersonate][:name].empty? user = User.find_by(name: params[:impersonate][:name]) overwrite_session end end end
display_error_msg
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 redirect_to :back end
overwrite_session
def overwrite_session #if not impersonatable, then original user's session remains if params[:impersonate].nil? 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? 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 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
check_if_special_char
def check_if_special_char if warn_for_special_chars(params[:user][:name], "Username") redirect_back return end end
do_main_operation
def do_main_operation(user) if user check_if_user_impersonateable else display_error_msg end end