CSC/ECE 517 Spring 2015/oss E1504 IMV

E1504. Refactoring the Bookmark Model

This page provides a description of the Expertiza based OSS project. This team project successfully refactored the Bookmark Model by removing the duplicate code, combining the methods, improving the search function, moving Bmapping model methods to their respective class, covering dependencies associated with changed segments and adding a User Interface for using bookmarks.

Introduction to Expertiza

Expertiza is a large project developed as a combined effort of students and faculty using the Ruby on Rails framework. The main advantage of using Expertiza, in an educational environment, is for the instructor to introduce peer reviewing among the students. Expertiza allows the instructor to create and customize assignments, create a list of topics the students can sign up for, have students work on teams and then review each other's assignments at the end. Expertiza supports submission of almost any document type, including the URLs and wiki pages.

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.

Problem Statement

Classes involved:

bookmark.rb
bmapping.rb
bookmarks_controller.rb
auth_controller.rb
auth_helper.rb

What they do: The bookmark model and controller work together to create and maintain user specific bookmarks, which can be added to different topics. Moreover, the user that created a bookmark can use his privilege to edit it at a later point in time. As a functionality added for convenience, any user can search bookmarks by either users who created them or their bookmark tags.

What needs to be done:

• The search methods in bookmarks model are being used used on a very granular level. This led to redundancy in bookmarks search methods. These methods include search_alltags_allusers, search_fortags_allusers, search_fortags_forusers, search_alltags_forusers. Hence duplicates in these methods are to be singled out.
• Following the CRUD declaration in Ruby on Rails, where the method name should specify the pseudo use of itself,methods such as add_new_bookmark and edit_this_bookmark have violate this naming convention. Therefore, we need to change the names of these two methods to create and edit, respectively. Upon changing the names, all the dependencies need to be updated, as well.
• Refactor add_topic_bookmark and add_this_bookmark by removing the repetitive code. These two methods differ only in 2 lines of code, which make it reasonable to merge them together into a single method that provides functionality of both individual methods.
• Fix the bug in add_bookmark method where only bookmarks without topic_id can be properly created. We add the functionality to this method to allow it to also create bookmarks with provided topic_id.
• Methods add_bmapping and add_bmapping_signuptopic need to be moved to their appropriate model (Bmapping model) rather than reside in Bookmarks model. Completing this task involves checking the dependencies and updating the function calls for these methods, accordingly.
• Add user interface for Bookmarks as this functionality has not been a part of Expertiza
• Create a functional tests suite covering the functionality of all the methods that have been changed by our team.
• Create a Cucumber integration test to show the functionality of added user interface for Bookmarks.

Changes Made

Bookmark Model

Method Name	Changes Made	Reason For Change
add_this_bookmark	Added functionality of add_topic_bookmark	To implement code reusability and remove repetitive code.
add_topic_bookmark	Merged the functionality into add_this_bookmarks and deleted add_topic_bookmark	To implement code reusability and remove repetitive code.
add_bookmark	Added a line that checks whether topic_id has been provided in the request	It allows creation of new Bmapping if no topic_id was given and removes the bug of searching for bookmark with nil topic_id later on
search_alltags_allusers	--------------	---------
search_alltags_foruser	--------------	---------
search_fortags_allusers	--------------	---------
search_fortags_foruser	--------------	---------
add_new_bookmark	Changed the method name to "create"	It is a much more meaningful and simpler name for a method that creates a new bookmark
edit_this_bookmark	Changed the name to "edit"	It is a much more appropriate name for a method that edits the bookmark it was called on
add_bmapping	Moved to class 'Bmapping'	This method helps in creating new bmapping. Hence it makes more sense if it belongs to bmapping class itself.
add_bmapping_signuptopic	Moved to class 'Bmapping'	This method helps in adding a signup topic to a newly created bmapping. Hence it makes more sense if it belongs to bmapping class itself.

Bookmark Controller

Method Name	Changes Made	Reason For Change
create	----------------	-----------
update	-----------------	--------
edit	--------------	---------
destroy	-----------------	---------
	---------------	----------
	----------	----------

Views

Method Name	Changes Made	Reason For Change
bookmarks/_result.html.erb	----------------	-----------
bookmarks/search_bookmarks.html.erb	-----------------	--------
bookmarks/view_bookmarks.html.erb	--------------	---------
bookmarks/_searchmine.html.erb	-----------------	---------
bookmarks/add_bookmark_form.html.erb	-----------------	---------
bookmarks/edit_bookmark_form.html.erb	-----------------	---------
bookmarks/managing_bookmarks.html.erb	-----------------	---------
layouts/application.html.erb	-----------------	---------
tree_display/list.html.erb	added a link to Manage Bookmarks page	This allows a user to see the link to Managing Bookmarks page as soon as the user logs in.

Re-factored Code Cases

Case 1 : Refactoring add_this_bookmark and add_topic_bookmark

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

 1:  def self.add_topic_bookmark(b_url, b_title, b_tags_text, b_description,session_user, topicid)
 2:    # Check if the bookmark exists and add / edit based on that
 3:    bookmark_exists = check_bookmark_exists(b_url)
 4:    bmapping_exists = check_bmapping_exists(b_url,session_user)
 5:    if (!bookmark_exists || !bmapping_exists)
 6:      Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user,topicid)
 7:    elsif (bmapping_exists)
 8:      Bookmark.edit_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
 9:    end
10:  end
11:
12:  def self.add_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
13:    bookmark_exists = check_bookmark_exists(b_url)
14:    bmapping_exists = check_bmapping_exists(b_url,session_user)
15:    if (!bookmark_exists || !bmapping_exists)
16:      Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
17:    elsif (bmapping_exists)
18:      Bookmark.edit_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user)
19:    end
20:  end

After Changes

  def self.add_this_bookmark(b_url, b_title, b_tags_text, b_description,session_user, topic_id)
    bookmark_exists = check_bookmark_exists(b_url)
    bmapping_exists = check_bmapping_exists(b_url,session_user)
    if (!bookmark_exists || !bmapping_exists)
      if(topic_id) # something has been passed for topic_id
        Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user,topic_id)
      else
        Bookmark.add_bookmark(b_url, b_title, b_tags_text, b_description,session_user, nil)
      end
    elsif (bmapping_exists)
      Bookmark.edit(b_url, b_title, b_tags_text, b_description,session_user)
    end
  end

Steps to verify changes manually

User2 Role

In order to inspect the changes made to the bookmark mode manually, you can use the following credentials:

    User Name:  user2
    Password:   any password

As you can notice in the credentials above, you can type in any password you wish in order to log in as user2. We have accomplished this by disabling the authentication and the reason behind it was to have an easily reachable user account for test and development purposes.

Automated Tests

Functional test suite

We have developed a suit of extensive unit tests covering all the functionality that we have add/modified in the forked version of Expertiza. The test suite consists of 9 tests providing 14 assertions that cover the functionality of all the method changes described in the former sections of this wiki page. The source code for these tests is stored in test/models/bookmark_test.rb file. You can run this test suite by simply navigating to expertiza project directory and running command:

ruby -I test test/models/bookmark_test.rb

Integration (Cucumber) tests

In addition to functional tests, we have also provided a Cucumber test as an integration test. The goal of this test is to show that a user can access the Manage Bookmarks page upon successful log in, proving that bookmarks view have been successfully integrated into expertiza system. The construct of this test can be found in following files:

features/bookmark.feature 
features/step_definitions/bookmark_steps

To run this cucumber test, please navigate to expertiza project directory and run the following command:

rake cucumber