template: preserve quoted numeric strings in DeepCopyWithTemplate

[holger-waschke]

This fixes an issue where a quoted numeric string produced by DeepCopyWithTemplate could end up being serialized as an integer instead of a string.

One concrete case is Jira custom fields that expect a payload like:

{"id":"15129"}

Before this change, the rendered field could become:

{"customfield_11209":{"id":15129}}

In that form, id was sent as an integer, which Jira rejected.

With this change, quoted numeric values are preserved as strings, so the generated payload keeps the expected type.

A regression test was added to cover this case.

Summary by CodeRabbit

• Bug Fixes

  • Improved template handling for YAML-parsed string values, including better treatment of empty strings and unquoted numeric strings.

• Tests

  • Added test coverage verifying quoted-numeric and nested template string processing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e91d2b9b-6443-4940-9523-071f36250316

📥 Commits

Reviewing files that changed from the base of the PR and between 3a36ca5 and 0da5231.

📒 Files selected for processing (1)

• template/template_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• template/template_test.go

---

📝 Walkthrough

Walkthrough

String-valued template output is now unwrapped when YAML parsing yields an inline string; non-string inline YAML values are recursively processed, and empty/whitespace template results are guarded against to avoid further recursion. Tests added to verify unquoting of quoted numeric strings, including nested cases.

Changes

Cohort / File(s)	Summary
Template Processing Logic template/template.go	Adjusts handling of string-valued templates: unwraps YAML-parsed inline strings, recurses for non-string inline YAML values, and returns original parsed result when the template output is empty/whitespace.
Template Tests template/template_test.go	Adds test cases verifying that a TemplateFunc returning a quoted numeric string (e.g., "\"123\"") becomes the unquoted string ("123"), including a nested/map variant to exercise recursive processing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble YAML wraps, then set them free,
Quoted digits loosened by my plea,
I hop through maps and nested trees,
Unwrapping strings with gentle ease,
A rabbit’s cheer for tidy keys 🐇✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description explains the bug, provides a concrete example, describes the fix, and mentions a regression test was added, but lacks a release notes entry and issue linkage required by the template.	Add release notes entry (e.g., '[BUGFIX]') and link to the related issue using 'Fixes #' format as specified in the template.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: preserving quoted numeric strings in DeepCopyWithTemplate, which matches the core issue being fixed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

[coderabbitai]

🧹 Nitpick comments (1)

template/template_test.go (1)

696-703: Add one nested payload regression case for end-to-end parity.

Current case validates scalar behavior well, but a nested object case (Jira-style field map) would better lock the original regression path.

🧪 Suggested test case extension

@@
 		{
 			title: "quoted numeric string stays string",
 			input: "hello",
 			fn: TemplateFunc(func(string) (string, error) {
 				return "\"123\"", nil
 			}),
 			want: "123",
 		},
+		{
+			title: "quoted numeric string stays string in nested map",
+			input: map[string]any{
+				"customfield_11209": map[string]any{
+					"id": "TOKEN",
+				},
+			},
+			fn: TemplateFunc(func(s string) (string, error) {
+				if s == "TOKEN" {
+					return "\"15129\"", nil
+				}
+				return s, nil
+			}),
+			want: map[string]any{
+				"customfield_11209": map[string]any{
+					"id": "15129",
+				},
+			},
+		},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@template/template_test.go` around lines 696 - 703, Add a new table test case
to cover the nested-object regression: create a test row (e.g., title "quoted
numeric string in nested payload stays string") alongside the existing case in
template_test.go that uses TemplateFunc to return a nested payload like a
Jira-style fields map where the value is a quoted numeric string (for example a
map {"fields": {"customfield_12345": "\"123\""}}), and assert the processed
output preserves the string value "123" (not numeric). Reference the existing
test harness and TemplateFunc usage so the new case mirrors the scalar case but
with the nested object shape to lock the regression.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@template/template_test.go`:
- Around line 696-703: Add a new table test case to cover the nested-object
regression: create a test row (e.g., title "quoted numeric string in nested
payload stays string") alongside the existing case in template_test.go that uses
TemplateFunc to return a nested payload like a Jira-style fields map where the
value is a quoted numeric string (for example a map {"fields":
{"customfield_12345": "\"123\""}}), and assert the processed output preserves
the string value "123" (not numeric). Reference the existing test harness and
TemplateFunc usage so the new case mirrors the scalar case but with the nested
object shape to lock the regression.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14633b63-013a-4316-bf02-0988e8a40f0a

📥 Commits

Reviewing files that changed from the base of the PR and between 35cbf8d and 3a36ca5.

📒 Files selected for processing (2)

• template/template.go
• template/template_test.go

[TheMeier]

Thank you @holger-waschke. LGTM
The addtional checks from proposed from rabbitai sound sensible to me, what do you think?

[TheMeier]

Loosely related to #5012

[holger-waschke]

Thank you @holger-waschke. LGTM The addtional checks from proposed from rabbitai sound sensible to me, what do you think?

Looks reasonable to me, I added the unit test to the template_test.go