Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DXCDT-383: Integrate preserveKeywords function into export process #751

Merged

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Feb 22, 2023

🔧 Changes

This PR takes the highest-order preserveKeywords function (#745) and introduces it into the export process. It does this by introducing a AUTH0_PRESERVE_KEYWORDS boolean configuration property that indicates wether to call that function, otherwise it will run as normal.

The majority of these changes are to alter the existing export process to disable keyword replacement. Previously, there was never a need to retrieve the raw, unreplaced resource configuration files, but with keyword preservation there needs to be a mechanism to disable replacement to identify where the raw keyword markers exist in the asset tree.

📚 References

Related PR: #745

🔬 Testing

Introducing unit tests for both the directory and YAML context handlers.

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@willvedd willvedd requested a review from a team as a code owner February 22, 2023 17:45
Comment on lines 124 to 142

const shouldPreserveKeywords =
//@ts-ignore because the string=>boolean conversion may not have happened if passed-in as env var
this.config.AUTH0_PRESERVE_KEYWORDS === 'true' ||
this.config.AUTH0_PRESERVE_KEYWORDS === true;
if (shouldPreserveKeywords) {
await this.loadAssetsFromLocal({ disableKeywordReplacement: true });
const localAssets = { ...this.assets };
//@ts-ignore
delete this['assets'];

this.assets = preserveKeywords(
localAssets,
auth0.assets,
this.config.AUTH0_KEYWORD_REPLACE_MAPPINGS || {}
);
} else {
this.assets = auth0.assets;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the integration points; into the yaml context handler.

Comment on lines 80 to 98
await this.loadAssetsFromLocal({ disableKeywordReplacement: true }); //Need to disable keyword replacement to retrieve the raw keyword markers (ex: ##KEYWORD##)

const shouldPreserveKeywords =
//@ts-ignore because the string=>boolean conversion may not have happened if passed-in as env var
this.config.AUTH0_PRESERVE_KEYWORDS === 'true' ||
this.config.AUTH0_PRESERVE_KEYWORDS === true;
if (shouldPreserveKeywords) {
const localAssets = { ...this.assets };
//@ts-ignore
delete this['assets'];

this.assets = preserveKeywords(
localAssets,
auth0.assets,
this.config.AUTH0_KEYWORD_REPLACE_MAPPINGS || {}
);
} else {
this.assets = auth0.assets;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the integration points; into the directory context handler.

@@ -205,6 +205,7 @@ export type Config = {
AUTH0_ALLOW_DELETE: boolean;
AUTH0_EXCLUDED?: AssetTypes[];
AUTH0_INCLUDED_ONLY?: AssetTypes[];
AUTH0_PRESERVE_KEYWORDS: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Introduction of the configuration to enable user opt-in behavior. Will be integrated as a flag in a later PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 🤔 What if we keep it as a config key only?

@willvedd willvedd force-pushed the DXCDT-383-integrate-preserve-keywords-function-into-export branch from a02e799 to df66ab0 Compare February 22, 2023 18:33
@willvedd willvedd enabled auto-merge (squash) February 22, 2023 18:35
@codecov-commenter
Copy link

Codecov Report

Base: 83.93% // Head: 84.30% // Increases project coverage by +0.36% 🎉

Coverage data is based on head (f1f6a40) compared to base (666e89d).
Patch coverage: 87.50% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #751      +/-   ##
==========================================
+ Coverage   83.93%   84.30%   +0.36%     
==========================================
  Files         115      115              
  Lines        3462     3486      +24     
  Branches      650      659       +9     
==========================================
+ Hits         2906     2939      +33     
+ Misses        324      311      -13     
- Partials      232      236       +4     
Impacted Files Coverage Δ
src/context/directory/handlers/customDomains.ts 50.00% <ø> (ø)
src/context/directory/handlers/emailProvider.ts 82.60% <ø> (ø)
src/context/directory/handlers/emailTemplates.ts 80.55% <ø> (ø)
...ectory/handlers/guardianPhoneFactorMessageTypes.ts 94.44% <ø> (ø)
...ry/handlers/guardianPhoneFactorSelectedProvider.ts 94.44% <ø> (ø)
src/context/directory/handlers/guardianPolicies.ts 94.44% <ø> (ø)
src/context/directory/handlers/logStreams.ts 35.00% <0.00%> (ø)
src/context/directory/handlers/pages.ts 82.92% <ø> (ø)
src/context/directory/handlers/resourceServers.ts 35.00% <0.00%> (ø)
src/context/directory/handlers/rulesConfigs.ts 46.15% <0.00%> (ø)
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@willvedd willvedd merged commit cabf4ea into master Feb 23, 2023
@willvedd willvedd deleted the DXCDT-383-integrate-preserve-keywords-function-into-export branch February 23, 2023 08:32
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.

3 participants