From fa68893510e7de7e86c480d5459484f65c044b22 Mon Sep 17 00:00:00 2001 From: Benjamin ter Kuile Date: Fri, 7 Dec 2012 13:25:51 +0100 Subject: [PATCH] Use safe dynamic finders of simply stored for suppliers sections --- Gemfile | 3 +- Gemfile.lock | 4 + .../suppliers/sections_controller.rb | 16 +- app/models/section.rb | 8 +- .../suppliers/sections_controller_spec.rb | 191 ++++++++++++++++++ 5 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 spec/controllers/suppliers/sections_controller_spec.rb diff --git a/Gemfile b/Gemfile index 201672ce..e5a0035b 100644 --- a/Gemfile +++ b/Gemfile @@ -59,7 +59,8 @@ end group :test do gem 'pry' gem 'steak' - #gem 'rb-fsevent', :require => false if RUBY_PLATFORM =~ /darwin/i + gem 'rb-fsevent', :require => false if RUBY_PLATFORM =~ /darwin/i + gem 'ruby_gntp' gem 'guard-rspec' gem 'factory_girl_rails' end diff --git a/Gemfile.lock b/Gemfile.lock index 88f57935..34278d8e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -180,6 +180,7 @@ GEM rdoc (~> 3.4) thor (>= 0.14.6, < 2.0) rake (10.0.2) + rb-fsevent (0.9.2) rdoc (3.12) json (~> 1.4) rest-client (1.6.7) @@ -200,6 +201,7 @@ GEM rspec-core (~> 2.12.0) rspec-expectations (~> 2.12.0) rspec-mocks (~> 2.12.0) + ruby_gntp (0.3.4) rubyzip (0.9.9) sass (3.2.3) sass-rails (3.2.5) @@ -274,8 +276,10 @@ DEPENDENCIES pry-remote rack-cors rails (= 3.2.9) + rb-fsevent rqrcode rspec-rails + ruby_gntp sass-rails (~> 3.2.3) simple_form simply_stored! diff --git a/app/controllers/suppliers/sections_controller.rb b/app/controllers/suppliers/sections_controller.rb index fc890fdf..6dd8bb28 100644 --- a/app/controllers/suppliers/sections_controller.rb +++ b/app/controllers/suppliers/sections_controller.rb @@ -18,7 +18,7 @@ module Suppliers # GET /sections/1 # GET /sections/1.json def show - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) respond_to do |format| format.html # show.html.erb @@ -40,7 +40,7 @@ module Suppliers # GET /sections/1/edit def edit - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) end # POST /sections @@ -63,7 +63,7 @@ module Suppliers # PUT /sections/1 # PUT /sections/1.json def update - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) respond_to do |format| if @section.update_attributes(params[:section]) @@ -79,7 +79,7 @@ module Suppliers # DELETE /sections/1 # DELETE /sections/1.json def destroy - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) @section.destroy respond_to do |format| @@ -91,7 +91,7 @@ module Suppliers # GET /sections/1/manage_tables # GET /sections/1/manage_tables.json def manage_tables - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) respond_to do |format| format.html # show.html.erb @@ -102,7 +102,7 @@ module Suppliers # GET /sections/1/tables_view # GET /sections/1/tables_view.json def tables_view - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) respond_to do |format| format.html # show.html.erb @@ -114,7 +114,7 @@ module Suppliers # POST /sections/1/add_tables {number_start: 1423, number_end: 234234} def add_tables - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) number_start = params[:number_start].to_i number_end = params[:number_end].to_i for table_number in number_start..number_end @@ -130,7 +130,7 @@ module Suppliers # POST /sections/1/arrange_tables {number_start: 1423, number_end: 234234} def arrange_tables - @section = Section.find_by_supplier_and_id(current_supplier, params[:id]) + @section = Section.find_by_supplier_id_and_id!(current_supplier.id, params[:id]) case params[:option] when 'distributed' then @section.arrange_tables_in_grid when 'by_row' then @section.arrange_tables_in_rows_of(params[:row_count].to_i) diff --git a/app/models/section.rb b/app/models/section.rb index 9b291e24..ff3305c6 100644 --- a/app/models/section.rb +++ b/app/models/section.rb @@ -14,13 +14,7 @@ class Section validates :title, presence: true validates :supplier_id, presence: true - # Probably faster to directly retreive the document and return nil - # if the supplier does not match - def self.find_by_supplier_and_id(supplier, id) - section = find(id) - return nil unless section.supplier_id == supplier.id - section - end + view :by_supplier_id_and_id, key: [:supplier_id, :_id] def occupied_tables return @occupied_tables if @occupied_tables.present? diff --git a/spec/controllers/suppliers/sections_controller_spec.rb b/spec/controllers/suppliers/sections_controller_spec.rb new file mode 100644 index 00000000..14acba42 --- /dev/null +++ b/spec/controllers/suppliers/sections_controller_spec.rb @@ -0,0 +1,191 @@ +# encoding: UTF-8 +require 'spec_helper' + +describe Suppliers::SectionsController do + before :each do + @supplier = Supplier.find_by_email('supplier@qwaiter.com') || Supplier.create(name: 'Supplier', email: 'supplier@qwaiter.com', password: 'secret') + sign_in @supplier + end + + describe "GET #index" do + it "populates an array of sections" do + base_section = @supplier.sections.first + section = create :section, supplier: @supplier + get :index + assigns(:sections).should =~[base_section, section].compact + end + + it "does not include sections from another supplier" do + base_section = @supplier.sections.first + section1 = create :section, supplier: @supplier + section2 = create :section + get :index + assigns(:sections).should =~[base_section, section1].compact + end + + it "should render without errors when no objects are present" do + get :index + expect{ render_template :index }.not_to raise_error + end + + it "renders the :index view" do + get :index + response.should render_template :index + end + end + + describe "GET #show" do + it "assigns the requested section to @section" do + section = create :section, supplier: @supplier + get :show, id: section + assigns(:section).should eq(section) + end + + it "should not display a section of another supplier" do + section = create :section + get :show, id: section + response.status.should == 404 + end + + it "renders the #show view" do + section = create :section, supplier: @supplier + get :show, id: section + response.should render_template :show + end + end + + describe "GET #new" do + it "assigns a new section to @section" do + get :new + assigns(:section).should be_a Section + end + + it "renders the #show view" do + get :new + response.should render_template :new + end + end + + describe "POST #create" do + context "with valid attributes" do + it "creates a new section" do + expect{ + post :create, section: attributes_for(:section, supplier: @supplier) + }.to change(Section, :count).by(2) + end + + it "redirects to the new section" do + post :create, section: attributes_for(:section, title: 'Created section 45', supplier: @supplier) + response.should redirect_to [:suppliers, Section.find_by_title('Created section 45')] + end + + it "should not be possible to create a section for another supplier" do + supplier2 = create :supplier + post :create, section: attributes_for(:section, title: 'Trying to hack', supplier: supplier2) + Section.find_by_title('Trying to hack').supplier_id.should == @supplier.id + end + end + + context "with invalid attributes" do + it "does not save the new section" do + expect{ + post :create, section: {title: ''} + }.to_not change(Section, :count) + end + + it "re-renders the new method" do + post :create, section: {title: ''} + response.should render_template :new + end + end + end + + describe 'PUT update' do + before :each do + @section = create :section, supplier: @supplier + end + + context "valid attributes" do + it "located the requested section" do + put :update, id: @section, section: attributes_for(:section, supplier: @supplier) + @section.reload + assigns(:section).should eq(@section) + end + + it "changes @section's attributes" do + put :update, id: @section, section: attributes_for(:section, title: "ChangedByTest", supplier: @supplier) + @section.reload + @section.title.should eq("ChangedByTest") + end + + it "redirects to the updated section" do + put :update, id: @section, section: attributes_for(:section, supplier: @supplier) + response.should redirect_to [:suppliers, @section] + end + it "should not be possible to update a section to another supplier" do + supplier2 = create :supplier + put :update, id: @section, section: attributes_for(:section, title: "Trying to hack", supplier: supplier2) + Section.find_by_title('Trying to hack').supplier_id.should == @supplier.id + end + + it "should not be possible to update a section of another supplier" do + section = create :section, title: 'Other supplier section' + put :update, id: section, section: {title: "Trying to hack"} + section.reload + section.title.should == 'Other supplier section' + end + end + + context "invalid attributes" do + it "locates the requested section" do + put :update, id: @section, section: {title: ''} + assigns(:section).should eq(@section) + end + + it "re-renders the edit method" do + put :update, id: @section, section: {title: ''} + response.should render_template :edit + end + end + end + + describe 'DELETE destroy' do + before :each do + @section = create :section, supplier: @supplier + end + + it "deletes the section" do + expect{ + delete :destroy, id: @section + }.to change(Section, :count).by(-1) + end + + it "redirects to sections#index" do + delete :destroy, id: @section + response.should redirect_to [:suppliers, :sections] + end + + it "should not be possible to delete a section of another supplier" do + section = create :section + expect{ + delete :destroy, id: section + }.to_not change(Section, :count) + end + end + + describe 'GET #manage_tables' do + pending "Add tests" + end + + describe 'GET #tables_view' do + pending "Add tests" + end + + describe 'POST #add_tables' do + pending "Add tests" + end + + describe 'POST #arrange_tables' do + pending "Add tests" + end +end