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

M365DSCUtil: Fix #4333 #4334

Merged
merged 15 commits into from
Feb 26, 2024
Merged

M365DSCUtil: Fix #4333 #4334

merged 15 commits into from
Feb 26, 2024

Conversation

ricmestre
Copy link
Contributor

@ricmestre ricmestre commented Feb 15, 2024

Pull Request (PR) description

If a resource has Name as key then it should be exported to the blueprint with it in its ResourceInstanceName title instead of appearing with Id, this is already done when the key is DisplayName but for Name is not working due to having the condition after Id.

EDIT: Just checked and there are several resources which have Name as Key or Required, hopefully this won't break any setup since Keys are supposed to be unique so having the ResourceInstanceName with the Id or Name should not conflict with other resources of the same type regardless of the key being used.

This Pull Request (PR) fixes the following issues

@ricmestre ricmestre changed the title M365DSCUtil M365DSCUtil: Fix #4333 Feb 15, 2024
Copy link
Collaborator

@andikrueger andikrueger left a comment

Choose a reason for hiding this comment

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

Please have a look at my comment. I haven't run a test but am guessing there might be challenges with other resources.

Modules/Microsoft365DSC/Modules/M365DSCUtil.psm1 Outdated Show resolved Hide resolved
@ricmestre ricmestre marked this pull request as draft February 20, 2024 11:15
@ricmestre ricmestre marked this pull request as ready for review February 21, 2024 14:18
@ricmestre
Copy link
Contributor Author

@andikrueger I had a little bit of time to look into it again, could you please check?

I'm calling Get-DscResource on the whole module to cache the information, so it will take longer to export the first resource but then all others will use the cached info otherwise it'd take longer to call Get-DscResource everytime per each resource.

Then I added as heuristics the usual mandatory keys plus Title since I think can be unique amongst other mandatory keys, all others if there are many will be joined together or if it's a single key then it will use that one, all remaining code afterwards was kept.

Copy link
Collaborator

@andikrueger andikrueger left a comment

Choose a reason for hiding this comment

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

Thanks for looking into it and the change. Just some additional ideas and suggestions.

Copy link
Collaborator

@andikrueger andikrueger left a comment

Choose a reason for hiding this comment

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

Please have a look at L3402

Modules/Microsoft365DSC/Modules/M365DSCUtil.psm1 Outdated Show resolved Hide resolved
Copy link
Collaborator

@andikrueger andikrueger left a comment

Choose a reason for hiding this comment

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

LGTM

@ricmestre
Copy link
Contributor Author

@andikrueger Forgot about resources that are IsSingleInstance should end up with ResourceInstanceName = ResourceName so I updated the condition to only iterate through the Keys for resources that aren't IsSingleInstance.

@andikrueger
Copy link
Collaborator

Thanks for having a second look at it.

@ykuijs ykuijs merged commit b3438f9 into microsoft:Dev Feb 26, 2024
2 checks passed
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.

IntuneSettingCatalogCustomPolicyWindows10: Export gets ResourceInstanceName with the Id instead of Name
3 participants