Skip to content

Conversation

@milov-dmitriy
Copy link
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings February 3, 2026 13:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors several LDAP protocol helpers to improve encapsulation and align computer Kerberos principal handling with the sAMAccountName attribute, while doing a small pagination refactor and removing a now-unnecessary host_principal property.

Changes:

  • Extracts a _validate_query helper in PaginationResult to check that Select queries have an order_by clause.
  • Converts ModifyRequest.old_vals into a private Pydantic attribute _old_vals and slightly adjusts a ModifyForbiddenError message.
  • Updates computer add/delete flows to derive Kerberos principals from the sAMAccountName attribute, and removes the Directory.host_principal property.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
interface Updates the submodule pointer to a new commit, pulling in external changes.
app/ldap_protocol/utils/pagination.py Adds a reusable _validate_query helper and reuses it in PaginationResult.get for order-by validation.
app/ldap_protocol/ldap_requests/modify.py Switches the cached old-values store to a Pydantic PrivateAttr and tweaks an error message.
app/ldap_protocol/ldap_requests/delete.py Changes computer principal deletion to use sAMAccountName-based host principals, with new error behavior when the attribute is missing.
app/ldap_protocol/ldap_requests/add.py Sets sAMAccountName for computers earlier, threads it through to Kerberos principal creation, and adjusts default group / userAccountControl handling.
app/entities.py Removes the now-unused host_principal property from Directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Misha-Shvets Misha-Shvets self-requested a review February 3, 2026 13:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@milov-dmitriy milov-dmitriy requested a review from Copilot February 3, 2026 13:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rimu-stack rimu-stack changed the title Rename entry refactor: ldap requests Feb 3, 2026
@rimu-stack rimu-stack merged commit b82c5b1 into dev Feb 3, 2026
11 of 12 checks passed
@rimu-stack rimu-stack deleted the rename_entry_task_1110 branch February 3, 2026 14:47
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.

4 participants