commit 459e0feb5c15f2ff8c175e2d46e372b4d83c6a4c
Author: Paul Kelly <pikelly@blueyonder.co.uk>
Date:   Fri May 14 16:01:39 2010 +0100

    Fixes #249 - Remove AS from reports
    
    Added a searchbar for hostname, reporting period and the type of report.

diff --git a/app/controllers/domain_parameters_controller.rb b/app/controllers/domain_parameters_controller.rb
deleted file mode 100644
index 2024dce78..000000000
--- a/app/controllers/domain_parameters_controller.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-class DomainParametersController < ApplicationController
-  active_scaffold :domain_parameters do |config|
-    config.columns = [ :name, :value ]
-  end
-end
diff --git a/app/controllers/hosts_controller.rb b/app/controllers/hosts_controller.rb
index b952c7e9d..86a3e21c7 100644
--- a/app/controllers/hosts_controller.rb
+++ b/app/controllers/hosts_controller.rb
@@ -6,7 +6,7 @@ class HostsController < ApplicationController
   before_filter :find_multiple, :only => [:multiple_actions, :update_multiple_parameters,
     :select_multiple_hostgroup, :select_multiple_environment, :multiple_parameters]
 
-  helper :hosts
+  helper :hosts, :reports
 
   def index
     @search = Host.search(params[:search])
@@ -170,6 +170,12 @@ class HostsController < ApplicationController
   # shows all reports for a certain host
   def reports
     @host = Host.find(params[:id])
+    # set defaults search order - cant use default scope due to bug in AR
+    # http://github.com/binarylogic/searchlogic/issues#issue/17
+    params[:search] ||= {}
+    params[:search][:order] ||=  "ascend_by_reported_at"
+    @search = Report.search(params[:search])
+    @reports = @search.paginate(:page => params[:page], :conditions => {:host_id => @host}, :include => :host)
   end
 
   def query
diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb
index cdff43fc9..fa1cea485 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -7,12 +7,23 @@ class ReportsController < ApplicationController
   # avoids storing the report data in the log files
   filter_parameter_logging :report
 
-  active_scaffold :reports do |config|
-    config.label   = "Puppet reports"
-    config.actions = [:list, :search, :delete]
-    config.columns = [:host, :reported_at, :applied, :restarted, :failed, :failed_restarts, :skipped, :config_retrival, :runtime]
-    config.list.sorting   = { :reported_at => :desc }
-    config.action_links.add 'show', :label => 'Details', :inline => false, :type => :record
+  def index
+    search_cmd  = "Report"
+    for condition in Report::METRIC
+      search_cmd += ".with('#{condition.to_s}', #{params[condition]})" if params.has_key? condition
+    end
+    search_cmd += ".search(params[:search])"
+    # set defaults search order - cant use default scope due to bug in AR
+    # http://github.com/binarylogic/searchlogic/issues#issue/17
+    params[:search] ||=  {}
+    params[:search][:order] ||=  "ascend_by_reported_at"
+
+    @search  = eval search_cmd
+    @reports = @search.paginate :page => params[:page], :include => [{:host => :domain}]
+  end
+
+  def show
+    @report = Report.find(params[:id])
   end
 
   def create
@@ -27,8 +38,13 @@ class ReportsController < ApplicationController
     end
   end
 
-  def show
+  def destroy
     @report = Report.find(params[:id])
+    if @report.destroy
+      flash[:foreman_notice] = "Successfully destroyed report."
+    else
+      flash[:foreman_error] = @report.errors.full_messages.join("<br>")
+    end
+    redirect_to reports_url
   end
-
 end
diff --git a/app/helpers/reports_helper.rb b/app/helpers/reports_helper.rb
index 67ed03be0..54f5af37f 100644
--- a/app/helpers/reports_helper.rb
+++ b/app/helpers/reports_helper.rb
@@ -1,9 +1,5 @@
 module ReportsHelper
 
-  def host_column(record)
-    link_to record.host.shortname, reports_host_path(record.host)
-  end
-
   def reported_at_column(record)
     if record.error?
       img = "hosts/warning"
@@ -15,4 +11,15 @@ module ReportsHelper
     image_tag("#{img}.png", :size => "18x18") + " " +
       link_to(time_ago_in_words(record.reported_at.getlocal), report_path(record))
   end
+
+  def reports_since builder
+    choices = [30,60,90].map{|i| OpenStruct.new :name => "#{i} minutes ago", :value => i.minutes.ago }
+    choices += (1..7).map{|i| OpenStruct.new :name => "#{pluralize(i, 'day')} ago", :value => i.days.ago }
+    choices += [OpenStruct.new :name => "1 week ago",   :value =>  1.week.ago]
+    choices += [OpenStruct.new :name => "2 weeks ago",  :value =>  2.week.ago]
+    choices += [OpenStruct.new :name => "1 month ago",  :value =>  1.month.ago]
+    choices += [OpenStruct.new :name => "3 months ago", :value =>  3.month.ago]
+    choices += [OpenStruct.new :name => "All Reports", :value =>  Report.first(:select => "created_at").created_at]
+    builder.collection_select :reported_at_gt, choices, :value, :name, {:include_blank => "Select a period"}
+  end
 end
diff --git a/app/models/host.rb b/app/models/host.rb
index 88c469801..a2d326c79 100644
--- a/app/models/host.rb
+++ b/app/models/host.rb
@@ -387,10 +387,10 @@ class Host < Puppet::Rails::Host
     data = {}
     data[:runtime] = []
     data[:resources] = []
-    data[:runtime_labels] = [ ['datetime', "Time" ],['number', "Config Retrival"], ['number', 'Total']]
+    data[:runtime_labels] = [ ['datetime', "Time" ],['number', "Config Retrieval"], ['number', 'Total']]
     data[:resources_labels] = [ ['datetime','Time']] + Report::METRIC.map{|metric| ['number', metric] }
     reports.recent(timerange).each do |r|
-      data[:runtime] << [r.reported_at.getlocal, r.config_retrival, r.runtime ]
+      data[:runtime] << [r.reported_at.getlocal, r.config_retrieval, r.runtime ]
       data[:resources] << [r.reported_at.getlocal, r.status.sort.map(&:last)].flatten
     end
     return data
diff --git a/app/models/report.rb b/app/models/report.rb
index 1b8f16d94..fd7b30e5e 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -21,6 +21,9 @@ class Report < ActiveRecord::Base
   # with_changes
   named_scope :with_changes, {:conditions => "status != 0"}
 
+  def self.per_page
+    20
+  end
   # a method that save the report values (e.g. values from METRIC)
   # it is not supported to edit status values after it has been written once.
   def status=(st)
@@ -63,7 +66,7 @@ class Report < ActiveRecord::Base
     "#{host.name} / #{reported_at.to_s}"
   end
 
-  def config_retrival
+  def config_retrieval
     t = validate_meteric("time", :config_retrieval)
     t.round_with_precision(2) if t
   end
@@ -98,13 +101,14 @@ class Report < ActiveRecord::Base
       # 1. It might be auto imported, therefore might not be valid (e.g. missing partition table etc)
       # 2. We want this to be fast and light on the db.
       # at this point, the report is important, not as much of the host
-      host.save_with_validation(perform_validation = false)
+      host.save_with_validation(false)
 
       # and save our report
       self.create! :host => host, :reported_at => report.time.utc, :log => report, :status => st
 
     rescue Exception => e
-      logger.warn "failed to process report for #{report.host} due to:#{e}"
+      logger.warn "Failed to process report for #{report.host} due to:#{e}"
+      false
     end
   end
 
diff --git a/app/views/hosts/reports.html.erb b/app/views/hosts/reports.html.erb
new file mode 100644
index 000000000..451c75c90
--- /dev/null
+++ b/app/views/hosts/reports.html.erb
@@ -0,0 +1,3 @@
+<% title "Reports for #{@host}" %>
+<%= render 'reports/list' %>
+<p><%= link_to "Host details", @host %></p>
diff --git a/app/views/hosts/reports.rhtml b/app/views/hosts/reports.rhtml
deleted file mode 100644
index 45ab85b4e..000000000
--- a/app/views/hosts/reports.rhtml
+++ /dev/null
@@ -1,2 +0,0 @@
-<%= render :active_scaffold => 'reports', :constraints  => { :host => @host}, :label => "Reports for #{@host.name}"%>
-<%= link_to 'Back', :back %>
diff --git a/app/views/layouts/standard.rhtml b/app/views/layouts/standard.rhtml
index f34edcdf9..a21c58188 100644
--- a/app/views/layouts/standard.rhtml
+++ b/app/views/layouts/standard.rhtml
@@ -17,7 +17,7 @@
       </div>
       <%= render :partial => "home/menu" -%>
     </div>
-    <%= render "common/searchbar" rescue "<!-- No searchbar facility available in this controller -->" %>
+    <%= render "common/searchbar" rescue "<!-- The search facility is not available in this controllers -->" unless params[:action] == 'show' %>
     <%= content_tag('div', flash[:foreman_error],  :class => 'flash error')  if flash[:foreman_error] -%>
     <%= content_tag('div', flash[:foreman_notice], :class => 'flash notice') if flash[:foreman_notice] -%>
     <div id="content">
diff --git a/app/views/puppetclasses/_form.html.erb b/app/views/puppetclasses/_form.html.erb
index f8a084265..d3604d417 100644
--- a/app/views/puppetclasses/_form.html.erb
+++ b/app/views/puppetclasses/_form.html.erb
@@ -5,16 +5,16 @@
   <%= f.text_field :name %>
   </p>
   <p>
-  <% field_set_tag("Operating Systems") do %> 
+  <% field_set_tag("Operating Systems") do %>
     <%= edit_habtm @puppetclass, Operatingsystem %>
   <% end -%>
   </p>
   <p>
-  <% field_set_tag("Puppet Environments") do %> 
+  <% field_set_tag("Puppet Environments") do %>
     <%= edit_habtm @puppetclass, Environment %>
   <% end -%>
   </p>
-  <% field_set_tag("Class Groups") do %> 
+  <% field_set_tag("Class Groups") do %>
     <p>
     <%= edit_habtm @puppetclass, Hostgroup %>
     </p>
diff --git a/app/views/reports/_list.html.erb b/app/views/reports/_list.html.erb
new file mode 100644
index 000000000..8448eeb9a
--- /dev/null
+++ b/app/views/reports/_list.html.erb
@@ -0,0 +1,37 @@
+<table class="list">
+  <tr>
+    <% unless defined? @host -%>
+      <th><%= order @search, :by => :host_name, :as => "Host" %></th>
+    <% end -%>
+    <th><%= order @search, :by => :reported_at %></th>
+    <th>Applied</th>
+    <th>Restarted</th>
+    <th>Failed</th>
+    <th>Failed restarts</th>
+    <th>Skipped</th>
+    <th>Config retrieval</th>
+    <th>Runtime</th>
+    <th></th>
+  </tr>
+  <% for report in @reports %>
+    <tr class="<%= cycle("even", "odd") -%>">
+      <% unless defined? @host -%>
+        <td><%= link_to h(report.host.shortname), reports_host_path(report.host) %></td>
+      <% end -%>
+      <td><%= reported_at_column(report) %></td>
+      <td><%= h report.applied %></td>
+      <td><%= h report.restarted %></td>
+      <td><%= h report.failed %></td>
+      <td><%= h report.failed_restarts %></td>
+      <td><%= h report.skipped %></td>
+      <td><%= h report.config_retrieval %></td>
+      <td><%= h report.runtime %></td>
+      <td align="right">
+        <%= link_to "Details", report %>
+        <%= link_to "Destroy", report, :confirm => "Delete report for #{report.host.name}?", :method => :delete %>
+      </td>
+    </tr>
+  <% end %>
+</table>
+<%= page_entries_info @reports %>
+<%= will_paginate @reports %>
diff --git a/app/views/reports/_search_line.html.erb b/app/views/reports/_search_line.html.erb
new file mode 100644
index 000000000..94930a5f5
--- /dev/null
+++ b/app/views/reports/_search_line.html.erb
@@ -0,0 +1,14 @@
+<% form_for @search do |f|-%>
+  <span title="A substring match">
+    <%= f.label      :host_name_like, "Host" %>
+    <%= f.text_field :host_name_like, :size => 10 %>
+  </span>
+  <span title="Time period">
+    <%= reports_since f  %>
+  </span> |
+  <% for status in Report::METRIC -%>
+    <%= check_box_tag status, "1", params[status] %>
+    <small><%= status.capitalize %></small> |
+  <% end -%>
+  <%= f.submit "Go" %> Save as tab: <%= text_field_tag :tab_name, "", :size => 10 %>
+<% end %>
diff --git a/app/views/reports/index.html.erb b/app/views/reports/index.html.erb
new file mode 100644
index 000000000..e5ea999f6
--- /dev/null
+++ b/app/views/reports/index.html.erb
@@ -0,0 +1,3 @@
+<% title "Reports" %>
+
+<%= render :partial => 'list' %>
diff --git a/app/views/reports/show.rhtml b/app/views/reports/show.rhtml
index eb9ffd475..7d01e4771 100644
--- a/app/views/reports/show.rhtml
+++ b/app/views/reports/show.rhtml
@@ -1,12 +1,8 @@
-<div class="active-scaffold-header">
-  <h2 style="border-bottom-style:none;" ><%= link_to @report.host.name, host_path(@report.host)  %> </h2>
-  Reported at <%= @report.reported_at.getlocal %>, which is <b><%= time_ago_in_words(@report.reported_at) %> ago</b>
-</div>
+<% title @report.host.name %>
+Reported at <%= @report.reported_at.getlocal %>, which is <b><%= time_ago_in_words(@report.reported_at) %> ago</b>
 
 <% if @report.log.logs.size > 0 -%>
-  <div>
-    <%= render :partial => 'output', :locals => { :logs => @report.log.logs} %>
-  </div>
+  <%= render :partial => 'output', :locals => { :logs => @report.log.logs} %>
 <% end -%>
 
 <div class="flash">
@@ -32,6 +28,7 @@
   </table>
 </div>
 
-<%= link_to 'Back', :back %>
-<%= link_to 'Delete', delete_report_path(@report) -%> or
+<%= link_to 'Back', :back %> |
+<%= link_to 'Delete', report_path(@report, :method => :delete) -%> |
+<%= link_to "Host details", @report.host %> |
 <%= link_to "Other reports for this hosts", reports_host_path(@report.host) %>
diff --git a/config/routes.rb b/config/routes.rb
index 3474bc789..141bd22ac 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -2,6 +2,7 @@ ActionController::Routing::Routes.draw do |map|
   map.resources :usergroups
   map.root :controller => "hosts"
 
+  map.resources :reports
   map.connect "node/:name", :controller => 'hosts', :action => 'externalNodes',
     :requirements => { :name => /[^\.][\w\.-]+/ }
   map.connect "/hosts/query", :controller => 'hosts', :action => 'query'
@@ -17,9 +18,6 @@ ActionController::Routing::Routes.draw do |map|
   map.audit '/audit', :controller => 'audit'
   map.statistics '/statistics', :controller => 'statistics'
   map.settings '/settings', :controller => 'home', :action => 'settings'
-  map.connect   "/reports/expire_reports", :controller => "reports", :action => "expire_reports"
-  map.connect   "/reports/expire_good_reports", :controller => "reports", :action => "expire_good_reports"
-  map.resources :reports, :active_scaffold => true
   map.resources :lookup_keys
   map.connect   "/lookup", :controller => "lookup_keys", :action => "q"
   map.resources :domains
diff --git a/test/fixtures/parameters.yml b/test/fixtures/parameters.yml
index b48c40a85..37b31a442 100644
--- a/test/fixtures/parameters.yml
+++ b/test/fixtures/parameters.yml
@@ -1,11 +1,18 @@
 # Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
 
-one:
+common:
   name: test
   value: myvalue
   type: CommonParameter
 
-two:
+domain:
   name: parameter
   value: value1
+  type: DomainParameter
+  domain: one
 
+host:
+  name: host1
+  value: host1
+  type: HostParameter
+  host: one
\ No newline at end of file
diff --git a/test/fixtures/reports.yml b/test/fixtures/reports.yml
index 5bf02933a..86d03fbaf 100644
--- a/test/fixtures/reports.yml
+++ b/test/fixtures/reports.yml
@@ -1,5 +1,10 @@
 # Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
 
+report:
+  host: one
+  log: <%= Puppet::Transaction::Report.new %>
+  reported_at: <%= 1.week.ago %>
+
 # one:
 #   column: value
 #
diff --git a/test/functional/reports_controller_test.rb b/test/functional/reports_controller_test.rb
index bcf27bbb9..6a599fcd8 100644
--- a/test/functional/reports_controller_test.rb
+++ b/test/functional/reports_controller_test.rb
@@ -1,37 +1,39 @@
 require 'test_helper'
 
 class ReportsControllerTest < ActionController::TestCase
-  def setup
-    @host = Host.create   :name => "myfullhost", :mac => "aabbecddeeff", :ip => "123.05.02.03",
-                          :domain => Domain.find_or_create_by_name("company.com"),
-                          :operatingsystem => Operatingsystem.create(:name => "linux", :major => 389),
-                          :architecture => Architecture.find_or_create_by_name("i386"),
-                          :environment => Environment.find_or_create_by_name("envy"),
-                          :disk => "empty partition"
+  def test_index
+    get :index
+    assert_response :success
+    assert_not_nil assigns('reports')
+    assert_template 'index'
   end
 
-  test "ActiveScaffold should look for Report model" do
-    assert_not_nil ReportsController.active_scaffold_config
-    assert ReportsController.active_scaffold_config.model == Report
+  def test_show
+    create_a_puppet_transaction_report
+    Report.last.update_attribute :log, @log
+    get :show, :id => Report.last.id
+    assert_template 'show'
   end
 
-  test "should get index" do
-    get :index, {}, set_session_user
-    assert_response :success
-    assert_not_nil assigns(:records)
+  def test_create_invalid
+    create_a_puppet_transaction_report
+    @log.host = nil
+    post :create, :report => @log.to_yaml
+    assert_response :error
   end
 
-  # couldn't test create, cause uses Report.import which needs yaml, but create passes it hash
-
-  # test "should create report" do
-  #   create_a_puppet_transaction_report
-
-    # assert_difference('Report.count') do
-    #   post :create, :report => { :commit => "Create", :record => { :host => h, :log => p, :reported_at => d } }
-    # end
+  def test_create_valid
+    create_a_puppet_transaction_report
+    post :create, :report => @log.to_yaml
+    assert_response :success
+  end
 
-  #   assert_redirected_to reports_path
-  # end
+  def test_destroy
+    report = Report.first
+    delete :destroy, :id => report
+    assert_redirected_to reports_url
+    assert !Report.exists?(report.id)
+  end
 
   test "should show report" do
     create_a_report
@@ -55,16 +57,20 @@ class ReportsControllerTest < ActionController::TestCase
   def create_a_report
     create_a_puppet_transaction_report
 
-    @report = Report.create :host => @host, :log => @log, :reported_at => Time.new
+    @report = Report.create :host => hosts(:one), :log => @log, :reported_at => Time.new
   end
 
   def create_a_puppet_transaction_report
     @log = Puppet::Transaction::Report.new
+    @log.time = Time.now.utc
     @log.metrics["time"] = Puppet::Util::Metric.new(:info)
-    @log.metrics["time"].values = [[:user, "User", 0.0135350227355957], [:total, "Total", 16.5941832065582], [:service, "Service", 1.46307373046875], [:package, "Package", 0.608669757843018], [:file, "File", 5.92631697654724], [:ssh_authorized_key, "Ssh authorized key", 0.00355410575866699], [:group, "Group", 0.00506687164306641], [:schedule, "Schedule", 0.00130486488342285], [:cron, "Cron", 0.0011448860168457], [:config_retrieval, "Config retrieval", 7.56520414352417], [:exec, "Exec", 1.00578165054321], [:filebucket, "Filebucket", 0.000531196594238281]]
+    @log.metrics["time"].values = [[:user, "User", 0.0135350227355957], [:total, "Total", 16.5941832065582], [:service, "Service", 1.46307373046875], [:package, "Package", 0.608669757843018], [:file, "File", 5.92631697654724], [:ssh_authorized_key, "Ssh authorize
+d key", 0.00355410575866699], [:group, "Group", 0.00506687164306641], [:schedule, "Schedule", 0.00130486488342285], [:cron, "Cron", 0.0011448860168457], [:config_retrieval, "Config retrieval", 7.56520414352417], [:exec, "Exec", 1.00578165054321], [:filebucket, "F
+ilebucket", 0.000531196594238281]]
+    @log.metrics["resources"] = Puppet::Util::Metric.new(:info)
+    @log.metrics["resources"].values = [[:total, "Total", 1273], [:applied, "Applied", 1], [:restarted, "Restarted", 0], [:skipped, "Skipped", 0], [:out_of_sync, "Out of sync", 1], [:scheduled, "Scheduled", 786], [:failed_restarts, "Failed restarts", 0], [:failed, "Failed", 0]]
     l = Puppet::Util::Log.new(:level => "notice", :message => :foo, :tags => %w{foo bar})
     @log.logs << l
     @log.save
   end
 end
-
