sensei icon indicating copy to clipboard operation
sensei copied to clipboard

Avoid caching when getting course structure

Open merkushin opened this issue 1 year ago • 6 comments

Resolves #7146

Proposed Changes

  • Avoid caching when getting course structure

Testing Instructions

  1. Go to Sensei LMS > Courses, create a course with modules and lessons in them.
  2. Go to Sensei LMS > Modules, change the order of the modules for the course. Save it.
  3. Go to Sensei LMS > Lessons, change the order of the lessons in modules and outside the modules. Save it.
  4. Open the course on the frontend and check the order is the same you set on previous steps.

Pre-Merge Checklist

  • [ ] PR title and description contain sufficient detail and accurately describe the changes
  • [ ] Acceptance criteria is met
  • [ ] Decisions are publicly documented
  • [ ] 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
  • [ ] New UIs match the designs
  • [ ] Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • [ ] Legacy courses (course without blocks) are tested
  • [ ] Code is tested on the minimum supported PHP and WordPress versions
  • [ ] User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • [ ] "Needs Documentation" label is added if this change requires updates to documentation
  • [ ] Known issues are created as new GitHub issues

merkushin avatar Aug 01 '24 22:08 merkushin

Test the previous changes of this PR with WordPress Playground.

github-actions[bot] avatar Aug 01 '24 22:08 github-actions[bot]

Test the previous changes of this PR with WordPress Playground.

github-actions[bot] avatar Aug 01 '24 23:08 github-actions[bot]

@merkushin I noticed this one is in Draft but assigned to Nexus. Is it ready for review?

donnapep avatar Aug 02 '24 13:08 donnapep

Hey @merkushin! As per your concern about performance that you mentioned to me, I think it's very important and it would be worth checking where this cache would be skipped to see if it could have a negative impact.

That said, I tested it in Pressable and I still see the issue here:

https://github.com/user-attachments/assets/a6d93804-6767-47ed-bb97-95394de4913d

renatho avatar Aug 02 '24 18:08 renatho

@donnapep not yet. It is unfinished yet, especially considering Renatho's reply.

I asked Renatho to look at it as he had some experience with addressing a similar issue. And I had a concern about performance (Renatho mentioned it in his comment here: https://github.com/Automattic/sensei/pull/7654#issuecomment-2265928786).

merkushin avatar Aug 02 '24 18:08 merkushin

I asked Renatho to look at it as he had some experience with addressing a similar issue.

OK, in that case I'm going to remove Nexus as reviewer so that I don't get notifications in my inbox. Not sure there's a way to disable those.

donnapep avatar Aug 02 '24 19:08 donnapep