#748 Suganthi|Yekkanti Search results should display the children reg…

…istered by limited user if limited user logged in
rapidftr · Oct 17, 2012 · bd2bbd6 · bd2bbd6
1 parent e576ca7
commit bd2bbd6
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 24 deletions.
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -13,7 +13,9 @@ class ApplicationController < ActionController::Base
   rescue_from( AuthenticationFailure ) { |e| handle_authentication_failure(e) }
   rescue_from( AuthorizationFailure ) { |e| handle_authorization_failure(e) }
   rescue_from( ErrorResponse ) { |e| render_error_response(e) }
-
+  rescue_from CanCan::AccessDenied do |exception|
+    render :file => "#{Rails.root}/public/403.html", :status => 403, :layout => false
+  end
   def render_error_response(ex)
     @exception = ex
 

diff --git a/app/controllers/children_controller.rb b/app/controllers/children_controller.rb
@@ -184,7 +184,7 @@ def search
     if (params[:query])
       @search = Search.new(params[:query])
       if @search.valid?
-        @results = Child.search(@search)
+        search_by_user_access
       else
         render :search
       end
@@ -304,4 +304,13 @@ def children_by_user_access
     end
   end
 
+  def search_by_user_access
+    if can? :view_all, Child
+      @results = Child.search(@search)
+    else
+      @results = Child.search_by_created_user(@search, app_session.user_name)
+    end
+  end
+
+
 end
diff --git a/app/models/child.rb b/app/models/child.rb
@@ -198,14 +198,22 @@ def self.duplicates_of(id)
     duplicates
   end
 
-  def self.search(search)
+  def self.search_by_created_user(search, created_by)
+    created_by_criteria = [SearchCriteria.new(:field => "created_by", :value => created_by, :join => "AND")]
+    search(search, created_by_criteria, created_by)
+  end
+
+  def self.search(search, criteria = [], created_by = "")
     return [] unless search.valid?
 
     query = search.query
-    children = sunspot_search("unique_identifier_text:#{query}")
+    solr_query = "unique_identifier_text:#{query}"
+    solr_query = solr_query + "AND created_by_text:#{created_by}" unless created_by.empty?
+    children = sunspot_search(solr_query)
     return children if children.length > 0
 
-    SearchService.search [ SearchCriteria.new(:field => "name", :value => query) ]
+    search_criteria = [SearchCriteria.new(:field => "name", :value => search.query)].concat(criteria)
+    SearchService.search search_criteria
   end
 
   def self.flagged

diff --git a/capybara_features/basic_search_for_limited_user.feature b/capybara_features/basic_search_for_limited_user.feature
@@ -0,0 +1,20 @@
+Feature: So that I can find a child that has been entered in to RapidFTR
+  As a limited user of the website
+  I want to enter a search query in to a search box and see all children registered by me.
+
+  Scenario: Limited user should not see children registered by other users in search results
+    Given a user "Tim" with "Access all data" permission
+    And a user "John" with "limited" permission
+    And the following children exist in the system:
+      | name   | created_by |
+      | Andrew | Tim    |
+      | Peter  | John  |
+    When I am logged in as "John"
+    And I am on the child search page
+    When I fill in "Andrew" for "Name or Unique ID"
+    And I press "Search"
+    Then I should see "No results found"
+    When I fill in "Peter" for "Name or Unique ID"
+    And I press "Search"
+    Then I should not see "No results found"
+
diff --git a/public/403.html b/public/403.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+       "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+
+<head>
+  <meta http-equiv="content-type" content="text/html; charset=UTF-8" />
+  <title>The page you were looking for doesn't exist (404)</title>
+	<style type="text/css">
+		body { background-color: #fff; color: #666; text-align: center; font-family: arial, sans-serif; }
+		div.dialog {
+			width: 25em;
+			padding: 0 4em;
+			margin: 4em auto 0 auto;
+			border: 1px solid #ccc;
+			border-right-color: #999;
+			border-bottom-color: #999;
+		}
+		h1 { font-size: 100%; color: #f00; line-height: 1.5em; }
+	</style>
+</head>
+
+<body>
+  <!-- This file lives in public/404.html -->
+  <div class="dialog">
+    <h1>Not Authorized</h1>
+    <p>You are not authorized to access this page.</p>
+  </div>
+</body>
+</html>
diff --git a/spec/controllers/children_controller_spec.rb b/spec/controllers/children_controller_spec.rb
@@ -32,27 +32,32 @@ def mock_child(stubs={})
     describe 'collection' do
       it "GET index" do
         @controller.current_ability.should_receive(:can?).with(:list, Child).and_return(false);
-        expect { get :index }.to raise_error(CanCan::AccessDenied)
+        get :index
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "GET search" do
         @controller.current_ability.should_receive(:can?).with(:list, Child).and_return(false);
-        expect { get :search }.to raise_error(CanCan::AccessDenied)
+        get :search
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "GET export_data" do
         @controller.current_ability.should_receive(:can?).with(:list, Child).and_return(false);
-        expect { get :export_data }.to raise_error(CanCan::AccessDenied)
+        get :export_data
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "GET new" do
         @controller.current_ability.should_receive(:can?).with(:create, Child).and_return(false);
-        expect { get :new }.to raise_error(CanCan::AccessDenied)
+        get :new
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "POST create" do
         @controller.current_ability.should_receive(:can?).with(:create, Child).and_return(false);
-        expect { post :create }.to raise_error(CanCan::AccessDenied)
+        post :create
+        response.should render_template("#{Rails.root}/public/403.html")
       end
     end
 
@@ -64,37 +69,44 @@ def mock_child(stubs={})
 
       it "GET show" do
         @controller.current_ability.should_receive(:can?).with(:read, @child_arg).and_return(false);
-        expect { get :show, :id => @child.id }.to raise_error(CanCan::AccessDenied)
+         get :show, :id => @child.id
+         response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "PUT update" do
         @controller.current_ability.should_receive(:can?).with(:edit, @child_arg).and_return(false);
-        expect { put :update, :id => @child.id }.to raise_error(CanCan::AccessDenied)
+        put :update, :id => @child.id
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "PUT edit_photo" do
         @controller.current_ability.should_receive(:can?).with(:edit, @child_arg).and_return(false);
-        expect { put :edit_photo, :id => @child.id }.to raise_error(CanCan::AccessDenied)
+        put :edit_photo, :id => @child.id
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "PUT update_photo" do
         @controller.current_ability.should_receive(:can?).with(:edit, @child_arg).and_return(false);
-        expect { put :update_photo, :id => @child.id }.to raise_error(CanCan::AccessDenied)
+        put :update_photo, :id => @child.id
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "PUT select_primary_photo" do
         @controller.current_ability.should_receive(:can?).with(:edit, @child_arg).and_return(false);
-        expect { put :select_primary_photo, :child_id => @child.id, :photo_id => 0 }.to raise_error(CanCan::AccessDenied)
+        put :select_primary_photo, :child_id => @child.id, :photo_id => 0
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "GET export_photo_to_pdf" do
         @controller.current_ability.should_receive(:can?).with(:read, @child_arg).and_return(false);
-        expect { get :export_photo_to_pdf, :id => @child.id }.to raise_error(CanCan::AccessDenied)
+        get :export_photo_to_pdf, :id => @child.id
+        response.should render_template("#{Rails.root}/public/403.html")
       end
 
       it "DELETE destroy" do
         @controller.current_ability.should_receive(:can?).with(:destroy, @child_arg).and_return(false);
-        expect { delete :destroy, :id => @child.id }.to raise_error(CanCan::AccessDenied)
+        delete :destroy, :id => @child.id
+        response.should render_template("#{Rails.root}/public/403.html")
       end
     end    
   end
@@ -449,7 +461,22 @@ def mock_child(stubs={})
 
 			get(:search, :format => 'csv', :query => 'blah')
     end
-	end
+  end
+  describe "Limited search" do
+    before :each do
+      @session = fake_limited_login
+    end
+    it "should only list the children which limited user has registered" do
+      search = mock("search", :query => 'some_name', :valid? => true)
+      Search.stub!(:new).and_return(search)
+
+      fake_results = [:fake_child,:fake_child]
+      Child.should_receive(:search_by_created_user).with(search, @session.user_name).and_return(fake_results)
+
+      get(:search, :query => 'some_name')
+      assigns[:results].should == fake_results
+    end
+  end
   describe "GET photo_pdf" do
 
     it 'extracts multiple selected ids from post params in correct order' do

diff --git a/spec/controllers/roles_controller_spec.rb b/spec/controllers/roles_controller_spec.rb
@@ -6,9 +6,12 @@
 
     it "should not allow non-admin user to access any roles action" do
       fake_limited_login
-      expect { get :index }.to raise_error(CanCan::AccessDenied)
-      expect { post :create }.to raise_error(CanCan::AccessDenied)
-      expect { get :new }.to raise_error(CanCan::AccessDenied)
+      get :index
+      response.should render_template("#{Rails.root}/public/403.html")
+      post :create
+      response.should render_template("#{Rails.root}/public/403.html")
+      get :new
+      response.should render_template("#{Rails.root}/public/403.html")
     end
 
   end

diff --git a/spec/integration/search_service_spec.rb b/spec/integration/search_service_spec.rb
@@ -110,5 +110,16 @@
     result.should =~ [child1, child2]
   end
 
-
+  it "Should find children by created_by" do
+    child1 = Child.create( :name => "tim", :company => "fireman", :created_by => "john")
+    child2 = Child.create( :name => "tom", :company => "student", :created_by => "jill")
+    child3 = Child.create( :name => "tox", :company => "student", :created_by => "jill")
+
+    criteria1 = SearchCriteria.new(:field => "name", :value => "t")
+    criteria2 = SearchCriteria.new(:field => "created_by", :value => "jill", :join => "AND")
+
+    result = SearchService.search [criteria1,criteria2]
+    result.should == [child2, child3]
+  end
+
 end
diff --git a/spec/models/child_spec.rb b/spec/models/child_spec.rb
@@ -123,6 +123,19 @@
       search = mock("search", :query => "timo coch", :valid? => true)                 
       Child.search(search).map(&:name).should =~ ["timothy cochran"]
     end
+
+    it "should return the children registered by the user if the user has limited permission" do
+      create_child("suganthi", {"created_by" => "thirumani"})
+      create_child("kavitha", {"created_by" => "rajagopalan"})
+      search = mock("search", :query => "kavitha", :valid? => true)
+      Child.search_by_created_user(search, "rajagopalan").map(&:name).should =~ ["kavitha"]
+    end
+    it "should not return any results if a limited user searches with unique id of a child registerd by a different user" do
+      create_child("suganthi", {"created_by" => "thirumani", "unique_identifier" => "thirumanixxx12345"})
+      create_child("kavitha", {"created_by" => "rajagopalan", "unique_identifier" => "rajagopalanxxx12345"})
+      search = mock("search", :query => "thirumanixxx12345", :valid? => true)
+      Child.search_by_created_user(search, "rajagopalan").map(&:name).should =~ []
+    end
   end
 
   describe "update_properties_with_user_name" do
@@ -1078,8 +1091,9 @@
 
   private
 
-  def create_child(name)
-    Child.create("name" => name, "last_known_location" => "new york")
+  def create_child(name, options={})
+    options.merge!("name" => name, "last_known_location" => "new york")
+    Child.create(options)
   end
 
   def create_duplicate(parent)