ADGroup: Updating means to retrieve group members

[jeremyciak]

Pull Request (PR) description

This Pull Request is intended to offer a viable mechanism for the Get-TargetResource aspect of the ADGroup resource when dealing with group members through a one-way trust.

This Pull Request (PR) fixes the following issues

• Fixes #616

Task list

• [x] Added an entry to the change log under the Unreleased section of the file CHANGELOG.md. Entry should say what was changed and how that affects users (if applicable), and reference the issue being resolved (if applicable).
• [ ] Resource documentation added/updated in README.md.
• [ ] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
• [ ] Comment-based help added/updated.
• [x] Localization strings added/updated in all localization files as appropriate.
• [ ] Examples appropriately added/updated.
• [x] Unit tests added/updated. See DSC Community Testing Guidelines.
• [ ] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
• [x] New/changed code adheres to DSC Community Style Guidelines.

---

This change is 

[jeremyciak]

616

[codecov[bot]]

Codecov Report

Merging #617 into master will increase coverage by 0%. The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #617   +/-   ##
=====================================
  Coverage      98%    98%           
=====================================
  Files          24     24           
  Lines        3110   3154   +44     
=====================================
+ Hits         3049   3093   +44     
  Misses         61     61           

[X-Guardian]

@jeremyciak, please continue the discussion on the issue thread, before you make any further additions to this PR.

[jeremyciak]

@X-Guardian, I apologize! I had a reply ready to comment on the issue thread and never actually hit the button to send it through. I have added that comment now.

[X-Guardian]

@jeremyciak, can you edit the PR description and add some detail and link to the issue.

[jeremyciak]

@X-Guardian I have added a description and link to the issue in the initial PR comment.

[X-Guardian]

Hi @jeremyciak, don't know if you have used Reviewable before, but if you haven't, press the Reviewable button in the PR description to see all the review comments in the context of the code. You can then respond to each one individually there.

[X-Guardian]

---

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 237 at r6 (raw file):

Previously, jeremyciak wrote…

I haven't yet made any code changes to represent this request. I will do so once I get feedback on my comment. One question I have is at least if I am moving to `New-InvalidOperationException` away from `Write-Error` then should my syntax update be to change this: ```powershell Write-Error -Exception $_.Exception ``` ... to this: ```powershell New-InvalidOperationException -ErrorRecord $_ ```

It just needs a generic message, something like 'Error retrieving membership for group. (ADG0014)'.

[X-Guardian]

This is getting pretty close @jeremyciak. I've tested your module updates in my lab and it is looking good. Just the last few review comments to cover.

For future info, when you have forked a GitHub repo, for each PR, create yourself a new branch on your fork for making any changes in, rather than using master. This makes it easier for reviewers to add your repo is a remote reference and check out the updated branch.

[jeremyciak]

Woohoo! Successful code coverage!

@X-Guardian, thank you so much for all of your time and effort spent guiding and educating me in all of this. I have learned a ton and I feel like a much more worthy contributor now.

[X-Guardian]

Regarding updating the ActiveDirectoryDsc.Common docs, you just need install the platyps module, change your path to the source\Modules\ActiveDirectoryDsc.Common folder and then run the Build-Readme.ps1 script. If you get any errors, let me know what they are.

[X-Guardian]

@jeremyciak, can you try Build-Readme.ps1 with PowerShell 5.x rather than 7?

[X-Guardian]

OK @jeremyciak, looking good. My one-way trust tests work and I've just successfully run the ADGroup integration tests, so just the ActiveDirectoryDsc.Common docs update and we should be good to go!

[jeremyciak]

@X-Guardian, I just got the docs to successfully create and those changes are in the pipeline now. Seems like if you import the platyPS module first and then let the build mechanism do its thing then the powershell-yaml module does not have issue on Powershell 5.1. Probably something that warrants investigation at some point!

[briantist]

I'ms o happy to see this fixed! https://github.com/dsccommunity/ActiveDirectoryDsc/issues/235