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

AADUser advanced query filter support #4430

Merged
merged 7 commits into from
Apr 3, 2024
Merged

Conversation

ifinch
Copy link
Contributor

@ifinch ifinch commented Mar 8, 2024

Pull Request (PR) description

advanced query filter support

This Pull Request (PR) fixes the following issues

-Fixes #2430

@ifinch ifinch requested a review from NikCharlebois as a code owner March 8, 2024 14:34
@bartvermeersch
Copy link
Contributor

Hello @ifinch

I noticed some tenant specific info in your PR:

'extension_4750531c75d84e8698ecd21bbd5da9f5_fruitPreference',
'extension_4750531c75d84e8698ecd21bbd5da9f5_test',

Also, have you considered the addition of a new switch parameters AdvancedQuery to set the CountVariable and ConsistencyLevel instead of adding the extra logic? In all available Graph SDK's the user has the responsibility to specify if the query is "advanced" (or not).

Keeping up with future changes in the graph for these query types and properties seems a lot of extra work. Similar logic will have to be implemented on other DSC resource (AADGroups, ...) as well.

@ifinch
Copy link
Contributor Author

ifinch commented Mar 10, 2024

Hey @bartvermeersch ,

Those ones are precented in the advanced query doc
https://learn.microsoft.com/en-us/graph/aad-advanced-queries?tabs=http#user-properties

extension_4750531c75d84e8698ecd21bbd5da9f5_fruitPreference
extension_4750531c75d84e8698ecd21bbd5da9f5_fruitPreference/any(p:p)
extension_4750531c75d84e8698ecd21bbd5da9f5_test

However, there is difference how MgUser vs MsBetaUsers and as MgUser used, some of the extensions should be cleaned up including those you pointed out.

I have not considered AdvancedQuery switch this far.

@ifinch
Copy link
Contributor Author

ifinch commented Mar 15, 2024

Hi @NikCharlebois , anything extra needed to follow up on this removing from On-Hold state?

@NikCharlebois
Copy link
Collaborator

My only concern here is that we will need to manage a hardcoded list of possible filters.

check if $Filter is null implemented
@ifinch
Copy link
Contributor Author

ifinch commented Mar 18, 2024

My only concern here is that we will need to manage a hardcoded list of possible filters.

completely agreed, but not that many options we have now to make differently AFAIK

@ifinch
Copy link
Contributor Author

ifinch commented Apr 2, 2024

Hi @NikCharlebois any ETA on this to be merged?

Copy link
Contributor Author

@ifinch ifinch left a comment

Choose a reason for hiding this comment

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

removed printing "Filter is null" on export with a Write-Host

@NikCharlebois NikCharlebois merged commit 6d73894 into microsoft:Dev Apr 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AADUser: Export Filtering doesn't work as expected
3 participants