CSC/ECE 517 Spring 2015/oss E1501 YWS

E1501: Refactoring Assignments.rb

This page provides information about a project which aims to refactor Assignment.rb file present in Expertiza OSS.

Introduction to Expertiza

Expertiza 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 from GitHub. This application provides an efficient way to manage assignments, grades and reviews, which makes the process easier and faster when the class strength is large.

Assignment.rb is a file which contains the assignment model which is used to create and edit assignments and the related functionality. It also contains the grading module which is refactored to make the file more clean and easy to read.

Problem Statement

Classes involved:

assignment.rb

What needs to be done: The Topics tab in the Assignments --> Edit view is not usable right now by an instructor when he/she is editing an assignment. The functionality that displays topics also needs to be usable by the student view, since students also have to view the list of topics (when they sign up for a topic). Instructor and student functionality is a little different: instructors can add topics and change the number of slots, but they can’t sign up for topics. The Topics tab needs to be fixed, edit/delete topic functionality is to be added for instructors and common/duplicate code needs to be refactored.

The functionality to display the topics under an assignment is duplicated between the AssignmentController views and the SignupSheetController views. Both the instructor and the student can view the topics by selecting an assignment and we were required to remove this duplication in the views. Consequently, most of our work dealt with modifying the SignupSheetController and its corresponding views.

Changes Made

SignupSheet Controller

Method Name	Changes Made	Reason For Change
create	Fixed redirection after topic creation to Topics tab	On successful creation of a topic, the action was merely redirecting to the assignments/edit view
update	Fixed redirection after topic updation to Topics tab	Once a topic was successfully edited, the view rendered was just a list of topics
edit	Initialized @assignment_id variable	The variable is used in the edit view
destroy	Renamed the 'delete' method to 'destroy'	This was done for better Rails 4 routing compatibility
	Initialized the assignment_id parameter	This is used further on and is not passed from the link URL
	Fixed redirection after topic deletion to Topics tab	Once a topic was successfully deleted, the view rendered was just a list of topics

Views

View Name	Changes Made	Reason For Change
sign_up_sheet/_add_signup_topcis	Fixed title	The page title displayed for an instructor was "Sign-up sheet for ... "
	Commented out View/Manage bookmark links	These links were broken, and according to existing code were not meant for the instructor view.
	Added variable initialization for @sign_up_topics	The variable was referenced further on in the view, which was causing an 'nil value' error
	Fixed flash message rendering for new topic creation	The flash message was rendering incorrectly with HTML tags visible on the web page
	Added full path names to partial names	The shared partials were also being used by views in the Assignments controller, so full paths were needed
sign_up_sheet/_add_topics	Added separation between the Import Topics and Manage Bookmarks links	The two links were clumped together and it was difficult to distinguish between them
sign_up_sheet/_actions	Replaced rendering of the reserve_topic partial with the _all_actions partial	The reserve_topic partial is no longer being used and is replaced by the newly created _all_actions partial
sign_up_sheet/_all_actions	Created this new partial	We created the _all_actions partial to conditionally render the "Actions" table field for both instructors and students. This now also includes working links through which instructors or administrators can edit or delete topics.
sign_up_sheet/_table_line	Gave edit/delete rights to Super Admin	The Super-Administrator user wasn't included in the list of users with these permissions
sign_up_sheet/edit	Modified title and included @assignment_id variable to be passed as a parameter variable to the update action	The view was initially trying to access the params[:assignment_id] variable which was not getting initialized
assignments/edit	Changed the partial rendered to /sign_up_sheet/add_signup_topcis.html	The assignments/edit/add_signup_topics partial (with duplicated code) was otherwise being rendered

Re-factored Code Cases

Case 1 : Refactoring the AssignmentsController and SignupSheetController

Removing duplicate _add_signup_topics partials

The AssignmentsController displayed topics to the instructor while he was editing an assignment. It used _add_signup_topics.html.erb partial to render topics under the topics tab. The SignupSheetController displayed topics to a student who wanted to view signup topics under an assignment. It was found that it had its own copy namely _add_signup_topcis.html.erb to display topics to the student. We wanted to have these two controllers use the same views to render the same thing which was list of topics under an assignment. We made the AssignmentsController reuse the _add_signup_topcis.html.erb partial under the SignupSheetController.

view:assignments/edit.html.erb
This view contains all the tabs to display like general,rubrics,review_strategy, topics which are related to the selected assignment. tabs-5 is the topics tab which we are interested in.

   <div id="tabs-3">
        <%= render 'assignments/edit/review_strategy' %>
   </div>
   <div id="tabs-4">
        <%= render 'assignments/edit/due_dates' %>
   </div>
   <div id="tabs-5">
        <%= render 'assignments/edit/add_signup_topics' %>
   </div>

After Changes

We have redundant code in two methods so we merged add_signup_topics_staggered and add_signup_topics into add_signup_topics. The action calls single method for both type of assignments and renders views based on condition of staggered or non-staggered assignment.

   <div id="tabs-3">
        <%= render 'assignments/edit/review_strategy' %>
   </div>
   <div id="tabs-4">
        <%= render 'assignments/edit/due_dates' %>
   </div>
   <div id="tabs-5">
        <%= render '/sign_up_sheet/add_signup_topcis.html' %>
   </div>

Refactoring view code into relevant views

There were no edit/delete links to edit/delete a topic in the instructor view. These had to be conditionally added based on the role of the currently logged in user. This was to prevent students from editing/deleting topics and to keep instructors from signing up for a topic. This was implemented as follows.

Adding Edit/Delete functionality to topics tab

The current expertiza project had two views to display the Actions column in the topics table. These were _reserve_topics.html.erb and _actions.html. The first one was displaying the signup button for the student and the latter was displaying the bookmarks and rendering the _reserve_topics.html.erb after the bookmarks. On top of this we had to add the edit/delete links for the instructor. A refactor was called for. We decided to create a _all_actions.html.erb partial which had the responsibility to display the relevant actions depending upon the role of the currently logged in user. This helped us remove the separate partial to render actions pertaining to a student, namely _reserve_topics.html.erb . Also the _actions.html.erb only took care of displaying the bookmark links rather than dealing with both bookmarks and actions.

partial_view:_all_actions,html.erb

<% if ['Instructor', 'Teaching Assistant', 'Administrator', 'Super-Administrator'].include? current_user_role?.name %>
    <td align="center">
      <%= link_to image_tag('edit_icon.png', :border => 0, :title => 'Edit Topic', :align => 'middle'), :controller=>'sign_up_sheet', :action=> 'edit', :id => topic.id %>
      <%= link_to image_tag('delete_icon.png', :border => 0, :title => 'Delete Topic', :align => 'middle'), sign_up_sheet_path(topic.id), method: :delete, data: {confirm: 'Are you sure?'} %>
<%elsif @show_actions %> 
    ... 
    #Render signup button for student here. The @show_actions will be true if this partial is navigated from the SignupSheetController which is accessible only to students.

Case 2 : Cleaning up unused code

The Refactors mentioned above enable us to delete unused partials namely:

• assignments controller partials :
  • _add_signup_topics.html.erb
  • _reserve_topic.html.erb

• signup_sheet_controller partials :
  • _reserve_topic.html.erb

See Also

1. Expertiza on GitHub
2. GitHub Project Repository Fork
3. The live Expertiza website
4. VCL link - This might not be available after Nov. 2014
5. Expertiza project documentation wiki
6. The OSS project requirements doc (Fall 2014 - Expertiza)

References

Expertiza
Code Refactoring
Code smell