autolab / Autolab

Course management service that enables auto-graded programming assignments.
http://www.autolabproject.com/
Apache License 2.0
757 stars 215 forks source link

Faulty nil checks after using a find method #2128

Closed damianhxy closed 3 months ago

damianhxy commented 6 months ago

In at least several controllers, .nil? checks are performed after calling .find( ... ). However, find throws an exception if an id is not found, so the checks are never reached.

They should be replaced with find_by where possible.

Some instances (found with grep -nr ".find(" -A 2)

course_user_data_controller.rb

    @editCUD = @course.course_user_data.find(params[:id])
    if @editCUD.nil?
      flash[:error] = "Can't find user in the course"
      redirect_to(action: "index") && return
    end

groups_controller.rb

    ass = @course.assessments.find(params[:ass])
    if !ass
      flash[:error] = "Assessment not found."
      redirect_to(action: :index) && return

users_controller.rb

    user = User.find(params[:id])
    if user.nil?
      flash[:error] = "Failed to edit user: user does not exist."
      redirect_to(users_path) && return
    end

lti_nrps_controller.rb

    lcd = LtiCourseDatum.find(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