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

Fix property naming and settings catalog handling #5382

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

FabienTschanz
Copy link
Contributor

Pull Request (PR) description

This PR fixes three issues with the settings catalog: One was a not handled case of a property name simplification, the second one in the same resource was an incorrectly generated property name, and the last issue in the second resource was a upper vs lower-case property name.

This Pull Request (PR) fixes the following issues

@ricmestre
Copy link
Contributor

I'm still trying to figure out what's going on with IntuneDeviceConfigurationCustomPolicyWindows10 and I'll test your diff once I'm finished with that one, but at least for IntuneAccountProtectionLocalAdministratorPasswordSolutionPolicy are you sure that PasswordAgeDays_AAD is the only property that needs the casing changed?

Additionally I remember that I already had to change the casing on that specific property a couple of times in my tests to cope with these issues, isn't there a better way to handle this?

@FabienTschanz
Copy link
Contributor Author

FabienTschanz commented Nov 11, 2024

The issue is that we use hashtables for the property lookups. Hashtables are case-sensitive regarding their properties, but I can try and check if the casing can be ignored. Unfortunately that will generate a bit of overhead.

Edit: Yes, I'm sure of it. That's because this is one of the two properties with the name PasswordAgeDays, and this one only differs from the other one in its definition id.

@ricmestre
Copy link
Contributor

Both seem to be working with this diff 👍

I'll run all my unit/integration tests again just in case.

@NikCharlebois
Copy link
Collaborator

@ricmestre were your tests successful?

@ricmestre
Copy link
Contributor

Unfortunately I found a couple of issues, of my own not DSC, so I had to fix them in the meantime.

I'm currently running the tests again right now, I'll let you know how it went in a couple of hours.

@NikCharlebois NikCharlebois merged commit 9f2e0a6 into microsoft:Dev Nov 12, 2024
2 checks passed
@FabienTschanz FabienTschanz deleted the fix/intune-local-admin branch November 12, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants