Skip to content

refactor(user_ldap): add type hints, harden TTL restore, improve comments/docblock in dn2ocname#58811

Draft
joshtrichards wants to merge 5 commits intomasterfrom
jtr/refactor-ldap-Access-dn2ocname
Draft

refactor(user_ldap): add type hints, harden TTL restore, improve comments/docblock in dn2ocname#58811
joshtrichards wants to merge 5 commits intomasterfrom
jtr/refactor-ldap-Access-dn2ocname

Conversation

@joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Mar 9, 2026

  • Resolves: #

Summary

  • Adds more type hints and aligns the method docblock with actual behavior and arguments
  • Clarifies the purpose of the $intermediates static variable
  • Refactors cache TTL logic to ensure TTL is always restored and reduce code duplication
  • Updates related comments and clarifies conditionals for easier maintenance

TODO

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Josh <josh.t.richards@gmail.com>
…fe with finally

Even without the robustness improvement, eliminates duplicate lines/logic.

Also made conditional easier to read and clarified comments for this code block.

Signed-off-by: Josh <josh.t.richards@gmail.com>
New comment
Code clarity

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added feature: ldap ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Mar 9, 2026
@joshtrichards joshtrichards requested review from blizzz and come-nc March 9, 2026 16:10
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Comment on lines -599 to -602
//a new user/group! Add it only if it doesn't conflict with other backend's users or existing groups
//disabling Cache is required to avoid that the new user is cached as not-existing in fooExists check
//NOTE: mind, disabling cache affects only this instance! Using it
// outside of core user management will still cache the user as non-existing.
Copy link
Contributor

Choose a reason for hiding this comment

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

The reformulation loses some of the information in the comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: ldap ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants