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

IntuneDeviceCompliancePolicyAndroidDeviceOwner/IntuneDeviceCompliancePolicyAndroidWorkProfile enhancements #5640

Closed
wants to merge 21 commits into from

Conversation

dannyKBjj
Copy link
Contributor

Pull Request (PR) description

MgGraph Update- / PATCH problem:

microsoftgraph/msgraph-sdk-powershell#2177
microsoftgraph/msgraph-metadata#245

This Pull Request (PR) fixes the following issues

- Fixes #5592
- Fixes #5593

Task list

  • 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 parameter descriptions added/updated in the schema.mof.
  • Resource documentation added/updated in README.md.
  • Resource settings.json file contains all required permissions.
  • Examples appropriately added/updated.
  • Unit tests added/updated.
  • New/changed code adheres to DSC Community Style Guidelines.

…ntuneDeviceCompliancePolicyAndroidWorkProfile

* IntuneDeviceCompliancePolicyAndroidDeviceOwner
  * Added missing properties for androidForWorkCompliancePolicy resource type: https://learn.microsoft.com/en-us/graph/api/resources/intune-deviceconfig-androiddeviceownercompliancepolicy?view=graph-rest-beta.
  * Non-compliance actions now supported for Export- Test- Get-
  * Non-compliance actions now supported for Set- when creating a policy, but not when updating a policy. Update can't be supported due to MgGraph bug.
    FIXES [microsoft#5593](microsoft#5593)
* IntuneDeviceCompliancePolicyAndroidWorkProfile
  * Added missing properties for androidForWorkCompliancePolicy resource type: https://learn.microsoft.com/en-us/graph/api/resources/intune-deviceconfig-androidforworkcompliancepolicy?view=graph-rest-beta.
  * Non-compliance actions now supported for Export- Test- Get-
  * Non-compliance actions now supported for Set- when creating a policy, but not when updating a policy. Update can't be supported due to MgGraph bug.
    FIXES [microsoft#5592](microsoft#5592)

Graph issue with Update- / PATCH
microsoftgraph/msgraph-sdk-powershell#2177
microsoftgraph/msgraph-metadata#245
…DeviceCompliancePolicyAndroidWorkProfile

* IntuneDeviceCompliancePolicyAndroidDeviceOwner
  * Added missing properties for androidForWorkCompliancePolicy resource type: https://learn.microsoft.com/en-us/graph/api/resources/intune-deviceconfig-androiddeviceownercompliancepolicy?view=graph-rest-beta.
  * Non-compliance actions now supported for Export- Test- Get-
  * Non-compliance actions now supported for Set- when creating a policy, but not when updating a policy. Update can't be supported due to MgGraph bug.
    FIXES [microsoft#5593](microsoft#5593)
* IntuneDeviceCompliancePolicyAndroidWorkProfile
  * Added missing properties for androidForWorkCompliancePolicy resource type: https://learn.microsoft.com/en-us/graph/api/resources/intune-deviceconfig-androidforworkcompliancepolicy?view=graph-rest-beta.
  * Non-compliance actions now supported for Export- Test- Get-
  * Non-compliance actions now supported for Set- when creating a policy, but not when updating a policy. Update can't be supported due to MgGraph bug.
    FIXES [microsoft#5592](microsoft#5592)

MgGraph issues:
microsoftgraph/msgraph-sdk-powershell#2177
microsoftgraph/msgraph-metadata#245
asked to change to pascal case on another PR, this PR has the same issue, so fixed it
@dannyKBjj
Copy link
Contributor Author

Can't make much sense of the conflicts in the script...

@dannyKBjj
Copy link
Contributor Author

dannyKBjj commented Jan 20, 2025

Still unable to make sense of conflict. It is conflicting with a PR that deletes all the region telemetry?!

For example MSFT_IntuneDeviceCompliancePolicyAndroidDeviceOwner.psm1

My PR:
image

The Conflict:

image

It does not look like the code I uploaded and I'm unsure what the $script bit it conflicts with is supposed to be doing anyway!

@ykuijs
Copy link
Member

ykuijs commented Jan 21, 2025

@dannyKBjj, we recently applied some performance improvement. This means that in an export, we do not have to connect over-and-over again and send telemetry. That is why we added the if-statement in the Get method that checks for the presence of the exportedInstance script variable:

If that is set, the export routine called the Get method and it already retrieved information from the Graph. So we will be using that instead of retrieving it all over again.

So in your case, please implement the changes on the code that exists within the if-statement.

Another thing: To prevent you having to copy/paste screenshots of lines of code: You can refer to actual lines in the code by browsing to that line in GitHub, selecting it (multi-line selection also possible by shift-select last line), clicking the three dots that appear in front of the line and clicking "Copy permalink":
image

This will copy a link to your clipboard, which you then can paste in an issue or PR:
https://github.com/microsoft/Microsoft365DSC/blob/2cd3bfe68431cfb0b2d6c8e532c85c975945e2b1/Modules/Microsoft365DSC/DSCResources/MSFT_AADDomain/MSFT_AADDomain.psm1#L20

GitHub will then automatically convert that into something readable:

@dannyKBjj
Copy link
Contributor Author

That's great, thanks for the tip!

@dannyKBjj
Copy link
Contributor Author

Requested changes made.

@dannyKBjj
Copy link
Contributor Author

Unit tests failed, but they run fine locally. Don't think the problem is with my code?! Besides which I've done nothing but resolve the conflicts with the code changes asked for since all the unit test passed (last time).

I've re-fetched the origin and run both unit tests and they work fine...

pointless edit to re-run unit test.
@dannyKBjj dannyKBjj mentioned this pull request Jan 27, 2025
@dannyKBjj
Copy link
Contributor Author

There appears to be nothing wrong with the Unit tests, but all checks are failing on the PR?!

image

All unit tests work fine. When PR is initiated all the unit tests fail.
@dannyKBjj
Copy link
Contributor Author

pushing again....

@ykuijs ykuijs mentioned this pull request Jan 27, 2025
7 tasks
@ykuijs
Copy link
Member

ykuijs commented Jan 27, 2025

You are now running unit tests individually, which might have something to do with it. Can you please try this:

  • Browse to your local clone
  • Run Import-Module './Tests/TestHarness.psm1' -Force
  • Then run Invoke-TestHarness -IgnoreCodeCoverage

This will run all unit tests sequentially.

If that doesn't work, please make sure there are no other versions of M365DSC installed on your machine. I have seen issues in the past where other versions of M365DSC, for example installed in the Program Files folder, were causing issues in unit tests.

@dannyKBjj
Copy link
Contributor Author

dannyKBjj commented Jan 28, 2025

I tried doing as you suggested and ran all the tests from my local clone. I got a number of errors, but none of them were with my module .test scrips!

@dannyKBjj
Copy link
Contributor Author

dannyKBjj commented Jan 28, 2025

Unit tests working fine locally
Resource behaving correctly when tested against my live environment.

@dannyKBjj
Copy link
Contributor Author

Merged with \Dev to resolve conflicts, new files were added that are not mine. Now these files are failing the unit tests?

image

@FabienTschanz
Copy link
Collaborator

Are you fully up to date? Do they fail locally too? I have two PRs open that worked with the latest changes.

@FabienTschanz
Copy link
Collaborator

Ouuuh and it seems like you goofed something up during the merge, there shouldn't be 518 changes files. Please undo the merge (by e.g. resetting your branch to your last commit before the merge) and then try again.

@dannyKBjj
Copy link
Contributor Author

Just cannot get unit tests to work, since last merge with dev, pester environment completely broken.

Created new PR #5803 on which unit tests work fine.

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.

3 participants