diff --git a/app/assets/javascripts/course_user_data_edit.js b/app/assets/javascripts/course_user_data_edit.js index 3ebb189d3..b7a26c327 100644 --- a/app/assets/javascripts/course_user_data_edit.js +++ b/app/assets/javascripts/course_user_data_edit.js @@ -1,28 +1,18 @@ function formvalidation(form){ function DoubleByte(str) { for (var i = 0, n = str.length; i < n; i++) { - if (str[i].charCodeAt() > 127) { return true; } + if (str.charCodeAt(i) > 127) { return true; } } return false; } - var formlog = 'Your nickname '; - function flag(msg, nickname){ - nickname.setAttribute('style','background-color:#FFF352'); - if (formlog!= 'Your nickname '){formlog+=' and ';} - formlog +=msg; + var nickname = document.getElementById('course_user_datum_nickname'); + if (DoubleByte(nickname.value)){ + nickname.setAttribute('style','background-color:#FFF352'); nickname.focus(); - } - var nickname = document.getElementById('user_nickname'); - var approve = true; - - if (nickname.value.length>20){approve = false; flag('is too long',nickname);} - if (DoubleByte(nickname.value)===true){approve = false; flag('has non-ASCII characters',nickname);} - - if (approve){ - form.submit(); + alert("Your nickname has non-ASCII characters"); } else { - alert(formlog); + form.submit(); } } diff --git a/app/controllers/course_user_data_controller.rb b/app/controllers/course_user_data_controller.rb index a78b209ee..3973c7b6a 100755 --- a/app/controllers/course_user_data_controller.rb +++ b/app/controllers/course_user_data_controller.rb @@ -72,7 +72,7 @@ def create redirect_to([:users, @course]) && return end else - error_msg = "Adding user failed:" + error_msg = "Creation failed." if !@newCUD.valid? @newCUD.errors.full_messages.each do |msg| error_msg += "
#{msg}" @@ -160,7 +160,9 @@ def update else COURSE_LOGGER.log(@editCUD.errors.full_messages.join(", ")) # error details are shown separately in the view - flash[:error] = "Update failed. Check all fields" + flash[:error] = "Update failed.
" + flash[:error] += @editCUD.errors.full_messages.join("
") + flash[:html_safe] = true redirect_to(action: :edit) && return end end diff --git a/app/controllers/scoreboards_controller.rb b/app/controllers/scoreboards_controller.rb index a89b1643a..54fc3daa9 100755 --- a/app/controllers/scoreboards_controller.rb +++ b/app/controllers/scoreboards_controller.rb @@ -208,7 +208,7 @@ def emitColSpec(colspec) str += " | " end str += hash["hdr"].to_s.upcase - str += hash["asc"] ? " [asc]" : " [desc]" if i < 3 + str += hash["asc"] ? " [asc]" : " [desc]" i += 1 end str @@ -318,46 +318,22 @@ def scoreboardOrderSubmissions(a, b) # in descending order. Lab authors can modify the default # direction with the "asc" key in the column spec. else - a0 = a[:entry][0].to_f - a1 = a[:entry][1].to_f - a2 = a[:entry][2].to_f - b0 = b[:entry][0].to_f - b1 = b[:entry][1].to_f - b2 = b[:entry][2].to_f - begin parsed = ActiveSupport::JSON.decode(@scoreboard.colspec) rescue StandardError => e Rails.logger.error("Error in scoreboards controller updater: #{e.message}") end - if a0 != b0 - if parsed && parsed["scoreboard"] && - !parsed["scoreboard"].empty? && - parsed["scoreboard"][0]["asc"] - a0 <=> b0 # ascending order - else - b0 <=> a0 # descending order - end - elsif a1 != b1 - if parsed && parsed["scoreboard"] && - parsed["scoreboard"].size > 1 && - parsed["scoreboard"][1]["asc"] - a1 <=> b1 # ascending order - else - b1 <=> a1 # descending order - end - elsif a2 != b2 - if parsed && parsed["scoreboard"] && - parsed["scoreboard"].size > 2 && - parsed["scoreboard"][2]["asc"] - a2 <=> b2 # ascending order - else - b2 <=> a2 # descending order - end - else - a[:time] <=> b[:time] # ascending by submission time + # Validations ensure that colspec is of the correct format + parsed["scoreboard"].each_with_index do |v, i| + ai = a[:entry][i].to_f + bi = b[:entry][i].to_f + next unless ai != bi + return ai <=> bi if v["asc"] # ascending + + return bi <=> ai # descending otherwise end + a[:time] <=> b[:time] # ascending by submission time to tiebreak end end end diff --git a/app/models/course_user_datum.rb b/app/models/course_user_datum.rb index 4c7eebb4b..342e43767 100755 --- a/app/models/course_user_datum.rb +++ b/app/models/course_user_datum.rb @@ -63,17 +63,15 @@ def after_update end def valid_nickname? - if !nickname - true - elsif nickname.length > 32 + return if nickname.nil? + + if nickname.length > 32 errors.add("nickname", "is too long (maximum is 32 characters)") - false - elsif !nickname.ascii_only? - errors.add("nickname", "can only contain ASCII characters") - false - else - true end + + return if nickname.ascii_only? + + errors.add("nickname", "can only contain ASCII characters") end ## diff --git a/app/models/scoreboard.rb b/app/models/scoreboard.rb index ee217136d..e93493199 100755 --- a/app/models/scoreboard.rb +++ b/app/models/scoreboard.rb @@ -24,14 +24,17 @@ def colspec_is_well_formed # The parse will throw an exception if the string has a JSON syntax error begin - # Quote JSON keys and values if they are not already quoted - quoted = colspec.gsub(/([a-zA-Z0-9]+):/, '"\1":').gsub(/:([a-zA-Z0-9]+)/, ':"\1"') - parsed = ActiveSupport::JSON.decode(quoted) + parsed = ActiveSupport::JSON.decode(colspec) rescue StandardError => e errors.add "colspec", e.to_s return end + unless parsed.is_a? Hash + errors.add "colspec", "must be a hash object" + return + end + # Colspecs must include a scoreboard array object unless parsed["scoreboard"] errors.add "colspec", "missing 'scoreboard' array object" @@ -60,12 +63,6 @@ def colspec_is_well_formed errors.add "colspec", "unknown key('#{k}') in scoreboard[#{i}]" return end - - next unless k == "asc" && i > 2 - - errors.add "colspec", - "'asc' key in col #{i} ignored because only the first", - "three columns are sorted." end end end diff --git a/app/views/course_user_data/_fields.html.erb b/app/views/course_user_data/_fields.html.erb index d9a5daa13..34fca9863 100644 --- a/app/views/course_user_data/_fields.html.erb +++ b/app/views/course_user_data/_fields.html.erb @@ -7,7 +7,7 @@ <% end %> - <%= f.text_field :nickname, help_text: "Anonymous nickname to display on the public scoreboards", placeholder: "droh" %> + <%= f.text_field :nickname, help_text: "Anonymous nickname to display on the public scoreboards (max length: 32)", placeholder: "droh", maxlength: 32 %> <%= f.text_field :course_number, help_text: "The course number", placeholder: "15213", disabled: !@cud.instructor? %> diff --git a/app/views/course_user_data/edit.html.erb b/app/views/course_user_data/edit.html.erb index c2bccf0cc..4dd199e11 100755 --- a/app/views/course_user_data/edit.html.erb +++ b/app/views/course_user_data/edit.html.erb @@ -4,29 +4,13 @@ <%= javascript_include_tag "course_user_data_edit" %> <% end %> -

Editing <%= @editCUD.user.full_name %>'s Enrollment in <%= @course.display_name %>

<%= form_for @editCUD, url: course_course_user_datum_path(@course, @editCUD), builder: FormBuilderWithDateTimeInput do |f| %> -<% if @editCUD.errors.any? %> -
-

<%= pluralize(@editCUD.errors.count, "error") %> - prohibited the data from being saved:

- - -
-<% end %> - -

+
<%= render partial: "fields", locals: {f: f, cud: @editCUD, edit: true} %>
-
- - + -<%end %> +<% end %> diff --git a/app/views/course_user_data/new.html.erb b/app/views/course_user_data/new.html.erb index 5d5c4923a..8bb0b08e3 100755 --- a/app/views/course_user_data/new.html.erb +++ b/app/views/course_user_data/new.html.erb @@ -1,4 +1,5 @@ <% content_for :javascripts do %> + <%= javascript_include_tag "course_user_data_edit" %>