sensei icon indicating copy to clipboard operation
sensei copied to clipboard

Fix module teacher name not showing for modules added to course in legacy way

Open Imran92 opened this issue 3 years ago • 1 comments
trafficstars

Fixes https://github.com/Automattic/sensei/issues/4044

Changes proposed in this Pull Request

  • When adding a module to a course via the legacy method, we didn't modify the module slug to prefix the course teacher id in the front of the slug. As we depended on the prefixed it to show the teacher name or to determine the module teacher, the name of the teacher of the module didn't appear beside the module name in SenseiLMS-> Modules and in other places.

Recently in https://github.com/Automattic/sensei/pull/5207, we allowed custom slug to be added to modules, to prefixing teacher id to slug is not a hard requirement now. In case of custom slugs, we are keeping the teacher id in the term meta. So here in this PR, upon adding a module to a course, we're saving the teacher id to module's meta, so we can show the teacher name using that.

However, I think there's no restriction (didn't find any) to add any module to any course in the legacy system, so you can add a module to course taught by Teacher A, then add the same module to another course taught by Teacher B even though a module should only be taught by one teacher. These cases are handled in our updated block based system, but remains in legacy way. Didn't try to fix that as probably that'll be out of the scope of this ticket. Also, I think it'll be a lot of work just for fixing the legacy way.

Testing instructions

  • Log-in as admin.
  • Create a teacher.
  • Create a course and remove all the blocks.
  • Add the teacher to the course.
  • Save, and refresh the page.
  • Add some modules to the course through the meta box.
  • Save the course.
  • Make sure the modules appear with the teacher name appended to the module title in SenseiLMS->Modules
  • Change the course teacher and check again to see if the new teacher name appears
  • Remove the module from the course and make sure the teacher name is also removed

Imran92 avatar Jul 21 '22 22:07 Imran92

Codecov Report

Merging #5376 (de6e238) into trunk (a3be583) will increase coverage by 0.03%. The diff coverage is 86.20%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #5376      +/-   ##
============================================
+ Coverage     44.95%   44.98%   +0.03%     
- Complexity     9452     9462      +10     
============================================
  Files           475      475              
  Lines         33457    33486      +29     
  Branches        272      272              
============================================
+ Hits          15039    15065      +26     
- Misses        18216    18219       +3     
  Partials        202      202              
Impacted Files Coverage Δ
includes/class-sensei-modules.php 37.15% <86.20%> (+1.61%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8fc1ce1...de6e238. Read the comment docs.

codecov[bot] avatar Jul 21 '22 23:07 codecov[bot]