Merge branch 'develop'

autolab · Mar 17, 2015 · 0c03794 · 0c03794
2 parents 4998572 + 5560750
commit 0c03794
Show file tree

Hide file tree

Showing 8 changed files with 132 additions and 85 deletions.
diff --git a/app/assets/javascripts/confirm_discard_changes.js b/app/assets/javascripts/confirm_discard_changes.js
@@ -0,0 +1,29 @@
+;(function() {
+  // Check for changes before navigating away if there are unsubmitted changes.
+  // TODO: This script won't recognize text that has been changed twice
+  //       resulting in no net effect to the input.
+
+  var changes = false;
+
+  $(document).ready(function() {
+    $('input').on('change', function(e) {
+      changes = true;
+    });
+    $('textarea').on('change', function(e) {
+      changes = true;
+    });
+
+    window.onbeforeunload = function() {
+      if (changes) {
+        return 'It looks like you might have unsubmitted changes. Are you sure you want to continue?';
+      }
+      else {
+        return null;
+      }
+    }
+
+    $('form').on('submit', function() {
+      changes = false;
+    });
+  });
+})();
diff --git a/app/assets/javascripts/initialize_datetimepickers.js b/app/assets/javascripts/initialize_datetimepickers.js
@@ -7,27 +7,30 @@
     var lessThans = $(this).data('dateLessThan');
     var greaterThans = $(this).data('dateGreaterThan');
 
+    function compareDates(dp_id, cmp) {
+      if (dp_id) {
+        var theirID = '#' + dp_id;
+        var theirDateData = $(theirID).data('DateTimePicker');
+
+        // Check whether the other target has been initialized
+        if (theirDateData) {
+          var theirDate = theirDateData.date();
+          $(theirID).data('DateTimePicker').date(cmp(theirDate, ourDate));
+        }
+      }
+    }
+
     // We are less than (older than) each of these elements
     if (lessThans) {
       lessThans.split(' ').forEach(function(dp_id, idx, arr) {
-        if (dp_id) {
-          var theirID = '#' + dp_id;
-          var theirDate = $(theirID).data('DateTimePicker').date();
-
-          $(theirID).data('DateTimePicker').date(moment.max(theirDate, ourDate));
-        }
+        compareDates(dp_id, moment.max);
       });
     }
 
     // We are greater than (younger than) each of these elements
     if (greaterThans) {
       greaterThans.split(' ').forEach(function(dp_id, idx, arr) {
-        if (dp_id) {
-          var theirID = '#' + dp_id;
-          var theirDate = $(theirID).data('DateTimePicker').date();
-
-          $(theirID).data('DateTimePicker').date(moment.min(theirDate, ourDate));
-        }
+        compareDates(dp_id, moment.min);
       });
     }
   }

diff --git a/app/controllers/announcements_controller.rb b/app/controllers/announcements_controller.rb
@@ -1,3 +1,5 @@
+# Extend the Rails ApplicationController to define some RESTful endpoints for
+# dealing with Announcements.
 class AnnouncementsController < ApplicationController
   action_auth_level :index, :instructor
   def index
@@ -10,16 +12,19 @@ def index
 
   action_auth_level :new, :instructor
   def new
-    # for consistency with REST
+    @announcement = Announcement.new
   end
 
   action_auth_level :create, :instructor
   def create
     if !@cud.user.administrator? && params[:announcement][:system]
-      flash[:error] = "You don't have the permission to create system announcements!"
+      flash[:error] = "You don't have the permission " \
+        "to create system announcements!"
       redirect_to(action: "index") && return
     end
+
     @announcement = @course.announcements.create(announcement_params)
+
     if @announcement.save
       flash[:success] = "Create success!"
       redirect_to(course_announcements_path(@course)) && return
@@ -37,11 +42,13 @@ def show
   action_auth_level :edit, :instructor
   def edit
     @announcement = Announcement.find(params[:id])
+
     # Prevent non-admin from entering system announcements edit page
-    if !@cud.user.administrator? && @announcement.system
-      flash[:error] = "You don't have the permission to edit system announcements!"
-      redirect_to(action: "index") && return
-    end
+    return unless @cud.user.administrator? || @announcement.system
+
+    flash[:error] = "You don't have the permission " \
+      "to edit system announcements!"
+    redirect_to(action: "index") && return
   end
 
   action_auth_level :update, :instructor
@@ -60,7 +67,8 @@ def update
   def destroy
     @announcement = Announcement.find(params[:id])
     if !@cud.user.administrator? && @announcement.system
-      flash[:error] = "You don't have the permission to destroy system announcements!"
+      flash[:error] = "You don't have the permission " \
+        "to destroy system announcements!"
       redirect_to(action: "index") && return
     end
     @announcement.destroy
@@ -70,6 +78,7 @@ def destroy
   private
 
   def announcement_params
-    params.require(:announcement).permit(:title, :description, :start_date, :end_date, :system, :persistent)
+    params.require(:announcement).permit(:title, :description, :start_date,
+                                         :end_date, :system, :persistent)
   end
 end
diff --git a/app/form_builders/form_builder_with_date_time_input.rb b/app/form_builders/form_builder_with_date_time_input.rb
@@ -1,3 +1,8 @@
+# Extend the Rails FormBuilder class.
+#
+# The naming is unfortunate, as this FormBuilder does more than just add a
+# custom datetimepicker. In reality, it's goal is to wrap common form builder
+# methods in Bootstrap boilerplate code.
 class FormBuilderWithDateTimeInput < ActionView::Helpers::FormBuilder
   %w(text_field text_area).each do |method_name|
     # retain access to default textfield, etc. helpers
@@ -21,7 +26,8 @@ def score_adjustment_input(name, *args)
       @template.content_tag :div, class: "score-adjustment input-group" do
         (f.vanilla_text_field :value, class: "form-control value") +
         (@template.content_tag :div, class: "input-group-addon" do
-          f.select(:kind, { "points" => "points", "%" => "percent" }, {}, class: "form-control kind input-group-addon")
+          f.select(:kind, { "points" => "points", "%" => "percent" }, {},
+                   class: "form-control kind input-group-addon")
         end)
       end
     end
@@ -68,21 +74,33 @@ def datetime_select(name, options = {}, _html_options = {})
   # :greater_than properties to initialize relationships between datepicker
   # fields.
   def date_helper(name, options, strftime, date_format)
-    existing_time = @object.send(name)
-    formatted_datetime = existing_time.to_time.strftime(strftime) if existing_time.present?
+    begin
+      existing_time = @object.send(name)
+    rescue
+      existing_time = nil
+    end
+
+    if existing_time.present?
+      formatted_datetime = existing_time.to_time.strftime(strftime)
+    else
+      formatted_datetime = ""
+    end
 
-    field = vanilla_text_field(name, :value => formatted_datetime,
-                                     :class => "form-control datetimepicker",
-                                     :"data-date-format" => date_format,
-                                     :"data-date-less-than" => options[:less_than],
-                                     :"data-date-greater-than" => options[:greater_than])
+    field = vanilla_text_field(
+      name,
+      :value => formatted_datetime,
+      :class => "form-control datetimepicker",
+      :"data-date-format" => date_format,
+      :"data-date-less-than" => options[:less_than],
+      :"data-date-greater-than" => options[:greater_than])
 
     wrap_field name, field, options[:help_text]
   end
 
   def wrap_field(name, field, help_text, display_name = nil)
     @template.content_tag :div, class: "form-group" do
-      label(name, display_name, class: "control-label") + field + help_text(name, help_text)
+      label(name, display_name, class: "control-label") +
+        field + help_text(name, help_text)
     end
   end
 

diff --git a/app/views/announcements/_announcementFields.html.erb b/app/views/announcements/_announcementFields.html.erb
@@ -1,31 +1,29 @@
-<table class="verticalTable">
-  <tr>
-    <th>Title:</th>
-    <td><%= f.text_field :title %></td>
-  </tr>
-  <tr>
-    <th>Description:</th>
-    <td><%= f.text_area :description %></td>
-  </tr>
-  <tr>
-    <th>Start Date:</th>
-    <td><%= f.datetime_select :start_date, step: 1 %></td>
-  </tr>
-  <tr>
-    <th>End Date:</th>
-    <td><%= f.datetime_select :end_date, step: 1 %></td>
-  </tr>
-  <tr>
-    <th>Persistent Announcement?</th>
-    <td><%= f.check_box (:persistent) %></td>
-  </tr>
-  <% if @cud.administrator? then %>
-    <tr>
-      <th>System Announcement?</th>
-      <td><%= f.check_box (:system) %></td>
-    </tr>
-  <% end %>
-  <tr>
-    <td colspan=2><%= f.submit 'Submit' , {:class=>"btn primary"} %></td>
-  </tr>
-</table>
+<% content_for :javascripts do %>
+  <%= javascript_include_tag "initialize_datetimepickers" %>
+  <%= javascript_include_tag "confirm_discard_changes" %>
+<% end %>
+
+<%= f.text_field :title,
+  help_text: "The best titles are short and sweet." %>
+
+<%= f.text_area :description,
+  help_text: "There'll also be limited space for the description text. If you need something long-form, perhaps link to a longer announcement posted on external media." %>
+
+<%= f.datetime_select :start_date,
+  help_text: "When should the announcement appear?",
+  less_than: "announcement_end_date" %>
+
+<%= f.datetime_select :end_date,
+  help_text: "When should the announcement come down?",
+  greater_than: "announcement_start_date" %>
+
+<!-- TODO: custom title 'Persistent Announcement?' -->
+<%= f.check_box :persistent,
+  help_text: "A persistent announcement is shown on every page in the course." %>
+
+<% if @cud.administrator? then %>
+  <%= f.check_box :system,
+    help_text: "Check this to make a system-wide announcement." %>
+<% end %>
+
+<%= f.submit 'Submit' %>
diff --git a/app/views/announcements/edit.html.erb b/app/views/announcements/edit.html.erb
@@ -1,4 +1,8 @@
 <h2>Edit Announcement</h2>
-<%= form_for :announcement, url: course_announcement_path(@course, @announcement), method: :patch do |f| %>
-  <%= render partial:'announcementFields', locals:{f:f} %>
-<% end %>
+<div class="row">
+  <div class="col-md-6">
+    <%= form_for @announcement, url: course_announcement_path(@course, @announcement), method: :patch, builder: FormBuilderWithDateTimeInput do |f| %>
+      <%= render partial: 'announcementFields', locals: {f:f} %>
+    <% end %>
+  </div>
+</div>
diff --git a/app/views/announcements/new.html.erb b/app/views/announcements/new.html.erb
@@ -1,4 +1,8 @@
 <h2>Create Announcement</h2>
-<%= form_for :announcement, url:{action:"create"} do |f| %>
-  <%= render partial:'announcementFields', locals:{f:f} %>
-<% end %>
+<div class="row">
+  <div class="col-md-6">
+    <%= form_for @announcement, url:{action:"create"}, builder: FormBuilderWithDateTimeInput do |f| %>
+      <%= render partial: 'announcementFields', locals: {f:f} %>
+    <% end %>
+  </div>
+</div>
diff --git a/app/views/assessments/edit.html.erb b/app/views/assessments/edit.html.erb
@@ -6,25 +6,7 @@
 <% end %>
 
 <% content_for :javascripts do %>
-  <script type="text/javascript">
-    var changes = false;
-
-    $('input').on('change', function(e) {
-      changes = true;
-    });
-
-    window.onbeforeunload = function() {
-      if (changes) {
-        var message = "You have some unsaved changes. \n Are you sure you want to navigate away from this page?";
-        if (confirm(message)) return true;
-        else return false;
-      }
-    }
-
-    $('form').on('submit', function() {
-      changes = false;
-    });
-  </script>
+  <%= javascript_include_tag "confirm_discard_changes" %>
 <% end %>
 
 <ul id="tabs" class="nav nav-tabs">