CSC/ECE 517 Fall 2023 - E2357. Refactor sign up sheet controller.rb

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

Introduction

Expertiza is an 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.

About Controller

The sign up sheet controller contains all functions related to the management of the signup sheet for an assignment:

Allows an instructor to add/remove topics to an assignment.
Allows an instructor to edit properties of topics.
Allows an instructor to assign/remove students to topics
Allows a student to see the list of available topics which they can bid on for a given OSS assignment.

Login Credentials

UserId: instructor6

Password: password

Issues/Problem Statement

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.
Update method has a plethora of instance variables defined before updating. These might not be necessary (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.]
Add_signup_topics_staggered does not do anything different from add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set. [Assignment 1042 has staggered deadlines]
Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)
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.
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.
Refactor participants variable in load_add_signup_topics

[In retrospect, the meaning of this is not clear. The @participants variable is used in a way that is very obscure, with code spread across several views, and no comments saying what it is doing. It used to be that participants (individual students) signed up for topics. Now, only teams can sign up for topics. So @participants does not make sense.

Signup_as_instructor_action has if-else ladder. It can be made more elegant. [If it is worth keeping this method at all.]
Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.
Code Climate issues

Previous Version of Project

Solutions

The naming inconsistency issue has already been fixed by the previous version of the project. It has been verified by us.

Refactoring the Update method

The refactored version is more concise and readable. It uses the Rails 'update_attributes' method to update multiple attributes of the @topic object in a single call. It also sets a success flash message when the topic is updated successfully, which was missing in the original code. Additionally, it removes redundant calls to @topic.save after attribute assignments.Here is the link to GitHub commit.

After refactoring

def update
   @topic = SignUpTopic.find(params[:id])
   if @topic
     update_max_choosers @topic
     @topic.update_attributes(topic_identifier: params[:topic][:topic_identifier], category: params[:topic][:category],
     topic_name: params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description],link:params[:topic][:link] )
     undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
     flash[:success] = 'The topic has been updated.'
   else
     flash[:error] = 'The topic could not be updated.'
   end
   # Akshay - correctly changing the redirection url to topics tab in edit assignment view.
   redirect_to edit_assignment_path(params[:assignment_id]) + '#tabs-2'
 end

Improved Method names as follows

Here is the link to GitHub commit.

signup_as_instructor function signup_as_instructor_action is used to sign up a student for a particular topic for an assignment as an instructor. signup_as_instructor function does nothing. It is an empty function. The function is not being called anywhere in the code. The method is not needed and hence removed. Here is the link to GitHub commit.

def signup_as_instructor; end

Refactoring List Method

The refactored code organizes the list method by breaking it down into smaller, more specialized methods (handle_intelligent_assignment and handle_deadlines). Here, the refactored version improves code readability, maintainability, and organization by breaking down the method into smaller, specialized functions and separating concerns, resulting in a more structured and manageable codebase. Here is the link to GitHub commit.

Refactored Code

def list
   # Fetch the AssignmentParticipant based on the given ID
   @participant = AssignmentParticipant.find(params[:id].to_i)
   # Retrieve the assignment associated with the participant
   @assignment = @participant.assignment
   # Find the number of slots filled and waitlisted for sign-up topics
   @slots_filled = SignUpTopic.find_slots_filled(@assignment.id)
   @slots_waitlisted = SignUpTopic.find_slots_waitlisted(@assignment.id)  
   @show_actions = true
   @priority = 0
   # Fetch sign-up topics for the assignment
   @sign_up_topics = SignUpTopic.where(assignment_id: @assignment.id, private_to: nil)
   @max_team_size = @assignment.max_team_size
   # Fetch the team ID if exists
   team_id = @participant.team.try(:id)
   @use_bookmark = @assignment.use_bookmark
   if @assignment.is_intelligent
     handle_intelligent_assignment
   end
   # Extract code related to intelligent assignment handling
   # Find the number of sign-up topics available
   @num_of_topics = @sign_up_topics.size
   # Retrieve important deadline information
   @signup_topic_deadline = @assignment.due_dates.find_by(deadline_type_id: 7)
   @drop_topic_deadline = @assignment.due_dates.find_by(deadline_type_id: 6)
   # Fetch student bids if available
   @student_bids = team_id.nil? ? [] : Bid.where(team_id: team_id)
   # Check if certain deadlines have passed to determine whether to show actions
   handle_deadlines
   # Rendering the appropriate view based on the assignment type
   render('sign_up_sheet/intelligent_topic_selection') && return if @assignment.is_intelligent
 end
 # Handle intelligent assignment scenarios
 def handle_intelligent_assignment
   # Fetch the team ID (if it exists)
   team_id = @participant.team.try(:id)
   # Fetch bids for the team (if any) and order them by priority
   @bids = team_id.nil? ? [] : Bid.where(team_id: team_id).order(:priority)
   signed_up_topics = []
   @bids.each do |bid|
     sign_up_topic = SignUpTopic.find_by(id: bid.topic_id)
     signed_up_topics << sign_up_topic if sign_up_topic
   end
   # Find topics that have been signed up and update the available sign-up topics
   signed_up_topics &= @sign_up_topics
   @sign_up_topics -= signed_up_topics
   @bids = signed_up_topics
 end
 # Extract code related to handling deadlines into a separate method
 def handle_deadlines
   unless @assignment.due_dates.find_by(deadline_type_id: 1).nil?
     if !@assignment.staggered_deadline? && (@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
     users_team = SignedUpTeam.find_team_users(@assignment.id, session[:user].id)
     @selected_topics = if users_team.empty?
                          nil
                        else
                          SignedUpTeam.find_user_signup_topics(@assignment.id, users_team.first.t_id)
                        end
   end  
 end

Refactoring Signup_as_instructor_action

The refactored version assigns params[:assignment_id] and params[:topic_id] to assignment_id and topic_id variables, respectively, to avoid repetitive use of these parameters and increase code readability. The refactored version enhances readability by using meaningful variable names (assignment_id and topic_id), the string interpolation ("The student is not registered for the assignment: #{user.id}") is used, providing a more elegant way of embedding user.id within the string. The redirection statement is cleaner, referencing the previously defined assignment_id variable (redirect_to(controller: 'assignments', action: 'edit', id: assignment_id)), reducing repetition and enhancing code clarity. Here is the link to GitHub commit.

Refactored Code

 # Signs up a student for a specific topic in an assignment by an instructor
 def signup_as_instructor_action
   user = User.find_by(name: params[:username])
 # validate invalid user
   if user.nil?
     flash[:error] = 'That student does not exist!'
   else
     #assign params[:assignment_id] and params[:topic_id] to variables and use these variables to avoid repetition 
     assignment_id = params[:assignment_id]
     topic_id = params[:topic_id]
     if AssignmentParticipant.exists?(user_id: user.id, parent_id: assignment_id)
       if SignUpSheet.signup_team(assignment_id, user.id, 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: #{topic_id}"))
       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: assignment_id)
 end

Refactoring Delete_signup and delete_signup_as_instructor

A new method, delete_signup_common, centralizes the shared functionality, reducing redundancy and improving maintainability. The refactored code uses the who_user parameter to determine the type of user performing the deletion action, allowing a single method to handle both cases. By utilizing a shared method and parameters, the code is more concise, easier to understand, and maintain. Here is the link to GitHub commit.

Refactored Code

 def delete_signup_common(who_user,participant, assignment, drop_topic_deadline, topic_id)
 if !participant.team.submitted_files.empty? || !participant.team.hyperlinks.empty?
   if who_user == "instructor"
     flash[:error] = 'The student has already submitted their work, so you are not allowed to remove them.'
   else
     flash[:error] = 'You have already submitted your work, so you are not allowed to drop your topic.'
   end
   ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, "Dropping topic for already submitted work: #{topic_id}")
 elsif !drop_topic_deadline.nil? && (Time.now > drop_topic_deadline.due_at)
   if who_user == "instructor"
     flash[:error] = 'You cannot drop a student after the drop topic deadline!'
     ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
   else
     flash[:error] = 'You cannot drop your topic after the drop topic deadline!'
     ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
   end
 else
   if who_user == "instructor"
     delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
     flash[:success] = 'You have successfully dropped the student from the topic!'
     ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
   else
     delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
     flash[:success] = 'You have successfully dropped your topic!'
     ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
   end
 end
end
def delete_signup
 who_user = "student"
 participant = AssignmentParticipant.find(params[:id])
 assignment = participant.assignment
 drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
 topic_id = params[:topic_id]
 delete_signup_common(who_user,participant, assignment, drop_topic_deadline, topic_id)
 redirect_to action: 'list', id: params[:id]
end
def delete_signup_as_instructor
 who_user = "instructor"
 team = Team.find(params[:id])
 assignment = Assignment.find(team.parent_id)
 user = TeamsUser.find_by(team_id: team.id).user
 participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
 drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
 topic_id = params[:topic_id]
 delete_signup_common(who_user,participant, assignment, drop_topic_deadline, topic_id)
 redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end

Code Climate Issues

unexpected token error (Using Ruby 2.1 parser; configure using TargetRubyVersion parameter, under AllCops): Safe navigation operator &. is available from Ruby 2.3. Currently replaced it with nil check conditional statement.

"After Refactoring" code is the replacement of the safe navigation operator (&.) used in the original code with a combination of nil? and empty? checks to maintain the same functionality while being compatible with Ruby 2.1.Here is the link to GitHub commit.

After Refactoring

  flash[:error] = error_id unless error_id.nil? || error_id.empty?
   flash[:note] = message_id unless message_id.nil? || message_id.empty?

Method new_feedback has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring

The refactored code utilizes if params[:id].present? instead, which is a more concise way to check if the params[:id] is not nil or not an empty string, maintaining the same functionality. We have assigned session[:user].id to current_user and review.map.assignment to assignment, which enhances readability and code clarity. The refactored code consolidates the logic to find or create a FeedbackResponseMap based on certain conditions and maintains better code structure with straightforward if-else logic.

After Refactoring

def new_feedback
     # Replaced unless params[:id].nil? with if params[:id].present? for a more concise condition
   review = Response.find(params[:id]) if params[:id].present?
 
   if review
     #Assigned session[:user] and  review.map.assignment to variables for clarity
     current_user = session[:user]
     assignment = review.map.assignment
     reviewer = AssignmentParticipant.where(user_id: current_user.id, parent_id: assignment.id).first
     map = FeedbackResponseMap.where(reviewed_object_id: review.id, reviewer_id: reviewer.id).first
     
     # if no feedback exists by dat user den only create for dat particular response/review
     if map.nil?
       map = FeedbackResponseMap.create(
         reviewed_object_id: review.id,
         reviewer_id: reviewer.id,
         reviewee_id: review.map.reviewer.id
       )
     end
 
     redirect_to action: 'new', id: map.id, return: 'feedback'
   else
     redirect_back fallback_location: root_path
   end
 end

Method edit has 29 lines of code (exceeds 25 allowed). Method edit has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring

The edit method is now broken down into smaller, more focused methods such as load_response_data and handle_team_reviewing.Each method is responsible for specific tasks: load_response_data handles the loading and processing of response data, while handle_team_reviewing deals with team reviewing scenarios.This separation of concerns improves code organization and readability.

After Refactoring

def edit
   assign_action_parameters
   load_response_data
   render action: 'response'
 end
 
 
 def handle_team_reviewing
   return unless @map.team_reviewing_enabled
 
   @response = Lock.get_lock(@response, current_user, Lock::DEFAULT_TIMEOUT)
   response_lock_action if @response.nil?
 end
 
 def load_response_data
   @prev = Response.where(map_id: @map.id)
   @review_scores = @prev.to_a
 
   if @prev.present?
     @sorted = @review_scores.sort do |m1, m2|
       if m1.version_num.to_i && m2.version_num.to_i
         m2.version_num.to_i <=> m1.version_num.to_i
       else
         m1.version_num ? -1 : 1
       end
     end
     @largest_version_num = @sorted[0]
   end
 
   @map = @response.map
   handle_team_reviewing
   @modified_object = @response.response_id
   set_content
   @review_scores = []
   @review_questions.each do |question|
     @review_scores << Answer.where(response_id: @response.response_id, question_id: question.id).first
   end
   @questionnaire = questionnaire_from_response
 end

Method redirect has 27 lines of code (exceeds 25 allowed). Consider refactoring.

The refactored code changes the conditional check for setting flash messages to use nil? and empty? instead of the safe navigation operator (&.), as the safe navigation operator is available from Ruby 2.3 onwards. The refactored version separates the logic for redirection based on different params[:return] values into the redirect_based_on_return method. It centralizes and isolates the redirection logic for improved readability and maintainability.

After Refactoring

def redirect
   error_id = params[:error_msg]
   message_id = params[:msg]
   flash[:error] = error_id unless error_id.nil? || error_id.empty?
   # Safe navigation operator &. is available from Ruby 2.3. Currently replaced it with nil check conditional statement 
   flash[:note] = message_id unless message_id.nil? || message_id.empty?
   
   @map = Response.find_by(map_id: params[:id])
   
   redirect_based_on_return
 end
  def redirect_based_on_return
   case params[:return]
   when 'feedback'
     redirect_to controller: 'grades', action: 'view_my_scores', id: @map.reviewer.id
   when 'teammate'
     redirect_to view_student_teams_path student_id: @map.reviewer.id
   when 'instructor'
     redirect_to controller: 'grades', action: 'view', id: @map.response_map.assignment.id
   when 'assignment_edit'
     redirect_to controller: 'assignments', action: 'edit', id: @map.response_map.assignment.id
   when 'selfreview'
     redirect_to controller: 'submitted_content', action: 'edit', id: @map.response_map.reviewer_id
   when 'survey'
     redirect_to controller: 'survey_deployment', action: 'pending_surveys'
   when 'bookmark'
     bookmark = Bookmark.find(@map.response_map.reviewee_id)
     redirect_to controller: 'bookmarks', action: 'list', id: bookmark.topic_id
   when 'ta_review' # Page should be directed to list_submissions if TA/instructor performs the review
     redirect_to controller: 'assignments', action: 'list_submissions', id: @map.response_map.assignment.id
   else
     # if reviewer is team, then we have to get the id of the participant from the team. the id in reviewer_id is of an AssignmentTeam
     reviewer_id = @map.response_map.reviewer.get_logged_in_reviewer_id(current_user.try(:id))
     redirect_to controller: 'student_review', action: 'list', id: reviewer_id
   end
 end

Method create has a Cognitive Complexity of 8 (exceeds 5 allowed). Method create has 29 lines of code (exceeds 25 allowed). Consider refactoring:

Before Refactoring

def create
   map_id = params[:id]
   unless params[:map_id].nil?
     map_id = params[:map_id]
   end # pass map_id as a hidden field in the review form
   @map = ResponseMap.find(map_id)
   if params[:review][:questionnaire_id]
     @questionnaire = Questionnaire.find(params[:review][:questionnaire_id])
     @round = params[:review][:round]
   else
     @round = nil
   end
   is_submitted = (params[:isSubmit] == 'Yes')
   # There could be multiple responses per round, when re-submission is enabled for that round.
   # Hence we need to pick the latest response.
   @response = Response.where(map_id: @map.id, round: @round.to_i).order(created_at: :desc).first
   if @response.nil?
     @response = Response.create(map_id: @map.id, additional_comment: params[:review][:comments],
                                 round: @round.to_i, is_submitted: is_submitted)
   end
   was_submitted = @response.is_submitted
   # ignore if autoupdate try to save when the response object is not yet created.s
   @response.update(additional_comment: params[:review][:comments], is_submitted: is_submitted)
   # :version_num=>@version)
   # Change the order for displaying questions for editing response views.
   questions = sort_questions(@questionnaire.questions)
   create_answers(params, questions) if params[:responses]
   msg = 'Your response was successfully saved.'
   error_msg = 
   # only notify if is_submitted changes from false to true
   if (@map.is_a? ReviewResponseMap) && (!was_submitted && @response.is_submitted) && @response.significant_difference?
     @response.notify_instructor_on_difference
     @response.email
   end
   redirect_to controller: 'response', action: 'save', id: @map.map_id,
               return: params.permit(:return)[:return], msg: msg, error_msg: error_msg, review: params.permit(:review)[:review], save_options: params.permit(:save_options)[:save_options]
 end

After refactoring

def create
 setup_response
 process_response
 finalize_response
end

def setup_response
 map_id = params[:map_id].presence || params[:id]
 @map = ResponseMap.find(map_id)
 if params[:review][:questionnaire_id]
   @questionnaire = Questionnaire.find(params[:review][:questionnaire_id])
   @round = params[:review][:round]
 else
   @round = nil
 end
end

def process_response
 is_submitted = (params[:isSubmit] == 'Yes')
 @response = find_or_create_response
 was_submitted = @response.is_submitted
 # ignore if autoupdate try to save when the response object is not yet created.s
 @response.update(additional_comment: params[:review][:comments], is_submitted: is_submitted)
 # :version_num=>@version)
end

def finalize_response
 # Change the order for displaying questions for editing response views.
 questions = sort_questions(@questionnaire.questions)
 create_answers(params, questions) if params[:responses]
 msg = 'Your response was successfully saved.'
 error_msg = 
 # only notify if is_submitted changes from false to true
 if should_notify_instructor_on_difference?(was_submitted)
   @response.notify_instructor_on_difference
   @response.email
 end
 redirect_to controller: 'response', action: 'save', id: @map.map_id,
             return: params.permit(:return)[:return], msg: msg, error_msg: error_msg, review: params.permit(:review)[:review], save_options: params.permit(:save_options)[:save_options]
end

def find_or_create_response
 round = @round.to_i
 is_submitted = params[:isSubmit] == 'Yes'
   # There could be multiple responses per round, when re-submission is enabled for that round.
   # Hence we need to pick the latest response.
 @response = Response.where(map_id: @map.id, round: round).order(created_at: :desc).first
 @response ||= Response.create(map_id: @map.id, additional_comment: params[:review][:comments], round: round, is_submitted: is_submitted)
end

def should_notify_instructor_on_difference?(was_submitted)
 @map.is_a?(ReviewResponseMap) && (!was_submitted && @response.is_submitted) && @response.significant_difference?
end

Method action_allowed? has a Cognitive Complexity of 6 (exceeds 5 allowed).

Before Refactoring

def action_allowed?
   response = user_id = nil
   action = params[:action]
   # Initialize response and user id if action is edit or delete or update or view.
   if %w[edit delete update view].include?(action)
     response = Response.find(params[:id])
     user_id = response.map.reviewer.user_id if response.map.reviewer
   end
   case action
   when 'edit'
     # If response has been submitted, no further editing allowed.
     return false if response.is_submitted
     # Else, return true if the user is a reviewer for that response.
     current_user_is_reviewer?(response.map, user_id)
   # Deny access to anyone except reviewer & author's team
   when 'delete', 'update'
     current_user_is_reviewer?(response.map, user_id)
   when 'view'
     response_edit_allowed?(response.map, user_id)
   else
     user_logged_in?
   end
 end

After Refactoring

def action_allowed?
   response = user_id = nil
   action = params[:action]
   response = find_response_for_actions(action)
   case action
   when 'edit'
     # If response has been submitted, no further editing allowed.
     return false if response&.is_submitted
     # Else, return true if the user is a reviewer for that response.
     current_user_is_reviewer?(response.map, response_reviewer_user_id(response))
   # Deny access to anyone except reviewer & author's team
   when 'delete', 'update'
     current_user_is_reviewer?(response.map, response_reviewer_user_id(response))
   when 'view'
     response_edit_allowed?(response.map, response_reviewer_user_id(response))
   else
     user_logged_in?
   end
 end
 
 def find_response_for_actions(action)
   # Initialize response and user id if action is edit or delete or update or view.
   if %w[edit delete update view].include?(action)
     Response.find(params[:id])
   end
 end
 
 def response_reviewer_user_id(response)
   response.map.reviewer&.user_id
 end

Method update has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring.

Before Refactoring

def update
   render nothing: true unless action_allowed?
   msg = 
   begin
     # the response to be updated
     # Locking functionality added for E1973, team-based reviewing
     if @map.team_reviewing_enabled && !Lock.lock_between?(@response, current_user)
       response_lock_action
       return
     end
     @response.update_attribute('additional_comment', params[:review][:comments])
     @questionnaire = questionnaire_from_response
     questions = sort_questions(@questionnaire.questions)
     # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
     create_answers(params, questions) unless params[:responses].nil?
     if params['isSubmit'] && params['isSubmit'] == 'Yes'
       @response.update_attribute('is_submitted', true)
     end
     if (@map.is_a? ReviewResponseMap) && @response.is_submitted && @response.significant_difference?
       @response.notify_instructor_on_difference
     end
   rescue StandardError
     msg = "Your response was not saved. Cause:189 #{$ERROR_INFO}"
   end
   ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Your response was submitted: #{@response.is_submitted}", request)
   redirect_to controller: 'response', action: 'save', id: @map.map_id,
               return: params.permit(:return)[:return], msg: msg, review: params.permit(:review)[:review],
               save_options: params.permit(:save_options)[:save_options]
 end

After Refactoring

def update
   render nothing: true unless action_allowed?
   msg = 
    # the response to be updated
    # Locking functionality added for E1973, team-based reviewing
   begin
     if @map.team_reviewing_enabled && !Lock.lock_between?(@response, current_user)
       response_lock_action
       return
     end
     update_response_attributes
     process_questionnaire
     create_answers_if_needed
     submit_response_if_requested
   rescue StandardError
     msg = "Your response was not saved. Cause:189 #{$ERROR_INFO}"
   end
   log_and_redirect_response_save(msg)
 end
 
 def update_response_attributes
   @response.update_attribute('additional_comment', params[:review][:comments])
   @response.update_attribute('is_submitted', true) if params['isSubmit'] && params['isSubmit'] == 'Yes'
 end
 
 def process_questionnaire
   @questionnaire = questionnaire_from_response
   @questions = sort_questions(@questionnaire.questions)
 end
 
 # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric)
 def create_answers_if_needed
   create_answers(params, @questions) unless params[:responses].nil?
 end
 
 def submit_response_if_requested
   @response.notify_instructor_on_difference if @map.is_a?(ReviewResponseMap) && @response.is_submitted && @response.significant_difference?
 end
 
 def log_and_redirect_response_save(msg)
   ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Your response was submitted: #{@response.is_submitted}", request)
   redirect_to controller: 'response', action: 'save', id: @map.map_id,
               return: params.permit(:return)[:return], msg: msg, review: params.permit(:review)[:review],
               save_options: params.permit(:save_options)[:save_options]
 end

Modified Files

sign_up_sheet_controller.rb
sign_up_sheet_controller_spec.rb
response_controller.rb
badges_controller_spec.rb
pair_programming_controller_spec.rb
participants_helper_spec.rb
collusion_cycle_spec.rb

Test Plan

Edge Cases

When dropping topic if submission already done.
When deadline from dropping topic has passed.
Deleting topic when topic cannot be found.
Signup in case when user cannot be found.
Create signup_sheet when sign_up_topic cannot be found.
Destroy signup_sheet when other topics with the same assignment exists.

RSpec

The RSpec test cases for the `SignUpSheetController` in `signup_sheet_controller.rb` are designed to thoroughly evaluate the functionality of managing sign-up topics for assignments. These test cases cover various scenarios, including creating new topics, updating topic properties, deleting topics, switching from original to suggested topics, and more. They verify that the controller correctly handles actions by students and instructors, respecting deadlines and submission status. Additionally, the test cases ensure that actions such as setting priorities for topics and saving topic deadlines work as intended.

All these tests aim to validate that the controller's actions respond appropriately to user input, and that the underlying logic, database interactions, and authorization checks are functioning as expected. With these tests, developers can have confidence that the `SignUpSheetController` is working correctly and that it provides a robust platform for managing sign-up topics in an educational setting.

Manual Testing

Follow these instructions to manually test the below functionalities:

->The instructor assigns a student to a topic:

Log in as an instructor using the credentials: username - instructor6, password - password.
Select Manage -> Assignments, go to Etc, there add the student to participant.
Then select a topic which you want to assign to that student.

->Set/updates deadlines to that topic

To set/update deadlines, select Manage -> Assignments
Enable the staggered deadline checkbox
Go to Topics, then select start show/ due date
There you can assign the deadline, and click on save

->Drop/Delete student from the topic

Now go to Manage-> Assignments ->Topics
We can see the assigned users to the topic
Click on the ‘drop student’, then that student would be dropped from the topic.

Here is the Demo Link for manual testing.

Team

Mentor

Ameya Vaichalikar (agvaicha)

Team Members

Sravya Karanam (skarana)
Sucharitha Nadendla (snadend3)
Abhishek Desai (adesai7)

Pull Request

Pull_Request

CSC/ECE 517 Fall 2023 - E2357. Refactor sign up sheet controller.rb

Contents

Introduction

About Controller

Login Credentials

Issues/Problem Statement

Previous Version of Project

Solutions

Modified Files

Test Plan

Edge Cases

RSpec

Manual Testing

Team

Mentor

Team Members

Pull Request

Navigation menu

CSC/ECE 517 Fall 2023 - E2357. Refactor sign up sheet controller.rb

Introduction

About Controller

Login Credentials

Issues/Problem Statement

Previous Version of Project

Solutions

Modified Files

Test Plan

Edge Cases

RSpec

Manual Testing

Team

Mentor

Team Members

Pull Request

Navigation menu

Search