Correct description plan logic based on the description in the import JSON

.changelog/319.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+`resource/davinci_flow`: Fix issue whereby descriptions are not updated.
+```
+
+```release-note:bug
+`resource/davinci_flow`: Where a description is not provided in the Terraform schema, the description from the flow export will be applied as a fallback.
+```

docs/resources/flow.md

@@ -64,7 +64,7 @@ resource "davinci_flow" "my_awesome_main_flow" {
 
 - `connection_link` (Block Set) Mappings to connections that this flow depends on.  Connections should be managed (with the `davinci_connection` resource) or retrieved (with the `davinci_connection` data source) to provide the mappings needed for this configuration block. (see [below for nested schema](#nestedblock--connection_link))
 - `deploy` (Boolean, Deprecated) **Deprecation notice:** This attribute is deprecated and will be removed in a future release.  Flows are automatically deployed on import. A boolean that specifies whether to deploy the flow after import.  Defaults to `true`.
-- `description` (String) A string that specifies a description of the flow.  If the field is left blank, a description value will be derived by the service.
+- `description` (String) A string that specifies a description of the flow.  If the field is left undefined, the description from the flow export will be used.  If this field is left undefined and the flow export does not contain a description, the service will define a description on import.
 - `name` (String) A string that identifies the flow name after import.  If the field is left blank, a flow name will be derived by the service from the name in the import JSON (the `flow_json` parameter).
 - `subflow_link` (Block Set) Child flows of this resource, where the `flow_json` contains reference to subflows.  If the `flow_json` contains subflows, this one `subflow_link` block is required per contained subflow. (see [below for nested schema](#nestedblock--subflow_link))
 

internal/acctest/flows/full-minimal.json

@@ -8,7 +8,7 @@
     "currentVersion": 4,
     "customerId": "db5f4450b2bd8a56ce076dec0c358a9a",
     "deployedDate": 1707837221226,
-    "description": "Imported on Wed Jan 31 2024 13:29:13 GMT+0000 (Coordinated Universal Time)",
+    "description": "This is a fallback description",
     "flowStatus": "enabled",
     "isOutputSchemaSaved": false,
     "name": "simple",

internal/service/davinci/resource_flow.go

@@ -138,7 +138,7 @@ func (r *FlowResource) Schema(ctx context.Context, req resource.SchemaRequest, r
 	)
 
 	descriptionDescription := framework.SchemaAttributeDescriptionFromMarkdown(
-		"A string that specifies a description of the flow.  If the field is left blank, a description value will be derived by the service.",
+		"A string that specifies a description of the flow.  If the field is left undefined, the description from the flow export will be used.  If this field is left undefined and the flow export does not contain a description, the service will define a description on import.",
 	)
 
 	deployDescription := framework.SchemaAttributeDescriptionFromMarkdown(
@@ -220,6 +220,10 @@ func (r *FlowResource) Schema(ctx context.Context, req resource.SchemaRequest, r
 				MarkdownDescription: descriptionDescription.MarkdownDescription,
 				Optional:            true,
 				Computed:            true,
+
+				Validators: []validator.String{
+					stringvalidator.LengthAtLeast(attrMinLength),
+				},
 			},
 
 			"flow_json": schema.StringAttribute{
@@ -436,6 +440,10 @@ func (p *FlowResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRe
 			resp.Plan.SetAttribute(ctx, path.Root("name"), flowObject.Name)
 		}
 
+		if v := flowObject.Description; config.Description.IsNull() && v != nil && *v != "" {
+			resp.Plan.SetAttribute(ctx, path.Root("description"), *v)
+		}
+
 		flowConfigObject = flowObject.FlowConfiguration
 
 		var d diag.Diagnostics
@@ -640,7 +648,7 @@ func (r *FlowResource) Create(ctx context.Context, req resource.CreateRequest, r
 		Description:             createFlow.Description,
 	}
 
-	resp.Diagnostics.Append(r.updateFlow(ctx, environmentID, createFlow.FlowID, &flowUpdate)...)
+	resp.Diagnostics.Append(r.updateFlow(ctx, environmentID, createFlow.FlowID, &flowUpdate, true)...)
 	if resp.Diagnostics.HasError() {
 		return
 	}
@@ -773,7 +781,20 @@ func (r *FlowResource) Update(ctx context.Context, req resource.UpdateRequest, r
 			return
 		}
 
-		resp.Diagnostics.Append(r.updateFlow(ctx, environmentID, flowID, daVinciUpdate)...)
+		resp.Diagnostics.Append(r.updateFlow(ctx, environmentID, flowID, daVinciUpdate, true)...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+	} else if !plan.Description.Equal(state.Description) || !plan.Name.Equal(state.Name) {
+		daVinciUpdate := &davinci.FlowUpdate{
+			Name:        plan.Name.ValueStringPointer(),
+			Description: plan.Description.ValueStringPointer(),
+		}
+
+		resp.Diagnostics.Append(r.updateFlow(ctx, environmentID, flowID, daVinciUpdate, false)...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
 	}
 
 	// Do an export for state
@@ -811,7 +832,7 @@ func (r *FlowResource) Update(ctx context.Context, req resource.UpdateRequest, r
 	resp.Diagnostics.Append(resp.State.Set(ctx, state)...)
 }
 
-func (r *FlowResource) updateFlow(ctx context.Context, environmentID, flowID string, daVinciUpdate *davinci.FlowUpdate) diag.Diagnostics {
+func (r *FlowResource) updateFlow(ctx context.Context, environmentID, flowID string, daVinciUpdate *davinci.FlowUpdate, deploy bool) diag.Diagnostics {
 	var diags diag.Diagnostics
 
 	_, err := sdk.DoRetryable(
@@ -832,19 +853,21 @@ func (r *FlowResource) updateFlow(ctx context.Context, environmentID, flowID str
 		return diags
 	}
 
-	_, err = sdk.DoRetryable(
-		ctx,
-		r.Client,
-		environmentID,
-		func() (any, *http.Response, error) {
-			return r.Client.DeployFlowWithResponse(environmentID, flowID)
-		},
-	)
-	if err != nil {
-		diags.AddError(
-			"Error deploying flow",
-			fmt.Sprintf("Error deploying flow, this might indicate a misconfiguration of the flow, or an unmapped node connection: %s", err),
+	if deploy {
+		_, err = sdk.DoRetryable(
+			ctx,
+			r.Client,
+			environmentID,
+			func() (any, *http.Response, error) {
+				return r.Client.DeployFlowWithResponse(environmentID, flowID)
+			},
 		)
+		if err != nil {
+			diags.AddError(
+				"Error deploying flow",
+				fmt.Sprintf("Error deploying flow, this might indicate a misconfiguration of the flow, or an unmapped node connection: %s", err),
+			)
+		}
 	}
 
 	return diags
@@ -955,6 +978,14 @@ func (p *FlowResourceModel) expand(ctx context.Context) (*davinci.FlowImport, di
 		return nil, diags
 	}
 
+	if data.Name == nil {
+		data.Name = &data.FlowInfo.Name
+	}
+
+	if data.Description == nil && data.FlowInfo.Description != nil {
+		data.Description = data.FlowInfo.Description
+	}
+
 	data.FlowNameMapping = map[string]string{
 		data.FlowInfo.FlowID: p.Name.ValueString(),
 	}
@@ -1038,13 +1069,11 @@ func (p *FlowResourceModel) expandUpdate(state FlowResourceModel) (*davinci.Flow
 	}
 
 	if !p.Name.IsNull() {
-		name := p.Name.ValueString()
-		data.Name = &name
+		data.Name = p.Name.ValueStringPointer()
 	}
 
 	if !p.Description.IsNull() {
-		description := p.Description.ValueString()
-		data.Description = &description
+		data.Description = p.Description.ValueStringPointer()
 	}
 
 	err = davinci.Unmarshal([]byte(state.FlowConfigurationJSON.ValueString()), &stateData, davinci.ExportCmpOpts{})
@@ -1113,8 +1142,8 @@ func (p *FlowResourceModel) toState(apiObject *davinci.Flow) diag.Diagnostics {
 
 	p.Name = framework.StringToTF(apiObject.Name)
 
-	if apiObject.Description != nil {
-		p.Description = framework.StringToTF(*apiObject.Description)
+	if v := apiObject.Description; v != nil {
+		p.Description = framework.StringToTF(*v)
 	}
 
 	var d diag.Diagnostics

internal/service/davinci/resource_flow_test.go

@@ -121,6 +121,7 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) {
 				"mutable": regexp.MustCompile(`^true$`),
 				"name":    regexp.MustCompile(`^fdgdfgfdg$`),
 				"type":    regexp.MustCompile(`^property$`),
+				//"type":    regexp.MustCompile(`^string$`),
 			}),
 			resource.TestMatchTypeSetElemNestedAttrs(resourceFullName, "flow_variables.*", map[string]*regexp.Regexp{
 				"context": regexp.MustCompile(`^flow$`),
@@ -131,6 +132,8 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) {
 				"mutable": regexp.MustCompile(`^true$`),
 				"name":    regexp.MustCompile(`^test123$`),
 				"type":    regexp.MustCompile(`^property$`),
+				//"type":    regexp.MustCompile(`^number$`),
+				//"value":   regexp.MustCompile(`^10$`),
 			}),
 		),
 	}
@@ -146,7 +149,7 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) {
 			resource.TestMatchResourceAttr(resourceFullName, "id", verify.P1DVResourceIDRegexpFullString),
 			resource.TestMatchResourceAttr(resourceFullName, "environment_id", verify.P1ResourceIDRegexpFullString),
 			resource.TestCheckResourceAttr(resourceFullName, "name", "simple"),
-			resource.TestMatchResourceAttr(resourceFullName, "description", regexp.MustCompile(`^Imported on `)),
+			resource.TestCheckResourceAttr(resourceFullName, "description", "This is a fallback description"),
 			resource.TestCheckResourceAttr(resourceFullName, "flow_json", fmt.Sprintf("%s\n", minimalStepJson)),
 			resource.TestCheckResourceAttr(resourceFullName, "connection_link.#", "1"),
 			resource.TestCheckResourceAttr(resourceFullName, "deploy", "true"),
@@ -155,14 +158,19 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) {
 		),
 	}
 
+	updateStepHcl, updateStepJson, err := testAccResourceFlow_Minimal_WithMappingIDs_Update_HCL(resourceName, name, withBootstrapConfig)
+	if err != nil {
+		t.Fatalf("Failed to get HCL: %v", err)
+	}
+
 	updateStep := resource.TestStep{
-		Config: minimalStepHcl,
+		Config: updateStepHcl,
 		Check: resource.ComposeTestCheckFunc(
 			resource.TestMatchResourceAttr(resourceFullName, "id", verify.P1DVResourceIDRegexpFullString),
 			resource.TestMatchResourceAttr(resourceFullName, "environment_id", verify.P1ResourceIDRegexpFullString),
 			resource.TestCheckResourceAttr(resourceFullName, "name", "simple"),
-			resource.TestMatchResourceAttr(resourceFullName, "description", regexp.MustCompile(`^$`)),
-			resource.TestCheckResourceAttr(resourceFullName, "flow_json", fmt.Sprintf("%s\n", minimalStepJson)),
+			resource.TestCheckResourceAttr(resourceFullName, "description", "This is an updated description"),
+			resource.TestCheckResourceAttr(resourceFullName, "flow_json", fmt.Sprintf("%s\n", updateStepJson)),
 			resource.TestCheckResourceAttr(resourceFullName, "connection_link.#", "1"),
 			resource.TestCheckResourceAttr(resourceFullName, "deploy", "true"),
 			resource.TestCheckResourceAttr(resourceFullName, "subflow_link.#", "0"),
@@ -184,20 +192,21 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) {
 		ErrorCheck:               acctest.ErrorCheck(t),
 		CheckDestroy:             davinci.Flow_CheckDestroy(),
 		Steps: []resource.TestStep{
-			// Create from scratch
+			// Create full from scratch
 			fullStep,
 			{
 				Config:  fullStepHcl,
 				Destroy: true,
 			},
-			// Create from scratch
+			// Create minimal from scratch
 			minimalStep,
 			{
 				Config:  minimalStepHcl,
 				Destroy: true,
 			},
 			// Test updates
 			fullStep,
+			minimalStep,
 			updateStep,
 			fullStep,
 			// Test importing the resource
@@ -219,6 +228,7 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) {
 					"connection_link",
 					"subflow_link",
 					"flow_json",
+					"flow_export_json",
 				},
 			},
 		},
@@ -1046,6 +1056,41 @@ EOT
 }`, acctest.PingoneEnvironmentSsoHcl(resourceName, withBootstrapConfig), commonHcl, resourceName, mainFlowJson), mainFlowJson, nil
 }
 
+func testAccResourceFlow_Minimal_WithMappingIDs_Update_HCL(resourceName, name string, withBootstrapConfig bool) (hcl, mainFlowJson string, err error) {
+
+	mainFlowJson, err = acctest.ReadFlowJsonFile("flows/full-minimal.json")
+	if err != nil {
+		return "", "", err
+	}
+
+	commonHcl, err := testAccResourceFlow_Common_WithMappingIDs_HCL(resourceName, name)
+	if err != nil {
+		return "", "", err
+	}
+
+	return fmt.Sprintf(`
+%[1]s
+
+%[2]s
+
+resource "davinci_flow" "%[3]s" {
+  environment_id = pingone_environment.%[3]s.id
+
+  description = "This is an updated description"
+
+  flow_json = <<EOT
+%[4]s
+EOT
+
+  // Error connector
+  connection_link {
+    id                           = davinci_connection.%[3]s-error.id
+    name                         = davinci_connection.%[3]s-error.name
+    replace_import_connection_id = "53ab83a4a4ab919d9f2cb02d9e111ac8"
+  }
+}`, acctest.PingoneEnvironmentSsoHcl(resourceName, withBootstrapConfig), commonHcl, resourceName, mainFlowJson), mainFlowJson, nil
+}
+
 func testAccResourceFlow_Common_WithoutMappingIDs_HCL_Resources(resourceName, name string) (hcl string, err error) {
 
 	subFlow1Json, err := acctest.ReadFlowJsonFile("flows/full-basic-subflow-1.json")