Commit e390e540994169cb0b13e454dc8e45ce67419b88
1 parent
6d763d65
Exists in
colab
and in
4 other branches
Correct error handling for resource finding in controllers
Error handling for finding resources in controllers used a find_resource method. It was broken and was letting code pass-through after errors. In addition, cases for find methods for things other than simple IDs weren't handled. Replace it with an error handler in ApplicationController capturing RecordNotFound exceptions and returning the appropriate error pages (or JSON). Also, fix usages of find that should throw errors (using ActiveRecord methods ending with !) but didn't. Correspondly, fix all the tests that mocked the find_resource method, and correct expectations where needed. Signed-off-by: Daniel Miranda <danielkza2@gmail.com>
Showing
25 changed files
with
74 additions
and
122 deletions
Show diff stats
app/controllers/application_controller.rb
| ... | ... | @@ -10,6 +10,9 @@ class ApplicationController < ActionController::Base |
| 10 | 10 | before_filter :configure_permitted_parameters, if: :devise_controller? |
| 11 | 11 | before_filter :set_locale |
| 12 | 12 | |
| 13 | + rescue_from ActiveRecord::RecordNotFound, with: :not_found | |
| 14 | + rescue_from KalibroClient::Errors::RecordNotFound, with: :not_found | |
| 15 | + | |
| 13 | 16 | class << self |
| 14 | 17 | # This is necessary for correct devise routing with locales: https://github.com/plataformatec/devise/wiki/How-To:--Redirect-with-locale-after-authentication-failure |
| 15 | 18 | def default_url_options |
| ... | ... | @@ -28,6 +31,13 @@ class ApplicationController < ActionController::Base |
| 28 | 31 | |
| 29 | 32 | protected |
| 30 | 33 | |
| 34 | + def not_found(exception) | |
| 35 | + respond_to do |format| | |
| 36 | + format.html { render file: "#{Rails.root}/public/404", layout: false, status: :not_found } | |
| 37 | + format.json { head :not_found } | |
| 38 | + end | |
| 39 | + end | |
| 40 | + | |
| 31 | 41 | # We don't have a way to test this unless we have the Devise controllers among our code. |
| 32 | 42 | # Since creating the controllers looks wronger than not testing this two |
| 33 | 43 | # lines. I think we can live without 100% of coverage | ... | ... |
app/controllers/base_metric_configurations_controller.rb
app/controllers/concerns/metric_configurations_concern.rb
| ... | ... | @@ -2,6 +2,6 @@ module MetricConfigurationsConcern |
| 2 | 2 | extend ActiveSupport::Concern |
| 3 | 3 | |
| 4 | 4 | def set_metric_configuration |
| 5 | - @metric_configuration = find_resource(MetricConfiguration, params[:id].to_i) | |
| 5 | + @metric_configuration = MetricConfiguration.find(params[:id].to_i) | |
| 6 | 6 | end |
| 7 | 7 | end | ... | ... |
app/controllers/concerns/resource_finder.rb
| ... | ... | @@ -1,15 +0,0 @@ |
| 1 | -module ResourceFinder | |
| 2 | - extend ActiveSupport::Concern | |
| 3 | - | |
| 4 | - def find_resource(klass, id) | |
| 5 | - begin | |
| 6 | - klass.find(id) | |
| 7 | - rescue KalibroClient::Errors::RecordNotFound | |
| 8 | - respond_to do |format| | |
| 9 | - format.html { render file: "#{Rails.root}/public/404", layout: false, status: :not_found } | |
| 10 | - end | |
| 11 | - | |
| 12 | - return | |
| 13 | - end | |
| 14 | - end | |
| 15 | -end | |
| 16 | 0 | \ No newline at end of file |
app/controllers/kalibro_configurations_controller.rb
| 1 | 1 | include OwnershipAuthentication |
| 2 | -include ResourceFinder | |
| 3 | 2 | |
| 4 | 3 | class KalibroConfigurationsController < ApplicationController |
| 5 | 4 | before_action :authenticate_user!, except: [:index, :show] |
| ... | ... | @@ -54,7 +53,7 @@ class KalibroConfigurationsController < ApplicationController |
| 54 | 53 | # DELETE /kalibro_configurations/1.json |
| 55 | 54 | def destroy |
| 56 | 55 | set_kalibro_configuration |
| 57 | - current_user.kalibro_configuration_ownerships.find_by_kalibro_configuration_id(@kalibro_configuration.id).destroy | |
| 56 | + current_user.kalibro_configuration_ownerships.find_by_kalibro_configuration_id!(@kalibro_configuration.id).destroy | |
| 58 | 57 | @kalibro_configuration.destroy |
| 59 | 58 | respond_to do |format| |
| 60 | 59 | format.html { redirect_to kalibro_configurations_url } |
| ... | ... | @@ -66,7 +65,7 @@ class KalibroConfigurationsController < ApplicationController |
| 66 | 65 | private |
| 67 | 66 | # Use callbacks to share common setup or constraints between actions. |
| 68 | 67 | def set_kalibro_configuration |
| 69 | - @kalibro_configuration = find_resource(KalibroConfiguration, params[:id].to_i) | |
| 68 | + @kalibro_configuration = KalibroConfiguration.find(params[:id].to_i) | |
| 70 | 69 | end |
| 71 | 70 | |
| 72 | 71 | # Never trust parameters from the scary internet, only allow the white list through. | ... | ... |
app/controllers/kalibro_ranges_controller.rb
| 1 | 1 | include OwnershipAuthentication |
| 2 | -include ResourceFinder | |
| 3 | 2 | |
| 4 | 3 | class KalibroRangesController < ApplicationController |
| 5 | 4 | before_action :authenticate_user!, except: [:show] |
| ... | ... | @@ -60,7 +59,8 @@ class KalibroRangesController < ApplicationController |
| 60 | 59 | end |
| 61 | 60 | |
| 62 | 61 | def format_metric_configuration_path(format, notice) |
| 63 | - @metric_configuration = MetricConfiguration.find @kalibro_range.metric_configuration_id | |
| 62 | + @metric_configuration = MetricConfiguration.find(@kalibro_range.metric_configuration_id) | |
| 63 | + | |
| 64 | 64 | if(@metric_configuration.metric.is_a? KalibroClient::Entities::Miscellaneous::CompoundMetric) |
| 65 | 65 | format.html { redirect_to kalibro_configuration_compound_metric_configuration_path( |
| 66 | 66 | @kalibro_configuration_id, @metric_configuration_id), notice: notice } |
| ... | ... | @@ -93,6 +93,6 @@ class KalibroRangesController < ApplicationController |
| 93 | 93 | end |
| 94 | 94 | |
| 95 | 95 | def set_kalibro_range |
| 96 | - @kalibro_range = find_resource(KalibroRange, params[:id].to_i) | |
| 96 | + @kalibro_range = KalibroRange.find(params[:id].to_i) | |
| 97 | 97 | end |
| 98 | 98 | end | ... | ... |
app/controllers/metric_configurations_controller.rb
| ... | ... | @@ -7,6 +7,7 @@ class MetricConfigurationsController < BaseMetricConfigurationsController |
| 7 | 7 | |
| 8 | 8 | def new |
| 9 | 9 | super |
| 10 | + # find_by_name throws an exception instead of returning nil, unlike ActiveRecord's API | |
| 10 | 11 | metric_configuration.metric = KalibroClient::Entities::Processor::MetricCollectorDetails.find_by_name(params[:metric_collector_name]).find_metric_by_code params[:metric_code] |
| 11 | 12 | end |
| 12 | 13 | ... | ... |
app/controllers/modules_controller.rb
| 1 | -include ResourceFinder | |
| 2 | - | |
| 3 | 1 | class ModulesController < ApplicationController |
| 4 | 2 | # POST /modules/1/metric_history |
| 5 | 3 | def metric_history |
| 6 | - @module_result = find_resource(ModuleResult, params[:id].to_i) | |
| 4 | + @module_result = ModuleResult.find(params[:id].to_i) | |
| 7 | 5 | @container = params[:container] |
| 8 | 6 | @metric_name = params[:metric_name] |
| 9 | 7 | end |
| 10 | 8 | |
| 11 | 9 | # POST /modules/1/tree |
| 12 | 10 | def load_module_tree |
| 13 | - @root_module_result = find_resource(ModuleResult, params[:id].to_i) | |
| 11 | + @root_module_result = ModuleResult.find(params[:id].to_i) | |
| 14 | 12 | end |
| 15 | -end | |
| 16 | 13 | \ No newline at end of file |
| 14 | +end | ... | ... |
app/controllers/projects_controller.rb
| 1 | 1 | include OwnershipAuthentication |
| 2 | -include ResourceFinder | |
| 3 | 2 | |
| 4 | 3 | class ProjectsController < ApplicationController |
| 5 | 4 | before_action :authenticate_user!, |
| ... | ... | @@ -55,7 +54,7 @@ class ProjectsController < ApplicationController |
| 55 | 54 | # DELETE /project/1.json |
| 56 | 55 | def destroy |
| 57 | 56 | set_project |
| 58 | - current_user.project_attributes.find_by_project_id(@project.id).destroy | |
| 57 | + current_user.project_attributes.find_by_project_id!(@project.id).destroy | |
| 59 | 58 | @project.destroy |
| 60 | 59 | respond_to do |format| |
| 61 | 60 | format.html { redirect_to projects_url } |
| ... | ... | @@ -66,7 +65,7 @@ class ProjectsController < ApplicationController |
| 66 | 65 | private |
| 67 | 66 | # Use callbacks to share common setup or constraints between actions. |
| 68 | 67 | def set_project |
| 69 | - @project = find_resource(Project, params[:id].to_i) | |
| 68 | + @project = Project.find(params[:id].to_i) | |
| 70 | 69 | end |
| 71 | 70 | |
| 72 | 71 | # Never trust parameters from the scary internet, only allow the white list through. | ... | ... |
app/controllers/reading_groups_controller.rb
| 1 | 1 | include OwnershipAuthentication |
| 2 | -include ResourceFinder | |
| 3 | 2 | |
| 4 | 3 | class ReadingGroupsController < ApplicationController |
| 5 | 4 | before_action :authenticate_user!, except: [:index, :show] |
| ... | ... | @@ -45,7 +44,7 @@ class ReadingGroupsController < ApplicationController |
| 45 | 44 | # DELETE /reading_group/1 |
| 46 | 45 | # DELETE /reading_group/1.json |
| 47 | 46 | def destroy |
| 48 | - current_user.reading_group_ownerships.find_by_reading_group_id(@reading_group.id).destroy | |
| 47 | + current_user.reading_group_ownerships.find_by_reading_group_id!(@reading_group.id).destroy | |
| 49 | 48 | @reading_group.destroy |
| 50 | 49 | respond_to do |format| |
| 51 | 50 | format.html { redirect_to reading_groups_url } |
| ... | ... | @@ -57,7 +56,7 @@ class ReadingGroupsController < ApplicationController |
| 57 | 56 | |
| 58 | 57 | # Use callbacks to share common setup or constraints between actions. |
| 59 | 58 | def set_reading_group |
| 60 | - @reading_group = find_resource(ReadingGroup, params[:id].to_i) | |
| 59 | + @reading_group = ReadingGroup.find(params[:id].to_i) | |
| 61 | 60 | end |
| 62 | 61 | |
| 63 | 62 | # Never trust parameters from the scary internet, only allow the white list through. | ... | ... |
app/controllers/readings_controller.rb
| 1 | 1 | include OwnershipAuthentication |
| 2 | -include ResourceFinder | |
| 3 | 2 | |
| 4 | 3 | class ReadingsController < ApplicationController |
| 5 | 4 | before_action :authenticate_user!, except: [:index] |
| ... | ... | @@ -71,6 +70,6 @@ class ReadingsController < ApplicationController |
| 71 | 70 | end |
| 72 | 71 | |
| 73 | 72 | def set_reading |
| 74 | - @reading = find_resource(Reading, params[:id].to_i) | |
| 73 | + @reading = Reading.find(params[:id].to_i) | |
| 75 | 74 | end |
| 76 | 75 | end | ... | ... |
app/controllers/repositories_controller.rb
| 1 | 1 | include OwnershipAuthentication |
| 2 | -include ResourceFinder | |
| 3 | 2 | |
| 4 | 3 | class RepositoriesController < ApplicationController |
| 5 | 4 | before_action :authenticate_user!, except: [:show, :state, :state_with_date] |
| ... | ... | @@ -101,11 +100,13 @@ private |
| 101 | 100 | |
| 102 | 101 | # Use callbacks to share common setup or constraints between actions. |
| 103 | 102 | def set_repository |
| 104 | - @repository = find_resource(Repository, params[:id].to_i) | |
| 103 | + @repository = Repository.find(params[:id].to_i) | |
| 105 | 104 | end |
| 106 | 105 | |
| 107 | 106 | def set_kalibro_configuration |
| 108 | - @kalibro_configuration = KalibroConfiguration.find(@repository.kalibro_configuration_id) | |
| 107 | + if @repository | |
| 108 | + @kalibro_configuration = KalibroConfiguration.find(@repository.kalibro_configuration_id) | |
| 109 | + end | |
| 109 | 110 | end |
| 110 | 111 | |
| 111 | 112 | # Never trust parameters from the scary internet, only allow the white list through. | ... | ... |
app/helpers/metric_configurations_helper.rb
| ... | ... | @@ -14,6 +14,7 @@ module MetricConfigurationsHelper |
| 14 | 14 | end |
| 15 | 15 | |
| 16 | 16 | def supported_metrics_of(metric_collector_name) |
| 17 | + # find_by_name throws an exception instead of returning nil, unlike ActiveRecord's API | |
| 17 | 18 | KalibroClient::Entities::Processor::MetricCollectorDetails.find_by_name(metric_collector_name).supported_metrics |
| 18 | 19 | end |
| 19 | 20 | end | ... | ... |
spec/controllers/base_metric_configurations_controller_spec.rb
| ... | ... | @@ -117,7 +117,7 @@ describe InheritsFromBaseMetricConfigurationsController, :type => :controller do |
| 117 | 117 | context 'with a valid metric_configuration' do |
| 118 | 118 | before :each do |
| 119 | 119 | ReadingGroup.expects(:find).with(metric_configuration.reading_group_id).returns(reading_group) |
| 120 | - subject.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 120 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 121 | 121 | metric_configuration.expects(:kalibro_ranges).returns([kalibro_range]) |
| 122 | 122 | |
| 123 | 123 | get :show, kalibro_configuration_id: metric_configuration.kalibro_configuration_id.to_s, id: metric_configuration.id | ... | ... |
spec/controllers/compound_metric_configurations_controller_spec.rb
| ... | ... | @@ -74,7 +74,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 74 | 74 | |
| 75 | 75 | before :each do |
| 76 | 76 | ReadingGroup.expects(:find).with(compound_metric_configuration.reading_group_id).returns(reading_group) |
| 77 | - subject.expects(:find_resource).with(MetricConfiguration, compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 77 | + MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 78 | 78 | compound_metric_configuration.expects(:kalibro_ranges).returns([kalibro_range]) |
| 79 | 79 | |
| 80 | 80 | get :show, kalibro_configuration_id: compound_metric_configuration.kalibro_configuration_id.to_s, id: compound_metric_configuration.id |
| ... | ... | @@ -94,7 +94,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 94 | 94 | context 'when the user owns the compound metric configuration' do |
| 95 | 95 | before :each do |
| 96 | 96 | subject.expects(:metric_configuration_owner?).returns(true) |
| 97 | - subject.expects(:find_resource).with(MetricConfiguration, compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 97 | + MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 98 | 98 | MetricConfiguration.expects(:metric_configurations_of).with(kalibro_configuration.id).returns([compound_metric_configuration]) |
| 99 | 99 | get :edit, id: compound_metric_configuration.id, kalibro_configuration_id: compound_metric_configuration.kalibro_configuration_id.to_s |
| 100 | 100 | end |
| ... | ... | @@ -138,7 +138,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 138 | 138 | |
| 139 | 139 | context 'with valid fields' do |
| 140 | 140 | before :each do |
| 141 | - subject.expects(:find_resource).with(MetricConfiguration, compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 141 | + MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 142 | 142 | MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(true) |
| 143 | 143 | |
| 144 | 144 | post :update, kalibro_configuration_id: compound_metric_configuration.kalibro_configuration_id, id: compound_metric_configuration.id, metric_configuration: metric_configuration_params |
| ... | ... | @@ -150,7 +150,7 @@ describe CompoundMetricConfigurationsController, :type => :controller do |
| 150 | 150 | |
| 151 | 151 | context 'with an invalid field' do |
| 152 | 152 | before :each do |
| 153 | - subject.expects(:find_resource).with(MetricConfiguration, compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 153 | + MetricConfiguration.expects(:find).with(compound_metric_configuration.id).returns(compound_metric_configuration) | |
| 154 | 154 | MetricConfiguration.expects(:metric_configurations_of).with(kalibro_configuration.id).returns([compound_metric_configuration]) |
| 155 | 155 | MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(false) |
| 156 | 156 | ... | ... |
spec/controllers/concerns/metric_configurations_concern_spec.rb
| ... | ... | @@ -6,7 +6,7 @@ describe MetricConfigurationsConcern, type: :controller do |
| 6 | 6 | let! (:metric_configurations_controller) { MetricConfigurationsController.new } |
| 7 | 7 | |
| 8 | 8 | before :each do |
| 9 | - metric_configurations_controller.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 9 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 10 | 10 | metric_configurations_controller.params = {id: metric_configuration.id} |
| 11 | 11 | end |
| 12 | 12 | |
| ... | ... | @@ -14,4 +14,4 @@ describe MetricConfigurationsConcern, type: :controller do |
| 14 | 14 | metric_configurations_controller.set_metric_configuration |
| 15 | 15 | end |
| 16 | 16 | end |
| 17 | -end | |
| 18 | 17 | \ No newline at end of file |
| 18 | +end | ... | ... |
spec/controllers/concerns/resource_finder_spec.rb
| ... | ... | @@ -1,39 +0,0 @@ |
| 1 | -require 'rails_helper' | |
| 2 | - | |
| 3 | -describe ResourceFinder, type: :controller do | |
| 4 | - describe 'find_resource' do | |
| 5 | - let(:klass) { mock('Resource') } | |
| 6 | - let(:id) { 1 } | |
| 7 | - let!(:projects_controller) {ProjectsController.new} | |
| 8 | - | |
| 9 | - before do | |
| 10 | - projects_controller.extend(ResourceFinder) | |
| 11 | - end | |
| 12 | - | |
| 13 | - context 'when the resource exists' do | |
| 14 | - let!(:resource) { mock('resource') } | |
| 15 | - | |
| 16 | - before :each do | |
| 17 | - klass.expects(:find).with(id).returns(resource) | |
| 18 | - end | |
| 19 | - | |
| 20 | - it 'is expect to return the resource' do | |
| 21 | - expect(projects_controller.find_resource(klass, id)).to eq(resource) | |
| 22 | - end | |
| 23 | - end | |
| 24 | - | |
| 25 | - context 'when the resource does not exists' do | |
| 26 | - before :each do | |
| 27 | - klass.expects(:find).with(id).raises(KalibroClient::Errors::RecordNotFound) | |
| 28 | - end | |
| 29 | - | |
| 30 | - # FIXME: this is not the best test, but it it's the closest we can think of | |
| 31 | - # full coverage is achieved through projects_controller_spec.rb | |
| 32 | - it 'is expected to render the 404 page' do | |
| 33 | - projects_controller.expects(:respond_to) | |
| 34 | - | |
| 35 | - projects_controller.find_resource(klass, id) | |
| 36 | - end | |
| 37 | - end | |
| 38 | - end | |
| 39 | -end | |
| 40 | 0 | \ No newline at end of file |
spec/controllers/kalibro_configurations_controller_spec.rb
| ... | ... | @@ -65,7 +65,7 @@ describe KalibroConfigurationsController, :type => :controller do |
| 65 | 65 | |
| 66 | 66 | before :each do |
| 67 | 67 | kalibro_configuration.expects(:metric_configurations).returns(metric_configuration) |
| 68 | - subject.expects(:find_resource).with(KalibroConfiguration, kalibro_configuration.id).returns(kalibro_configuration) | |
| 68 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns(kalibro_configuration) | |
| 69 | 69 | |
| 70 | 70 | get :show, :id => kalibro_configuration.id |
| 71 | 71 | end |
| ... | ... | @@ -96,11 +96,11 @@ describe KalibroConfigurationsController, :type => :controller do |
| 96 | 96 | |
| 97 | 97 | #Those two mocks looks the same but they are necessary since params[:id] is a String and @configuration.id is an Integer :( |
| 98 | 98 | @ownerships.expects(:find_by_kalibro_configuration_id).with("#{@subject.id}").returns(@ownership) |
| 99 | - @ownerships.expects(:find_by_kalibro_configuration_id).with(@subject.id).returns(@ownership) | |
| 99 | + @ownerships.expects(:find_by_kalibro_configuration_id!).with(@subject.id).returns(@ownership) | |
| 100 | 100 | |
| 101 | 101 | User.any_instance.expects(:kalibro_configuration_ownerships).at_least_once.returns(@ownerships) |
| 102 | 102 | |
| 103 | - subject.expects(:find_resource).with(KalibroConfiguration, @subject.id).returns(@subject) | |
| 103 | + KalibroConfiguration.expects(:find).with(@subject.id).returns(@subject) | |
| 104 | 104 | delete :destroy, :id => @subject.id |
| 105 | 105 | end |
| 106 | 106 | |
| ... | ... | @@ -160,7 +160,7 @@ describe KalibroConfigurationsController, :type => :controller do |
| 160 | 160 | |
| 161 | 161 | context 'when the user owns the kalibro_configuration' do |
| 162 | 162 | before :each do |
| 163 | - subject.expects(:find_resource).with(KalibroConfiguration, @subject.id).returns(@subject) | |
| 163 | + KalibroConfiguration.expects(:find).with(@subject.id).returns(@subject) | |
| 164 | 164 | @ownerships.expects(:find_by_kalibro_configuration_id).with("#{@subject.id}").returns(@ownership) |
| 165 | 165 | |
| 166 | 166 | get :edit, :id => @subject.id |
| ... | ... | @@ -215,7 +215,7 @@ describe KalibroConfigurationsController, :type => :controller do |
| 215 | 215 | |
| 216 | 216 | context 'with valid fields' do |
| 217 | 217 | before :each do |
| 218 | - subject.expects(:find_resource).with(KalibroConfiguration, kalibro_configuration.id).returns(kalibro_configuration) | |
| 218 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns(kalibro_configuration) | |
| 219 | 219 | KalibroConfiguration.any_instance.expects(:update).with(kalibro_configuration_params).returns(true) |
| 220 | 220 | end |
| 221 | 221 | |
| ... | ... | @@ -240,7 +240,7 @@ describe KalibroConfigurationsController, :type => :controller do |
| 240 | 240 | |
| 241 | 241 | context 'with an invalid field' do |
| 242 | 242 | before :each do |
| 243 | - subject.expects(:find_resource).with(KalibroConfiguration, kalibro_configuration.id).returns(kalibro_configuration) | |
| 243 | + KalibroConfiguration.expects(:find).with(kalibro_configuration.id).returns(kalibro_configuration) | |
| 244 | 244 | KalibroConfiguration.any_instance.expects(:update).with(kalibro_configuration_params).returns(false) |
| 245 | 245 | |
| 246 | 246 | post :update, :id => kalibro_configuration.id, :kalibro_configuration => kalibro_configuration_params | ... | ... |
spec/controllers/kalibro_ranges_controller_spec.rb
| ... | ... | @@ -97,7 +97,7 @@ describe KalibroRangesController, :type => :controller do |
| 97 | 97 | before :each do |
| 98 | 98 | subject.expects(:metric_configuration_owner?).returns true |
| 99 | 99 | kalibro_range.expects(:destroy) |
| 100 | - subject.expects(:find_resource).with(KalibroRange, kalibro_range.id).returns(kalibro_range) | |
| 100 | + KalibroRange.expects(:find).with(kalibro_range.id).returns(kalibro_range) | |
| 101 | 101 | MetricConfiguration.expects(:find).with(kalibro_range.metric_configuration_id).returns(metric_configuration) |
| 102 | 102 | |
| 103 | 103 | delete :destroy, id: kalibro_range.id, metric_configuration_id: metric_configuration.id, kalibro_configuration_id: metric_configuration.kalibro_configuration_id |
| ... | ... | @@ -114,7 +114,7 @@ describe KalibroRangesController, :type => :controller do |
| 114 | 114 | before :each do |
| 115 | 115 | subject.expects(:metric_configuration_owner?).returns true |
| 116 | 116 | new_kalibro_range.expects(:destroy) |
| 117 | - subject.expects(:find_resource).with(KalibroRange, new_kalibro_range.id).returns(new_kalibro_range) | |
| 117 | + KalibroRange.expects(:find).with(new_kalibro_range.id).returns(new_kalibro_range) | |
| 118 | 118 | MetricConfiguration.expects(:find).with(new_kalibro_range.metric_configuration_id).returns(compound_metric_configuration) |
| 119 | 119 | |
| 120 | 120 | delete :destroy, id: new_kalibro_range.id, metric_configuration_id: compound_metric_configuration.id, kalibro_configuration_id: compound_metric_configuration.kalibro_configuration_id |
| ... | ... | @@ -156,7 +156,7 @@ describe KalibroRangesController, :type => :controller do |
| 156 | 156 | context 'when the user owns the kalibro range' do |
| 157 | 157 | before :each do |
| 158 | 158 | subject.expects(:metric_configuration_owner?).returns true |
| 159 | - subject.expects(:find_resource).with(KalibroRange, kalibro_range.id).returns(kalibro_range) | |
| 159 | + KalibroRange.expects(:find).with(kalibro_range.id).returns(kalibro_range) | |
| 160 | 160 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) |
| 161 | 161 | Reading.expects(:readings_of).with(metric_configuration.reading_group_id).returns([reading]) |
| 162 | 162 | get :edit, id: kalibro_range.id, kalibro_configuration_id: metric_configuration.kalibro_configuration_id, metric_configuration_id: metric_configuration.id |
| ... | ... | @@ -205,7 +205,7 @@ describe KalibroRangesController, :type => :controller do |
| 205 | 205 | |
| 206 | 206 | context 'with valid fields' do |
| 207 | 207 | before :each do |
| 208 | - subject.expects(:find_resource).with(KalibroRange, kalibro_range.id).returns(kalibro_range) | |
| 208 | + KalibroRange.expects(:find).with(kalibro_range.id).returns(kalibro_range) | |
| 209 | 209 | KalibroRange.any_instance.expects(:update).with(kalibro_range_params).returns(true) |
| 210 | 210 | MetricConfiguration.expects(:find).with(kalibro_range.metric_configuration_id).returns(metric_configuration) |
| 211 | 211 | |
| ... | ... | @@ -221,7 +221,7 @@ describe KalibroRangesController, :type => :controller do |
| 221 | 221 | let(:new_kalibro_range) { FactoryGirl.build(:kalibro_range_with_id, metric_configuration_id: compound_metric_configuration.id) } |
| 222 | 222 | |
| 223 | 223 | before :each do |
| 224 | - subject.expects(:find_resource).with(KalibroRange, new_kalibro_range.id).returns(new_kalibro_range) | |
| 224 | + KalibroRange.expects(:find).with(new_kalibro_range.id).returns(new_kalibro_range) | |
| 225 | 225 | KalibroRange.any_instance.expects(:update).with(new_kalibro_range.to_hash).returns(true) |
| 226 | 226 | MetricConfiguration.expects(:find).with(new_kalibro_range.metric_configuration_id).returns(compound_metric_configuration) |
| 227 | 227 | |
| ... | ... | @@ -234,7 +234,7 @@ describe KalibroRangesController, :type => :controller do |
| 234 | 234 | |
| 235 | 235 | context 'with an invalid field' do |
| 236 | 236 | before :each do |
| 237 | - subject.expects(:find_resource).with(KalibroRange, kalibro_range.id).returns(kalibro_range) | |
| 237 | + KalibroRange.expects(:find).with(kalibro_range.id).returns(kalibro_range) | |
| 238 | 238 | KalibroRange.any_instance.expects(:update).with(kalibro_range_params).returns(false) |
| 239 | 239 | MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) |
| 240 | 240 | Reading.expects(:readings_of).with(metric_configuration.reading_group_id).returns([reading]) | ... | ... |
spec/controllers/metric_configurations_controller_spec.rb
| ... | ... | @@ -97,7 +97,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 97 | 97 | |
| 98 | 98 | before :each do |
| 99 | 99 | ReadingGroup.expects(:find).with(metric_configuration.reading_group_id).returns(reading_group) |
| 100 | - subject.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 100 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 101 | 101 | metric_configuration.expects(:kalibro_ranges).returns([kalibro_range]) |
| 102 | 102 | |
| 103 | 103 | get :show, kalibro_configuration_id: metric_configuration.kalibro_configuration_id.to_s, id: metric_configuration.id |
| ... | ... | @@ -117,7 +117,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 117 | 117 | context 'when the user owns the metric configuration' do |
| 118 | 118 | before :each do |
| 119 | 119 | subject.expects(:metric_configuration_owner?).returns(true) |
| 120 | - subject.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 120 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 121 | 121 | get :edit, id: metric_configuration.id, kalibro_configuration_id: metric_configuration.kalibro_configuration_id.to_s |
| 122 | 122 | end |
| 123 | 123 | |
| ... | ... | @@ -160,7 +160,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 160 | 160 | |
| 161 | 161 | context 'with valid fields' do |
| 162 | 162 | before :each do |
| 163 | - subject.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 163 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 164 | 164 | MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(true) |
| 165 | 165 | |
| 166 | 166 | post :update, kalibro_configuration_id: metric_configuration.kalibro_configuration_id, id: metric_configuration.id, metric_configuration: metric_configuration_params |
| ... | ... | @@ -172,7 +172,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 172 | 172 | |
| 173 | 173 | context 'with an invalid field' do |
| 174 | 174 | before :each do |
| 175 | - subject.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 175 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 176 | 176 | MetricConfiguration.any_instance.expects(:update).with(metric_configuration_params).returns(false) |
| 177 | 177 | |
| 178 | 178 | post :update, kalibro_configuration_id: metric_configuration.kalibro_configuration_id, id: metric_configuration.id, metric_configuration: metric_configuration_params |
| ... | ... | @@ -205,7 +205,7 @@ describe MetricConfigurationsController, :type => :controller do |
| 205 | 205 | before :each do |
| 206 | 206 | subject.expects(:metric_configuration_owner?).returns true |
| 207 | 207 | metric_configuration.expects(:destroy) |
| 208 | - subject.expects(:find_resource).with(MetricConfiguration, metric_configuration.id).returns(metric_configuration) | |
| 208 | + MetricConfiguration.expects(:find).with(metric_configuration.id).returns(metric_configuration) | |
| 209 | 209 | |
| 210 | 210 | delete :destroy, id: metric_configuration.id, kalibro_configuration_id: metric_configuration.kalibro_configuration_id.to_s |
| 211 | 211 | end | ... | ... |
spec/controllers/modules_controller_spec.rb
| ... | ... | @@ -3,7 +3,7 @@ require 'rails_helper' |
| 3 | 3 | describe ModulesController, :type => :controller do |
| 4 | 4 | describe "load_module_tree" do |
| 5 | 5 | before :each do |
| 6 | - subject.expects(:find_resource).with(ModuleResult, 42).returns(FactoryGirl.build(:module_result)) | |
| 6 | + ModuleResult.expects(:find).with(42).returns(FactoryGirl.build(:module_result)) | |
| 7 | 7 | |
| 8 | 8 | post :load_module_tree, id: 42, format: :js |
| 9 | 9 | end |
| ... | ... | @@ -20,7 +20,7 @@ describe ModulesController, :type => :controller do |
| 20 | 20 | let! (:module_result){ FactoryGirl.build(:module_result) } |
| 21 | 21 | |
| 22 | 22 | before :each do |
| 23 | - subject.expects(:find_resource).with(ModuleResult, module_result.id).returns(module_result) | |
| 23 | + ModuleResult.expects(:find).with(module_result.id).returns(module_result) | |
| 24 | 24 | subject.expire_fragment("#{module_result.id}_#{metric_name}") |
| 25 | 25 | |
| 26 | 26 | xhr :get, :metric_history, {id: module_result.id, metric_name: metric_name, module_id: module_id} | ... | ... |
spec/controllers/projects_controller_spec.rb
| ... | ... | @@ -65,7 +65,7 @@ describe ProjectsController, :type => :controller do |
| 65 | 65 | context 'when the project exists' do |
| 66 | 66 | let(:repository) { FactoryGirl.build(:repository) } |
| 67 | 67 | before :each do |
| 68 | - subject.expects(:find_resource).with(Project, project.id).returns(project) | |
| 68 | + Project.expects(:find).with(project.id).returns(project) | |
| 69 | 69 | project.expects(:repositories).returns(repository) |
| 70 | 70 | get :show, :id => project.id |
| 71 | 71 | end |
| ... | ... | @@ -102,11 +102,11 @@ describe ProjectsController, :type => :controller do |
| 102 | 102 | |
| 103 | 103 | #Those two mocks looks the same but they are necessary since params[:id] is a String and @project.id is an Integer :( |
| 104 | 104 | @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(@project_attributes) |
| 105 | - @attributes.expects(:find_by_project_id).with(@subject.id).returns(@project_attributes) | |
| 105 | + @attributes.expects(:find_by_project_id!).with(@subject.id).returns(@project_attributes) | |
| 106 | 106 | |
| 107 | 107 | User.any_instance.expects(:project_attributes).at_least_once.returns(@attributes) |
| 108 | 108 | |
| 109 | - subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) | |
| 109 | + Project.expects(:find).with(@subject.id).returns(@subject) | |
| 110 | 110 | delete :destroy, :id => @subject.id |
| 111 | 111 | end |
| 112 | 112 | |
| ... | ... | @@ -168,7 +168,7 @@ describe ProjectsController, :type => :controller do |
| 168 | 168 | |
| 169 | 169 | context 'when the user owns the project' do |
| 170 | 170 | before :each do |
| 171 | - subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) | |
| 171 | + Project.expects(:find).with(@subject.id).returns(@subject) | |
| 172 | 172 | @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(@attribute) |
| 173 | 173 | |
| 174 | 174 | get :edit, :id => @subject.id |
| ... | ... | @@ -224,7 +224,7 @@ describe ProjectsController, :type => :controller do |
| 224 | 224 | |
| 225 | 225 | context 'with valid fields' do |
| 226 | 226 | before :each do |
| 227 | - subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) | |
| 227 | + Project.expects(:find).with(@subject.id).returns(@subject) | |
| 228 | 228 | Project.any_instance.expects(:update).with(@subject_params).returns(true) |
| 229 | 229 | @project_attributes.expects(:update).with(image_url: @subject_params[:image_url]).returns(true) |
| 230 | 230 | @subject.expects(:attributes).returns(@project_attributes) |
| ... | ... | @@ -251,7 +251,7 @@ describe ProjectsController, :type => :controller do |
| 251 | 251 | |
| 252 | 252 | context 'with an invalid field' do |
| 253 | 253 | before :each do |
| 254 | - subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) | |
| 254 | + Project.expects(:find).with(@subject.id).returns(@subject) | |
| 255 | 255 | Project.any_instance.expects(:update).with(@subject_params).returns(false) |
| 256 | 256 | |
| 257 | 257 | post :update, :id => @subject.id, :project => @subject_params | ... | ... |
spec/controllers/reading_groups_controller_spec.rb
| ... | ... | @@ -62,7 +62,7 @@ describe ReadingGroupsController, :type => :controller do |
| 62 | 62 | let!(:reading_group) { FactoryGirl.build(:reading_group_with_id) } |
| 63 | 63 | let(:reading) { FactoryGirl.build(:reading_with_id) } |
| 64 | 64 | before :each do |
| 65 | - subject.expects(:find_resource).with(ReadingGroup, reading_group.id).returns(reading_group) | |
| 65 | + ReadingGroup.expects(:find).with(reading_group.id).returns(reading_group) | |
| 66 | 66 | get :show, :id => reading_group.id |
| 67 | 67 | end |
| 68 | 68 | |
| ... | ... | @@ -89,11 +89,11 @@ describe ReadingGroupsController, :type => :controller do |
| 89 | 89 | |
| 90 | 90 | #Those two mocks looks the same but they are necessary since params[:id] is a String and @ReadingGroup.id is an Integer :( |
| 91 | 91 | @ownerships.expects(:find_by_reading_group_id).with("#{@subject.id}").returns(@ownership) |
| 92 | - @ownerships.expects(:find_by_reading_group_id).with(@subject.id).returns(@ownership) | |
| 92 | + @ownerships.expects(:find_by_reading_group_id!).with(@subject.id).returns(@ownership) | |
| 93 | 93 | |
| 94 | 94 | User.any_instance.expects(:reading_group_ownerships).at_least_once.returns(@ownerships) |
| 95 | 95 | |
| 96 | - subject.expects(:find_resource).with(ReadingGroup, @subject.id).returns(@subject) | |
| 96 | + ReadingGroup.expects(:find).with(@subject.id).returns(@subject) | |
| 97 | 97 | delete :destroy, :id => @subject.id |
| 98 | 98 | end |
| 99 | 99 | |
| ... | ... | @@ -153,7 +153,7 @@ describe ReadingGroupsController, :type => :controller do |
| 153 | 153 | |
| 154 | 154 | context 'when the user owns the reading group' do |
| 155 | 155 | before :each do |
| 156 | - subject.expects(:find_resource).with(ReadingGroup, @subject.id).returns(@subject) | |
| 156 | + ReadingGroup.expects(:find).with(@subject.id).returns(@subject) | |
| 157 | 157 | @ownerships.expects(:find_by_reading_group_id).with("#{@subject.id}").returns(@ownership) |
| 158 | 158 | |
| 159 | 159 | get :edit, :id => @subject.id |
| ... | ... | @@ -210,7 +210,7 @@ describe ReadingGroupsController, :type => :controller do |
| 210 | 210 | |
| 211 | 211 | context 'with valid fields' do |
| 212 | 212 | before :each do |
| 213 | - subject.expects(:find_resource).with(ReadingGroup, @subject.id).returns(@subject) | |
| 213 | + ReadingGroup.expects(:find).with(@subject.id).returns(@subject) | |
| 214 | 214 | ReadingGroup.any_instance.expects(:update).with(@subject_params).returns(true) |
| 215 | 215 | end |
| 216 | 216 | |
| ... | ... | @@ -235,7 +235,7 @@ describe ReadingGroupsController, :type => :controller do |
| 235 | 235 | |
| 236 | 236 | context 'with an invalid field' do |
| 237 | 237 | before :each do |
| 238 | - subject.expects(:find_resource).with(ReadingGroup, @subject.id).returns(@subject) | |
| 238 | + ReadingGroup.expects(:find).with(@subject.id).returns(@subject) | |
| 239 | 239 | ReadingGroup.any_instance.expects(:update).with(@subject_params).returns(false) |
| 240 | 240 | |
| 241 | 241 | post :update, :id => @subject.id, :reading_group => @subject_params | ... | ... |
spec/controllers/readings_controller_spec.rb
| ... | ... | @@ -74,7 +74,7 @@ describe ReadingsController, :type => :controller do |
| 74 | 74 | context 'when the user owns the reading' do |
| 75 | 75 | before :each do |
| 76 | 76 | subject.expects(:reading_owner?).returns true |
| 77 | - subject.expects(:find_resource).with(Reading, reading.id).returns(reading) | |
| 77 | + Reading.expects(:find).with(reading.id).returns(reading) | |
| 78 | 78 | get :edit, id: reading.id, reading_group_id: reading_group.id.to_s |
| 79 | 79 | end |
| 80 | 80 | |
| ... | ... | @@ -117,7 +117,7 @@ describe ReadingsController, :type => :controller do |
| 117 | 117 | |
| 118 | 118 | context 'with valid fields' do |
| 119 | 119 | before :each do |
| 120 | - subject.expects(:find_resource).with(Reading, reading.id).returns(reading) | |
| 120 | + Reading.expects(:find).with(reading.id).returns(reading) | |
| 121 | 121 | Reading.any_instance.expects(:update).with(reading_params).returns(true) |
| 122 | 122 | |
| 123 | 123 | post :update, reading_group_id: reading_group.id, id: reading.id, reading: reading_params |
| ... | ... | @@ -129,7 +129,7 @@ describe ReadingsController, :type => :controller do |
| 129 | 129 | |
| 130 | 130 | context 'with an invalid field' do |
| 131 | 131 | before :each do |
| 132 | - subject.expects(:find_resource).with(Reading, reading.id).returns(reading) | |
| 132 | + Reading.expects(:find).with(reading.id).returns(reading) | |
| 133 | 133 | Reading.any_instance.expects(:update).with(reading_params).returns(false) |
| 134 | 134 | |
| 135 | 135 | post :update, reading_group_id: reading_group.id, id: reading.id, reading: reading_params |
| ... | ... | @@ -169,7 +169,7 @@ describe ReadingsController, :type => :controller do |
| 169 | 169 | before :each do |
| 170 | 170 | subject.expects(:reading_owner?).returns true |
| 171 | 171 | reading.expects(:destroy) |
| 172 | - subject.expects(:find_resource).with(Reading, reading.id).returns(reading) | |
| 172 | + Reading.expects(:find).with(reading.id).returns(reading) | |
| 173 | 173 | |
| 174 | 174 | delete :destroy, id: reading.id, reading_group_id: reading.reading_group_id.to_s |
| 175 | 175 | end | ... | ... |
spec/controllers/repositories_controller_spec.rb
| ... | ... | @@ -94,7 +94,7 @@ describe RepositoriesController, :type => :controller do |
| 94 | 94 | processing = FactoryGirl.build(:processing) |
| 95 | 95 | |
| 96 | 96 | KalibroConfiguration.expects(:find).with(repository.id).returns(FactoryGirl.build(:kalibro_configuration_with_id)) |
| 97 | - subject.expects(:find_resource).with(Repository, repository.id).returns(repository) | |
| 97 | + Repository.expects(:find).with(repository.id).returns(repository) | |
| 98 | 98 | |
| 99 | 99 | get :show, id: repository.id.to_s, project_id: project.id.to_s |
| 100 | 100 | end |
| ... | ... | @@ -108,7 +108,7 @@ describe RepositoriesController, :type => :controller do |
| 108 | 108 | processing = FactoryGirl.build(:processing) |
| 109 | 109 | |
| 110 | 110 | KalibroConfiguration.expects(:find).with(repository.id).returns(FactoryGirl.build(:kalibro_configuration_with_id)) |
| 111 | - subject.expects(:find_resource).with(Repository, repository.id).returns(repository) | |
| 111 | + Repository.expects(:find).with(repository.id).returns(repository) | |
| 112 | 112 | |
| 113 | 113 | get :show, id: repository.id.to_s, project_id: project.id.to_s |
| 114 | 114 | end | ... | ... |