CSC/ECE 517 Fall 2021 - E2123. Refactor sign up sheet controller.rb

This wiki page is for the information regarding the changes made for the E2123 OSS assignment for Fall 2021, CSC/ECE 517.

Introduction

Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

Peer Review Information

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:

• Instructor login: username -> instructor6, password -> password

About Controller

The sign up sheet controller

• Allows an instructor to add/remove topics
• Allows an instructor to assign/remove students to topics

Issues

1. Naming is inconsistent. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf. “sign_up_as_instructor”. Please make the naming consistent. Of course, this will result in changes in calling methods as well.

2. Create method has and if-else condition determining if create or update should be called. Create method should not be responsible for calling update. Identify why the if-else condition exists and rectify it. [This may have already been fixed.]

3. Update method has a plethora of instance variables defined before updating. These might not be necessary (For e.g., look at update method of bookmarks_controller)Decide whether so many instance variables are really needed. Refactor the variables not needed out. [This may have already been fixed, but save_topic_deadline also has these problems.]

4. Add_signup_topics_staggered does not do anything different from add add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set.

5. Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)

6. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.

7. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.

8. Refactor participants variable in load_add_signup_topics

9. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

10. Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

Solutions

1.

2.

3.

4.

5.

6.

7.

8.

9. Previously signup_as_instructor_action provided unhelpful feedback to the instructor by only stating if the instructor could/couldn't assign a topic for a student. The refactored code provides the instructor with the student id of the person that they are assigning, the team that the student is on, and the topic that the instructor is assigning to the team.

Before:

def signup_as_instructor_action
    user = User.find_by(name: params[:username])
    if user.nil? # validate invalid user
      flash[:error] = "That student does not exist!"
    else
      if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
        if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
          flash[:success] = "You have successfully signed up the student for the topic!"
          ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up student for topic: ' + params[:topic_id].to_s)
        else
          flash[:error] = "The student has already signed up for a topic!"
          ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a student who already has a topic')
        end
      else
        flash[:error] = "The student is not registered for the assignment!"
        ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'The student is not registered for the assignment: ' << user.id)
      end
    end
    redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
  end
  

10.