CSC/ECE 517 Fall 2021 - E2122. Refactor impersonate controller.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 30: Line 30:
== Code changes ==
== Code changes ==


* Controller
This part shows the changes in ./app/controllers/impersonate_controller.rb
app/controllers/impersonate_controller.rb


<syntaxhighlight lang="python" line>
===In function overwrite_session===
def quick_sort(arr):
 
less = []
'''Before'''
pivot_list = []
<pre>
more = []
def overwrite_session
if len(arr) <= 1:
    # If not impersonatable, then original user's session remains
return arr
    if params[:impersonate].nil?
else:
      # E1991 : check whether instructor is currently in anonymized view
pass
      user = User.anonymized_view?(session[:ip]) ? User.real_user_from_anonymized_name(params[:user][:name]) : user = User.find_by(name: params[:user][:name])
</syntaxhighlight>
      session[:super_user] = session[:user] if session[:super_user].nil?
* Tests
      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
</pre>
'''After'''
<pre>
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
</pre>
 
===In function check_if_special_char===
 
'''Before'''
<pre>
  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
</pre>
'''After'''
<pre>
  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
</pre>
 
===In function check_if_user_impersonateable===
'''Before'''
<pre>
  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
</pre>
'''After'''
<pre>
  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
</pre>


= Test Plan =
= Test Plan =

Revision as of 02:06, 21 October 2021

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

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)