From 1a01fc0c964defe60c9dc99fa2b5fa0387f3246a Mon Sep 17 00:00:00 2001 From: Sebastian Ziebell Date: Wed, 13 Feb 2013 17:45:05 +0100 Subject: [PATCH 1/3] API: tests to show incorrect behavior when reaching project limit When reaching the project limit the API returns an error code 404 on the last possible project. The project itself is created and is available in the database (seems valid). A similar behavior can be observed when reaching the project limit via web client, but in this case the user is notified that the maximum number of projects is reached. The project itself is still created and can be accessed. Tests are added to check the behavior when reaching the project limit or one tries to exceed it via the API. --- spec/requests/api/projects_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 16fd1b9307c..3256cde6b82 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -33,6 +33,20 @@ describe Gitlab::API do end describe "POST /projects" do + context "maximum number of projects reached" do + before do + (1..user2.projects_limit).each do |project| + post api("/projects", user2), name: "foo#{project}" + end + end + + it "should not create new project" do + expect { + post api("/projects", user2), name: 'foo' + }.to change {Project.count}.by(0) + end + end + it "should create new project without path" do expect { post api("/projects", user), name: 'foo' }.to change {Project.count}.by(1) end @@ -41,6 +55,12 @@ describe Gitlab::API do expect { post api("/projects", user) }.to_not change {Project.count} end + it "should create last project before reaching project limit" do + (1..user2.projects_limit-1).each { |p| post api("/projects", user2), name: "foo#{p}" } + post api("/projects", user2), name: "foo" + response.status.should == 201 + end + it "should respond with 201 on success" do post api("/projects", user), name: 'foo' response.status.should == 201 From 7e45ba700415a180998cee5eb36158727be5f051 Mon Sep 17 00:00:00 2001 From: Sebastian Ziebell Date: Thu, 14 Feb 2013 11:14:52 +0100 Subject: [PATCH 2/3] API: fixes return code when creating last project before reaching limit When creating the last project via API when reaching the project limit a status code of 404 (Not found) is returned instead of 201 (Created). The fix checks now correctly if the project could be saved. --- lib/api/projects.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index d416121a78a..e0c1e338290 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -44,7 +44,7 @@ module Gitlab :merge_requests_enabled, :wiki_enabled] @project = ::Projects::CreateContext.new(current_user, attrs).execute - if @project.saved? + if @project.persisted? present @project, with: Entities::Project else not_found! From 3025824415184c16f86bb67276474113de48d813 Mon Sep 17 00:00:00 2001 From: Sebastian Ziebell Date: Thu, 14 Feb 2013 12:58:33 +0100 Subject: [PATCH 3/3] API: refactored last fix, project limit in web client is fixed too The previous call `saved?` is restored in the `POST /projects` method in the API. It is refactored to check if the record is persisted. This is useful to not validate the record again after saving. This fixes the returned status code in the web client too. If the last project is created via web client instead of error notification the project page is shown. --- app/models/project.rb | 2 +- lib/api/projects.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 15b2d858b62..ea8e0ecfd40 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -146,7 +146,7 @@ class Project < ActiveRecord::Base end def saved? - id && valid? + id && persisted? end def import? diff --git a/lib/api/projects.rb b/lib/api/projects.rb index e0c1e338290..d416121a78a 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -44,7 +44,7 @@ module Gitlab :merge_requests_enabled, :wiki_enabled] @project = ::Projects::CreateContext.new(current_user, attrs).execute - if @project.persisted? + if @project.saved? present @project, with: Entities::Project else not_found!