Commit 182e8f5ae578797276a5e74cc58c332fbd284795
1 parent
abdd2edc
Exists in
colab
and in
4 other branches
RepositoriesController checks for Project ownership only if necessary
Actions new and create check if project_id was provided before trying to check the project ownership
Showing
2 changed files
with
95 additions
and
48 deletions
Show diff stats
app/controllers/repositories_controller.rb
... | ... | @@ -2,7 +2,7 @@ include OwnershipAuthentication |
2 | 2 | |
3 | 3 | class RepositoriesController < ApplicationController |
4 | 4 | before_action :authenticate_user!, except: [:show, :state, :state_with_date] |
5 | - before_action :project_owner?, only: [:new, :create] | |
5 | + before_action :project_owner?, only: [:new, :create], if: "!params[:project_id].nil?" | |
6 | 6 | before_action :repository_owner?, only: [:edit, :update, :destroy, :process_repository] |
7 | 7 | before_action :set_repository, only: [:show, :edit, :update, :destroy, :state, :state_with_date, :process_repository] |
8 | 8 | before_action :set_project_id_repository_types_and_configurations, only: [:new, :edit] | ... | ... |
spec/controllers/repositories_controller_spec.rb
1 | 1 | require 'rails_helper' |
2 | 2 | |
3 | 3 | describe RepositoriesController, :type => :controller do |
4 | - let(:project) { FactoryGirl.build(:project_with_id) } | |
5 | - | |
6 | 4 | describe 'new' do |
7 | 5 | context 'with a User logged in' do |
8 | 6 | let!(:user) { FactoryGirl.create(:user) } |
... | ... | @@ -11,30 +9,48 @@ describe RepositoriesController, :type => :controller do |
11 | 9 | sign_in user |
12 | 10 | end |
13 | 11 | |
14 | - context 'when the current user owns the project' do | |
15 | - before :each do | |
16 | - KalibroConfiguration.expects(:public_or_owned_by_user).with(user).returns(kalibro_configurations) | |
17 | - Repository.expects(:repository_types).returns([]) | |
18 | - subject.expects(:project_owner?).returns true | |
12 | + context 'when a project_id is provided' do | |
13 | + let(:project) { FactoryGirl.build(:project_with_id) } | |
14 | + | |
15 | + context 'and the current user owns the project' do | |
16 | + before :each do | |
17 | + KalibroConfiguration.expects(:public_or_owned_by_user).with(user).returns(kalibro_configurations) | |
18 | + Repository.expects(:repository_types).returns([]) | |
19 | + subject.expects(:project_owner?).returns true | |
19 | 20 | |
20 | - get :new, project_id: project.id.to_s | |
21 | + get :new, project_id: project.id.to_json | |
22 | + end | |
23 | + | |
24 | + it { is_expected.to respond_with(:success) } | |
25 | + it { is_expected.to render_template(:new) } | |
21 | 26 | end |
22 | 27 | |
23 | - it { is_expected.to respond_with(:success) } | |
24 | - it { is_expected.to render_template(:new) } | |
28 | + context "and the current user doesn't own the project" do | |
29 | + before :each do | |
30 | + get :new, project_id: project.id.to_s | |
31 | + end | |
32 | + | |
33 | + it { is_expected.to redirect_to(projects_url) } | |
34 | + it { is_expected.to respond_with(:redirect) } | |
35 | + end | |
25 | 36 | end |
26 | 37 | |
27 | - context "when the current user doesn't own the project" do | |
38 | + context 'when no project_id is provided' do | |
28 | 39 | before :each do |
29 | - get :new, project_id: project.id.to_s | |
40 | + KalibroConfiguration.expects(:public_or_owned_by_user).with(user).returns(kalibro_configurations) | |
41 | + Repository.expects(:repository_types).returns([]) | |
42 | + | |
43 | + get :new | |
30 | 44 | end |
31 | 45 | |
32 | - it { is_expected.to redirect_to(projects_url) } | |
33 | - it { is_expected.to respond_with(:redirect) } | |
46 | + it { is_expected.to respond_with(:success) } | |
47 | + it { is_expected.to render_template(:new) } | |
34 | 48 | end |
35 | 49 | end |
36 | 50 | |
37 | 51 | context 'with no user logged in' do |
52 | + let(:project) { FactoryGirl.build(:project_with_id) } | |
53 | + | |
38 | 54 | before :each do |
39 | 55 | get :new, project_id: project.id.to_s |
40 | 56 | end |
... | ... | @@ -46,23 +62,63 @@ describe RepositoriesController, :type => :controller do |
46 | 62 | describe 'create' do |
47 | 63 | let!(:kalibro_configurations) { [FactoryGirl.build(:kalibro_configuration)] } |
48 | 64 | let!(:user) { FactoryGirl.create(:user) } |
49 | - let (:repository) { FactoryGirl.build(:repository, project_id: project.id) } | |
50 | 65 | let(:repository_params) { FactoryGirl.build(:repository).to_hash } |
51 | 66 | |
52 | 67 | before do |
53 | 68 | sign_in user |
54 | 69 | end |
55 | 70 | |
56 | - context 'when the current user owns the project' do | |
57 | - before :each do | |
58 | - subject.expects(:project_owner?).returns true | |
71 | + context 'when a project_id is provided' do | |
72 | + let(:project) { FactoryGirl.build(:project_with_id) } | |
73 | + let(:repository) { FactoryGirl.build(:repository, project_id: project.id) } | |
74 | + | |
75 | + context 'and the current user owns the project' do | |
76 | + before :each do | |
77 | + subject.expects(:project_owner?).returns true | |
78 | + end | |
79 | + | |
80 | + context 'with valid fields' do | |
81 | + before :each do | |
82 | + Repository.any_instance.expects(:save).returns(true) | |
83 | + | |
84 | + post :create, project_id: project.id, repository: repository_params | |
85 | + end | |
86 | + | |
87 | + it { is_expected.to redirect_to(repository_process_path(id: repository.id)) } | |
88 | + it { is_expected.to respond_with(:redirect) } | |
89 | + end | |
90 | + | |
91 | + context 'with an invalid field' do | |
92 | + before :each do | |
93 | + KalibroConfiguration.expects(:public_or_owned_by_user).with(user).returns(kalibro_configurations) | |
94 | + Repository.any_instance.expects(:save).returns(false) | |
95 | + Repository.expects(:repository_types).returns([]) | |
96 | + | |
97 | + post :create, project_id: project.id.to_s, repository: repository_params | |
98 | + end | |
99 | + | |
100 | + it { is_expected.to render_template(:new) } | |
101 | + end | |
102 | + end | |
103 | + | |
104 | + context "and the current user doesn't own the project " do | |
105 | + before :each do | |
106 | + post :create, project_id: project.id, repository: repository_params | |
107 | + end | |
108 | + | |
109 | + it { is_expected.to redirect_to(projects_url) } | |
110 | + it { is_expected.to respond_with(:redirect) } | |
59 | 111 | end |
112 | + end | |
113 | + | |
114 | + context 'when no project_id is provided' do | |
115 | + let(:repository) { FactoryGirl.build(:repository) } | |
60 | 116 | |
61 | 117 | context 'with valid fields' do |
62 | 118 | before :each do |
63 | 119 | Repository.any_instance.expects(:save).returns(true) |
64 | 120 | |
65 | - post :create, project_id: project.id, repository: repository_params | |
121 | + post :create, repository: repository_params | |
66 | 122 | end |
67 | 123 | |
68 | 124 | it { is_expected.to redirect_to(repository_process_path(id: repository.id)) } |
... | ... | @@ -75,21 +131,12 @@ describe RepositoriesController, :type => :controller do |
75 | 131 | Repository.any_instance.expects(:save).returns(false) |
76 | 132 | Repository.expects(:repository_types).returns([]) |
77 | 133 | |
78 | - post :create, project_id: project.id.to_s, repository: repository_params | |
134 | + post :create.to_s, repository: repository_params | |
79 | 135 | end |
80 | 136 | |
81 | 137 | it { is_expected.to render_template(:new) } |
82 | 138 | end |
83 | 139 | end |
84 | - | |
85 | - context "when the current user doesn't own the project " do | |
86 | - before :each do | |
87 | - post :create, project_id: project.id, repository: repository_params | |
88 | - end | |
89 | - | |
90 | - it { is_expected.to redirect_to(projects_url) } | |
91 | - it { is_expected.to respond_with(:redirect) } | |
92 | - end | |
93 | 140 | end |
94 | 141 | |
95 | 142 | describe 'show' do |
... | ... | @@ -100,7 +147,7 @@ describe RepositoriesController, :type => :controller do |
100 | 147 | KalibroConfiguration.expects(:find).with(repository.id).returns(FactoryGirl.build(:kalibro_configuration, :with_id)) |
101 | 148 | Repository.expects(:find).with(repository.id).returns(repository) |
102 | 149 | |
103 | - get :show, id: repository.id.to_s, project_id: project.id.to_s | |
150 | + get :show, id: repository.id.to_s | |
104 | 151 | end |
105 | 152 | |
106 | 153 | it { is_expected.to render_template(:show) } |
... | ... | @@ -112,7 +159,7 @@ describe RepositoriesController, :type => :controller do |
112 | 159 | KalibroConfiguration.expects(:find).with(repository.id).returns(FactoryGirl.build(:kalibro_configuration, :with_id)) |
113 | 160 | Repository.expects(:find).with(repository.id).returns(repository) |
114 | 161 | |
115 | - get :show, id: repository.id.to_s, project_id: project.id.to_s | |
162 | + get :show, id: repository.id.to_s | |
116 | 163 | end |
117 | 164 | |
118 | 165 | it { is_expected.to render_template(:show) } |
... | ... | @@ -133,7 +180,7 @@ describe RepositoriesController, :type => :controller do |
133 | 180 | repository.expects(:destroy) |
134 | 181 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
135 | 182 | |
136 | - delete :destroy, id: repository.id, project_id: project.id.to_s | |
183 | + delete :destroy, id: repository.id | |
137 | 184 | end |
138 | 185 | |
139 | 186 | it { is_expected.to redirect_to(projects_path) } |
... | ... | @@ -142,7 +189,7 @@ describe RepositoriesController, :type => :controller do |
142 | 189 | |
143 | 190 | context "when the user doesn't own the project" do |
144 | 191 | before :each do |
145 | - delete :destroy, id: repository.id, project_id: project.id.to_s | |
192 | + delete :destroy, id: repository.id | |
146 | 193 | end |
147 | 194 | |
148 | 195 | it { is_expected.to redirect_to(projects_url) } |
... | ... | @@ -152,7 +199,7 @@ describe RepositoriesController, :type => :controller do |
152 | 199 | |
153 | 200 | context 'with no User logged in' do |
154 | 201 | before :each do |
155 | - delete :destroy, id: repository.id, project_id: project.id.to_s | |
202 | + delete :destroy, id: repository.id | |
156 | 203 | end |
157 | 204 | |
158 | 205 | it { is_expected.to redirect_to new_user_session_path } |
... | ... | @@ -175,7 +222,7 @@ describe RepositoriesController, :type => :controller do |
175 | 222 | subject.expects(:repository_owner?).returns true |
176 | 223 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
177 | 224 | Repository.expects(:repository_types).returns(["SUBVERSION"]) |
178 | - get :edit, id: repository.id, project_id: project.id.to_s | |
225 | + get :edit, id: repository.id | |
179 | 226 | end |
180 | 227 | |
181 | 228 | it { is_expected.to render_template(:edit) } |
... | ... | @@ -183,7 +230,7 @@ describe RepositoriesController, :type => :controller do |
183 | 230 | |
184 | 231 | context 'when the user does not own the repository' do |
185 | 232 | before do |
186 | - get :edit, id: repository.id, project_id: project.id.to_s | |
233 | + get :edit, id: repository.id | |
187 | 234 | end |
188 | 235 | |
189 | 236 | it { is_expected.to redirect_to(projects_url) } |
... | ... | @@ -194,7 +241,7 @@ describe RepositoriesController, :type => :controller do |
194 | 241 | |
195 | 242 | context 'with no user logged in' do |
196 | 243 | before :each do |
197 | - get :edit, id: repository.id, project_id: project.id.to_s | |
244 | + get :edit, id: repository.id | |
198 | 245 | end |
199 | 246 | |
200 | 247 | it { is_expected.to redirect_to new_user_session_path } |
... | ... | @@ -223,7 +270,7 @@ describe RepositoriesController, :type => :controller do |
223 | 270 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
224 | 271 | Repository.any_instance.expects(:update).with(repository_params).returns(true) |
225 | 272 | |
226 | - post :update, project_id: project.id.to_s, id: repository.id, repository: repository_params | |
273 | + post :update, id: repository.id, repository: repository_params | |
227 | 274 | end |
228 | 275 | |
229 | 276 | it { is_expected.to redirect_to(repository_path(repository.id)) } |
... | ... | @@ -237,7 +284,7 @@ describe RepositoriesController, :type => :controller do |
237 | 284 | Repository.any_instance.expects(:update).with(repository_params).returns(false) |
238 | 285 | Repository.expects(:repository_types).returns([]) |
239 | 286 | |
240 | - post :update, project_id: project.id.to_s, id: repository.id, repository: repository_params | |
287 | + post :update, id: repository.id, repository: repository_params | |
241 | 288 | end |
242 | 289 | |
243 | 290 | it { is_expected.to render_template(:edit) } |
... | ... | @@ -246,7 +293,7 @@ describe RepositoriesController, :type => :controller do |
246 | 293 | |
247 | 294 | context 'when the user does not own the repository' do |
248 | 295 | before :each do |
249 | - post :update, project_id: project.id.to_s, id: repository.id, repository: repository_params | |
296 | + post :update, id: repository.id, repository: repository_params | |
250 | 297 | end |
251 | 298 | |
252 | 299 | it { is_expected.to redirect_to projects_path } |
... | ... | @@ -255,7 +302,7 @@ describe RepositoriesController, :type => :controller do |
255 | 302 | |
256 | 303 | context 'with no user logged in' do |
257 | 304 | before :each do |
258 | - post :update, project_id: project.id.to_s, id: repository.id, repository: repository_params | |
305 | + post :update, id: repository.id, repository: repository_params | |
259 | 306 | end |
260 | 307 | |
261 | 308 | it { is_expected.to redirect_to new_user_session_path } |
... | ... | @@ -272,7 +319,7 @@ describe RepositoriesController, :type => :controller do |
272 | 319 | repository.expects(:last_processing).returns(ready_processing) |
273 | 320 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
274 | 321 | |
275 | - xhr :get, :state, {project_id: project.id.to_s, id: repository.id, last_state: 'ANALYZING'} | |
322 | + xhr :get, :state, {id: repository.id, last_state: 'ANALYZING'} | |
276 | 323 | end |
277 | 324 | |
278 | 325 | it { is_expected.to respond_with(:success) } |
... | ... | @@ -286,7 +333,7 @@ describe RepositoriesController, :type => :controller do |
286 | 333 | repository.expects(:last_processing).returns(processing) |
287 | 334 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
288 | 335 | |
289 | - xhr :get, :state, {project_id: project.id.to_s, id: repository.id, last_state: 'LOADING'} | |
336 | + xhr :get, :state, {id: repository.id, last_state: 'LOADING'} | |
290 | 337 | end |
291 | 338 | |
292 | 339 | it { is_expected.to respond_with(:success) } |
... | ... | @@ -297,7 +344,7 @@ describe RepositoriesController, :type => :controller do |
297 | 344 | before :each do |
298 | 345 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
299 | 346 | |
300 | - xhr :get, :state, {project_id: project.id.to_s, id: repository.id, last_state: 'READY'} | |
347 | + xhr :get, :state, {id: repository.id, last_state: 'READY'} | |
301 | 348 | end |
302 | 349 | |
303 | 350 | it { is_expected.to respond_with(:ok) } |
... | ... | @@ -312,7 +359,7 @@ describe RepositoriesController, :type => :controller do |
312 | 359 | repository.expects(:last_processing).returns(errored_processing) |
313 | 360 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
314 | 361 | |
315 | - xhr :get, :state, {project_id: project.id.to_s, id: repository.id, last_state: 'ANALYZING'} | |
362 | + xhr :get, :state, {id: repository.id, last_state: 'ANALYZING'} | |
316 | 363 | end |
317 | 364 | |
318 | 365 | it { is_expected.to respond_with(:success) } |
... | ... | @@ -328,7 +375,7 @@ describe RepositoriesController, :type => :controller do |
328 | 375 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
329 | 376 | Processing.expects(:processing_with_date_of).with(repository.id, "2013-11-11").returns(processing) |
330 | 377 | |
331 | - xhr :get, :state_with_date, {project_id: project.id.to_s, id: repository.id, day: '11', month: '11', year: '2013'} | |
378 | + xhr :get, :state_with_date, {id: repository.id, day: '11', month: '11', year: '2013'} | |
332 | 379 | end |
333 | 380 | |
334 | 381 | it { is_expected.to respond_with(:ok) } |
... | ... | @@ -343,7 +390,7 @@ describe RepositoriesController, :type => :controller do |
343 | 390 | repository.expects(:process) |
344 | 391 | Repository.expects(:find).at_least_once.with(repository.id).returns(repository) |
345 | 392 | KalibroConfiguration.expects(:find).with(repository.id).returns(FactoryGirl.build(:kalibro_configuration, :with_id)) |
346 | - get :process_repository, project_id: project.id.to_s, id: repository.id | |
393 | + get :process_repository, id: repository.id | |
347 | 394 | end |
348 | 395 | it { is_expected.to redirect_to(repository_path(repository.id)) } |
349 | 396 | end | ... | ... |