Conversation
Store RemoteDesktop_ActiveDirectoryImportLdapSearchBase after a successful directory query and prefill the import dialog on next open.
|
Thanks. I will review it once i am done with the PR for the Firewall Module :) I don't have much time right now, so it might take a while. But I think it's a great feature. |
|
@copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
Pull request overview
Adds a Remote Desktop UI flow to import computer accounts from on-premises Active Directory (LDAP subtree search) into a selected profile group, with filtering and persisted “last search base” for UX.
Changes:
- Adds an “Import computers from Active Directory…” button to the Remote Desktop host view and a new child window for import options.
- Introduces
ActiveDirectoryComputerSearcher(+ record type) to query AD using current Windows credentials and return candidate computers. - Persists the last successful LDAP search base in settings and adds new localized strings for the import UX.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/NETworkManager/Views/RemoteDesktopHostView.xaml | Adds toolbar button to open the AD import dialog. |
| Source/NETworkManager/Views/ImportAdComputersChildWindow.xaml | New child window UI for LDAP search base, target group, and options. |
| Source/NETworkManager/Views/ImportAdComputersChildWindow.xaml.cs | Child window sizing/focus behavior. |
| Source/NETworkManager/ViewModels/RemoteDesktopHostViewModel.cs | Adds command to open the AD import dialog. |
| Source/NETworkManager/ViewModels/ImportAdComputersViewModel.cs | Implements import workflow, filtering, and summary dialog. |
| Source/NETworkManager/ProfileDialogManager.cs | Wires up new import child window + VM. |
| Source/NETworkManager.Validators/GroupNameValidator.cs | Avoids exceptions by treating null/empty as valid (paired with EmptyValidator). |
| Source/NETworkManager.Utilities/ActiveDirectory/ActiveDirectoryComputerSearcher.cs | LDAP query logic for AD computers. |
| Source/NETworkManager.Utilities/ActiveDirectory/ActiveDirectoryComputerRecord.cs | Record struct for returned AD computer data. |
| Source/NETworkManager.Settings/SettingsInfo.cs | Adds persisted setting for last LDAP search base. |
| Source/NETworkManager.Localization/Resources/Strings.resx | Adds new strings for AD import UI. |
| Source/NETworkManager.Localization/Resources/Strings.Designer.cs | Updates generated resource accessors. |
| CONTRIBUTORS.md | Adds contributor entry. |
Files not reviewed (1)
- Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var profile = CreateRemoteDesktopProfileForImportedComputer(computer.ProfileName, dnsHostName, | ||
| targetGroup); | ||
|
|
||
| ProfileManager.AddProfile(profile); | ||
| existingProfileNames.Add(computer.ProfileName); | ||
| importedCount++; | ||
| } |
There was a problem hiding this comment.
Import loop calls ProfileManager.AddProfile(profile) for every AD computer, and AddProfile triggers ProfilesUpdated() each time. For large OUs this can cause significant UI churn/slowdowns. Consider batching (e.g., add a ProfileManager.AddProfiles(...)/BeginUpdate-EndUpdate mechanism or otherwise suppress repeated OnProfilesUpdated until after the loop) so the UI refresh happens once.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Extend the ProfileManager with AddProfiles()
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Changes proposed in this pull request
ActiveDirectoryComputerSearcherinNETworkManager.Utilitiesto handle secure AD queries using current Windows credentialsRemoteDesktop_ActiveDirectoryImportLdapSearchBase) for better UXRelated issue(s)
Copilot generated summary
Provide a Copilot generated summary of the changes in this pull request.
Copilot summary
Summary
Adds Remote Desktop functionality to import computer accounts from an on-premises Active Directory using an LDAP subtree search from a user-supplied search base (OU DN). Imported machines are added to a single target profile group. Each profile is created with Remote Desktop enabled, utilizing the
dNSHostNameproperty.User-visible behavior
ProfileManager.Save()after a successful import.Implementation notes
NETworkManager.Utilities.ActiveDirectoryComputerSearcherusingSystem.DirectoryServices.GroupNameValidatorto prevent exceptions on editable group combos.To-Do
Contributing
By submitting this pull request, I confirm the following: