Fixed faulty nil checks (#2139)

* faulty nil checks

* deleted unused code

* fixed find_by parameters

app/controllers/assessment/grading.rb

@@ -294,7 +294,13 @@ def submission_popover
   end
 
   def score_grader_info
-    score = Score.find(params[:score_id])
+    score = Score.find_by(id: params[:score_id])
+    if score.nil?
+      flash[:error] = "Could not find score #{params[:score_id]}."
+      redirect_to action: :show
+      return
+    end
+
     grader = (if score then score.grader else nil end)
     grader_info = ""
     if grader

app/controllers/assessments_controller.rb

@@ -456,8 +456,16 @@ def raw_score(scores)
   end
 
   def grade
-    @problem = @assessment.problems.find(params[:problem])
-    @submission = @assessment.submissions.find(params[:submission])
+    @problem = @assessment.problems.find_by(id: params[:problem])
+    if @problem.nil?
+      flash[:error] = "Could not find problem #{params[:problem]}"
+      redirect_to(course_assessment_path(@course, @assessment)) && return
+    end
+    @submission = @assessment.submissions.find_by(id: params[:submission])
+    if @submission.nil?
+      flash[:error] = "Could not find submission #{params[:submission]}"
+      redirect_to(course_assessment_path(@course, @assessment)) && return
+    end
     # Shows a form which has the submission on top, and feedback on bottom
     begin
       subFile = Rails.root.join("courses", @course.name, @assessment.name,

app/controllers/course_user_data_controller.rb

@@ -88,23 +88,25 @@ def create
 
   action_auth_level :show, :student
   def show
-    @requestedUser = @cud.course.course_user_data.find(params[:id])
+    @requestedUser = @cud.course.course_user_data.find_by(id: params[:id])
+    if @requestedUser.nil?
+      flash[:error] = "Could not find user #{params[:id]}"
+      redirect_to([:users, @course]) && return
+    end
     respond_to do |format|
       if @requestedUser
         format.html
         format.json { render json: @requestedUser.to_json }
-      else
-        format.json { head :bad_request }
       end
     end
   end
 
   action_auth_level :edit, :student
   def edit
-    @editCUD = @course.course_user_data.find(params[:id])
+    @editCUD = @course.course_user_data.find_by(id: params[:id])
     if @editCUD.nil?
       flash[:error] = "Can't find user in the course"
-      redirect_to(action: "index") && return
+      redirect_to(action: "show") && return
     end
 
     if (@editCUD.id != @cud.id) && !@cud.instructor? &&
@@ -124,7 +126,7 @@ def update
     # ensure presence of nickname
     # isn't a User model validation since users can start off without nicknames
     # application_controller's authenticate_user redirects here if nickname isn't set
-    @editCUD = @course.course_user_data.find(params[:id])
+    @editCUD = @course.course_user_data.find_by(id: params[:id])
     redirect_to(action: "index") && return if @editCUD.nil?
 
     if @cud.student?
@@ -172,28 +174,6 @@ def update
     end
   end
 
-  action_auth_level :destroy, :instructor
-  def destroy
-    @destroyCUD = @course.course_user_data.find(params[:id])
-    if @destroyCUD && @destroyCUD != @cud && params[:yes1] && params[:yes2] && params[:yes3]
-      @destroyCUD.destroy # awwww!!!
-    end
-    flash[:success] = "Success: User #{@editCUD.email} has been deleted from the course"
-    redirect_to([:users, @course]) && return
-  end
-
-  # Non-RESTful paths below
-
-  # this GET page confirms that the instructor wants to destroy the user
-  action_auth_level :destroyConfirm, :instructor
-  def destroyConfirm
-    @destroyCUD = @course.course_user_data.find(params[:id])
-    return unless @destroyCUD.nil?
-
-    flash[:error] = "The user to be deleted is not in the course"
-    redirect_to(action: :index) && return
-  end
-
   action_auth_level :sudo, :instructor
   def sudo
     unless @cud.can_sudo? || session[:sudo]

app/controllers/courses_controller.rb

@@ -635,7 +635,7 @@ def run_moss
 
     # First, validate access on each of the requested assessments
     assessmentIDs&.keys&.each do |aid|
-      assessment = Assessment.find(aid)
+      assessment = Assessment.find_by(id: aid)
       unless assessment
         flash[:error] = "Invalid Assessment ID: #{aid}"
         redirect_to(action: :moss) && return

app/controllers/groups_controller.rb

@@ -152,7 +152,7 @@ def destroy
   #
   action_auth_level :import, :instructor
   def import
-    ass = @course.assessments.find(params[:ass])
+    ass = @course.assessments.find_by(id: params[:ass])
     if !ass
       flash[:error] = "Assessment not found."
       redirect_to(action: :index) && return
@@ -360,7 +360,7 @@ def group_params
   #
   def get_member_cud
     cud = if params[:member_id]
-            @course.course_user_data.find(params[:member_id].to_i)
+            @course.course_user_data.find_by(id: params[:member_id].to_i)
           elsif params[:member_email]
             @course.course_user_data.joins(:user).find_by(users: { email: params[:member_email] })
           end

app/controllers/lti_nrps_controller.rb

@@ -89,7 +89,7 @@ def request_access_token
   # NRPS endpoint for Autolab to send an NRPS request to LTI Advantage Platform
   action_auth_level :sync_roster, :instructor
   def sync_roster
-    lcd = LtiCourseDatum.find(params[:lcd_id])
+    lcd = LtiCourseDatum.find_by(id: params[:lcd_id])
     if lcd.nil? || lcd.membership_url.nil? || lcd.course_id.nil?
       raise LtiLaunchController::LtiError.new("Unable to update roster", :bad_request)
     end

app/controllers/schedulers_controller.rb

@@ -86,8 +86,8 @@ def visual_run
 
   action_auth_level :update, :instructor
   def update
-    @scheduler = Scheduler.find(params[:id])
-    if @scheduler.update(scheduler_params)
+    @scheduler = Scheduler.find_by(id: params[:id])
+    if @scheduler&.update(scheduler_params)
       flash[:success] = "Scheduler updated."
       redirect_to(course_schedulers_path(@course))
     else
@@ -98,8 +98,8 @@ def update
 
   action_auth_level :destroy, :instructor
   def destroy
-    @scheduler = Scheduler.find(params[:id])
-    if @scheduler.destroy
+    @scheduler = Scheduler.find_by(id: params[:id])
+    if @scheduler&.destroy
       flash[:success] = "Scheduler destroyed."
       redirect_to(course_schedulers_path(@course))
     else

app/controllers/users_controller.rb

@@ -128,7 +128,7 @@ def create
   # GET users/:id/edit
   action_auth_level :edit, :student
   def edit
-    user = User.find(params[:id])
+    user = User.find_by(id: params[:id])
     if user.nil?
       flash[:error] = "Failed to edit user: user does not exist."
       redirect_to(users_path) && return
@@ -195,7 +195,7 @@ def download_all_submissions
   # PATCH users/:id/
   action_auth_level :update, :student
   def update
-    user = User.find(params[:id])
+    user = User.find_by(id: params[:id])
     if user.nil?
       flash[:error] = "Failed to update user: user does not exist."
       redirect_to(users_path) && return
@@ -238,7 +238,7 @@ def destroy
       redirect_to(users_path) && return
     end
 
-    user = User.find(params[:id])
+    user = User.find_by(id: params[:id])
     if user.nil?
       flash[:error] = "Failed to delete user: user doesn't exist."
       redirect_to(users_path) && return
@@ -405,7 +405,7 @@ def change_password_for_user
   end
 
   def update_password_for_user
-    @user = User.find(params[:id])
+    @user = User.find_by(id: params[:id])
     return if params[:user].nil? || params[:user].is_a?(String) || @user.nil?
 
     if params[:user][:password] != params[:user][:password_confirmation]
@@ -474,7 +474,7 @@ def set_gh_oauth_client
   end
 
   def set_user
-    @user = User.find(params[:id])
+    @user = User.find_by(id: params[:id])
     return unless @user.nil?
 
     flash[:error] = "User doesn't exist."