Merge branch 'issue_23242' into 'master'

Fix project features default values

closes #23242

See merge request !7181
Signed-off-by: Rémy Coutable <remy@rymai.me>

CHANGELOG.md

@@ -7,6 +7,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Allow owners to fetch source code in CI builds. !6943
 - Reduce the overhead to calculate number of open/closed issues and merge requests within the group or project. !7123
 - Fix builds tab visibility. !7178
+- Fix project features default values. !7181
 
 ## 8.13.3 (2016-11-02)
 

app/models/concerns/project_features_compatibility.rb

@@ -31,7 +31,7 @@ module ProjectFeaturesCompatibility
   def write_feature_attribute(field, value)
     build_project_feature unless project_feature
 
-    access_level = value == "true" ? ProjectFeature::ENABLED : ProjectFeature::DISABLED
+    access_level = Gitlab::Utils.to_boolean(value) ? ProjectFeature::ENABLED : ProjectFeature::DISABLED
     project_feature.update_attribute(field, access_level)
   end
 end

app/models/project.rb

@@ -30,6 +30,11 @@ class Project < ActiveRecord::Base
   default_value_for :container_registry_enabled, gitlab_config_features.container_registry
   default_value_for(:repository_storage) { current_application_settings.repository_storage }
   default_value_for(:shared_runners_enabled) { current_application_settings.shared_runners_enabled }
+  default_value_for :issues_enabled, gitlab_config_features.issues
+  default_value_for :merge_requests_enabled, gitlab_config_features.merge_requests
+  default_value_for :builds_enabled, gitlab_config_features.builds
+  default_value_for :wiki_enabled, gitlab_config_features.wiki
+  default_value_for :snippets_enabled, gitlab_config_features.snippets
 
   after_create :ensure_dir_exist
   after_create :create_project_feature, unless: :project_feature

lib/api/helpers.rb

 module API
   module Helpers
+    include Gitlab::Utils
+
     PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN"
     PRIVATE_TOKEN_PARAM = :private_token
     SUDO_HEADER = "HTTP_SUDO"
     SUDO_PARAM = :sudo
 
-    def to_boolean(value)
-      return value if [true, false].include?(value)
-      return true if value =~ /^(true|t|yes|y|1|on)$/i
-      return false if value =~ /^(false|f|no|n|0|off)$/i
-
-      nil
-    end
-
     def private_token
       params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
     end

lib/gitlab/utils.rb

@@ -13,5 +13,13 @@ module Gitlab
     def force_utf8(str)
       str.force_encoding(Encoding::UTF_8)
     end
+
+    def to_boolean(value)
+      return value if [true, false].include?(value)
+      return true if value =~ /^(true|t|yes|y|1|on)$/i
+      return false if value =~ /^(false|f|no|n|0|off)$/i
+
+      nil
+    end
   end
 end

spec/lib/gitlab/utils_spec.rb

+describe Gitlab::Utils, lib: true do
+  def to_boolean(value)
+    described_class.to_boolean(value)
+  end
+
+  describe '.to_boolean' do
+    it 'accepts booleans' do
+      expect(to_boolean(true)).to be(true)
+      expect(to_boolean(false)).to be(false)
+    end
+
+    it 'converts a valid string to a boolean' do
+      expect(to_boolean(true)).to be(true)
+      expect(to_boolean('true')).to be(true)
+      expect(to_boolean('YeS')).to be(true)
+      expect(to_boolean('t')).to be(true)
+      expect(to_boolean('1')).to be(true)
+      expect(to_boolean('ON')).to be(true)
+
+      expect(to_boolean('FaLse')).to be(false)
+      expect(to_boolean('F')).to be(false)
+      expect(to_boolean('NO')).to be(false)
+      expect(to_boolean('n')).to be(false)
+      expect(to_boolean('0')).to be(false)
+      expect(to_boolean('oFF')).to be(false)
+    end
+
+    it 'converts an invalid string to nil' do
+      expect(to_boolean('fals')).to be_nil
+      expect(to_boolean('yeah')).to be_nil
+      expect(to_boolean('')).to be_nil
+      expect(to_boolean(nil)).to be_nil
+    end
+  end
+end

spec/models/concerns/project_features_compatibility_spec.rb

@@ -22,4 +22,18 @@ describe ProjectFeaturesCompatibility do
       expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED)
     end
   end
+
+  it "converts fields from true to ProjectFeature::ENABLED" do
+    features.each do |feature|
+      project.update_attribute("#{feature}_enabled".to_sym, true)
+      expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::ENABLED)
+    end
+  end
+
+  it "converts fields from false to ProjectFeature::DISABLED" do
+    features.each do |feature|
+      project.update_attribute("#{feature}_enabled".to_sym, false)
+      expect(project.project_feature.public_send("#{feature}_access_level")).to eq(ProjectFeature::DISABLED)
+    end
+  end
 end

spec/models/project_spec.rb

@@ -67,11 +67,11 @@ describe Project, models: true do
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
     it { is_expected.to have_many(:forks).through(:forked_project_links) }
 
-    context 'after create' do
-      it "creates project feature" do
+    context 'after initialized' do
+      it "has a project_feature" do
         project = FactoryGirl.build(:project)
 
-        expect { project.save }.to change{ project.project_feature.present? }.from(false).to(true)
+        expect(project.project_feature.present?).to be_present
       end
     end
 

spec/requests/api/api_helpers_spec.rb

@@ -265,36 +265,6 @@ describe API::Helpers, api: true do
     end
   end
 
-  describe '.to_boolean' do
-    it 'accepts booleans' do
-      expect(to_boolean(true)).to be(true)
-      expect(to_boolean(false)).to be(false)
-    end
-
-    it 'converts a valid string to a boolean' do
-      expect(to_boolean(true)).to be(true)
-      expect(to_boolean('true')).to be(true)
-      expect(to_boolean('YeS')).to be(true)
-      expect(to_boolean('t')).to be(true)
-      expect(to_boolean('1')).to be(true)
-      expect(to_boolean('ON')).to be(true)
-
-      expect(to_boolean('FaLse')).to be(false)
-      expect(to_boolean('F')).to be(false)
-      expect(to_boolean('NO')).to be(false)
-      expect(to_boolean('n')).to be(false)
-      expect(to_boolean('0')).to be(false)
-      expect(to_boolean('oFF')).to be(false)
-    end
-
-    it 'converts an invalid string to nil' do
-      expect(to_boolean('fals')).to be_nil
-      expect(to_boolean('yeah')).to be_nil
-      expect(to_boolean('')).to be_nil
-      expect(to_boolean(nil)).to be_nil
-    end
-  end
-
   describe '.handle_api_exception' do
     before do
       allow_any_instance_of(self.class).to receive(:sentry_enabled?).and_return(true)

spec/services/projects/create_service_spec.rb

@@ -69,7 +69,7 @@ describe Projects::CreateService, services: true do
 
       context 'wiki_enabled false does not create wiki repository directory' do
         before do
-          @opts.merge!( { project_feature_attributes: { wiki_access_level: ProjectFeature::DISABLED } })
+          @opts.merge!(wiki_enabled: false)
           @project = create_project(@user, @opts)
           @path = ProjectWiki.new(@project, @user).send(:path_to_repo)
         end