Fix project features default valuesissue_23242

10 files changed, 70 insertions, 43 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 55e475df25e..ac3fc5b6388 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -39,6 +39,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Shortened merge request modal to let clipboard button not overlap
 
 ## 8.13.2
+  - Fix Project Features default values
   - Fix builds dropdown overlapping bug !7124
   - Fix and improve `Sortable.highest_label_priority`
 
diff --git a/app/models/concerns/project_features_compatibility.rb b/app/models/concerns/project_features_compatibility.rb
index 9216122923e..6d88951c713 100644
--- a/app/models/concerns/project_features_compatibility.rb
+++ b/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
diff --git a/app/models/project.rb b/app/models/project.rb
index ae57815c620..c71c9cd9943 100644
--- a/app/models/project.rb
+++ b/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
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 8025581d3ca..3c9d7b1aaef 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -1,18 +1,12 @@
 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
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index e59ead5d76c..4c395b4266e 100644
--- a/lib/gitlab/utils.rb
+++ b/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
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
new file mode 100644
index 00000000000..d5d87310874
--- /dev/null
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -0,0 +1,35 @@
+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
diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb
index 5363aea4d22..9041690023f 100644
--- a/spec/models/concerns/project_features_compatibility_spec.rb
+++ b/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
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f4dda1ee558..aef277357cf 100644
--- a/spec/models/project_spec.rb
+++ b/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
 
diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb
index f7fe4c10873..01bb9e955e0 100644
--- a/spec/requests/api/api_helpers_spec.rb
+++ b/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)
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index 3ea1273abc3..876bfaf085c 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/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