Skip to content

Conversation

@m1r0
Copy link
Member

@m1r0 m1r0 commented Mar 18, 2025

Resolves https://github.com/Automattic/sensei-pro/issues/2640

Proposed Changes

  • Reset the enrolment provider state store to avoid storing too much data in memory when rendering one of the Students screens.

Testing Instructions

  1. Have Sensei Pro enabled.
  2. Have a course with multiple enrolments.
  3. In code, add a breakpoint in the Sensei_Enrolment_Provider_State_Store::persist_all method to inspect the $instances property. Note that this method is called on the shutdown action so just adding a var_dump(self::$instances) should work as well.
  4. Go to Students -> Click on a course that has multiple enrolled users -> Click Manually Enrolled Students (the URL should be similar to /wp-admin/admin.php?page=sensei_learners&course_id=578669&view=learners&enrolment_status=manual)
  5. Make sure the $instances property is empty.
  6. Make sure the screen works as before.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • Code is tested on the minimum supported PHP and WordPress versions

@m1r0 m1r0 self-assigned this Mar 18, 2025
@m1r0 m1r0 added this to the 4.24.6 milestone Mar 18, 2025
@m1r0 m1r0 requested a review from a team March 18, 2025 17:20
@github-actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@renatho renatho self-requested a review March 18, 2025 20:04
Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

Thank you for solving this quickly! This works as described! =)


return $manual_enrolment_provider->is_enrolled( $user_id, $this->course_id );
// Reset the enrolment state store to avoid out of memory issues.
// TODO: This can be removed once the state store no longer holds all the data in memory.
Copy link
Contributor

Choose a reason for hiding this comment

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

once the state store no longer holds all the data in memory.

Do we have an issue to implement this already?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review, Renatho! I'm not sure. It's possible that the Sensei_Enrolment_Provider_State_Store is storing everything in memory to avoid querying the database multiple times per request. I didn't have the time to investigate all the cases.

@m1r0 m1r0 merged commit 95c8e19 into trunk Mar 19, 2025
21 of 22 checks passed
@m1r0 m1r0 deleted the fix/out-of-memory-on-students-screen branch March 19, 2025 08:25
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.

3 participants