Skip to content

feat: add config-manager push iga-workflows command#103

Open
dallinjsevy wants to merge 1 commit into
feature/config-manager-pull-iga-workflowsfrom
feature/config-manager-push-iga-workflows
Open

feat: add config-manager push iga-workflows command#103
dallinjsevy wants to merge 1 commit into
feature/config-manager-pull-iga-workflowsfrom
feature/config-manager-push-iga-workflows

Conversation

@dallinjsevy

Copy link
Copy Markdown

No description provided.

Comment on lines +9 to +14
const { CLOUD_DEPLOYMENT_TYPE_KEY, FORGEOPS_DEPLOYMENT_TYPE_KEY } =
frodo.utils.constants;
const deploymentTypes = [
CLOUD_DEPLOYMENT_TYPE_KEY,
FORGEOPS_DEPLOYMENT_TYPE_KEY,
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workflows are a Cloud only deployment, remove forgeops from this


export default function setup() {
const program = new FrodoCommand(
'frodo config-manager pull iga-workflows',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pull should be push

)
)
.addOption(
new Option('--draft', 'Push as draft version instead of publishing.')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a shorthand to this, use -d (config-manger uses -r, but I think that was a typo, it should be -d)

command
);

if (await getTokens(false, true, deploymentTypes)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before getTokens, I would add an if-statement checking if the tenant is an IGA tenant, and fail early if not, as I did for the Frodo workflow commands:

if (!state.getIsIGA()) {
printMessage(
'Command not supported for non-IGA cloud tenants',
'error'
);
process.exitCode = 1;
return;
}

Comment on lines +43 to +50
if (await getTokens(false, true, deploymentTypes)) {
verboseMessage('Importing config entity iga-workflows');
const outcome = await configManagerImportIgaWorkflows(
options.name,
options.draft
);
if (!outcome) process.exitCode = 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this code to look something like this:

const getTokensIsSuccessful = await getTokens(
false,
true,
deploymentTypes
);
if (!getTokensIsSuccessful) process.exit(1);
verboseMessage('Importing email provider configuration.');
const outcome = await configManagerImportEmailProvider();
if (!outcome) process.exitCode = 1;

This is almost correct, although we need to make sure we are setting the exit code to 1 in the event that getTokens fails, which this example does.

);

if (await getTokens(false, true, deploymentTypes)) {
verboseMessage('Importing config entity iga-workflows');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would reword this message to be "Importing IGA workflows...", since config entity is typically associated with IDM entities.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For test configurations, I would update these workflows to have one of each possible node in them so we can test to verify that importing works for all nodes. Currently your test workflows only have the wait node.

Comment on lines +72 to +73
const { stdout } = await exec(CMD, igaEnv);
expect(removeAnsiEscapeCodes(stdout)).toMatchSnapshot();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change these tests to use the await testSuccess(CMD, igaEnv); helper function I created to test success, as it helps simplify the tests some more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants