Skip to content

Conversation

@renatho
Copy link
Contributor

@renatho renatho commented Mar 31, 2025

This reverts commit 19a00b2.

See issue reported p1743440666861309-slack-C07418EJ0

Proposed Changes

At some point, we made a change to improve a performance issue noticed on Learnomattic.

Now it's causing an issue in atomic sites on WPCOM, not ordering the courses at all. Given that currently Learnomattic doesn't have a page to list all the courses, it should not be a problem anymore, and we should prioritize user sites.

If we face this performance issue again for a specific type of environment, we need to find a different solution. Or if the reverted solution still works in this environment, we could try a conditional with the 2 approaches depending on the environment.

Testing Instructions

  1. Apply this change on atomic.
  2. Create some courses.
  3. Navigate to WP Admin > Sensei LMS > Courses.
  4. Click on "Order Courses".
  5. Reorder your courses.
  6. Visit your courses list on your site frontend.
  7. Check that the reordering worked properly.
  8. Repeat the reorder, and make sure it still works (I noticed that sometimes it was working the first time).

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

@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-assigned this Mar 31, 2025
@renatho renatho changed the title Revert "Improve performance on course ordering" Fix courses ordering on WPCOM Mar 31, 2025
@renatho renatho requested a review from a team March 31, 2025 22:18
@renatho renatho added this to the 4.24.6 milestone Mar 31, 2025
'menu_order' => $i,
);

// If you face performance issues on Simple Sites, see https://github.com/Automattic/sensei/pull/7799.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this comment, so in case we face this issue again in the future, it's easier to find our previous tries and possible solutions.

@renatho renatho marked this pull request as ready for review March 31, 2025 22:20
Copy link
Contributor

@bogiii bogiii left a comment

Choose a reason for hiding this comment

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

Works as described. Nice finding. 👍

@bogiii bogiii self-requested a review April 1, 2025 11:23
@renatho renatho merged commit 3b7a129 into trunk Apr 1, 2025
22 of 23 checks passed
@renatho renatho deleted the fix/course-ordering branch April 1, 2025 13:44
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