Properly render all entered information when saving a draft failed

5 files changed, 96 insertions, 28 deletions

diff --git a/app/assets/stylesheets/screen.css.erb b/app/assets/stylesheets/screen.css.erb
index d7706d7..d825737 100644
--- a/app/assets/stylesheets/screen.css.erb
+++ b/app/assets/stylesheets/screen.css.erb
@@ -293,6 +293,11 @@ tr.odd, p.odd {
   background-color: #D3CFE5;
 }
 
+tr.invalid, tr.invalid td {
+  background-color: #a40000 !important;
+  color: white;
+}
+
 td {
   margin: 0;
   border: none;
diff --git a/app/controllers/glsa_controller.rb b/app/controllers/glsa_controller.rb
index 5668a38..657af5c 100644
--- a/app/controllers/glsa_controller.rb
+++ b/app/controllers/glsa_controller.rb
@@ -144,23 +144,13 @@ class GlsaController < ApplicationController
     return unless check_object_access!(@glsa)
     @rev = @glsa.last_revision
     
-    # Packages
-    @rev.vulnerable_packages.build(:comp => "<", :arch => "*") if @rev.vulnerable_packages.length == 0
-    @rev.unaffected_packages.build(:comp => ">=", :arch => "*") if @rev.unaffected_packages.length == 0
-    
-    # References
-    @rev.references.build if @rev.references.length == 0
-
-    @templates = {}
-    GLSAMAKER_TEMPLATE_TARGETS.each do |target|
-      @templates[target] = Template.where(:target => target).all
-    end
+    set_up_editing
   end
 
   def update
     @glsa = Glsa.find(params[:id])
     return unless check_object_access!(@glsa)
-    @prev_latest_rev = @glsa.last_revision
+    @rev = @glsa.last_revision
 
     if @glsa.nil?
       flash[:error] = "Unknown GLSA ID"
@@ -196,14 +186,17 @@ class GlsaController < ApplicationController
     revision.resolution = params[:glsa][:resolution]
 
     unless revision.save
-      flash.now[:error] = "Errors occurred while saving the Revision object: #{revision.errors.full_messages.join ', '}"
+      flash[:error] = "Errors occurred while saving the Revision object: #{revision.errors.full_messages.join ', '}"
+      set_up_editing
       render :action => "edit"
       return
     end
 
     unless @glsa.save
       flash[:error] = "Errors occurred while saving the GLSA object"
+      set_up_editing
       render :action => "edit"
+      return
     end
 
     # Bugs
@@ -216,15 +209,19 @@ class GlsaController < ApplicationController
         begin
           b = Glsamaker::Bugs::Bug.load_from_id(bug)
 
-          revision.bugs.create(
+          revision.bugs.create!(
             :bug_id => bug,
             :title => b.summary,
             :whiteboard => b.status_whiteboard,
             :arches => b.arch_cc.join(', ')
           )
+        rescue ActiveRecord::RecordInvalid => e
+          flash[:error] = "Errors occurred while saving a bug: #{e.record.errors.full_messages.join ', '}"
+          set_up_editing
+          render :action => "edit"
+          return
         rescue Exception => e
           log_error e
-          logger.info { e.inspect }
           # In case of bugzilla errors, just keep the bug #
           revision.bugs.create(
             :bug_id => bug
@@ -234,13 +231,22 @@ class GlsaController < ApplicationController
       end
     end
 
-    logger.debug params[:glsa][:package].inspect
+    logger.debug "Packages: " + params[:glsa][:package].inspect
 
     # Packages
-    params[:glsa][:package].each do |package|
+    packages = params[:glsa][:package] || []
+    packages.each do |package|
       logger.debug package.inspect
       next if package[:atom].strip == ''
-      revision.packages.create(package)
+
+      begin
+        revision.packages.create!(package)
+      rescue ActiveRecord::RecordInvalid => e
+        flash[:error] = "Errors occurred while saving a package: #{e.record.errors.full_messages.join ', '}"
+        set_up_editing
+        render :action => "edit"
+        return
+      end
     end
 
     # References
@@ -255,7 +261,14 @@ class GlsaController < ApplicationController
           reference[:url] = "http://nvd.nist.gov/nvd.cfm?cvename=#{reference[:title].strip}"
         end
 
-        revision.references.create(reference)
+        begin
+          revision.references.create(reference)
+        rescue ActiveRecord::RecordInvalid => e
+          flash[:error] = "Errors occurred while saving a reference: #{e.record.errors.full_messages.join ', '}"
+          set_up_editing
+          render :action => "edit"
+          return
+        end
       end
     end
 
@@ -442,6 +455,56 @@ class GlsaController < ApplicationController
   end
   
   protected
+  def set_up_editing
+    # Packages
+    @rev.vulnerable_packages.build(:comp => "<", :arch => "*") if @rev.vulnerable_packages.length == 0
+    @rev.unaffected_packages.build(:comp => ">=", :arch => "*") if @rev.unaffected_packages.length == 0
+
+    # References
+    if params.has_key? :glsa and params[:glsa].has_key? :reference
+      @references = []
+      params[:glsa][:reference].each do |reference|
+        @references << Reference.new(reference)
+      end
+    elsif @rev.references.length == 0
+      @references = [Reference.new]
+    else
+      @references = @rev.references
+    end
+
+    # Bugs
+    if params.has_key? :glsa and params[:glsa].has_key? :bugs
+      @bugs = []
+      params[:glsa][:bugs].each do |bug|
+        @bugs << Bug.new(:bug_id => bug)
+      end
+    else
+      @bugs = @rev.bugs
+    end
+
+    # Packages
+    if params.has_key? :glsa and params[:glsa].has_key? :package
+      @unaffected_packages = []
+      @vulnerable_packages = []
+      params[:glsa][:package].each do |package|
+        if package[:my_type] == 'vulnerable'
+          @vulnerable_packages << Package.new(package)
+        elsif package[:my_type] == 'unaffected'
+          @unaffected_packages << Package.new(package)
+        end
+      end
+    else
+      @unaffected_packages = @rev.unaffected_packages
+      @vulnerable_packages = @rev.vulnerable_packages
+    end
+
+    @templates = {}
+    GLSAMAKER_TEMPLATE_TARGETS.each do |target|
+      @templates[target] = Template.where(:target => target).all
+    end
+  end
+
+
   def rev_diff(glsa, rev_old, rev_new, format = :unified, context_lines = 3)
     @glsa = glsa
     old_text = ""
diff --git a/app/views/glsa/_package.html.erb b/app/views/glsa/_package.html.erb
index 304e991..308eda5 100644
--- a/app/views/glsa/_package.html.erb
+++ b/app/views/glsa/_package.html.erb
@@ -1,5 +1,5 @@
 <% comps = %w{ >= > *>= *> <= < *<= *< == } -%>
-<tr class="entry">
+<tr class="entry <%= 'invalid' if package.invalid? %>">
 <%= fields_for "glsa[package][]", package do |pf| %>
   <td><%= pf.text_field :atom, :class => :nice, :index => nil %></td>
   <td class="odd"><%= pf.select :comp, comps, {}, :index => nil %></td>
diff --git a/app/views/glsa/_template_popups.html.erb b/app/views/glsa/_template_popups.html.erb
index a5e9766..8abedfc 100644
--- a/app/views/glsa/_template_popups.html.erb
+++ b/app/views/glsa/_template_popups.html.erb
@@ -2,7 +2,7 @@
 <div id="templates-<%= target %>" style="display: none;" class="popup">
   <%= form_tag({:controller => 'tools', :action => 'template'}, :remote => true) do -%>
   <%= hidden_field('template', 'target', :value => target) %>
-  <%= select('template', 'id', templates[target].collect {|template| [template.name, template.id]}) %><%= image_submit_tag 'icons/next.png', :style => 'margin-top: .3em; vertical-align: top;' %>
+  <%= select('template', 'id', (@templates[target] || []).collect {|template| [template.name, template.id]}) %><%= image_submit_tag 'icons/next.png', :style => 'margin-top: .3em; vertical-align: top;' %>
   <% end -%>
 </div>
 <% end %>
\ No newline at end of file
diff --git a/app/views/glsa/edit.html.erb b/app/views/glsa/edit.html.erb
index 8dc208a..100e4ff 100644
--- a/app/views/glsa/edit.html.erb
+++ b/app/views/glsa/edit.html.erb
@@ -17,16 +17,16 @@
     <%= box_title('Metadata', :icon => 'icons/metadata.png') %>
     <p style="float: left; width: 50%;"><label for="access"><%= image_tag 'icons/access.png' %> Access:</label>
       <select name="glsa[access]" id="access">
-        <%= options_for_select [['local', 'local'], ['remote', 'remote'], ['local, remote', 'both']], @rev.access %>
+        <%= options_for_select [['local', 'local'], ['remote', 'remote'], ['local, remote', 'both']], lastrev_content(@glsa, 'access') %>
       </select>
     </p>
     <p><label for="severity"><%= image_tag 'icons/severity.png' %> Severity:</label>
       <select name="glsa[severity]" id="severity">
-        <%= options_for_select %w{ low normal high }, @rev.severity %>
+        <%= options_for_select %w{ low normal high }, lastrev_content(@glsa, 'severity') %>
       </select>
     </p>
     <p><label for="keyword"><%= image_tag 'icons/keyword.png' %> GLSA Keyword:</label>
-      <%= text_field_tag "glsa[product]", @rev.product, :class => "nice" %>
+      <%= text_field_tag "glsa[product]", lastrev_content(@glsa, 'product'), :class => "nice" %>
     </p>
   </div>
 
@@ -41,7 +41,7 @@
     <%= box_title('Bugs', :icon => 'icons/bug.png',
                   :toolbar => [{:uri => "javascript:GLSAMaker.editing.bugs.add_dialog(#{@glsa.id})", :title => 'Add bug', :icon => 'icons/plus.png'}]) %>
     <table id="bugtable">
-      <%= render :partial => "edit_bug_row", :collection => @rev.bugs, :as => :bug %>
+      <%= render :partial => "edit_bug_row", :collection => @bugs, :as => :bug %>
     </table>
   </div>
 
@@ -82,7 +82,7 @@
         <td><%= image_tag 'icons/affected.png' %> Vulnerable
         (<%= add_vulnerable_package_link(image_tag('icons/package-add.png')) %>)</td>
       </tr>
-      <%= render :partial => 'package', :collection => @rev.vulnerable_packages %>
+      <%= render :partial => 'package', :collection => @vulnerable_packages %>
     </table>
 
     <table class="padcells swap centercells" id="packages_table_unaffected">
@@ -90,7 +90,7 @@
         <td><%= image_tag 'icons/unaffected.png' %> Unaffected
             (<%= add_unaffected_package_link(image_tag('icons/package-add.png')) %>)</td>
       </tr>
-      <%= render :partial => 'package', :collection => @rev.unaffected_packages %>
+      <%= render :partial => 'package', :collection => @unaffected_packages %>
 
     </table>
   </div>
@@ -155,7 +155,7 @@
       <th></th>
     </tr>
 
-    <%= render :partial => 'reference', :collection => @rev.references %>
+    <%= render :partial => 'reference', :collection => @references %>
     </table>
   </div>