Merge branch 'master' of git://github.com/MarkUsProject/Markus into d…

…epotOfficiel
akrassikov · Oct 20, 2011 · ffff2e3 · ffff2e3
2 parents f52a91b + d156662
commit ffff2e3
Show file tree

Hide file tree

Showing 19 changed files with 183 additions and 133 deletions.
diff --git a/app/controllers/admins_controller.rb b/app/controllers/admins_controller.rb
@@ -48,9 +48,7 @@ def create
 
       redirect_to :action => 'index'
     else
-      flash[:error] = reason_for_error(
-                        @user.errors,
-                        I18n.t('admins.create.error'))
+      flash[:error] = I18n.t('admins.create.error')
       render "new"
     end
   end

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -8,7 +8,6 @@ class ApplicationController < ActionController::Base
 
   layout "content"
 
-  helper_method :reason_for_error #include this helper in both controllers and views
   helper :all # include all helpers in the views, all the time
 
   # activate i18n for renaming constants in views
@@ -69,20 +68,4 @@ def set_locale
     I18n.load_path -= [locale_path]
     I18n.locale = I18n.default_locale
   end
-
-  # Given an error object returned from a failed database update/save, returns
-  # a nicely formatted string listing the errors. Prepends msg if set.
-  # Example: {:name => "can't be blank", :random => "is too random"} results in
-  #          "Name can't be blank, and Random is too random."
-  def reason_for_error(errors, msg='')
-    if errors.empty?
-      return msg
-    else
-      reasons = []
-      errors.each do |key, value|
-        reasons << "#{key}".capitalize + " #{value}"
-      end
-      return msg.empty? ? "#{reasons.to_sentence}." : "#{msg} #{reasons.to_sentence}."
-    end
-  end
 end
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
@@ -46,6 +46,7 @@ def index
   # gets the objects for groupings on first load.
   def new
     new_retrieve
+    @note = Note.new
   end
 
   def create

diff --git a/app/controllers/sections_controller.rb b/app/controllers/sections_controller.rb
@@ -23,8 +23,8 @@ def create
       redirect_to :action => 'index'
       return
     else
-      flash[:error] = reason_for_error(@section.errors, I18n.t('section.create.error'))
-      redirect_to :action => 'new'
+      flash[:error] = I18n.t('section.create.error')
+      render :action => 'new'
     end
   end
 
@@ -41,8 +41,8 @@ def update
                                :name => @section.name)
       redirect_to :action => 'index'
     else
-      flash[:error] = reason_for_error(@section.errors, I18n.t('section.update.error'))
-      redirect_to :action => 'edit'
+      flash[:error] = I18n.t('section.update.error')
+      render :action => 'edit'
     end
   end
 end
diff --git a/app/controllers/students_controller.rb b/app/controllers/students_controller.rb
@@ -36,6 +36,7 @@ def update
     # update_attributes supplied by ActiveRecords
     if !@user.update_attributes(attrs)
       flash[:error] = I18n.t("students.update.error")
+      @sections = Section.find(:all, :order => "name")
       render :action => :edit
     else
       flash[:success] = I18n.t("students.update.success",
@@ -89,9 +90,7 @@ def create
       redirect_to :action => 'index' # Redirect
     else
       @sections = Section.find(:all, :order => "name")
-      flash[:error] = reason_for_error(
-                          @user.errors,
-                          I18n.t('students.create.error'))
+      flash[:error] = I18n.t('students.create.error')
       render :action => 'new'
     end
   end

diff --git a/app/views/assignments/_form.html.erb b/app/views/assignments/_form.html.erb
@@ -19,7 +19,8 @@
 <% end %>
 
 <%= form_for @assignment do |f| %>
-<%= f.error_messages%>
+  <%= render :partial => "shared/error_explanation",
+      :locals => { :model => @assignment } %>
 <fieldset>
   <legend><%= t("assignment.properties") %></legend>
     <%= raw( f.label :short_identifier, t(:short_identifier) ) %>

diff --git a/app/views/grade_entry_forms/_form.html.erb b/app/views/grade_entry_forms/_form.html.erb
@@ -21,9 +21,8 @@
 </div>
 <% end %>
 
-<%= form_for @grade_entry_form,
-	     :as => :grade_entry_form do |f| %>
-<%= f.error_messages%>
+<%= form_for @grade_entry_form, :as => :grade_entry_form do |f| %>
+  <%= render :partial => "shared/error_explanation", :locals => { :model => @grade_entry_form } %>
 <fieldset>
   <legend><%= t('grade_entry_forms.properties') %></legend>
 

diff --git a/app/views/layouts/_userform.html.erb b/app/views/layouts/_userform.html.erb
@@ -1,12 +1,8 @@
 <div class="section">
-  <% if flash[:error] %>
-    <div class="error">
-      <%= flash[:error] %>
-    </div>
-  <% end%>
+  <%= form_for @user, :as => :user do |f| %>
 
-  <%= form_for @user,
-               :as => :user do |f| %>
+  <%= render :partial => "shared/error_explanation",
+      :locals => { :model => @user, :flash_message => flash[:error] } %>
 
     <%= raw(f.label(:user_name, I18n.t("user.user_name"))) %>
     <%= raw(f.text_field :user_name) %><br />

diff --git a/app/views/notes/edit.html.erb b/app/views/notes/edit.html.erb
@@ -2,25 +2,25 @@
 
 <div class="wrapLeft">
   <%= form_for @note, :as => :note, :url => {:action => 'update', :id => @note.id} do |f| %>
-  <%= f.error_messages %>
+    <%= render :partial => "shared/error_explanation", :locals => { :model => @note } %>
 
-  <p>
-    <b><%=I18n.t('notes.author')%>:</b> <%= h @note.user.user_name %>
-  </p>
+    <p>
+      <b><%=I18n.t('notes.author')%>:</b> <%= h @note.user.user_name %>
+    </p>
 
-  <p>
-    <b><%= I18n.t("notes.noteables.#{@note.noteable_type}") %>:</b> <%= @note.noteable.display_for_note %>
-  </p>
+    <p>
+      <b><%= I18n.t("notes.noteables.#{@note.noteable_type}") %>:</b> <%= @note.noteable.display_for_note %>
+    </p>
 
-  <p>
-    <b><%=I18n.t('notes.creation_time')%>:</b> <%= @note.format_date %>
-  </p>
+    <p>
+      <b><%=I18n.t('notes.creation_time')%>:</b> <%= @note.format_date %>
+    </p>
 
-  <%= f.label :notes_message, I18n.t('notes.note')+":" %>
-  <%= f.text_area :notes_message, :rows => 8, :cols => 50 %>
+    <%= f.label :notes_message, I18n.t('notes.note')+":" %>
+    <%= f.text_area :notes_message, :rows => 8, :cols => 50 %>
 
-  <p>
-    <%= f.submit I18n.t('save') %>
-  </p>
-<% end %>
+    <p>
+      <%= f.submit I18n.t('save') %>
+    </p>
+  <% end %>
 </div>
diff --git a/app/views/notes/new.html.erb b/app/views/notes/new.html.erb
@@ -1,39 +1,37 @@
 <div id="title_bar"><h1><%= I18n.t('notes.new.title') %></h1></div>
 
 <div class="wrapLeft">
-<%= form_for :note, :url => notes_path() do |f| %>
-  <%= f.error_messages %>
+  <%= form_for @note, :as => :note, :url => notes_path() do |f| %>
+    <%= render :partial => "shared/error_explanation", :locals => { :model => @note } %>
+    <fieldset>
+      <p>
+        <%= label_tag "noteable_type", I18n.t('notes.noteable') %>
+        <%= select_tag "noteable_type",
+           options_for_select(
+              Note::NOTEABLES.map {|item| [I18n.t('notes.noteables.' + item),
+                 item]}) %>
+        <%= observe_field :noteable_type,
+                          :on => 'onchange',
+                          :url => noteable_object_selector_notes_path(),
+                          :with => "noteable_type",
+                          :before => "$('loading_selector').show()",
+                          :success => "$('loading_selector').hide()" %>
+        <span id="loading_selector" style="display:none">
+          <%= image_tag('spinner.gif') %> <%= I18n.t('notes.new.loading_selector') %>
+        </span>
+      </p>
 
-<fieldset>
+      <div id="noteable_selector">
+        <%= render  :partial => 'grouping' %>
+      </div>
 
-  <p>
-    <%= label_tag "noteable_type", I18n.t('notes.noteable') %>
-    <%= select_tag "noteable_type",
-		   options_for_select(
-		      Note::NOTEABLES.map {|item| [I18n.t('notes.noteables.' + item),
-						   item]}) %>
-    <%= observe_field :noteable_type,
-		      :on => 'onchange',
-		      :url => noteable_object_selector_notes_path(),
-                      :with => "noteable_type",
-                      :before => "$('loading_selector').show()",
-		      :success => "$('loading_selector').hide()" %>
-    <span id="loading_selector" style="display:none">
-      <%= image_tag('spinner.gif') %> <%= I18n.t('notes.new.loading_selector') %>
-    </span>
-  </p>
+      <p>
+        <%= raw(f.label :notes_message, I18n.t("notes.note")) %>
+        <%= raw(f.text_area :notes_message, :rows => 10) %>
+      </p>
+    </fieldset>
 
-  <div id="noteable_selector">
-    <%= render  :partial => 'grouping' %>
-  </div>
-
-  <p>
-    <%= f.label :notes_message, I18n.t("notes.note") %>
-    <%= f.text_area :notes_message, :rows => 10 %>
-  </p>
-</fieldset>
-
-  <%= f.submit I18n.t('save') %>
-  <%= link_to I18n.t('cancel'), notes_path(), :class => "button" %>
-<% end %>
+    <%= f.submit I18n.t('save') %>
+    <%= link_to I18n.t('cancel'), notes_path(), :class => "button" %>
+  <% end %>
 </div>
diff --git a/app/views/sections/_form.html.erb b/app/views/sections/_form.html.erb
@@ -5,28 +5,21 @@
       <%= flash[:success] %>
     </div>
   <% end%>
-  <% if flash[:error] %>
-    <div class="error">
-      <%= flash[:error] %>
-    </div>
-  <% end%>
-
 </div>
 
-<%= form_for @section,
-	     :as => :section do |f| %>
-<fieldset>
-  <legend><%= I18n.t('section.legend') %></legend>
-  <%= f.label :name, t("section.name") %>
-  <%= f.text_field :name%>
-  <br />
-  <% if controller.action_name == "create_section"%>
-    <%= f.submit I18n.t(:submit) %>
-  <% else %>
-    <%= f.submit I18n.t(:save) %>
-  <% end %>
-
-</fieldset>
-
+<%= form_for @section, :as => :section do |f| %>
+  <%= render :partial => "shared/error_explanation",
+      :locals => { :model => @section, :flash_message => flash[:error] } %>
+  <fieldset>
+    <legend><%= I18n.t('section.legend') %></legend>
+    <%= raw(f.label :name, t("section.name")) %>
+    <%= raw(f.text_field :name) %>
+    <br />
+    <% if controller.action_name == "create_section"%>
+      <%= f.submit I18n.t(:submit) %>
+    <% else %>
+      <%= f.submit I18n.t(:save) %>
+    <% end %>
+  </fieldset>
 <% end %>
 
diff --git a/app/views/shared/_error_explanation.html.erb b/app/views/shared/_error_explanation.html.erb
@@ -0,0 +1,13 @@
+<% #requires locals: model - the object that may have an error and flash_message (optional) %>
+<% if model.errors.any? %>
+  <div class="errorExplanation" id="errorExplanation" >
+    <% unless defined?(flash_message) == nil %>
+      <h2><%= flash_message %></h2>
+    <% end %>
+    <ul>
+    <% model.errors.full_messages.each do |msg| %>
+      <li><%= msg %></li>
+    <% end %>
+    </ul>
+  </div>
+<% end %>
diff --git a/app/views/students/_student_form.html.erb b/app/views/students/_student_form.html.erb
@@ -1,14 +1,8 @@
 <div class="section">
+  <%= form_for @user, :as => :user do |f| %>
 
-  <% if flash[:error] %>
-    <div class="error">
-      <%= flash[:error] %>
-    </div>
-  <% end%>
-
-
-  <%= form_for @user,
-	       :as => :user do |f| %>
+    <%= render :partial => "shared/error_explanation",
+        :locals => { :model => @user, :flash_message => flash[:error] } %>
 
     <%= raw(f.label(:user_name, I18n.t("user.user_name"))) %>
     <%= raw(f.text_field :user_name) %><br />

diff --git a/app/views/tas/edit.html.erb b/app/views/tas/edit.html.erb
@@ -3,6 +3,7 @@
 </div>
 <div class="wrapLeft">
 <h2><%= I18n.t("graders.edit_grader") %></h2>
-<%= render :partial => "layouts/userform" %>
+<%= render :partial => "layouts/userform",
+    :locals => { :controller => 'tas', :action => 'edit'}%>
 </div>
 
diff --git a/app/views/tas/new.html.erb b/app/views/tas/new.html.erb
@@ -2,6 +2,8 @@
 <h1><%= I18n.t("graders.create_new_grader") %></h1>
 </div>
 <div class="wrapLeft">
-<%= render :partial => "layouts/userform" %>
+
+<%= render :partial => "layouts/userform", 
+    :locals => { :controller => 'tas', :action => 'create'} %>
 
 </div>
diff --git a/config/environment.rb b/config/environment.rb
@@ -1,5 +1,9 @@
 # Load the rails application
 require File.expand_path('../application', __FILE__)
 
+# Fix TA pluralization
+ActiveSupport::Inflector.inflections do |inflection| inflection.irregular "ta", "tas"
+end 
+
 # Initialize the rails application
 Markus::Application.initialize!
diff --git a/test/functional/sections_controller_test.rb b/test/functional/sections_controller_test.rb
@@ -52,18 +52,19 @@ def setup
       assert Section.find_by_name("section_01")
     end
 
-   should "be able to create a section with the same name as a existing one" do
+   should "not be able to create a section with the same name as a existing one" do
       section = Section.make
       post_as @admin, :create, {:section => {:name => section.name}}
-      assert respond_with :success
-      assert_equal flash[:error], I18n.t('section.create.error') + ' Name has already been taken.'
+      assert_response :success
+      assert_equal I18n.t('section.create.error'), flash[:error]
     end
 
-    should "be able to create a section with a blank name" do
+    should "not be able to create a section with a blank name" do
       section = Section.make
       post_as @admin, :create, {:section => {:name => ''}}
       assert respond_with :success
-      assert_equal flash[:error], I18n.t('section.create.error') + ' Name can\'t be blank.'
+      assert_nil Section.find_by_name('')
+      assert_equal I18n.t('section.create.error'), flash[:error]
     end
 
     should "be able to edit a section" do
@@ -92,8 +93,8 @@ def setup
               :update,
               :id => @section.id,
               :section => {:name => @section2.name}
-      assert_response :redirect
-      assert_equal flash[:error], I18n.t('section.update.error') + ' Name has already been taken.'
+      assert_response :success
+      assert_equal I18n.t('section.update.error'), flash[:error]
     end
   end
 

diff --git a/test/functional/students_controller_test.rb b/test/functional/students_controller_test.rb
@@ -72,11 +72,9 @@ def setup
                         :first_name => 'John'}
       assert_response :success
       assert_nil Student.find_by_user_name('jdoe')
-      assert_equal flash[:error],
-              I18n.t('student.create.error') + " Last_name can't be blank."
+      assert_equal I18n.t('student.create.error'), flash[:error]
     end
 
-
     should "be able to create a student with a section" do
       post_as @admin,
               :create,