CSC/ECE 517 Fall 2014/oss E1453 syy
E1453. Refactoring AssignmentsController
This page provides a description of the Expertiza based OSS project. This project aimed at refactoring the AssignmentsController as well as adding some missing functionality related to topics within an assignment.
Introduction to Expertiza
Expertiza is a project developed using Ruby on Rails platform. It provides features like peer review, team assignments and submission of projects. This can be achieved by submitting code base, URL of hosted code on remote server and Wiki submissions. It is an open source application and the code can be cloned from github. This application provides an efficient way to manage assignments, grades and reviews. This makes the process easier and faster when the class strength is large.
Expertiza is supported by National Science Foundation under Grant No. 0536558. Additional funding from the NCSU Learning in a Technology-Rich Environment (LITRE) program, the NCSU Faculty Center for Teaching and Learning, the NCSU STEM Initiative, and the Center for Advanced Computing and Communication.
Classes involved:
assignment_controller.rb sign_up_sheet_controller.rb assignment.rb sign_up_sheet.rb
What they do: The assignment model and controller are used to create and edit assignments. It provides tabs for rubrics, review strategy, due dates, and topics. The sign_up_sheet model and controller handle operations on the topics associated with each assignment.
What needs to be done: The Topics tab in the Assignments --> Edit view is not usable right now. 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.
Problem Statement
Currently the topics tab is not usable to the instructor when he is editing a current assignment or creating a new one. This functionality is necessary as it enables the instructor to create/delete/edit/view topics present under the selected assignment. Currently, the instructor achieves this using the pop-up menu that appears adjacent every assignment on the page that lists all the assignments. We were required to implement the topics tab.
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.
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
Steps to verify changes
Log into the application with the user having instructor's role (user3, password). VCL application link
1. Go to Manage -> Assignments and expand the assignments section.
2. Hover over the options symbol adjacent to the assignment and select the edit icon from the popup.
3. We have pre-created an assignment with multiple topics with id. 253, so you can click this link to edit it - http://152.46.18.5:3000/assignments/253/edit
4. Click on topics tab.
5. You can edit or delete any topic here as you are in instructor role. Also you can undo, redo any changes you have made.
4. Now once you have created two new topics , then Click on “Show start/due date”.
5. You should be able to view the new start and due dates of your topics confirming these are the staggered deadlines for “StaggeredDeadlines_Refactoring” which has many topics with different deadlines.
6. Now to view the difference between the Stagerred and Non-Staggered Deadlines, follow steps.
7. Go to Manage -> Assignments and hover over “SALT Demo” and click on “Edit Signup sheet”.
8. Click on “New Topic” and fill in the details for a new topic.
9. Now you will be able to see that there is no option for “Show Start/Due Date” which is because of the reason that this is not a staggered assignment.
10. Hence there is only one due date and has no dependencies on others.
Concise Report on Refactoring
The project had redundant code, multiple functionalities in a method and extra conditional statements in controller, model and view files. We analyzed each occurrence of these issues, thus creating a list which needed to be refactored. Controllers like sign_up_sheet_controler, assignment.rb, due_date.rb,deadline_types.rb along with other files were modified. It's very difficult to explain the sequential and detailed procedure, but it has been explained in the sections above within the capacity of the Wiki.
Additional Learning and Future Work
In current implementation, our database has two tables for staggered and non-staggered deadlines. For any future improvements, we can make use of STI (Single Table Inheritance) and analyze if applying STI would really impact query performance when application runs and extracts data from database.
See Also
- Expertiza on GitHub
- GitHub Project Repository Fork
- The live Expertiza website
- VCL link - This might not be available after Nov. 2014
- Expertiza project documentation wiki
- The OSS project requirements doc (Fall 2014 - Expertiza)
References
Expertiza
Single Table Inheritance
Code Refactoring
Code smell