CSC/ECE 517 Fall 2015/oss E1565 AAJ

E1565: Refactoring Admin Controller and Course Controller

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.

Introduction to Expertiza

Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

Problem Statement

Files involved

admin_controller.rb
views in app/views/admin folder
user_controller

What they do:

The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

What needs to be done:

• The text field for adding a new admin/instructor has to be removed, leaving only the button.
• When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
• Make sure that user creation works for super-admin/admin/instructor.
• Associate the users with the institution table
• Add a drop down to the view of creating users so that new account creator can select the institute and save it.

Changes Made

Admin Controller

Method Name	Changes Made	Reason For Change
show_super_admin	Added this method to controller	This method helps in displaying the specified super_admin
show_admin	Added this method to Admin Controller	This method helps in displaying the specified admin
remove_instructor	Added this method to Admin Controller	This method helps in destroying an instructor.
remove_administrator	Added this method to Admin Controller	This method helps in destroying an administrator.
remove_super_administrator	Added this method to Admin Controller.	This method helps in destroying a super_admin.

Institution Controller

Method Name	Changes Made	Reason For Change
create	Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])	Was throwing error

User Controller

Method Name	Changes Made	Reason For Change
create	Added the line @user.institutions_id=params[:users][:institutions_id]	To include institution for newly created users
user_params	Added the parameter institutions ID.	To include institution for newly created users.

Views

Method Name	Changes Made	Reason For Change
views/admin/list_administrators.html.erb	Removed the text field next to New Administrator button	The button redirects to a new user page and passes a hidden field with role:administrator
views/admin/list_administrators.html.erb	Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>	There is no show method in Admin Controller
views/admin/list_instructors.html.erb	Removed the text field next to New Instructor button	The button redirects to a new user page and passes a hidden field with role:Instructor
views/admin/list_super_administrators.html.erb	Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>	There is no show method in Admin Controller.
views/admin/list_super_administrators.html.erb	Removed the Parent column from line 4	Super admins don't have any parents.
views/admin/list_super_administrators.html.erb	Removed <%= link_to 'New Super administrator', :action => 'new' %>	There can be only one super admin.
views/admin/show_admin.html.erb	Added this file.	show_admin lists out all the administrator.
views/admin/show_super_admin.html.erb	Added this file.	Lists out the super administrator.
views/users/_institutions.html.erb	Added this file	To add institution drop down menu in the new user creation form.
views/users/_user.html.erb	Added line <%= render :partial => 'users/institutions' %>	To select the institution.
views/users/list.html.erb	Sending role:student as the default role for new user creation form	Was throwing an error
views/users/new.html	Default role is selected in new user creation form.	So that if new administrator is clicked the user creation form will have admin as the pre selected role.

Re-factored Code Cases

Case 1 : Refactoring admin_controller

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

Before Changes

     def list_instructors
    @users = User.
      instructors.
      order(:name).
      where("parent_id = ?", current_user.id).
      paginate(:page => params[:page], :per_page => 50)
  end

  def add_administrator
    @user = User.new
  end

  def new_administrator
    add_administrator
  end

  def save_administrator # saves newly created administrator to database
    PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
  end

  def create_administrator
    save_administrator
  end

  def list_administrators

After Changes

+  def show
+    redirect_to url_for(:controller => :users, :action => :new)
+  end
 
   def list_instructors
     @users = User.
       instructors.
       order(:name).
       where("parent_id = ?", current_user.id).
       paginate(:page => params[:page], :per_page => 50)
   end
 
   def add_administrator
     @user = User.new
+    redirect_to 'users#new'
   end
 
   def new_administrator
     add_administrator
   end
 
   def save_administrator # saves newly created administrator to database
     PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
   end
 
   def create_administrator
     save_administrator
+    redirect_to "users#new"
   end
 
   def list_administrators

Case 2 : Refactoring institution_controller

Before changes

  
  def index
    list
    render :action => 'list'
  end

  # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
  verify :method => :post, :only => [ :destroy, :create, :update ],
    :redirect_to => { :action => :list }

  def list
    @institutions = Institution.all
  end

After Changes

def index
-    list
+   # @institutions=Institution.All
+   list
     render :action => 'list'
   end
 
   # GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
   verify :method => :post, :only => [ :destroy, :create, :update ],
     :redirect_to => { :action => :list }
 
   def list
-    @institutions = Institution.all
+    @institutions = Institution.all 
+
   end

Case 3 : Refactoring user_controller

Before Changes

      @user = User.new(user_params)
      # record the person who created this new user
      @user.parent_id = (session[:user]).id
      # set the user's timezone to its parent's
      @user.timezonepref = User.find(@user.parent_id).timezonepref

After Changes

 @user = User.new(user_params)
+      @user.institutions_id=params[:users][:institutions_id]
       # record the person who created this new user
       @user.parent_id = (session[:user]).id
       # set the user's timezone to its parent's
      

Case 4 : Refactoring list_administrators

Before Changes

 <h1>Administrators</h1>

<table>
  <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
  <% for user in @users %>
    <tr>
      <td><%= link_to user.name, :action => 'show',
        :id => user.id %></td>
      <td><%= user.fullname %></td>
      <td><%= user.email %></td>
      <td><%= (User.find(user.parent_id)).name %></td>
    </tr>
  <% end -%>
</table>


<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

  <br>
  <%= text_field_tag('name', '', {'size' => 30}) %>
   
  <input type='submit' value='New Administrator'/>
<% end %>

After Changes

<h1>Administrators</h1>
 
 <table>
-  <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
-  <% for user in @users %>
+  <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+  <% for user in User.all %>
+   <% if user.role_id == 3%>
     <tr>
       <td><%= link_to user.name, :action => 'show',
-        :id => user.id %></td>
-      <td><%= user.fullname %></td>
-      <td><%= user.email %></td>
-      <td><%= (User.find(user.parent_id)).name %></td>
+        :id => user.id %></td><td> </td>
+      <td><%= user.fullname %></td><td> </td>
+      <td><%= user.email %></td><td> </td>
+      <td><%= (User.find(user.parent_id)).name %></td><td> </td>
     </tr>
   <% end -%>
+  <% end -%>
 </table>
-
-
 <hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
-  <br>
-  <%= text_field_tag('name', '', {'size' => 30}) %>
-   
-  <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %> 
+<input type='submit' value='New Administrator'/>
+<% end %> 

Case 5 : Refactoring list_instructors.html.erb

Before Changes

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
  <tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
  <% for user in @users %>
    <tr>
      <td><%= link_to user.name, :action => 'show_instructor',
        :id => user.id %></td>
      <td><%= user.fullname %></td>
      <td><%= user.email %></td>
      <td><%= (User.find(user.parent_id)).name  %></td>
    </tr>
  <% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

  <br>
  <%= text_field_tag('name', '', {'size' => 30}) %>
   
  <input type='submit' value='New Instructor'/>
<% end %>

After Changes

 <h1>Instructors</h1>
 
 <%= will_paginate @users %>
 
 <table>
   <tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
-  <% for user in @users %>
+  <% for user in User.all %>
+  <% if user.role_id == 2%>
     <tr>
       <td><%= link_to user.name, :action => 'show_instructor',
         :id => user.id %></td>
       <td><%= user.fullname %></td>
       <td><%= user.email %></td>
       <td><%= (User.find(user.parent_id)).name  %></td>
     </tr>
   <% end -%>
+  <% end -%>
 </table>
 
 <hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
-  <br>
-  <%= text_field_tag('name', '', {'size' => 30}) %>
-   
-  <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
 <% end %>

Case 6: Refactoring list_super_administrators.html.erb

Before Changes

<h1>Super-Administrators</h1>

<table>
  <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
  <% for user in @users %>
    <tr>
      <td><%= link_to user.name, :action => 'show',
        :id => user.id %></td>
      <td><%= user.fullname %></td>
      <td><%= user.email %></td>
      <td><%= (User.find(user.parent_id)).name  %></td>
    </tr>
  <% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

After Changes

 <h1>Super-Administrators</h1>
 
 <table>
   <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
   <% for user in @users %>
     <tr>
       <td><%= link_to user.name, :action => 'show',
         :id => user.id %></td>
       <td><%= user.fullname %></td>
       <td><%= user.email %></td>
-      <td><%= (User.find(user.parent_id)).name  %></td>
+      <td><%= (User.find(user.id)).name  %></td>
     </tr>
   <% end -%>
 </table>
 
 <hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+  <input type='submit' value='New Super Administrator'/>
+<% end %> 

Case 7: Addition of the institution id in the view

<p><label for="user_institutions">Institution</label><br/>
+    <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>

Before Changes

<%= error_messages_for 'user' %>

<!--[form:user]-->
<p><label for="user_name">Username</label><br/>
  <%= text_field 'user', 'name'  %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
<!--[eoform:user]-->
<br/>

After Changes

@@ -1,13 +1,14 @@
 <%= error_messages_for 'user' %>
 
 <!--[form:user]-->
 <p><label for="user_name">Username</label><br/>
   <%= text_field 'user', 'name'  %></p>
 
 <% new_user ||= false %>
 <%= render :partial => 'users/name' %>
 <%= render :partial => 'users/email' %>
 <%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
 <%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>
 <!--[eoform:user]-->
 <br/>

Before Changes

  resources :admin do
    collection do
      get :list_super_administrators
      get :list_administrators
      get :list_instructors
      get :new_administrator

After Changes

  resources :admin do
     collection do
+      get :create_any_user
       get :list_super_administrators
       get :list_administrators
       get :list_instructors