2128/confirm prompt validator not invoked as expected

.changeset/thick-owls-collect.md

@@ -0,0 +1,5 @@
+---
+'@sap-ux/abap-deploy-config-inquirer': patch
+---
+
+Handle validator method not being invoked for confirm field

packages/abap-deploy-config-inquirer/src/prompts/conditions.ts

@@ -1,6 +1,5 @@
 import { isAppStudio } from '@sap-ux/btp-utils';
 import { PromptState } from './prompt-state';
-import { clientDoesNotExistOrInvalid } from '../validator-utils';
 import { findBackendSystemByUrl, initTransportConfig } from '../utils';
 import { handleTransportConfigError } from '../error-handler';
 import { t } from '../i18n';
@@ -53,52 +52,45 @@ 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 isS4HanaCloudSystem - is S/4 HANA Cloud system
+ * @param scp - is SCP system
  * @returns boolean
  */
-function showClientCondition(isS4HanaCloudSystem?: boolean): boolean {
+function showClientCondition(scp?: boolean): boolean {
  return Boolean(
- !isAppStudio() &&
- clientDoesNotExistOrInvalid(PromptState.abapDeployConfig.client) &&
- !PromptState.abapDeployConfig.scp &&
- !isS4HanaCloudSystem
+ !isAppStudio() && !PromptState.abapDeployConfig?.isS4HC && !scp && !PromptState.abapDeployConfig?.scp
  );
 }
 
 /**
  * Determines if the client choice question should be shown.
  *
+ * @param previousAnswers - previous answers
  * @param client - client
- * @param isS4HanaCloudSystem - is S/4 HANA Cloud system
  * @returns boolean
  */
-export function showClientChoiceQuestion(client?: string, isS4HanaCloudSystem?: boolean): boolean {
+export function showClientChoiceQuestion(previousAnswers?: AbapDeployConfigAnswersInternal, client?: string): boolean {
  if (PromptState.isYUI || !client) {
  return false;
  }
-
- return showClientCondition(isS4HanaCloudSystem);
+ return showClientCondition(previousAnswers?.scp) && previousAnswers?.targetSystem === TargetSystemType.Url;
 }
 
 /**
- * Determines if the client question should be shown.
- * Note: In some instances, when a yaml conf is parsed, double quoted properties i.e. client: "100" are saved as a number instead of a string.
+ * Determines if the client question should be shown under very specific conditions.
  *
- * @param clientChoice - client choice from previous answers
- * @param client - client
- * @param isS4HanaCloudSystem - is S/4 HANA Cloud system
+ * @param previousAnswers - previous answers
  * @returns boolean
  */
-export function showClientQuestion(clientChoice?: string, client?: string, isS4HanaCloudSystem?: boolean): boolean {
- const clientCondition = showClientCondition(isS4HanaCloudSystem);
-
- if (clientCondition && client) {
- PromptState.abapDeployConfig.client = String(client);
- }
- const showOnCli = clientChoice === ClientChoiceValue.New || !client;
- return !PromptState.isYUI ? showOnCli && clientCondition : clientCondition;
+export function showClientQuestion(previousAnswers?: AbapDeployConfigAnswersInternal): boolean {
+ const clientCondition = showClientCondition(previousAnswers?.scp);
+ const isTargetUrl = previousAnswers?.targetSystem === TargetSystemType.Url;
+ const showCli = !PromptState.isYUI
+ ? previousAnswers?.clientChoice === ClientChoiceValue.New || isTargetUrl
+  : isTargetUrl;
+ const showYui = PromptState.isYUI ? isTargetUrl : false;
+ return (showYui && clientCondition) || (showCli && clientCondition);
 }
 
 /**

packages/abap-deploy-config-inquirer/src/prompts/questions/abap-target.ts

@@ -3,7 +3,6 @@ import {
  validateClientChoiceQuestion,
  validateClient,
  validateDestinationQuestion,
- validateScpQuestion,
  validateTargetSystem,
  validateTargetSystemUrlCli,
  validateUrl,
@@ -26,6 +25,7 @@ import {
 } from '../../types';
 import type { InputQuestion, ListQuestion, ConfirmQuestion, YUIQuestion } from '@sap-ux/inquirer-common';
 import type { Question } from 'inquirer';
+import { TargetSystemType } from '../../types';
 
 /**
  * Returns the destination prompt.
@@ -158,18 +158,31 @@ function getUrlPrompt(
  * @param backendTarget - backend target
  * @returns confirm question for scp
  */
-function getScpPrompt(backendTarget?: BackendTarget): Question<AbapDeployConfigAnswersInternal> {
- return {
- when: (previousAnswers: AbapDeployConfigAnswersInternal): boolean => showScpQuestion(previousAnswers),
- type: 'confirm',
- name: promptNames.scp,
- message: t('prompts.target.scp.message'),
- guiOptions: {
- breadcrumb: t('prompts.target.scp.breadcrumb')
+function getScpPrompt(backendTarget?: BackendTarget): Question<AbapDeployConfigAnswersInternal>[] {
+ const prompts: (ConfirmQuestion<AbapDeployConfigAnswersInternal> | Question)[] = [
+ {
+ when: (previousAnswers: AbapDeployConfigAnswersInternal): boolean => showScpQuestion(previousAnswers),
+ type: 'confirm',
+ name: promptNames.scp,
+ message: t('prompts.target.scp.message'),
+ guiOptions: {
+ breadcrumb: t('prompts.target.scp.breadcrumb')
+ },
+ default: (): boolean | undefined => backendTarget?.abapTarget?.scp
+ }
+ ];
+ // Setter prompt to ensure the state for both CLI and YUI is updated
+ prompts.push({
+ when: (answers: AbapDeployConfigAnswersInternal): boolean => {
+ const scpChoice = answers[promptNames.scp];
+ const targetChoice = answers[promptNames.targetSystem];
+ // scpChoice by default is true so only update state if target system is a URL
+ PromptState.abapDeployConfig.scp = !!(targetChoice === TargetSystemType.Url && scpChoice);
+ return false;
  },
- default: (): boolean | undefined => backendTarget?.abapTarget?.scp,
-  validate: (scp: boolean): boolean | string => validateScpQuestion(scp)
- } as ConfirmQuestion<AbapDeployConfigAnswersInternal>;
+ name: promptNames.scpSetter
+ } as Question);
+ return prompts;
 }
 
 /**
@@ -183,8 +196,8 @@ function getClientChoicePrompt(
 ): (YUIQuestion<AbapDeployConfigAnswersInternal> | Question)[] {
  const prompts: (ListQuestion<AbapDeployConfigAnswersInternal> | Question)[] = [
  {
- when: (): boolean =>
- showClientChoiceQuestion(backendTarget?.abapTarget?.client, PromptState.abapDeployConfig?.isS4HC),
+ when: (previousAnswers: AbapDeployConfigAnswersInternal): boolean =>
+ showClientChoiceQuestion(previousAnswers, backendTarget?.abapTarget?.client),
  type: 'list',
  name: promptNames.clientChoice,
  message: t('prompts.target.clientChoice.message'),
@@ -217,25 +230,20 @@ function getClientChoicePrompt(
 /**
  * Returns the client prompt.
  *
- * @param backendTarget - backend target
  * @returns input question for client
  */
-function getClientPrompt(backendTarget?: BackendTarget): Question<AbapDeployConfigAnswersInternal> {
+function getClientPrompt(): Question<AbapDeployConfigAnswersInternal> {
  return {
  when: (previousAnswers: AbapDeployConfigAnswersInternal): boolean => {
- return showClientQuestion(
- previousAnswers.clientChoice,
- backendTarget?.abapTarget?.client,
- PromptState.abapDeployConfig?.isS4HC
- );
+ return showClientQuestion(previousAnswers);
  },
  type: 'input',
  name: promptNames.client,
  message: t('prompts.target.client.message'),
  guiOptions: {
  breadcrumb: t('prompts.target.client.breadcrumb')
  },
- default: (): string | undefined => backendTarget?.abapTarget?.client,
+ default: (): string | undefined => PromptState.abapDeployConfig?.client, // Already set from previous step, if passed in from yaml config for example
  filter: (input: string): string => input?.trim(),
  validate: (client: string): boolean | string => validateClient(client)
  } as InputQuestion<AbapDeployConfigAnswersInternal>;
@@ -256,8 +264,8 @@ export async function getAbapTargetPrompts(
  ...getDestinationPrompt(abapSystemChoices, destinations, options.backendTarget),
  ...getTargetSystemPrompt(abapSystemChoices),
  getUrlPrompt(destinations, options.backendTarget),
- getScpPrompt(options.backendTarget),
+ ...getScpPrompt(options.backendTarget),
  ...getClientChoicePrompt(options.backendTarget),
- getClientPrompt(options.backendTarget)
+ getClientPrompt()
  ];
 }

packages/abap-deploy-config-inquirer/src/prompts/validators.ts

@@ -158,17 +158,6 @@ export function validateTargetSystemUrlCli(targetSystem?: string, choices?: Abap
  }
 }
 
-/**
- * Validates the SCP question and sets the SCP in the prompt state.
- *
- * @param input - if confirm was selected
- * @returns boolean
- */
-export function validateScpQuestion(input: boolean): boolean {
- PromptState.abapDeployConfig.scp = input;
- return true;
-}
-
 /**
  * Validates and updates the client property in the state.
  *

packages/abap-deploy-config-inquirer/src/types.ts

@@ -59,6 +59,7 @@ export enum promptNames {
  targetSystemCliSetter = 'targetSystemCliSetter',
  url = 'url',
  scp = 'scp',
+ scpSetter = 'scpSetter',
  clientChoice = 'clientChoice',
  clientChoiceCliSetter = 'clientChoiceCliSetter',
  client = 'client',

packages/abap-deploy-config-inquirer/src/validator-utils.ts

@@ -43,16 +43,6 @@ export function isValidClient(client: string): boolean {
  return !!regex.exec(client);
 }
 
-/**
- * Returns true if the input does not exist, or if it exists but it is invalid.
- *
- * @param client - input string
- * @returns boolean
- */
-export function clientDoesNotExistOrInvalid(client?: string): boolean {
- return Boolean(!client || (client && !isValidClient(client)));
-}
-
 /**
  * Returns the list of packages for the given input.
  *

packages/abap-deploy-config-inquirer/test/index.test.ts

@@ -16,7 +16,7 @@ describe('index', () => {
  });
  const prompts = await getPrompts({}, undefined, false);
  expect(prompts.answers).toBeDefined();
- expect(prompts.prompts.length).toBe(23);
+ expect(prompts.prompts.length).toBe(24);
  });
 
  it('should prompt with inquirer adapter', async () => {

packages/abap-deploy-config-inquirer/test/prompts/conditions.test.ts

@@ -1,5 +1,11 @@
 import { isAppStudio } from '@sap-ux/btp-utils';
-import { ClientChoiceValue, PackageInputChoices, TransportChoices, TransportConfig } from '../../src/types';
+import {
+ ClientChoiceValue,
+ PackageInputChoices,
+ TargetSystemType,
+ TransportChoices,
+ TransportConfig
+} from '../../src/types';
 import {
  defaultOrShowManualPackageQuestion,
  defaultOrShowManualTransportQuestion,
@@ -70,25 +76,93 @@ describe('Test abap deploy config inquirer conditions', () => {
  test('should show client choice question', () => {
  mockIsAppStudio.mockReturnValueOnce(false);
  PromptState.isYUI = false;
- expect(showClientChoiceQuestion('100', false)).toBe(true);
+ PromptState.abapDeployConfig.isS4HC = false;
+ 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();
  });
 
  test('should not show client choice question', () => {
  mockIsAppStudio.mockReturnValueOnce(false);
  PromptState.isYUI = false;
- expect(showClientChoiceQuestion(undefined, true)).toBe(false);
+ PromptState.abapDeployConfig.isS4HC = true;
+ expect(
+ showClientChoiceQuestion({ scp: true, targetSystem: TargetSystemType.Url, url: '', package: '' }, undefined)
+ ).toBe(false);
+ PromptState.resetAbapDeployConfig();
  });
 
- test('should show client question', () => {
- PromptState.isYUI = true;
- mockIsAppStudio.mockReturnValueOnce(false);
- expect(showClientQuestion(undefined, undefined, false)).toBe(true);
- });
+ it.each([
+ { isYui: true, scpEnabled: false, scpDisabled: true, clientChoice: undefined },
+ {
+ isYui: false,
+ scpEnabled: false,
+ scpDisabled: true,
+ clientChoice: ClientChoiceValue.New
+ },
+ { isYui: false, scpEnabled: false, scpDisabled: true, clientChoice: undefined }
+ ])(
+ 'Validate showClientQuestion for different environments isYui: $isYui, clientChoice: $clientChoice',
+ ({ isYui, scpEnabled, scpDisabled, clientChoice }) => {
+ PromptState.resetAbapDeployConfig();
+ PromptState.isYUI = isYui;
+ mockIsAppStudio.mockReturnValueOnce(false);
+ // Validate client question if SCP is enabled
+ PromptState.abapDeployConfig.isS4HC = false;
+ expect(showClientQuestion({ scp: true, targetSystem: TargetSystemType.Url, url: '', package: '' })).toBe(
+ scpEnabled
+ );
+ PromptState.resetAbapDeployConfig();
+ // Validate client question if SCP is disabled
+ PromptState.abapDeployConfig.client = '100';
+ PromptState.abapDeployConfig.isS4HC = false;
+ expect(
+ showClientQuestion({
+ scp: false,
+ clientChoice,
+ targetSystem: TargetSystemType.Url,
+ url: '',
+ package: ''
+ })
+ ).toBe(scpDisabled);
+ // Should always be shown if target system is not SCP and is URL for both CLI and YUI
+ expect(
+ showClientQuestion({
+ scp: false,
+ clientChoice: ClientChoiceValue.Blank,
+ targetSystem: TargetSystemType.Url,
+ url: '',
+ package: ''
+ })
+ ).toBe(true);
+ PromptState.resetAbapDeployConfig();
+ }
+ );
 
  test('should show client question (CLI)', () => {
  PromptState.isYUI = false;
  mockIsAppStudio.mockReturnValue(false);
- expect(showClientQuestion(ClientChoiceValue.New, undefined, false)).toBe(true);
+ expect(
+ showClientQuestion({
+ clientChoice: ClientChoiceValue.New,
+ targetSystem: TargetSystemType.Url,
+ url: '',
+ package: ''
+ })
+ ).toBe(true);
  });
 
  test('should show username question', async () => {