Fix custom_properties diffing when config uses property_name#978
Merged
decyjphr merged 5 commits intoMay 12, 2026
Conversation
Copilot
AI
changed the title
[WIP] Fix TypeError in CustomProperties normalize method
Fix CustomProperties normalize() regression with paginated response shape differences
May 12, 2026
Copilot
AI
changed the title
Fix CustomProperties normalize() regression with paginated response shape differences
Fix custom_properties diffing when config uses May 12, 2026
property_name
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a diffing bug in the custom_properties plugin where config entries authored with property_name (instead of name) were not normalized consistently, causing intended property updates to be missed.
Changes:
- Updated config entry normalization to accept both
nameandproperty_nameand lowercase the resolved key prior to diffing. - Hardened normalization of fetched custom properties to handle both
property_nameandnameshapes and skip invalid entries. - Added Jest regression tests covering
property_namein config and mixed response shapes during pagination normalization.
Show a summary per file
| File | Description |
|---|---|
| lib/plugins/custom_properties.js | Extends normalization logic to accept both name and property_name, adds basic guards, and ensures lowercased keys are used for comparisons. |
| test/unit/lib/plugins/custom_properties.test.js | Adds regression coverage for property_name-based config input and mixed key shapes in paginated results. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
lib/plugins/custom_properties.js:68
- Same issue as in
normalizeEntries:property.property_name || property.namewill ignore a validname/property_nameif the first field is present but non-string and truthy. Prefer checkingtypeoffor each candidate and using the first string value to avoid silently dropping valid records.
const propertyName = property.property_name || property.name
if (typeof propertyName !== 'string') {
return normalizedProperties
}
normalizedProperties.push({
name: propertyName.toLowerCase(),
value: property.value
- Files reviewed: 2/2 changed files
- Comments generated: 2
Comment on lines
+20
to
+28
| const entryName = entry.name || entry.property_name | ||
|
|
||
| if (typeof entryName !== 'string') { | ||
| return normalizedEntries | ||
| } | ||
|
|
||
| normalizedEntries.push({ | ||
| name: entryName.toLowerCase(), | ||
| value: entry.value |
Comment on lines
+66
to
+71
| it('should normalize paginated custom properties when property name shape differs', async () => { | ||
| const mockResponse = [ | ||
| { name: 'Owner', value: 'My Team' }, | ||
| { property_name: 'Criticality', value: 'High' }, | ||
| { value: 'ignored' } | ||
| ] |
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.
Bug Fix
What was the bug?
custom_propertiesentries in repo/suborg YAML were ignored when authored withproperty_nameinstead ofname. As a result, safe-settings did not detect/add expected property values (for example,ent-ownership: expert-services).How did you fix it?
Normalize config entries across both key shapes
CustomProperties.normalizeEntries()to accept eithernameorproperty_namefrom config entries.Harden entry handling
Add regression coverage
property_name.Testing
test/unit/lib/plugins/custom_properties.test.jsforproperty_name-based config input.This now normalizes to the same internal shape as
name, so the diff logic detects and applies the property update.