Skip to content

Conversation

simonLeary42
Copy link
Collaborator

@simonLeary42 simonLeary42 commented Jul 5, 2025

Fixes the slowness issues that created the need for redis cache. user-mgmt and pi-mgmt should no longer be relying on redis at all. UnityUser->getGroups still uses cache by default, but these changes should enable us to remove cache later.

  • UnityUser->getGroups() -> UnityUser->getPIGroupGIDs():
    • before:
      • query LDAP for all PI groups (all attributes)
      • initialize an object for each PI group
      • filter for PI groups with membership
    • after:
      • query LDAP for PI groups with this user as a member
  • pi-mgmt.php:
    • before (without cache):
      • query LDAP for all PI groups (all attributes)
      • initialize a UnityUser object for each PI group owner
      • query LDAP separately for name, UID, and email for each PI group owner
    • after:
      • query LDAP for all PI groups (GIDs only)
      • query LDAP for all users (gecos, UID, and mail only)
  • user-mgmt.php:
    • before (without cache):
      • query LDAP for all users (all attributes)
      • initialize a UnityUser object for each user
      • query LDAP separately for name, org, UID, and email for each user
    • after:
      • query LDAP for all users (name, org, UID, and email only)

also, replaced <p> <br> formatting for group members in user-mgmt.php with a nested table.

before:

image

after:

image

@simonLeary42 simonLeary42 marked this pull request as draft July 5, 2025 18:53
@simonLeary42 simonLeary42 changed the title optimize big pages remove cache from big pages Jul 5, 2025
@simonLeary42 simonLeary42 marked this pull request as ready for review July 6, 2025 22:49
@simonLeary42 simonLeary42 requested a review from bryank-cs July 6, 2025 22:52
@simonLeary42 simonLeary42 requested a review from Copilot August 26, 2025 18:29
Copilot

This comment was marked as outdated.

Copy link
Contributor

@Copilot 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 optimizes the performance of user management and PI management pages by removing dependency on Redis cache and implementing more efficient LDAP queries. The changes replace cached object initialization with direct attribute queries, significantly reducing LDAP overhead.

Key changes:

  • Replaced UnityUser->getGroups() with getPIGroupGIDs() to return group IDs instead of full objects
  • Optimized user-mgmt.php to query only necessary user attributes instead of initializing full user objects
  • Optimized pi-mgmt.php to query PI group owner attributes directly instead of creating objects for each PI group

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
webroot/panel/groups.php Updates group iteration to use GIDs and create UnityGroup objects on-demand
webroot/panel/account.php Refactors to use getPIGroupGIDs() and converts mixed PHP/HTML to echo statements
webroot/admin/user-mgmt.php Replaces user object initialization with direct LDAP attribute queries
webroot/admin/pi-mgmt.php Replaces PI group object initialization with direct owner attribute queries
test/functional/AccountDeletionRequestTest.php Updates test assertions to use new getPIGroupGIDs() method
resources/lib/UnityUser.php Refactors getGroups() to getPIGroupGIDs() returning array of group IDs
resources/lib/UnityLDAP.php Adds new methods for efficient attribute-only LDAP queries
resources/init.php Adds base DN parameter to LDAP constructor

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@simonLeary42 simonLeary42 merged commit 34b2135 into main Aug 26, 2025
3 checks passed
@simonLeary42 simonLeary42 deleted the optimize-big-pages branch August 26, 2025 21:31
@simonLeary42 simonLeary42 mentioned this pull request Aug 28, 2025
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.

2 participants