Skip to content

Commit 06bd5fa

Browse files
refactor: reduce complexity of execute_impl in create_work_item.rs (#1509)
Extract maybe_build_artifact_link_op helper to eliminate deep nesting in the artifact-link resolution block. The nested if-enabled → if-repo_name → match repo_id → match resolve chain (4 levels of nesting) is now a single call that returns Ok(None), Ok(Some((op,msg))), or Err(failure) — reducing the nesting depth of execute_impl from 7 to 4 and making the main execution path linear and easy to follow. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fc0674c commit 06bd5fa

1 file changed

Lines changed: 71 additions & 48 deletions

File tree

src/safe_outputs/create_work_item.rs

Lines changed: 71 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,60 @@ fn artifact_link_op(project: &str, repository_id: &str, branch: &str) -> serde_j
220220
})
221221
}
222222

223+
/// Resolve the artifact link patch op and a human-readable status message.
224+
///
225+
/// Returns `Ok(None)` when artifact linking is disabled.
226+
/// Returns `Ok(Some((op, message)))` when an op was built.
227+
/// Returns `Err(ExecutionResult)` when the caller should short-circuit with a failure.
228+
async fn maybe_build_artifact_link_op(
229+
client: &reqwest::Client,
230+
config: &ArtifactLinkConfig,
231+
ctx: &ExecutionContext,
232+
org_url: &str,
233+
project: &str,
234+
token: &str,
235+
) -> Result<Option<(serde_json::Value, String)>, ExecutionResult> {
236+
if !config.enabled {
237+
return Ok(None);
238+
}
239+
240+
let repo_name = match config.repository.as_ref().or(ctx.repository_name.as_ref()) {
241+
Some(name) => name,
242+
None => {
243+
return Ok(Some((
244+
serde_json::Value::Null,
245+
"skipped: no repository available".to_string(),
246+
)));
247+
}
248+
};
249+
250+
// Prefer the env-supplied repo ID to avoid an extra API call; fall back to lookup.
251+
let repo_id = if config.repository.is_none() {
252+
ctx.repository_id.clone()
253+
} else {
254+
None
255+
};
256+
257+
let repo_id = match repo_id {
258+
Some(id) => id,
259+
None => {
260+
match resolve_repository_id(client, org_url, project, token, repo_name).await {
261+
Ok(id) => id,
262+
Err(e) => {
263+
return Err(ExecutionResult::failure(format!(
264+
"Failed to resolve repository '{}': {}",
265+
repo_name, e
266+
)));
267+
}
268+
}
269+
}
270+
};
271+
272+
let op = artifact_link_op(project, &repo_id, &config.branch);
273+
let message = format!("linked to {}:{}", repo_name, config.branch);
274+
Ok(Some((op, message)))
275+
}
276+
223277
/// Resolve a repository name to its ID via Azure DevOps API
224278
async fn resolve_repository_id(
225279
client: &reqwest::Client,
@@ -388,55 +442,24 @@ impl Executor for CreateWorkItemResult {
388442
let client = reqwest::Client::new();
389443

390444
// Add artifact link if configured (included in creation request)
391-
let artifact_link_included = if config.artifact_link.enabled {
392-
// Get repository name from config override or context
393-
let repo_name = config
394-
.artifact_link
395-
.repository
396-
.as_ref()
397-
.or(ctx.repository_name.as_ref());
398-
399-
if let Some(repo_name) = repo_name {
400-
// If we already have the repo ID from environment, use it (avoids extra API call)
401-
let repo_id = if config.artifact_link.repository.is_none() {
402-
// Using default repo from environment - check if we have the ID already
403-
ctx.repository_id.clone()
404-
} else {
405-
None // Config overrides require lookup
406-
};
407-
408-
let repo_id = match repo_id {
409-
Some(id) => id,
410-
None => {
411-
// Resolve repo name to ID via API
412-
match resolve_repository_id(&client, org_url, project, token, repo_name)
413-
.await
414-
{
415-
Ok(id) => id,
416-
Err(e) => {
417-
return Ok(ExecutionResult::failure(format!(
418-
"Failed to resolve repository '{}': {}",
419-
repo_name, e
420-
)));
421-
}
422-
}
423-
}
424-
};
425-
426-
patch_doc.push(artifact_link_op(
427-
project,
428-
&repo_id,
429-
&config.artifact_link.branch,
430-
));
431-
Some(format!(
432-
"linked to {}:{}",
433-
repo_name, config.artifact_link.branch
434-
))
435-
} else {
436-
Some("skipped: no repository available".to_string())
445+
let artifact_link_included = match maybe_build_artifact_link_op(
446+
&client,
447+
&config.artifact_link,
448+
ctx,
449+
org_url,
450+
project,
451+
token,
452+
)
453+
.await
454+
{
455+
Err(failure) => return Ok(failure),
456+
Ok(None) => None,
457+
Ok(Some((op, message))) => {
458+
if !op.is_null() {
459+
patch_doc.push(op);
460+
}
461+
Some(message)
437462
}
438-
} else {
439-
None
440463
};
441464

442465
// Make the API call

0 commit comments

Comments
 (0)