-
Notifications
You must be signed in to change notification settings - Fork 159
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-384: Keyword preservation in auxiliary files #758
DXCDT-384: Keyword preservation in auxiliary files #758
Conversation
…T-375-rename-load-to-loadAssetsFromAuth0
…ort' into fix-make-keyword-preservation-more-flexible
…to DXCDT-382-standardize-resource-identifiers
{ | ||
mappings: context.mappings, | ||
disableKeywordReplacement: context.disableKeywordReplacement, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the diffs in this PR are due to the changing function signature of loadFileAndReplaceKeywords
which adds an option to disable keyword replacement.
|
||
constructor(config: Config, mgmtClient) { | ||
this.configFile = config.AUTH0_INPUT_FILE; | ||
this.config = config; | ||
this.mappings = config.AUTH0_KEYWORD_REPLACE_MAPPINGS || {}; | ||
this.mgmtClient = mgmtClient; | ||
this.disableKeywordReplacement = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, keyword replacement is enabled (not disabled). This configuration value is only enabled during the keyword preservation stage.
} | ||
|
||
async loadAssetsFromLocal(opts = { disableKeywordReplacement: false }) { | ||
// Allow to send object/json directly | ||
this.disableKeywordReplacement = opts.disableKeywordReplacement; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only location where disableKeywordReplacement
is set to true.
//Dumping without keyword preservation so we can assert that remote values will overwrite local values | ||
await dump({ | ||
output_folder: workDirectory, | ||
format: 'directory', | ||
config: { | ||
...config, | ||
AUTH0_PRESERVE_KEYWORDS: false, | ||
}, | ||
}); | ||
|
||
const jsonWithoutPreservation = JSON.parse( | ||
fs.readFileSync(path.join(workDirectory, 'tenant.json')).toString() | ||
); | ||
|
||
expect(jsonWithoutPreservation.friendly_name).to.equal('This tenant name should be preserved'); | ||
expect(jsonWithoutPreservation.enabled_locales).to.deep.equal(['en', 'es']); | ||
expect(jsonWithoutPreservation.support_email).to.equal('support@travel0.com'); | ||
expect(jsonWithoutPreservation.support_url).to.equal('https://travel0.com/support'); | ||
|
||
const emailTemplateJsonWithoutPreservation = JSON.parse( | ||
fs.readFileSync(path.join(workDirectory, 'emails', 'welcome_email.json')).toString() | ||
); | ||
|
||
expect(emailTemplateJsonWithoutPreservation.resultUrl).to.equal('https://travel0.com/welcome'); | ||
|
||
expect( | ||
fs.readFileSync(path.join(workDirectory, 'emails', 'welcome_email.html')).toString() | ||
).to.contain('This tenant name should be preserved'); | ||
|
||
emptyDirSync(workDirectory); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new E2E assertions are to ensure that keyword replacement worked when keyword preservation disabled. Otherwise, false positives could occur during testing.
const emailTemplateHTML = fs | ||
.readFileSync(path.join(workDirectory, 'emailTemplates', 'welcome_email.html')) | ||
.toString(); | ||
expect(emailTemplateHTML).to.contain('##TENANT_NAME##'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This E2E assertion proves that this functionality works 🎉
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #758 +/- ##
=======================================
Coverage 84.07% 84.08%
=======================================
Files 115 115
Lines 3510 3512 +2
Branches 670 670
=======================================
+ Hits 2951 2953 +2
Misses 322 322
Partials 237 237
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. |
🔧 Changes
Up to now, the keyword preservation only operated on properties that were directly defined in the resource configuration files, but not auxiliary files such as email HTML templates and action JS files, which are defined in a separate file. This limitation existed because the
loadFileAndReplaceKeywords
function, a fundamental building block of this repo, was always designed to replace keywords by default. However, in the context of keyword preservation, it is necessary to retrieve the "raw" files. This PR enables keyword preservation for auxiliary files too with a logic change inloadFileAndReplaceKeywords
which bifurcates the functionality and disables keyword replacement depending on the operating context.📚 References
Related Keyword Preservation RFC: #688
🔬 Testing
Added several new E2E test assertions. Updated
loadFileAndReplaceKeywords
function unit tests.📝 Checklist