From 0e0aa15910cf0bcccb87b7267b3349c4c2306edd Mon Sep 17 00:00:00 2001
From: J Long <john.long@sap.com>
Date: Tue, 17 Sep 2024 15:37:07 +0100
Subject: [PATCH] fix(abap-deploy-config-inquirer ): further cleanup based on
 testing

---
 .../src/prompts/conditions.ts                 | 17 ++++++++--------
 .../src/prompts/questions/abap-target.ts      |  2 +-
 .../test/prompts/conditions.test.ts           | 20 +++++++++++++++++--
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/packages/abap-deploy-config-inquirer/src/prompts/conditions.ts b/packages/abap-deploy-config-inquirer/src/prompts/conditions.ts
index 1abfb01e6d..e1688d8ad6 100644
--- a/packages/abap-deploy-config-inquirer/src/prompts/conditions.ts
+++ b/packages/abap-deploy-config-inquirer/src/prompts/conditions.ts
@@ -52,28 +52,29 @@ export function showScpQuestion(previousAnswers: AbapDeployConfigAnswersInternal
 }
 
 /**
- * Client condition to determine if the client question should be shown.
+ * Client condition to determine if the client question should be shown. Validates the SCP confirmation and project configuration properties.
  *
  * @param scp - is SCP system
- * @param isS4HanaCloudSystem - is S/4 HANA Cloud system
  * @returns boolean
  */
-function showClientCondition(scp?: boolean, isS4HanaCloudSystem?: boolean): boolean {
-    return Boolean(!isAppStudio() && !scp && !isS4HanaCloudSystem);
+function showClientCondition(scp?: boolean): boolean {
+    return Boolean(
+        !isAppStudio() && !PromptState.abapDeployConfig?.isS4HC && !scp && !PromptState.abapDeployConfig?.scp
+    );
 }
 
 /**
  * Determines if the client choice question should be shown.
  *
- * @param scp - SCP
+ * @param previousAnswers - previous answers
  * @param client - client
  * @returns boolean
  */
-export function showClientChoiceQuestion(scp?: boolean, client?: string): boolean {
+export function showClientChoiceQuestion(previousAnswers?: AbapDeployConfigAnswersInternal, client?: string): boolean {
     if (PromptState.isYUI || !client) {
         return false;
     }
-    return showClientCondition(scp, PromptState.abapDeployConfig?.isS4HC);
+    return showClientCondition(previousAnswers?.scp) && previousAnswers?.targetSystem === TargetSystemType.Url;
 }
 
 /**
@@ -83,7 +84,7 @@ export function showClientChoiceQuestion(scp?: boolean, client?: string): boolea
  * @returns boolean
  */
 export function showClientQuestion(previousAnswers?: AbapDeployConfigAnswersInternal): boolean {
-    const clientCondition = showClientCondition(previousAnswers?.scp, PromptState.abapDeployConfig?.isS4HC);
+    const clientCondition = showClientCondition(previousAnswers?.scp);
     const isTargetUrl = previousAnswers?.targetSystem === TargetSystemType.Url;
     const showCli = !PromptState.isYUI
         ? previousAnswers?.clientChoice === ClientChoiceValue.New || isTargetUrl
diff --git a/packages/abap-deploy-config-inquirer/src/prompts/questions/abap-target.ts b/packages/abap-deploy-config-inquirer/src/prompts/questions/abap-target.ts
index d03c0f84eb..ba7b508c5e 100644
--- a/packages/abap-deploy-config-inquirer/src/prompts/questions/abap-target.ts
+++ b/packages/abap-deploy-config-inquirer/src/prompts/questions/abap-target.ts
@@ -194,7 +194,7 @@ function getClientChoicePrompt(
     const prompts: (ListQuestion<AbapDeployConfigAnswersInternal> | Question)[] = [
         {
             when: (previousAnswers: AbapDeployConfigAnswersInternal): boolean =>
-                showClientChoiceQuestion(previousAnswers?.scp, backendTarget?.abapTarget?.client),
+                showClientChoiceQuestion(previousAnswers, backendTarget?.abapTarget?.client),
             type: 'list',
             name: abapDeployConfigInternalPromptNames.clientChoice,
             message: t('prompts.target.clientChoice.message'),
diff --git a/packages/abap-deploy-config-inquirer/test/prompts/conditions.test.ts b/packages/abap-deploy-config-inquirer/test/prompts/conditions.test.ts
index 5895c43477..1b43654157 100644
--- a/packages/abap-deploy-config-inquirer/test/prompts/conditions.test.ts
+++ b/packages/abap-deploy-config-inquirer/test/prompts/conditions.test.ts
@@ -78,7 +78,21 @@ describe('Test abap deploy config inquirer conditions', () => {
         mockIsAppStudio.mockReturnValueOnce(false);
         PromptState.isYUI = false;
         PromptState.abapDeployConfig.isS4HC = false;
-        expect(showClientChoiceQuestion(false, '100')).toBe(true);
+        expect(
+            showClientChoiceQuestion({ scp: false, targetSystem: TargetSystemType.Url, url: '', package: '' }, '100')
+        ).toBe(true);
+        PromptState.resetAbapDeployConfig();
+        // Should not show client choice question if SCP is enabled
+        PromptState.abapDeployConfig.isS4HC = false;
+        PromptState.abapDeployConfig.scp = true;
+        expect(
+            showClientChoiceQuestion({ scp: false, targetSystem: TargetSystemType.Url, url: '', package: '' }, '100')
+        ).toBe(false);
+        PromptState.resetAbapDeployConfig();
+        // Should not show client choice question if target system is not a URL
+        PromptState.abapDeployConfig.isS4HC = false;
+        PromptState.abapDeployConfig.scp = true;
+        expect(showClientChoiceQuestion({ scp: true, url: '', package: '' }, '100')).toBe(false);
         PromptState.resetAbapDeployConfig();
     });
 
@@ -86,7 +100,9 @@ describe('Test abap deploy config inquirer conditions', () => {
         mockIsAppStudio.mockReturnValueOnce(false);
         PromptState.isYUI = false;
         PromptState.abapDeployConfig.isS4HC = true;
-        expect(showClientChoiceQuestion(true, undefined)).toBe(false);
+        expect(
+            showClientChoiceQuestion({ scp: true, targetSystem: TargetSystemType.Url, url: '', package: '' }, undefined)
+        ).toBe(false);
         PromptState.resetAbapDeployConfig();
     });