Commit 95c2aa133f22b04dff7fc8b25f2af3edd78a6a84
1 parent
81301c44
Exists in
master
and in
1 other branch
Scope apps and errs when a regular user is logged in
Showing
8 changed files
with
134 additions
and
52 deletions
Show diff stats
README.md
| @@ -14,4 +14,5 @@ TODO | @@ -14,4 +14,5 @@ TODO | ||
| 14 | Add capistrano | 14 | Add capistrano |
| 15 | Add form.error_messages | 15 | Add form.error_messages |
| 16 | Add a deployment view | 16 | Add a deployment view |
| 17 | -Add ability for watchers to be configured for types of notifications they should receive | ||
| 18 | \ No newline at end of file | 17 | \ No newline at end of file |
| 18 | +Add ability for watchers to be configured for types of notifications they should receive | ||
| 19 | +Differentiate resolved errs | ||
| 19 | \ No newline at end of file | 20 | \ No newline at end of file |
app/controllers/apps_controller.rb
| 1 | class AppsController < ApplicationController | 1 | class AppsController < ApplicationController |
| 2 | 2 | ||
| 3 | before_filter :require_admin!, :except => [:index, :show] | 3 | before_filter :require_admin!, :except => [:index, :show] |
| 4 | + before_filter :find_app, :except => [:index, :new, :create] | ||
| 4 | 5 | ||
| 5 | def index | 6 | def index |
| 6 | @apps = current_user.admin? ? App.all : current_user.apps.all | 7 | @apps = current_user.admin? ? App.all : current_user.apps.all |
| 7 | end | 8 | end |
| 8 | 9 | ||
| 9 | def show | 10 | def show |
| 10 | - @app = current_user.admin? ? App.find(params[:id]) : current_user.apps.find_by_id!(params[:id]) | ||
| 11 | @errs = @app.errs.paginate | 11 | @errs = @app.errs.paginate |
| 12 | end | 12 | end |
| 13 | 13 | ||
| @@ -17,7 +17,6 @@ class AppsController < ApplicationController | @@ -17,7 +17,6 @@ class AppsController < ApplicationController | ||
| 17 | end | 17 | end |
| 18 | 18 | ||
| 19 | def edit | 19 | def edit |
| 20 | - @app = App.find(params[:id]) | ||
| 21 | @app.watchers.build if @app.watchers.none? | 20 | @app.watchers.build if @app.watchers.none? |
| 22 | end | 21 | end |
| 23 | 22 | ||
| @@ -32,9 +31,7 @@ class AppsController < ApplicationController | @@ -32,9 +31,7 @@ class AppsController < ApplicationController | ||
| 32 | end | 31 | end |
| 33 | end | 32 | end |
| 34 | 33 | ||
| 35 | - def update | ||
| 36 | - @app = App.find(params[:id]) | ||
| 37 | - | 34 | + def update |
| 38 | if @app.update_attributes(params[:app]) | 35 | if @app.update_attributes(params[:app]) |
| 39 | flash[:success] = "Good news everyone! '#{@app.name}' was successfully updated." | 36 | flash[:success] = "Good news everyone! '#{@app.name}' was successfully updated." |
| 40 | redirect_to app_path(@app) | 37 | redirect_to app_path(@app) |
| @@ -44,9 +41,18 @@ class AppsController < ApplicationController | @@ -44,9 +41,18 @@ class AppsController < ApplicationController | ||
| 44 | end | 41 | end |
| 45 | 42 | ||
| 46 | def destroy | 43 | def destroy |
| 47 | - @app = App.find(params[:id]) | ||
| 48 | @app.destroy | 44 | @app.destroy |
| 49 | flash[:success] = "'#{@app.name}' was successfully destroyed." | 45 | flash[:success] = "'#{@app.name}' was successfully destroyed." |
| 50 | redirect_to apps_path | 46 | redirect_to apps_path |
| 51 | end | 47 | end |
| 48 | + | ||
| 49 | + protected | ||
| 50 | + | ||
| 51 | + def find_app | ||
| 52 | + @app = App.find(params[:id]) | ||
| 53 | + | ||
| 54 | + # Mongoid Bug: could not chain: current_user.apps.find_by_id! | ||
| 55 | + # apparently finding by 'watchers.email' and 'id' is broken | ||
| 56 | + raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) | ||
| 57 | + end | ||
| 52 | end | 58 | end |
app/controllers/errs_controller.rb
| 1 | class ErrsController < ApplicationController | 1 | class ErrsController < ApplicationController |
| 2 | 2 | ||
| 3 | + before_filter :find_app, :except => [:index, :all] | ||
| 4 | + | ||
| 3 | def index | 5 | def index |
| 4 | - @errs = Err.unresolved.ordered.paginate(:page => params[:page]) | 6 | + app_scope = current_user.admin? ? App.all : current_user.apps |
| 7 | + @errs = Err.for_apps(app_scope).unresolved.ordered.paginate(:page => params[:page]) | ||
| 5 | end | 8 | end |
| 6 | 9 | ||
| 7 | def all | 10 | def all |
| 8 | - @errs = Err.ordered.paginate(:page => params[:page]) | 11 | + app_scope = current_user.admin? ? App.all : current_user.apps |
| 12 | + @errs = Err.for_apps(app_scope).ordered.paginate(:page => params[:page]) | ||
| 9 | end | 13 | end |
| 10 | 14 | ||
| 11 | def show | 15 | def show |
| 12 | - @app = App.find(params[:app_id]) | ||
| 13 | @err = @app.errs.find(params[:id]) | 16 | @err = @app.errs.find(params[:id]) |
| 14 | - @notices = @err.notices.ordered.paginate(:page => (params[:notice] || @err.notices.count), :per_page => 1) | 17 | + page = (params[:notice] || @err.notices.count) |
| 18 | + page = 1 if page.zero? | ||
| 19 | + @notices = @err.notices.ordered.paginate(:page => page, :per_page => 1) | ||
| 15 | @notice = @notices.first | 20 | @notice = @notices.first |
| 16 | end | 21 | end |
| 17 | 22 | ||
| 18 | def resolve | 23 | def resolve |
| 19 | - @app = App.find(params[:app_id]) | ||
| 20 | - @err = @app.errs.unresolved.find(params[:id]) | 24 | + @err = @app.errs.unresolved.find(params[:id]) |
| 21 | 25 | ||
| 22 | # Deal with bug in mogoid where find is returning an Enumberable obj | 26 | # Deal with bug in mogoid where find is returning an Enumberable obj |
| 23 | @err = @err.first if @err.respond_to?(:first) | 27 | @err = @err.first if @err.respond_to?(:first) |
| @@ -28,4 +32,14 @@ class ErrsController < ApplicationController | @@ -28,4 +32,14 @@ class ErrsController < ApplicationController | ||
| 28 | redirect_to errs_path | 32 | redirect_to errs_path |
| 29 | end | 33 | end |
| 30 | 34 | ||
| 35 | + protected | ||
| 36 | + | ||
| 37 | + def find_app | ||
| 38 | + @app = App.find(params[:app_id]) | ||
| 39 | + | ||
| 40 | + # Mongoid Bug: could not chain: current_user.apps.find_by_id! | ||
| 41 | + # apparently finding by 'watchers.email' and 'id' is broken | ||
| 42 | + raise(Mongoid::Errors::DocumentNotFound.new(App,@app.id)) unless current_user.admin? || current_user.watching?(@app) | ||
| 43 | + end | ||
| 44 | + | ||
| 31 | end | 45 | end |
app/models/app.rb
| @@ -22,7 +22,8 @@ class App | @@ -22,7 +22,8 @@ class App | ||
| 22 | 22 | ||
| 23 | # Mongoid Bug: find(id) on association proxies returns an Enumerator | 23 | # Mongoid Bug: find(id) on association proxies returns an Enumerator |
| 24 | def self.find_by_id!(app_id) | 24 | def self.find_by_id!(app_id) |
| 25 | - where(:id => app_id).first || raise(Mongoid::Errors::DocumentNotFound.new(self,key)) | 25 | + raise app_id.inspect |
| 26 | + where(:id => app_id).first || raise(Mongoid::Errors::DocumentNotFound.new(self,app_id)) | ||
| 26 | end | 27 | end |
| 27 | 28 | ||
| 28 | def self.find_by_api_key!(key) | 29 | def self.find_by_api_key!(key) |
app/models/err.rb
| @@ -19,6 +19,7 @@ class Err | @@ -19,6 +19,7 @@ class Err | ||
| 19 | scope :unresolved, where(:resolved => false) | 19 | scope :unresolved, where(:resolved => false) |
| 20 | scope :ordered, order_by(:last_notice_at.desc) | 20 | scope :ordered, order_by(:last_notice_at.desc) |
| 21 | scope :in_env, lambda {|env| where(:environment => env)} | 21 | scope :in_env, lambda {|env| where(:environment => env)} |
| 22 | + scope :for_apps, lambda {|apps| where(:app_id.in => apps.all.map(&:id))} | ||
| 22 | 23 | ||
| 23 | def self.for(attrs) | 24 | def self.for(attrs) |
| 24 | app = attrs.delete(:app) | 25 | app = attrs.delete(:app) |
app/models/user.rb
| @@ -20,7 +20,11 @@ class User | @@ -20,7 +20,11 @@ class User | ||
| 20 | end | 20 | end |
| 21 | 21 | ||
| 22 | def apps | 22 | def apps |
| 23 | - App.where('watchers.user_id' => id) | 23 | + App.where('watchers.user_id' => id.to_s) |
| 24 | + end | ||
| 25 | + | ||
| 26 | + def watching?(app) | ||
| 27 | + apps.all.include?(app) | ||
| 24 | end | 28 | end |
| 25 | 29 | ||
| 26 | protected | 30 | protected |
app/views/apps/_fields.html.haml
| @@ -12,7 +12,7 @@ | @@ -12,7 +12,7 @@ | ||
| 12 | %div.nested | 12 | %div.nested |
| 13 | %div | 13 | %div |
| 14 | = w.label :user | 14 | = w.label :user |
| 15 | - = w.select :user_id, User.all.map{|u| [u.name,u.id]}, :include_blank => '-- Select a User --' | 15 | + = w.select :user_id, User.all.map{|u| [u.name,u.id.to_s]}, :include_blank => '-- Select a User --' |
| 16 | %strong.option - OR - | 16 | %strong.option - OR - |
| 17 | %div | 17 | %div |
| 18 | = w.label :email | 18 | = w.label :email |
spec/controllers/errs_controller_spec.rb
| @@ -9,33 +9,59 @@ describe ErrsController do | @@ -9,33 +9,59 @@ describe ErrsController do | ||
| 9 | 9 | ||
| 10 | let(:app) { Factory(:app) } | 10 | let(:app) { Factory(:app) } |
| 11 | let(:err) { Factory(:err, :app => app) } | 11 | let(:err) { Factory(:err, :app => app) } |
| 12 | - | ||
| 13 | - before do | ||
| 14 | - sign_in Factory(:user) | ||
| 15 | - end | ||
| 16 | - | 12 | + |
| 17 | describe "GET /errs" do | 13 | describe "GET /errs" do |
| 18 | - it "gets a paginated list of unresolved errs" do | ||
| 19 | - errs = WillPaginate::Collection.new(1,30) | ||
| 20 | - 3.times { errs << Factory(:err) } | ||
| 21 | - Err.should_receive(:unresolved).and_return( | ||
| 22 | - mock('proxy', :ordered => mock('proxy', :paginate => errs)) | ||
| 23 | - ) | ||
| 24 | - get :index | ||
| 25 | - assigns(:errs).should == errs | 14 | + context 'when logged in as an admin' do |
| 15 | + it "gets a paginated list of unresolved errs" do | ||
| 16 | + sign_in Factory(:admin) | ||
| 17 | + errs = WillPaginate::Collection.new(1,30) | ||
| 18 | + 3.times { errs << Factory(:err) } | ||
| 19 | + Err.should_receive(:unresolved).and_return( | ||
| 20 | + mock('proxy', :ordered => mock('proxy', :paginate => errs)) | ||
| 21 | + ) | ||
| 22 | + get :index | ||
| 23 | + assigns(:errs).should == errs | ||
| 24 | + end | ||
| 25 | + end | ||
| 26 | + | ||
| 27 | + context 'when logged in as a user' do | ||
| 28 | + it 'gets a paginated list of unresolved errs for the users apps' do | ||
| 29 | + sign_in(user = Factory(:user)) | ||
| 30 | + unwatched_err = Factory(:err) | ||
| 31 | + watched_unresolved_err = Factory(:err, :app => Factory(:watcher, :user => user).app, :resolved => false) | ||
| 32 | + watched_resolved_err = Factory(:err, :app => Factory(:watcher, :user => user).app, :resolved => true) | ||
| 33 | + get :index | ||
| 34 | + assigns(:errs).should include(watched_unresolved_err) | ||
| 35 | + assigns(:errs).should_not include(unwatched_err, watched_resolved_err) | ||
| 36 | + end | ||
| 26 | end | 37 | end |
| 27 | end | 38 | end |
| 28 | 39 | ||
| 29 | describe "GET /errs/all" do | 40 | describe "GET /errs/all" do |
| 30 | - it "gets a paginated list of all errs" do | ||
| 31 | - errs = WillPaginate::Collection.new(1,30) | ||
| 32 | - 3.times { errs << Factory(:err) } | ||
| 33 | - 3.times { errs << Factory(:err, :resolved => true)} | ||
| 34 | - Err.should_receive(:ordered).and_return( | ||
| 35 | - mock('proxy', :paginate => errs) | ||
| 36 | - ) | ||
| 37 | - get :index | ||
| 38 | - assigns(:errs).should == errs | 41 | + context 'when logged in as an admin' do |
| 42 | + it "gets a paginated list of all errs" do | ||
| 43 | + sign_in Factory(:admin) | ||
| 44 | + errs = WillPaginate::Collection.new(1,30) | ||
| 45 | + 3.times { errs << Factory(:err) } | ||
| 46 | + 3.times { errs << Factory(:err, :resolved => true)} | ||
| 47 | + Err.should_receive(:ordered).and_return( | ||
| 48 | + mock('proxy', :paginate => errs) | ||
| 49 | + ) | ||
| 50 | + get :all | ||
| 51 | + assigns(:errs).should == errs | ||
| 52 | + end | ||
| 53 | + end | ||
| 54 | + | ||
| 55 | + context 'when logged in as a user' do | ||
| 56 | + it 'gets a paginated list of all errs for the users apps' do | ||
| 57 | + sign_in(user = Factory(:user)) | ||
| 58 | + unwatched_err = Factory(:err) | ||
| 59 | + watched_unresolved_err = Factory(:err, :app => Factory(:watcher, :user => user).app, :resolved => false) | ||
| 60 | + watched_resolved_err = Factory(:err, :app => Factory(:watcher, :user => user).app, :resolved => true) | ||
| 61 | + get :all | ||
| 62 | + assigns(:errs).should include(watched_resolved_err, watched_unresolved_err) | ||
| 63 | + assigns(:errs).should_not include(unwatched_err) | ||
| 64 | + end | ||
| 39 | end | 65 | end |
| 40 | end | 66 | end |
| 41 | 67 | ||
| @@ -44,28 +70,57 @@ describe ErrsController do | @@ -44,28 +70,57 @@ describe ErrsController do | ||
| 44 | 3.times { Factory(:notice, :err => err)} | 70 | 3.times { Factory(:notice, :err => err)} |
| 45 | end | 71 | end |
| 46 | 72 | ||
| 47 | - it "finds the app" do | ||
| 48 | - get :show, :app_id => app.id, :id => err.id | ||
| 49 | - assigns(:app).should == app | ||
| 50 | - end | 73 | + context 'when logged in as an admin' do |
| 74 | + before do | ||
| 75 | + sign_in Factory(:admin) | ||
| 76 | + end | ||
| 77 | + | ||
| 78 | + it "finds the app" do | ||
| 79 | + get :show, :app_id => app.id, :id => err.id | ||
| 80 | + assigns(:app).should == app | ||
| 81 | + end | ||
| 82 | + | ||
| 83 | + it "finds the err" do | ||
| 84 | + get :show, :app_id => app.id, :id => err.id | ||
| 85 | + assigns(:err).should == err | ||
| 86 | + end | ||
| 51 | 87 | ||
| 52 | - it "finds the err" do | ||
| 53 | - get :show, :app_id => app.id, :id => err.id | ||
| 54 | - assigns(:err).should == err | 88 | + it "paginates the notices, 1 at a time" do |
| 89 | + App.stub(:find).with(app.id).and_return(app) | ||
| 90 | + app.errs.stub(:find).with(err.id).and_return(err) | ||
| 91 | + err.notices.should_receive(:ordered).and_return(proxy = stub('proxy')) | ||
| 92 | + proxy.should_receive(:paginate).with(:page => 3, :per_page => 1). | ||
| 93 | + and_return(WillPaginate::Collection.new(1,1) << err.notices.first) | ||
| 94 | + get :show, :app_id => app.id, :id => err.id | ||
| 95 | + end | ||
| 55 | end | 96 | end |
| 56 | 97 | ||
| 57 | - it "paginates the notices, 1 at a time" do | ||
| 58 | - App.stub(:find).with(app.id).and_return(app) | ||
| 59 | - app.errs.stub(:find).with(err.id).and_return(err) | ||
| 60 | - err.notices.should_receive(:ordered).and_return(proxy = stub('proxy')) | ||
| 61 | - proxy.should_receive(:paginate).with(:page => 3, :per_page => 1). | ||
| 62 | - and_return(WillPaginate::Collection.new(1,1) << err.notices.first) | ||
| 63 | - get :show, :app_id => app.id, :id => err.id | 98 | + context 'when logged in as a user' do |
| 99 | + before do | ||
| 100 | + sign_in(@user = Factory(:user)) | ||
| 101 | + @unwatched_err = Factory(:err) | ||
| 102 | + @watched_app = Factory(:app) | ||
| 103 | + @watcher = Factory(:watcher, :user => @user, :app => @watched_app) | ||
| 104 | + @watched_err = Factory(:err, :app => @watched_app) | ||
| 105 | + end | ||
| 106 | + | ||
| 107 | + it 'finds the err if the user is watching the app' do | ||
| 108 | + get :show, :app_id => @watched_app.to_param, :id => @watched_err.id | ||
| 109 | + assigns(:err).should == @watched_err | ||
| 110 | + end | ||
| 111 | + | ||
| 112 | + it 'raises a DocumentNotFound error if the user is not watching the app' do | ||
| 113 | + lambda { | ||
| 114 | + get :show, :app_id => @unwatched_err.app_id, :id => @unwatched_err.id | ||
| 115 | + }.should raise_error(Mongoid::Errors::DocumentNotFound) | ||
| 116 | + end | ||
| 64 | end | 117 | end |
| 65 | end | 118 | end |
| 66 | 119 | ||
| 67 | describe "PUT /apps/:app_id/errs/:id/resolve" do | 120 | describe "PUT /apps/:app_id/errs/:id/resolve" do |
| 68 | before do | 121 | before do |
| 122 | + sign_in Factory(:admin) | ||
| 123 | + | ||
| 69 | @err = Factory(:err) | 124 | @err = Factory(:err) |
| 70 | App.stub(:find).with(@err.app.id).and_return(@err.app) | 125 | App.stub(:find).with(@err.app.id).and_return(@err.app) |
| 71 | @err.app.errs.stub(:unresolved). | 126 | @err.app.errs.stub(:unresolved). |