[#70106] Do not force-activate custom fields with a default value

app/services/projects/concerns/new_project_service.rb

@@ -42,7 +42,6 @@ def after_persist(attributes_call)
       new_project = attributes_call.result
 
       set_default_role(new_project) unless user.admin?
-      disable_custom_fields_with_empty_values(new_project)
       notify_project_created(new_project) if new_project.persisted?
 
       super
@@ -84,32 +83,12 @@ def send_project_creation_email(new_project)
       ProjectMailer.project_created(new_project, user:).deliver_later
     end
 
-    def disable_custom_fields_with_empty_values(new_project)
-      # Ideally, `build_missing_project_custom_field_project_mappings` would not activate custom fields
-      # with empty values, but:
-      # This hook is required as acts_as_customizable build custom values with their default value
-      # even if a blank value was provided in the project creation form.
-      # `build_missing_project_custom_field_project_mappings` will then activate the custom field,
-      # although the user explicitly provided a blank value. In order to not patch `acts_as_customizable`
-      # further, we simply identify these custom values and deactivate the custom field.
-
-      custom_field_ids = new_project.custom_values.select { |cv| cv.value.blank? && !cv.is_for_all? }.pluck(:custom_field_id)
-      custom_field_project_mappings = new_project.project_custom_field_project_mappings
-
-      custom_field_project_mappings
-        .where(custom_field_id: custom_field_ids)
-        .or(custom_field_project_mappings
-          .where.not(custom_field_id: new_project.available_custom_fields.select(:id)))
-        .destroy_all
-    end
-
     def build_missing_project_custom_field_project_mappings(project)
       # Activate all custom fields (via mapping table) that have no mapping, but are either
       # intended for all projects, or have a value provided by the user.
+      custom_field_ids = Set.new(activatable_custom_field_ids_from_project(project))
+      custom_field_ids.merge(activatable_custom_field_ids_from_params)
 
-      custom_field_ids = project.custom_values
-        .select { |cv| cv.value? || cv.is_for_all? }
-        .pluck(:custom_field_id).uniq
       activated_custom_field_ids = project.project_custom_field_project_mappings.pluck(:custom_field_id).uniq
 
       mappings = (custom_field_ids - activated_custom_field_ids).uniq
@@ -118,14 +97,38 @@ def build_missing_project_custom_field_project_mappings(project)
       project.project_custom_field_project_mappings.build(mappings)
     end
 
+    def activatable_custom_field_ids_from_project(project)
+      # We will activate fields with existing values, unless it's the default value, which might
+      # be generated automatically
+      project.custom_values
+             .select { |cv| cv.is_for_all? || (cv.value? && !cv.default?) }
+             .pluck(:custom_field_id)
+    end
+
+    def activatable_custom_field_ids_from_params
+      # We will activate fields that are explicitly set via params
+
+      # Extract custom field IDs from custom_field_values
+      via_cf_values = params.fetch(:custom_field_values, {})
+                            .keys
+                            .map { it.to_s.to_i }
+
+      # Extract custom field IDs from params keys that match 'custom_field_<id>'
+      via_cf = params.keys
+                     .grep(/\Acustom_field_\d+\z/)
+                     .map { |k| k.to_s[/custom_field_(\d+)/, 1].to_i }
+
+      via_cf_values + via_cf
+    end
+
     def update_calculated_value_custom_fields(model)
       changed_cf_ids = model.custom_values.map(&:custom_field_id)
 
       # Using unscope(where: :admin_only) to fix an issue when non admin user
       # edits a custom field which is used by an admin only calculated value
       # field. Without this unscoping, admin only value and all fields
       # referencing it (recursively) will not be recalculated and there will
-      # even be no place for that recalculatin to be triggered unless an admin
+      # even be no place for that recalculation to be triggered unless an admin
       # edits same value again.
       #
       # This may need to be handled differently to make it work for other custom

spec/features/projects/create_spec.rb

@@ -267,6 +267,14 @@
                project_custom_field_section:)
       end
 
+      shared_let(:required_inactive_custom_field_with_default_value) do
+        create(:text_project_custom_field,
+               name: "Required inactive with default value",
+               is_required: true,
+               default_value: "foo",
+               project_custom_field_section:)
+      end
+
       it "renders activated required custom fields for new" do
         visit new_project_path
 
@@ -290,6 +298,7 @@
 
         # Inactive fields, even if required, should not be shown
         expect(page).to have_no_field "Required Inactive *"
+        expect(page).to have_no_field "Required Inactive with default value *"
       end
     end
 

spec/features/projects/creation_wizard/project_creation_wizard_spec.rb

@@ -29,6 +29,10 @@
 #++
 require "spec_helper"
 
+# This is a feature spec for the project creation wizard, but only when creating
+# a new project from a template with the wizard enabled (Project Initiation Request, PIR).
+# The wizard that is shown when creating a new blank project from scratch is NOT tested here.
+# See `spec/features/projects/create_spec.rb` for that.
 RSpec.describe "Project creation wizard",
                :js,
                :with_cuprite do

spec/requests/api/v3/projects/create_resource_spec.rb

@@ -119,38 +119,38 @@
   end
 
   describe "custom fields" do
-    context "with an optional custom field" do
-      shared_let(:optional_custom_field) do
-        create(:text_project_custom_field,
-               name: "Department",
-               is_for_all: true)
+    shared_examples "creates a project with a custom value" do |custom_value|
+      it "responds with 201" do
+        expect(last_response).to have_http_status(:created)
       end
 
-      shared_examples "creates a project with an empty custom value" do
-        it "responds with 201" do
-          expect(last_response).to have_http_status(:created)
-        end
+      it "returns the newly created project" do
+        expect(last_response.body)
+          .to be_json_eql("Project".to_json)
+                .at_path("_type")
 
-        it "returns the newly created project" do
-          expect(last_response.body)
-            .to be_json_eql("Project".to_json)
-                  .at_path("_type")
+        expect(last_response.body)
+          .to be_json_eql("Project name".to_json)
+                .at_path("name")
+      end
 
-          expect(last_response.body)
-            .to be_json_eql("Project name".to_json)
-                  .at_path("name")
-        end
+      it "creates a project with an empty custom field value" do
+        project = Project.last
+        expect(project.typed_custom_value_for(shared_custom_field))
+          .to eq(custom_value)
+      end
 
-        it "creates a project with an empty custom field value" do
-          project = Project.last
-          expect(project.typed_custom_value_for(optional_custom_field))
-            .to eq("")
-        end
+      it "automatically activates the cf for project" do
+        expect(Project.last.project_custom_fields)
+          .to contain_exactly(shared_custom_field)
+      end
+    end
 
-        it "automatically activates the cf for project" do
-          expect(Project.last.project_custom_fields)
-            .to contain_exactly(optional_custom_field)
-        end
+    context "with an optional custom field" do
+      shared_let(:shared_custom_field) do
+        create(:text_project_custom_field,
+               name: "Department",
+               is_for_all: true)
       end
 
       context "when no custom field value is provided" do
@@ -161,63 +161,91 @@
           }.to_json
         end
 
-        it_behaves_like "creates a project with an empty custom value"
+        it_behaves_like "creates a project with a custom value", ""
       end
 
       context "when the custom field is provided but empty" do
         let(:body) do
           {
             identifier: "new_project_identifier",
             name: "Project name",
-            optional_custom_field.attribute_name(:camel_case) => {
+            shared_custom_field.attribute_name(:camel_case) => {
               raw: ""
             }
           }.to_json
         end
 
-        it_behaves_like "creates a project with an empty custom value"
+        it_behaves_like "creates a project with a custom value", ""
       end
 
       context "when the custom field value is provided and valid" do
         let(:body) do
           {
             identifier: "new_project_identifier",
             name: "Project name",
-            optional_custom_field.attribute_name(:camel_case) => {
+            shared_custom_field.attribute_name(:camel_case) => {
               raw: "Engineering"
             }
           }.to_json
         end
 
-        it "responds with 201" do
-          expect(last_response).to have_http_status(:created)
-        end
+        it_behaves_like "creates a project with a custom value", "Engineering"
+      end
+    end
 
-        it "returns the newly created project" do
-          expect(last_response.body)
-            .to be_json_eql("Project".to_json)
-                  .at_path("_type")
+    context "when a custom field has a default value" do
+      shared_let(:shared_custom_field) do
+        create(:text_project_custom_field,
+               name: "Location",
+               is_for_all: false,
+               default_value: "Default Location")
+      end
 
-          expect(last_response.body)
-            .to be_json_eql("Project name".to_json)
-                  .at_path("name")
+      context "when the custom field value is provided and valid" do
+        let(:body) do
+          {
+            identifier: "new_project_identifier",
+            name: "Project name",
+            shared_custom_field.attribute_name(:camel_case) => {
+              raw: "Custom Location"
+            }
+          }.to_json
         end
 
-        it "creates a project with the custom field value" do
-          project = Project.last
-          expect(project.typed_custom_value_for(optional_custom_field))
-            .to eq("Engineering")
+        it_behaves_like "creates a project with a custom value", "Custom Location"
+      end
+
+      context "when the custom field value is identical to the default" do
+        let(:body) do
+          {
+            identifier: "new_project_identifier",
+            name: "Project name",
+            shared_custom_field.attribute_name(:camel_case) => {
+              raw: "Default Location"
+            }
+          }.to_json
         end
 
-        it "automatically activates the cf for project if the value was provided" do
-          expect(Project.last.project_custom_fields)
-            .to contain_exactly(optional_custom_field)
+        it_behaves_like "creates a project with a custom value", "Default Location"
+      end
+
+      context "when the custom field value is blank" do
+        let(:body) do
+          {
+            identifier: "new_project_identifier",
+            name: "Project name",
+            shared_custom_field.attribute_name(:camel_case) => {
+              raw: ""
+            }
+          }.to_json
         end
+
+        it_behaves_like "creates a project with a custom value", ""
       end
     end
 
     context "with a required for_all custom field" do
-      shared_let(:required_custom_field) do
+      shared_let(:shared_custom_field) do
         create(:text_project_custom_field,
                name: "Department",
                is_required: true,
@@ -246,7 +274,7 @@
           {
             identifier: "new_project_identifier",
             name: "Project name",
-            required_custom_field.attribute_name(:camel_case) => {
+            shared_custom_field.attribute_name(:camel_case) => {
               raw: ""
             }
           }.to_json
@@ -266,36 +294,13 @@
           {
             identifier: "new_project_identifier",
             name: "Project name",
-            required_custom_field.attribute_name(:camel_case) => {
+            shared_custom_field.attribute_name(:camel_case) => {
               raw: "Engineering"
             }
           }.to_json
         end
 
-        it "responds with 201" do
-          expect(last_response).to have_http_status(:created)
-        end
-
-        it "returns the newly created project" do
-          expect(last_response.body)
-            .to be_json_eql("Project".to_json)
-            .at_path("_type")
-
-          expect(last_response.body)
-            .to be_json_eql("Project name".to_json)
-            .at_path("name")
-        end
-
-        it "creates a project with the custom field value" do
-          project = Project.last
-          expect(project.typed_custom_value_for(required_custom_field))
-            .to eq("Engineering")
-        end
-
-        it "automatically activates the cf for project if the value was provided" do
-          expect(Project.last.project_custom_fields)
-            .to contain_exactly(required_custom_field)
-        end
+        it_behaves_like "creates a project with a custom value", "Engineering"
       end
     end