CSC/ECE 517 Fall 2013/oss ssp

OSS E803 Refactoring and testing — signup_sheet model

Motivation

The motivation of this project was to provide functionality to manage Sign Up Sheets. The intent was to improve the design of the project by modularizing the code and maintaining good naming conventions. The methods relevant to wait-listing a team were all a part of the Sign Up sheet model and controller. in order to improve the readability of the code and to have a better organization, we intended to move these methods to a separate Waitlist Model. Also, some methods had too many functionalities, a lot more than what their names suggested. To have a more elegant code, the functionalities had to be moved to relevant methods. cancel_all_waitlists and waitlist_teams methods were created in the waitlist model for this purpose. Naming conventions are considered very important for making the code more readable and understandable. Ruby is known for its 'Convention over Configuration' ideology. Thus the naming conventions had to be taken care of. In the Sign Up sheet controller, we could see the usage of Camel cases in a lot of places. We intended to replace these by the convention of Ruby where underscore is used between two words in variable names with all the alphabets in small letters.

Design

Initial Design

• Initially the sign_up_sheet_controller had huge methods with no comments and multiple variable decalarations.
• The other major drawback with the initial design was there were redundant methods in sign_up_sheet_controller and sign_up_controller.

Modified Design

The refactoring required us to modularize and push some of the functionalities in sign_up_sheet_controller.rb to sign_up_topic.rb (model) and waitlist.rb ( model). This demanded to make calls from remove_team and reassign_topic from sign_up_topic model to signup_topics method in sign_up_sheet_controller. Similarly the controller also used methods waitlist_teams and cancel_all_waitlists from the waitlist model.

• Refactoring the sign_up_sheet_controller for modularity and flexibility:

1. By defining functions in the desired class and making only calls to the specific controller, we have implemented a Collaborative design based on CRC modelling:
   1. finding classes/ models - sign_up_topic.rb and waitlist.rb
   2. finding responsibilities - remove_team, reassign_topic in sign_up_topic.rb, cancel-all_waitlists, waitlist_teams in waitlist.rb
   3. defining the collaborators - confirm_topic, delete_signup_topic, signup_topics in sign_up_sheet_controller.rb
2. This has also made it more flexible for the developers as a change in the db or table design can be easily tracked.
   1. Easily implemented in the respective models and by passing the correct type/number parameters in the controllers.
   2. Reduces overhead of rewriting methods and keeping track of the method calls.

• Adding a waitlist model:

1. The newly formed waitlist model follows the Singleton pattern as it is an independent class and also the waitlist for a topic is a single queue or list on which multiple teams and individuals wait on.
2. The methods defined in waitlist model are singleton methods and thus are defined as class methods with the keyword "self" prepended to them.

• Removing Redundancy from sign_up_controller

1. The methods such as sign_up were redundantly present in both the controllers.
2. Some part of the code which had common functionality were removed and the function calls were also redirected accordingly to the specific views.

Issues

• Refactoring issues

1. Since we had two controllers for the signup sheet, there were many functions which had nested calls and needed to be taken care.
   1. For example, slotAvailble?() was called from both controller and the sign_up_topic model.
2. As waitlist was created as a singleton class with no table structure it was difficult to make use of attributes of other classes in it as attributes can be inherited or used only if they have associativity at schema level.
3. By defining the functions in the models, we had to take care of the dependencies of the instance variables defined and used in the controller and interaction between several models had to be taken care.
4. Controllers have the flexibility to directly make use of the parameters from the specific views whereas defining them in models had to carefully set and passed. Since some of the parameters were used in multiple views, it needed to be updated in several places.
5. Some of the methods that were modularised were conflicting as they were modified to class methods from being an instance method.

• Redundant Test issues

1. Since the code has been developed over the years and refactored by multiple developers, there were several redundant tests and identifying them was a huge task.
2. The rspec environment variable along with the spec_helper had to be freshly created for the test framework suite to be recognized.

• Deployment issues

1. Merging of branches from the local to the remote repository at the right level was initially being an overhead.
2. This was overcome by cloning a specific branch using: git clone -b <branch_name> <remote_url>.
3. Earlier commits do not have a proper commit message to refer.

Procedure

A brief description

Our OSS project required us to do refactoring and testing of the part of Expertiza that deals with signup,mainly sign_up_sheet controller. This controller manages functionalities of signup sheet for assignment. We have moved a few methods from controller to a more suitable model in order to attain the “thin controller, fat model” implementation. A major part includes the actions related to waitlist. We have put methods from sign_up_sheet controller to the waitlist model (which was earlier empty, so we created a waitlist model by adding functionalities to it) A brief description of the refactoring that we have done is given below.

Add teams to waiting list

We have moved the functionality that adds teams to waiting list when the topic is unavailable. We have moved this functionality from confirm_topic in sign_up_sheet controller to waitlist_teams method that we have created in waitlist controller.

This shows the method confirm_topic in sign_up_sheet controller that contains call to waitlist_teams method in waitlist controller.

This method has definition for waitlist_teams and is defined in waitlist model.

Cancel waitlist

We have moved the functionality that removes a team from the waiting list, from sign_up_topic model to waitlist model. The method name is cancel_all_waitlist.

Reassign a topic

We have moved the functionality that reassigns a topic to another team when a team holding that particular topic cancels its reservation. We have moved this functionality from delete_signup_for_topic in sign_up_sheet controller to a method named reassign_topic that we have created in sign_up_topic model.

This shows the method delete_signup_for_topic in sign_up_sheet controller that contains call to reassign_topic method in sign_up_topic model.

This method has definition for reassign_topic and is defined in sign_up_topic model.

Remove team from topic

We have moved the functionality that removes a team from a topic when all of the team’s members have left the team. We have moved this functionality from signup_topics in sign_up_sheet controller to a method named remove_team that we have created in sign_up_topic model.

This shows the method signup_topics in sign_up_sheet controller that contains call to remove_team method in sign_up_topic model.

This method has definition for remove_team and is defined in sign_up_topic model. Rest, we have done renaming at a few places in sign_up_sheet controller.

Snapshots

Here, we have a few snapshots that would guide one to walk-through our work and the areas wherein we did refactoring.

Login as an admin.

Go to manage->courses

Create an assignment by filling in the required fields and selecting the checkboxes.

Create users by filling in the required fields.

Again go to Manage courses and as shown in the snapshot above, one can see actions associated with an assignment. Click on add participants.

Add a user to an assignment as a participant.

The participant is added.

Also, through manage-> courses, add signup sheet to an assignment in a similar manner as adding participants.

When a signup sheet is created, add new topics to it in the similar manner

Topic is successfully created. Logout of admin system

Now login as an user.

Go to the assignments on top

Select one assignment from the list of assignments.

Select signup sheet

One can view signup sheet and signup for the topic

After signing up, here for example for topic 1234, the user has the topic and the no. of available slot shows 0

Now, if another user tries to sign up for the same topic, he is put on waitlist.

This is a brief illustration of the changes that we have made as a part of our project.

Testing

Testing was done using rspec as most of our work involved moving from the controller class and as rspec allows to create a single http request for each test case.

Set up environment for Testing

1. Go to Run -> Edit Configuraions ->RSpec -> Select All tests in folder and Test file name mask: **/*_spec.rb
2. Check the run the script in context of the bundle
3. Configuration -> Working directory must be pointed to the root of the application. Apply the changes
4. Run rspec:install form Rails Generator to create a rspec_helper.rb which will have the RAILS_ENV(= 'test')
5. rake db:migrate
6. rake spec
7. run all tests in spec.

Sample Test Case

describe "GET show" do
   it "assigns the requested signup_topic as @signup_topic" do
     signup_topic = SignupTopic.create! valid_attributes
     get :show, {:id => signup_topic.to_param}, valid_session
     assigns(:signup_topic).should eq(signup_topic)
   end
 end

 describe "GET new" do
   it "assigns a new signup_topic as @signup_topic" do
     get :new, {}, valid_session
     assigns(:signup_topic).should be_a_new(SignupTopic)
   end
 end

Future Work

The sign up sheet controller still has many methods relevant to other entities which need to be moved to other relevant models and controllers. save_topic_dependancies and save_topic_deadlines are two methods which can be put into SignUpTopic Model or controller. show_team method which is in sign_up_sheet controller can be moved to teams_controller. We also see another controller called SignUp controller which looks like a redundant one which has methods mostly from SignUpSheet controller. And these methods don't seem to be used. So this method can be looked into carefully and discarded if possible. There is also a function called slotAvailable? in the sign up sheet controller which merely makes a call to a method slotAvailable? in the SignUpTopic model and does nothing else. This looks like an unnecesary redirection. Dependencies can be resolved and this method can be removed and calls can directly be made to the slotAvailable? method in the SignUpTopic model. Also the method update_waitlisted_users can be moved from SignUpTopic model to Wailist Model to improve the readability of the code.