Preserve overwriteSettings when template project settings are merged after user repo settings#2233
Open
jeffreybulanadi wants to merge 1 commit intomicrosoft:mainfrom
Conversation
When a user repo settings file (AL-Go-Settings.json) uses overwriteSettings
to clear or replace an array property such as appDependencyProbingPaths, a
subsequent template-managed source file (AL-Go-TemplateProjectSettings.doNotEdit.json)
was able to re-merge its own values into that property because the
MergeCustomObjectIntoOrderedDictionary function had no awareness of
which settings had already been explicitly overwritten by a user source.
Changes:
- MergeCustomObjectIntoOrderedDictionary gains two optional parameters:
overwrittenSettings ([string[]]) - names of settings already overwritten
by a user-controlled source, and isTemplateSource ([bool]) - true when
the source file is a template-managed *.doNotEdit.json file.
When isTemplateSource is true and a property name appears in
overwrittenSettings, the merge of that property is skipped entirely.
- ReadSettings now tracks overwrittenSettings as it iterates the ordered
settings sources. After each non-template source is merged, any property
names declared in its overwriteSettings list are accumulated. Template
sources that follow receive the accumulated list so they cannot
re-introduce overwritten values.
- Two Pester tests added to ReadSettings.Test.ps1 covering:
1. overwriteSettings in a user source prevents template project settings
from re-merging that property.
2. overwriteSettings in a user source still allows later user project
settings to contribute additional values to that property.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the AL-Go settings merge logic so that overwriteSettings declared in user-controlled sources continues to take effect even when template-managed *.doNotEdit.json sources are merged later, preventing templates from re-introducing values that a user explicitly cleared/replaced.
Changes:
- Extend
MergeCustomObjectIntoOrderedDictionaryandReadSettingsto track settings explicitly overwritten by user sources and skip merging those settings from template sources. - Add Pester integration tests covering the overwrite-vs-template merge behavior for
appDependencyProbingPaths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Actions/.Modules/ReadSettings.psm1 | Tracks overwritten setting names across sources and prevents later template sources from re-merging them. |
| Tests/ReadSettings.Test.ps1 | Adds integration tests ensuring user overwrites win over template settings while still allowing later user sources to merge. |
Comment on lines
+509
to
+511
| if (-not $isTemplateSource -and $settingsJson.PSObject.Properties.Name -contains "overwriteSettings") { | ||
| $settingsJson.overwriteSettings | ForEach-Object { | ||
| if ($_ -notin $overwrittenSettings) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a repository uses
overwriteSettingsin.github/AL-Go-Settings.jsonto clear or replace an array property (such asappDependencyProbingPaths), a subsequent template-managed settings source (AL-Go-TemplateProjectSettings.doNotEdit.json) re-merges its own values for that property. This causes the user intent to be silently ignored.The root cause is in
MergeCustomObjectIntoOrderedDictionary: the function is called independently for each source. TheoverwriteSettingsmechanism removes a property from the destination only at the moment of the user source merge. Any template source that appears later in the ordered list has no knowledge that the property was already overwritten, so it re-applies its values unconditionally.Closes: #2228
Solution
Track which settings names have been explicitly overwritten by any user-controlled source as
ReadSettingsiterates the ordered sources list. Pass that accumulated list intoMergeCustomObjectIntoOrderedDictionaryalongside a flag that identifies whether the current source is a template-managed file. When both conditions are true, skip the merge for that property.Changes to
Actions/.Modules/ReadSettings.psm1MergeCustomObjectIntoOrderedDictionaryreceives two new optional parameters:[string[]] overwrittenSettings: names of settings already declared as overwritten by a prior user-controlled source.[bool] isTemplateSource: true when the source file is a template-managed*.doNotEdit.jsonfile.When
isTemplateSourceis true and the property name appears inoverwrittenSettings, the merge for that property is skipped in both the "add new properties" loop and the "update existing properties" loop.The
ReadSettingsforeach loop now:isTemplateSourcefrom the source file name.-overwrittenSettingsand-isTemplateSourceto bothMergeCustomObjectIntoOrderedDictionarycalls (main merge and conditional settings merge).overwriteSettingslist so subsequent template sources cannot re-introduce those values.Changes to
Tests/ReadSettings.Test.ps1Two integration tests added:
appDependencyProbingPathsremains empty after a user overwrite even when the template project settings file contains values for that property.Testing
.github/AL-Go-TemplateProjectSettings.doNotEdit.json, add an entry toappDependencyProbingPaths..github/AL-Go-Settings.json, setoverwriteSettings: ["appDependencyProbingPaths"]andappDependencyProbingPaths: [].appDependencyProbingPathsin the resolved settings contains the template entry. After this change, it is empty as the user specified.Unit tests can be run locally with Pester v5: