CSC/ECE 517 Spring 2015/oss E1509 lds

E1509. Refactoring SignUpController and SignUpSheetController

This page talks about an open source project based on Expertiza. As a part of contribution to Expertiza, this project aims to refactor SignUpController and SignUpSheetControllers.

Introduction to Expertiza

Expertiza is a web application developed by Ruby on Rails framework. It serves as a peer review system for professors and students at NC State and some other colleges and universities. Students can submit different assignments and peer-review reusable learning objects (articles, code, web sites, etc)<ref>https://github.com/expertiza/expertiza</ref>. The Expertiza project is supported by the National Science Foundation. And it is an open source application and the source code can be cloned on Github .

Problem Statement

Classes involved

* sign_up_sheet.rb 
* sign_up_topic.rb 
* assignments_controller.rb
* response.rb
* and possibly other model classes

What it does

Lists topics available for an assignment, checks whether a user is signed up for a topic, allows users to sign up for topics.

What's wrong with it

• These two controllers seem to do almost the same thing. They have many of the same methods. SignUpSheetController is much longer, and has many more recent mods, but some methods of SignUpController seem more sophisticated than SignUpSheetController. So, your first job is to figure out if both controllers are being used. If not, remove the unused controller. Or, move the functions to a single controller if that makes sense to do.
• Neither controller is at all RESTful; i.e.., its method names aren’t the standard names new, create, edit, delete, etc. Functionality is divided differently than in a standard controller.

   a. def confirm_topic(creator_id, topic_id, assignment_id)

   b. def delete_signup

   c. def delete_signup_for_topic(assignment_id,topic_id)

   d. def other_confirmed_topic_for_user(assignment_id, creator_id)

   e. def signup

   f. def slotAvailable?(topic_id)

• Functionality that should be in models is incorporated into the controller.
• Some methods are too long and appear to do more than one thing
• This class interfaces with assignments_controller so that a list of topics can be displayed when one is editing an assignment. Please be careful that your changes do not affect the functionality on the Topics tab of editing an assignment.
• Rename the controller(s) to SignupController and/or SignupSheetController. (“Sign up”, which gets written as SignUp in camel case, is a verb, whereas “Signup” is a noun.)

Modification We Made

RESTful style implementation

---

The purpose of the list method in the SignupSheet controller is to list all the topics in the specific assignments. According to RESTful rules, a method that returns a list of all available objects should be named as index. Therefore, we renamed the list method to the index method in the controller and in all the files that had references to the list method of the signup_sheet_controller.

Before Refactoring :
Method : list

def list
  @assignment_id = params[:id]
  @sign_up_topics = SignUpTopic.where( ['assignment_id = ?', params[:id]]).all
  @slots_filled = SignUpTopic.find_slots_filled(params[:id])
  @slots_waitlisted = SignUpTopic.find_slots_waitlisted(params[:id])
  @show_actions = true
  @priority = 0
  assignment=Assignment.find(params[:id])
        #end
  if assignment.due_dates.find_by_deadline_type_id(1)!= nil
    unless !(assignment.staggered_deadline? and assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now )
      @show_actions = false
    end
    users_team = SignedUpUser.find_team_users(params[:id],(session[:user].id))
    if users_team.size == 0
      @selected_topics = nil
    else
      @selected_topics = SignUpSheetController.other_confirmed_topic_for_user(params[:id], users_team[0].t_id)
    end
    SignUpTopic.remove_team(users_team, @assignment_id)
  end
end

      def index
        puts "2222222 call list"
        @assignment_id = params[:id]
        @sign_up_topics =SignUpTopic.find_with_assignment_id( params[:id]).all
        filled_and_waitlisted_topics(@assignment_id)
        @show_actions = true
        @priority = 0
        assignment=Assignment.find(params[:id])
        #end
        if assignment.due_dates.find_by_deadline_type_id(1)!= nil
          unless !(assignment.staggered_deadline? and assignment.due_dates.find_by_deadline_type_id(1).due_at < Time.now )
            @show_actions = false
          end
          #Find whether the user has signed up for any topics; if so the user won't be able to
          #sign up again unless the former was a waitlisted topic
          #if team assignment, then team id needs to be passed as parameter else the user's id
          users_team = SignedUpUser.find_team_users(params[:id],(session[:user].id))
          if users_team.size == 0
            @selected_topics = nil
          else
            #TODO: fix this; cant use 0
            @selected_topics = SignupSheetController.other_confirmed_topic_for_user(params[:id], users_team[0].t_id)
          end
          SignUpTopic.remove_team(users_team, @assignment_id)
          end
        end

References

<references/>