Merge pull request #69 from fxposter/refactoring

Small enhancements, more to go

Merge pull request #69 from fxposter/refactoring
Small enhancements, more to go
Nathan B
2 parents abf96589 9277301c
Showing 6 changed files with 50 additions and 48 deletions Show diff stats
app/controllers/application_controller.rb
app/controllers/apps_controller.rb
app/controllers/deploys_controller.rb
app/controllers/errs_controller.rb
app/models/err.rb
app/views/apps/show.html.haml
@@ -6,7 +6,7 @@ class ApplicationController &lt; ActionController::Base
   protected
     def require_admin!
-      redirect_to(root_path) and return(false) unless user_signed_in? && current_user.admin?
+      redirect_to root_path unless user_signed_in? && current_user.admin?
     end
 end
@@ -5,17 +5,14 @@ class AppsController &lt; InheritedResources::Base
   respond_to :html
   def show
-    where_clause = {}
     respond_to do |format|
       format.html do
-        where_clause[:environment] = params[:environment] if(params[:environment].present?)
-        if(params[:all_errs])
-          @errs = resource.errs.where(where_clause).ordered.paginate(:page => params[:page], :per_page => current_user.per_page)
-          @all_errs = true
-        else
-          @errs = resource.errs.unresolved.where(where_clause).ordered.paginate(:page => params[:page], :per_page => current_user.per_page)
-          @all_errs = false
-        end
+        @all_errs = !!params[:all_errs]
+
+        @errs = resource.errs
+        @errs = @errs.unresolved unless @all_errs
+        @errs = @errs.in_env(params[:environment]).ordered.paginate(:page => params[:page], :per_page => current_user.per_page)
+
         @deploys = @app.deploys.order_by(:created_at.desc).limit(5)
       end
       format.atom do
@@ -46,7 +43,6 @@ class AppsController &lt; InheritedResources::Base
     edit!
   end
-
   protected
     def initialize_subclassed_issue_tracker
       if params[:app][:issue_tracker_attributes] && tracker_type = params[:app][:issue_tracker_attributes][:type]
@@ -7,34 +7,40 @@ class DeploysController &lt; ApplicationController
   def create
     @app = App.find_by_api_key!(params[:api_key])
-    if params[:deploy]
-      deploy = {
-        :username     => params[:deploy][:local_username],
-        :environment  => params[:deploy][:rails_env],
-        :repository   => params[:deploy][:scm_repository],
-        :revision     => params[:deploy][:scm_revision],
-        :message      => params[:deploy][:message]
-      }
-    end
-
-    # handle Heroku's HTTP post deployhook format
-    deploy ||= {
-      :username     => params[:user],
-      :environment  => params[:rack_env].try(:downcase) || params[:app],
-      :repository   => "git@heroku.com:#{params[:app]}.git",
-      :revision     => params[:head],
-    }
-
-    @deploy = @app.deploys.create!(deploy)
+    @deploy = @app.deploys.create!(default_deploy || heroku_deploy)
     render :xml => @deploy
   end
   def index
     # See AppsController#find_app for the reasoning behind this code.
     app = App.find(params[:app_id])
-    raise(Mongoid::Errors::DocumentNotFound.new(App,app.id)) unless current_user.admin? || current_user.watching?(app)
+    raise Mongoid::Errors::DocumentNotFound.new(App, app.id) unless current_user.admin? || current_user.watching?(app)
-    @deploys = app.deploys.order_by(:created_at.desc).paginate(:page =>  params[:page], :per_page => 10)
+    @deploys = app.deploys.order_by(:created_at.desc).paginate(:page => params[:page], :per_page => 10)
   end
-
+  
+  private
+  
+    def default_deploy
+      if params[:deploy]
+        {
+          :username     => params[:deploy][:local_username],
+          :environment  => params[:deploy][:rails_env],
+          :repository   => params[:deploy][:scm_repository],
+          :revision     => params[:deploy][:scm_revision],
+          :message      => params[:deploy][:message]
+        }
+      end
+    end
+  
+    # handle Heroku's HTTP post deployhook format
+    def heroku_deploy
+      {
+        :username     => params[:user],
+        :environment  => params[:rack_env].try(:downcase) || params[:app],
+        :repository   => "git@heroku.com:#{params[:app]}.git",
+        :revision     => params[:head],
+      }
+    end
+    
 end
@@ -5,15 +5,12 @@ class ErrsController &lt; ApplicationController
   def index
     app_scope = current_user.admin? ? App.all : current_user.apps
-    where_clause = {}
-    where_clause[:environment] = params[:environment] if(params[:environment].present?)
+    @errs = Err.for_apps(app_scope).in_env(params[:environment]).unresolved.ordered
     respond_to do |format|
       format.html do
-        @errs = Err.for_apps(app_scope).where(where_clause).unresolved.ordered.paginate(:page => params[:page], :per_page => current_user.per_page)
-      end
-      format.atom do
-        @errs = Err.for_apps(app_scope).where(where_clause).unresolved.ordered
+        @errs = @errs.paginate(:page => params[:page], :per_page => current_user.per_page)
       end
+      format.atom
     end
   end
@@ -25,8 +25,11 @@ class Err
   scope :resolved, where(:resolved => true)
   scope :unresolved, where(:resolved => false)
   scope :ordered, order_by(:last_notice_at.desc)
-  scope :in_env, lambda {|env| where(:environment => env)}
   scope :for_apps, lambda {|apps| where(:app_id.in => apps.all.map(&:id))}
+  
+  def self.in_env(env)
+    env.present? ? where(:environment => env) : scoped
+  end
   def self.for(attrs)
     app = attrs.delete(:app)
@@ -13,14 +13,14 @@
   - if current_user.admin?
     = link_to 'edit', edit_app_path(@app), :class => 'button'
     = link_to 'destroy', app_path(@app), :method => :delete, :confirm => 'Seriously?', :class => 'button'
-    - if @all_errs == true
+    - if @all_errs
       = link_to 'unresolved errs', app_path(@app), :class => 'button'
     - else
-      = link_to 'all errs', app_path(@app, {:all_errs => true}), :class => 'button'
+      = link_to 'all errs', app_path(@app, :all_errs => true), :class => 'button'
-%h3{:id => 'watchers_toggle'}
+%h3#watchers_toggle
   Watchers
-  %span{:class => 'click_span'} (show/hide)
+  %span.click_span (show/hide)
 #watchers_div
   - if @app.notify_all_users
     %table.watchers
@@ -42,9 +42,9 @@
               %em Sadly, no one is watching this app
 - if @app.github_url?
-  %h3{:id => 'repository_toggle'}
+  %h3#repository_toggle
     Repository
-    %span{:class => 'click_span'} (show/hide)
+    %span.click_span (show/hide)
   #repository_div
     %table.repository
       %thead
@@ -54,9 +54,9 @@
         %tr
           %td= link_to(@app.github_url, @app.github_url, :target => '_blank')
-%h3{:id => 'deploys_toggle'}
+%h3#deploys_toggle
   Latest Deploys
-  %span{:class => 'click_span'} (show/hide)
+  %span.click_span (show/hide)
 #deploys_div
   - if @deploys.any?
     %table.deploys
	@@ -6,7 +6,7 @@ class ApplicationController < ActionController::Base		@@ -6,7 +6,7 @@ class ApplicationController < ActionController::Base
6	protected	6	protected
7		7
8	def require_admin!	8	def require_admin!
9	- redirect_to(root_path) and return(false) unless user_signed_in? && current_user.admin?	9	+ redirect_to root_path unless user_signed_in? && current_user.admin?
10	end	10	end
11		11
12	end	12	end
	@@ -13,14 +13,14 @@		@@ -13,14 +13,14 @@
13	- if current_user.admin?	13	- if current_user.admin?
14	= link_to 'edit', edit_app_path(@app), :class => 'button'	14	= link_to 'edit', edit_app_path(@app), :class => 'button'
15	= link_to 'destroy', app_path(@app), :method => :delete, :confirm => 'Seriously?', :class => 'button'	15	= link_to 'destroy', app_path(@app), :method => :delete, :confirm => 'Seriously?', :class => 'button'
16	- - if @all_errs == true	16	+ - if @all_errs
17	= link_to 'unresolved errs', app_path(@app), :class => 'button'	17	= link_to 'unresolved errs', app_path(@app), :class => 'button'
18	- else	18	- else
19	- = link_to 'all errs', app_path(@app, {:all_errs => true}), :class => 'button'	19	+ = link_to 'all errs', app_path(@app, :all_errs => true), :class => 'button'
20		20
21	-%h3{:id => 'watchers_toggle'}	21	+%h3#watchers_toggle
22	Watchers	22	Watchers
23	- %span{:class => 'click_span'} (show/hide)	23	+ %span.click_span (show/hide)
24	#watchers_div	24	#watchers_div
25	- if @app.notify_all_users	25	- if @app.notify_all_users
26	%table.watchers	26	%table.watchers
	@@ -42,9 +42,9 @@		@@ -42,9 +42,9 @@
42	%em Sadly, no one is watching this app	42	%em Sadly, no one is watching this app
43		43
44	- if @app.github_url?	44	- if @app.github_url?
45	- %h3{:id => 'repository_toggle'}	45	+ %h3#repository_toggle
46	Repository	46	Repository
47	- %span{:class => 'click_span'} (show/hide)	47	+ %span.click_span (show/hide)
48	#repository_div	48	#repository_div
49	%table.repository	49	%table.repository
50	%thead	50	%thead
	@@ -54,9 +54,9 @@		@@ -54,9 +54,9 @@
54	%tr	54	%tr
55	%td= link_to(@app.github_url, @app.github_url, :target => '_blank')	55	%td= link_to(@app.github_url, @app.github_url, :target => '_blank')
56		56
57	-%h3{:id => 'deploys_toggle'}	57	+%h3#deploys_toggle
58	Latest Deploys	58	Latest Deploys
59	- %span{:class => 'click_span'} (show/hide)	59	+ %span.click_span (show/hide)
60	#deploys_div	60	#deploys_div
61	- if @deploys.any?	61	- if @deploys.any?
62	%table.deploys	62	%table.deploys