CSC/ECE 517 Fall 2015/oss E1565 AAJ: Difference between revisions
(Added changes for admin_controller) |
|||
(37 intermediate revisions by 2 users not shown) | |||
Line 1: | Line 1: | ||
'''E1565: Refactoring Admin Controller and Course Controller''' | '''E1565: Refactoring Admin Controller and Course Controller''' | ||
This page | This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines. | ||
Refactoring | |||
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for: | |||
Programs that are difficult to read; | |||
Programs that does not follow the language specific guidelines; | |||
Programs that has high coupling; | |||
'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.''' | '''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== | ==Introduction to Expertiza== | ||
Expertiza is | [Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews. | ||
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read. | |||
==Problem Statement== | |||
<p>'''Files involved'''</p> | |||
<pre> | |||
admin_controller.rb | |||
views in app/views/admin folder | |||
user_controller</pre> | |||
<p>'''What they do: '''</p> | |||
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. | |||
''' | |||
'''What | <p>'''What needs to be done: ''' </p> | ||
* 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=== | |||
{| class="wikitable" | |||
|- | |||
! style="width:13%;"|Method Name | |||
! style="width:33%;"|Changes Made | |||
! style="width:43%;"|Reason For Change | |||
|- style="vertical-align:top;" | |||
| 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 === | ||
''' | {| class="wikitable" | ||
|- | |||
! style="width:13%;"|Method Name | |||
! style="width:33%;"|Changes Made | |||
! style="width:43%;"|Reason For Change | |||
|- style="vertical-align:top;" | |||
| create | |||
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name]) | |||
| Was throwing error | |||
|} | |||
===User Controller=== | |||
{| class="wikitable" | |||
|- | |||
! style="width:13%;"|Method Name | |||
! style="width:33%;"|Changes Made | |||
! style="width:43%;"|Reason For Change | |||
|- style="vertical-align:top;" | |||
| 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=== | |||
{| class="wikitable" | |||
|- | |||
! style="width:13%;"|Method Name | |||
! style="width:33%;"|Changes Made | |||
! style="width:43%;"|Reason For Change | |||
|- style="vertical-align:top;" | |||
| 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 controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way. | |||
===Before Changes=== | |||
<pre> | |||
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 | def add_administrator | ||
@user = User.new | @user = User.new | ||
end | 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 | def create_administrator | ||
save_administrator | save_administrator | ||
end | end | ||
def list_administrators | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
+ 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 | |||
</pre> | </pre> | ||
def | == Case 2 : Refactoring institution_controller == | ||
=== Before changes === | |||
<pre> | |||
def index | |||
list | |||
render :action => 'list' | |||
end | 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 | end | ||
</pre> | </pre> | ||
|} | |||
=== After Changes === | |||
<pre> | |||
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 | |||
</pre> | |||
== Case 3 : Refactoring user_controller == | |||
The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user. A mailer error is caused when a user/admin/instructor is created and the application crashes. But this is not because of an error with the functionality. Although, despite the error, one can check if the user has been created by going to Manage -> Users page in the user interface. | |||
===Before Changes=== | |||
<pre> | |||
@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 | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
@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 | |||
</pre> | |||
== Case 4 : Refactoring list_administrators == | |||
===Before Changes=== | |||
<pre> | |||
<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 %> | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
<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 %> | |||
</pre> | |||
== Case 5 : Refactoring list_instructors.html.erb == | |||
===Before Changes=== | |||
<pre> | |||
<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 %> | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
<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 %> | |||
</pre> | |||
== Case 6: Refactoring list_super_administrators.html.erb == | |||
===Before Changes=== | |||
<pre> | |||
<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> | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
<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 %> | |||
</pre> | |||
==Case 7: Addition of the institution id in the view== | |||
<pre> | |||
<p><label for="user_institutions">Institution</label><br/> | |||
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p> | |||
</pre> | |||
==Case 8: Adding the institution feature to user view== | |||
===Before Changes=== | |||
<pre> | |||
<%= 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/> | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
@@ -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/> | |||
</pre> | |||
==Case 9:Routes added == | |||
===Before Changes=== | |||
<pre> | |||
resources :admin do | |||
collection do | |||
get :list_super_administrators | |||
get :list_administrators | |||
get :list_instructors | |||
get :new_administrator | |||
</pre> | |||
===After Changes=== | |||
<pre> | |||
resources :admin do | |||
collection do | |||
+ get :create_any_user | |||
get :list_super_administrators | |||
get :list_administrators | |||
get :list_instructors | |||
</pre> | |||
=Steps to verify changes manually= | |||
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page. | |||
===Removing the text field from New Administrator=== | |||
[[File:List admin.png]] | |||
===Administrator as the default role=== | |||
[[File:New admin.png]] | |||
===Adding Institutions to User creation forms and having student as the default role=== | |||
[[File:Student.png]] |
Latest revision as of 22:04, 6 November 2015
E1565: Refactoring Admin Controller and Course Controller
This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines. Refactoring Refactoring[1] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for: Programs that are difficult to read; Programs that does not follow the language specific guidelines; Programs that has high coupling;
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][2] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews. Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.
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 controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
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
The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user. A mailer error is caused when a user/admin/instructor is created and the application crashes. But this is not because of an error with the functionality. Although, despite the error, one can check if the user has been created by going to Manage -> Users page in the user interface.
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>
Case 8: Adding the institution feature to user view
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/>
Case 9:Routes added
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
Steps to verify changes manually
In order to inspect the changes made, you can visit our deployed Expertiza and use the credentials mentioned at the top of the page.