Merge branch 'master' into joeywildman-fix-pluralize

app/assets/javascripts/course_user_data_edit.js

@@ -1,44 +1,53 @@
 function formvalidation(form){
-    function DoubleByte(str) {
-      for (var i = 0, n = str.length; i < n; i++) {
-        if (str.charCodeAt(i) > 127) { return true; }
-      }
-      return false;
+  function DoubleByte(str) {
+    for (var i = 0, n = str.length; i < n; i++) {
+      if (str.charCodeAt(i) > 127) { return true; }
     }
-    var nickname = document.getElementById('course_user_datum_nickname');
+    return false;
+  }
+  var nickname = document.getElementById('course_user_datum_nickname');
 
-    if (DoubleByte(nickname.value)){
-      nickname.setAttribute('style','background-color:#FFF352');
-      nickname.focus();
-      alert("Your nickname has non-ASCII characters");
-    } else {
-      form.submit();
-    }
+  if (DoubleByte(nickname.value)){
+    nickname.setAttribute('style','background-color:#FFF352');
+    nickname.focus();
+    alert("Your nickname has non-ASCII characters");
+  } else {
+    form.submit();
+  }
 }
 
-function disable_dropped(){
-    $('#course_user_datum_dropped').prop("checked",false);
-    $('#course_user_datum_dropped').prop("disabled",true);
-}
+// User can be at most one of the following: instructor, course assistant, or dropped (student)
+const $instructor_checkbox = $('#course_user_datum_instructor');
+const $course_assistant_checkbox = $('#course_user_datum_course_assistant');
+const $dropped_checkbox = $('#course_user_datum_dropped');
+const mutually_exclusive_fields = [
+  $instructor_checkbox, $course_assistant_checkbox, $dropped_checkbox
+];
 
-function enable_dropped(){
-    $('#course_user_datum_dropped').prop("disabled",false);
-}
+function mutual_exclusion() {
+  const fields = mutually_exclusive_fields; // For brevity
+  // Enable all fields
+  fields.forEach((field) => field.prop('disabled', false));
 
-function prevent_dropping_instructor_ca(){
-    if($('#course_user_datum_instructor').is(':checked') || 
-        $('#course_user_datum_course_assistant').is(':checked')){
-        disable_dropped();
-    }
-    else{
-        enable_dropped();
+  // If any field is checked, disable the rest
+  fields.forEach((field, idx) => {
+    if (field.is(':checked')) {
+      fields.forEach((field2, idx2) => {
+        if (idx !== idx2) {
+          field2.prop('disabled', true);
+          field2.prop('checked', false);
+        }
+      });
     }
+  });
 }
 
-$(document).ready(prevent_dropping_instructor_ca);
-
-$('#course_user_datum_instructor').on( "click", 
-    prevent_dropping_instructor_ca);
+$(document).ready(function(){
+  $('#user_submit').on("click", function(e){
+    formvalidation(this.closest('form'));
+    e.preventDefault();
+  });
 
-$('#course_user_datum_course_assistant').on( "click", 
-    prevent_dropping_instructor_ca);
+  mutual_exclusion();
+  mutually_exclusive_fields.forEach((field) => field.on("click", mutual_exclusion));
+});

app/controllers/announcements_controller.rb

@@ -2,6 +2,8 @@
 # dealing with Announcements.
 class AnnouncementsController < ApplicationController
   before_action :set_announcement, except: %i[index new create]
+  before_action :set_manage_course_breadcrumb
+  before_action :set_manage_announcement_breadcrumb, except: %i[index]
 
   action_auth_level :index, :instructor
   def index
@@ -80,4 +82,11 @@ def announcement_params
     params.require(:announcement).permit(:title, :description, :start_date,
                                          :end_date, :system, :persistent)
   end
+
+  def set_manage_announcement_breadcrumb
+    return if @course.nil?
+
+    @breadcrumbs << (view_context.link_to "Manage Announcements",
+                                          course_announcements_path(@course))
+  end
 end

app/controllers/application_controller.rb

@@ -228,7 +228,6 @@ def set_assessment
       redirect_to(course_assessments_path(@course)) && return
     end
 
-    @breadcrumbs << (view_context.current_assessment_link)
     ASSESSMENT_LOGGER.setAssessment(@assessment)
   end
 
@@ -310,6 +309,47 @@ def set_breadcrumbs
                     end
   end
 
+  ## Helpers for breadcrumbs
+
+  # Guarded against nil @assessment, so safe to use regardless of whether set_assessment was called
+  def set_assessment_breadcrumb
+    return if @course.nil? || @assessment.nil?
+
+    @breadcrumbs << (view_context.link_to @assessment.display_name,
+                                          course_assessment_path(@course, @assessment))
+  end
+
+  def set_edit_assessment_breadcrumb
+    return if @course.nil? || @assessment.nil? || !@cud.instructor
+
+    @breadcrumbs << (view_context.link_to "Edit Assessment",
+                                          edit_course_assessment_path(@course, @assessment))
+  end
+
+  def set_manage_course_breadcrumb
+    return if @course.nil? || !@cud.instructor
+
+    @breadcrumbs << (view_context.link_to "Manage Course",
+                                          manage_course_path(@course))
+  end
+
+  def set_manage_course_users_breadcrumb
+    return if @course.nil? || !@cud.instructor
+
+    @breadcrumbs << (view_context.link_to "Manage Course Users",
+                                          users_course_path(@course))
+  end
+
+  def set_users_list_breadcrumb
+    @breadcrumbs << (view_context.link_to "Users List", users_path)
+  end
+  ### END HELPERS
+
+  def pluralize(count, singular, plural = nil)
+    "#{count || 0} " +
+      (count == 1 || count =~ /^1(\.0+)?$/ ? singular : (plural || singular.pluralize))
+  end
+
   # make_dlist - Creates a string of emails that can be added as b/cc field.
   # @param section The section to email.  nil if we should email the entire
   # class.

app/controllers/assessments_controller.rb

@@ -26,6 +26,9 @@ class AssessmentsController < ApplicationController
   before_action :set_assessment, except: %i[index new create install_assessment
                                             importAsmtFromTar importAssessment
                                             log_submit local_submit autograde_done]
+  skip_before_action :set_breadcrumbs, only: %i[index]
+  before_action :set_assessment_breadcrumb, except: %i[index show install_assessment]
+  before_action :set_manage_course_breadcrumb, only: %i[install_assessment]
   before_action :set_submission, only: [:viewFeedback]
 
   # We have to do this here, because the modules don't inherit ApplicationController.

app/controllers/attachments_controller.rb

@@ -6,9 +6,10 @@ class AttachmentsController < ApplicationController
   # inherited from ApplicationController
   # this will also set an @is_assessment variable based on the result of is_assessment?
   before_action :set_assessment, if: :assessment?
+  before_action :set_assessment_breadcrumb
   before_action :set_attachment, except: %i[index new create]
-  before_action :add_attachments_breadcrumb
 
+  # This page shouldn't really be used
   action_auth_level :index, :instructor
   def index
     @attachments = if @is_assessment
@@ -150,15 +151,6 @@ def redirect_to_index
     end
   end
 
-  def add_attachments_breadcrumb
-    @breadcrumbs << if @is_assessment
-                      view_context.link_to "Assessment Attachments",
-                                           course_assessment_attachments_path(@course, @assessment)
-                    else
-                      view_context.link_to "Course Attachments", course_attachments_path(@course)
-                    end
-  end
-
   def attachment_params
     params.require(:attachment).permit(:name, :file, :category_name, :release_at, :mime_type)
   end

app/controllers/autograders_controller.rb

@@ -4,6 +4,7 @@
 class AutogradersController < ApplicationController
   before_action :set_assessment
   before_action :set_assessment_breadcrumb, only: [:edit]
+  before_action :set_edit_assessment_breadcrumb, only: [:edit]
   before_action :set_autograder, except: [:create]
 
   action_auth_level :create, :instructor
@@ -73,10 +74,6 @@ def upload
 
 private
 
-  def set_assessment_breadcrumb
-    @breadcrumbs << (view_context.link_to "Edit Assessment", [:edit, @course, @assessment])
-  end
-
   def set_autograder
     @autograder = @assessment.autograder
     redirect_to([@course, @assessment]) if @autograder.nil?

app/controllers/course_user_data_controller.rb

@@ -1,5 +1,7 @@
 class CourseUserDataController < ApplicationController
-  before_action :add_users_breadcrumb
+  before_action :set_manage_course_breadcrumb
+  before_action :set_manage_course_users_breadcrumb, except: %i[sudo]
+  # :set_course_user_breadcrumb called from within edit
 
   action_auth_level :index, :student
   def index
@@ -111,6 +113,9 @@ def edit
       redirect_to(action: "index") && return
     end
 
+    # This can't be a before_action callback since @editCUD is only defined here
+    set_course_user_breadcrumb
+
     @editCUD.tweak ||= Tweak.new
   end
 
@@ -161,7 +166,7 @@ def update
       # error details are shown separately in the view
       flash[:error] = "Update failed.<br>"
       flash[:error] += @editCUD.errors.full_messages.join("<br>")
-      flash[:notice] = ""
+      flash.delete(:notice)
       flash[:html_safe] = true
       redirect_to(action: :edit) && return
     end
@@ -239,10 +244,6 @@ def unsudo
 
 private
 
-  def add_users_breadcrumb
-    @breadcrumbs << (view_context.link_to "Users", [:users, @course]) if @cud.instructor
-  end
-
   def cud_params
     if @cud.administrator? || @cud.instructor?
       params.require(:course_user_datum).permit(:school, :major, :year, :course_number,
@@ -268,4 +269,11 @@ def edit_cud_params
       # user_attributes: [:first_name, :last_name])
     end
   end
+
+  def set_course_user_breadcrumb
+    return if @course.nil? || @editCUD.nil?
+
+    @breadcrumbs << (view_context.link_to @editCUD.user.full_name,
+                                          course_course_user_datum_path(@course, @editCUD))
+  end
 end

app/controllers/courses_controller.rb

@@ -5,11 +5,15 @@
 require "statistics"
 
 class CoursesController < ApplicationController
-  skip_before_action :set_course, only: %i[courses_redirect index new create]
+  skip_before_action :set_course, only: %i[courses_redirect index new create join_course]
   # you need to be able to pick a course to be authorized for it
-  skip_before_action :authorize_user_for_course, only: %i[courses_redirect index new create]
+  skip_before_action :authorize_user_for_course,
+                     only: %i[courses_redirect index new create join_course]
   # if there's no course, there are no persistent announcements for that course
-  skip_before_action :update_persistent_announcements, only: %i[courses_redirect index new create]
+  skip_before_action :update_persistent_announcements,
+                     only: %i[courses_redirect index new create join_course]
+  before_action :set_manage_course_breadcrumb, only: %i[edit users moss email upload_roster export]
+  before_action :set_manage_course_users_breadcrumb, only: %i[upload_roster]
 
   def index
     courses_for_user = User.courses_for_user current_user
@@ -34,6 +38,42 @@ def courses_redirect
     end
   end
 
+  def join_course
+    return unless params[:access_code]
+
+    # GET + access_code when using direct join link
+    # POST + access_code when using join course form
+
+    access_code = params[:access_code].upcase
+    unless Course::VALID_CODE_REGEX.match?(access_code)
+      flash[:error] = "Invalid access code format"
+      redirect_to(join_course_courses_path) && return
+    end
+
+    course = Course.find_by(access_code:)
+    if course.nil?
+      flash[:error] = "Invalid access code"
+      redirect_to(join_course_courses_path) && return
+    end
+
+    cud = course.course_user_data.find_by(user_id: current_user.id)
+
+    if cud.nil?
+      cud = course.course_user_data.new
+      cud.user = current_user
+      unless cud.save
+        flash[:error] = "An error occurred while joining the course"
+        redirect_to(join_course_courses_path) && return
+      end
+      # else, no point setting a flash because they will be redirected
+      # to set their nickname
+    else
+      flash[:success] = "You are already enrolled in this course"
+    end
+
+    redirect_to course_path(course)
+  end
+
   action_auth_level :show, :student
   def show
     redirect_to course_assessments_url(@course)
@@ -616,11 +656,32 @@ def new_course_params
   end
 
   def edit_course_params
-    params.require(:editCourse).permit(:semester, :website, :late_slack,
-                                       :grace_days, :display_name, :start_date, :end_date,
-                                       :disabled, :exam_in_progress, :version_threshold,
-                                       :gb_message, late_penalty_attributes: %i[kind value],
-                                                    version_penalty_attributes: %i[kind value])
+    att = params.require(:editCourse).permit(:semester, :website, :late_slack,
+                                             :grace_days, :display_name, :start_date, :end_date,
+                                             :disabled, :exam_in_progress, :allow_self_enrollment,
+                                             :version_threshold, :gb_message,
+                                             late_penalty_attributes: %i[kind value],
+                                             version_penalty_attributes: %i[kind value])
+
+    handle_self_enrollment(att)
+  end
+
+  def handle_self_enrollment(att)
+    if params[:allow_self_enrollment] && @course.access_code.blank?
+      att.merge!(access_code: generate_access_code)
+    elsif !params[:allow_self_enrollment]
+      att.merge!(access_code: nil)
+    end
+    att.except(:allow_self_enrollment)
+  end
+
+  def generate_access_code
+    loop do
+      code = SecureRandom.alphanumeric(6).upcase
+
+      # Possible race condition, but we also have a uniqueness validation
+      break code unless Course.where(access_code: code).exists?
+    end
   end
 
   def categorize_courses_for_listing(courses)

app/controllers/device_flow_activation_controller.rb

@@ -5,10 +5,10 @@ class DeviceFlowActivationController < ApplicationController
   skip_before_action :authorize_user_for_course
   skip_before_action :authenticate_for_action
   skip_before_action :update_persistent_announcements
+  before_action :set_users_list_breadcrumb
+  before_action :set_user_breadcrumb, only: %i[index]
 
-  def index
-    # just renders the index page
-  end
+  def index; end
 
   # target for the form on the index page
   def resolve
@@ -57,4 +57,12 @@ def authorization_callback
     flash[:success] = "Access granted"
     redirect_to(action: :index) && return
   end
+
+private
+
+  def set_user_breadcrumb
+    return if current_user.nil?
+
+    @breadcrumbs << (view_context.link_to current_user.display_name, user_path(current_user))
+  end
 end