Skip to content

Commit 2393d76

Browse files
csevcsev
committed
No duplicate Discussions.
1 parent 3fe5623 commit 2393d76

4 files changed

Lines changed: 80 additions & 32 deletions

File tree

‎src/UI/Lessons.php‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,19 +1220,25 @@ public function renderDiscussions($buffer=false)
12201220

12211221
foreach($this->lessons->modules as $module) {
12221222
if ( isset($module->hidden) && $module->hidden ) continue;
1223-
if ( isset($module->discussions) && is_array($module->discussions) ) {
1224-
foreach($module->discussions as $discussion) {
1225-
$discussions [] = $discussion;
1226-
}
1227-
}
1228-
// Scan items array for discussion items (Lessons 2 format)
1229-
if ( isset($module->items) && is_array($module->items) ) {
1223+
1224+
// Check if module uses items array (new format)
1225+
$has_items = isset($module->items) && is_array($module->items) && count($module->items) > 0;
1226+
1227+
if ( $has_items ) {
1228+
// New format: scan items array for discussion items
12301229
foreach($module->items as $item) {
12311230
$item_obj = is_array($item) ? (object)$item : $item;
12321231
if ( isset($item_obj->type) && $item_obj->type == 'discussion' ) {
12331232
$discussions [] = $item_obj;
12341233
}
12351234
}
1235+
} else {
1236+
// Legacy format: scan discussions array
1237+
if ( isset($module->discussions) && is_array($module->discussions) ) {
1238+
foreach($module->discussions as $discussion) {
1239+
$discussions [] = $discussion;
1240+
}
1241+
}
12361242
}
12371243
}
12381244

‎src/UI/Lessons2.php‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,19 +1437,25 @@ public function renderDiscussions($buffer=false)
14371437

14381438
foreach($this->lessons->modules as $module) {
14391439
if ( isset($module->hidden) && $module->hidden ) continue;
1440-
if ( isset($module->discussions) && is_array($module->discussions) ) {
1441-
foreach($module->discussions as $discussion) {
1442-
$discussions [] = $discussion;
1443-
}
1444-
}
1445-
// Scan items array for discussion items (Lessons 2 format)
1446-
if ( isset($module->items) && is_array($module->items) ) {
1440+
1441+
// Check if module uses items array (new format)
1442+
$has_items = isset($module->items) && is_array($module->items) && count($module->items) > 0;
1443+
1444+
if ( $has_items ) {
1445+
// New format: scan items array for discussion items
14471446
foreach($module->items as $item) {
14481447
$item_obj = is_array($item) ? (object)$item : $item;
14491448
if ( isset($item_obj->type) && $item_obj->type == 'discussion' ) {
14501449
$discussions [] = $item_obj;
14511450
}
14521451
}
1452+
} else {
1453+
// Legacy format: scan discussions array
1454+
if ( isset($module->discussions) && is_array($module->discussions) ) {
1455+
foreach($module->discussions as $discussion) {
1456+
$discussions [] = $discussion;
1457+
}
1458+
}
14531459
}
14541460
}
14551461

‎tests/UI/Lessons2Test.php‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ public function __construct() {
535535
/**
536536
* Test renderDiscussions() flattening logic - extracts discussions from items array
537537
* This tests the new functionality added to support items array format
538+
* IMPORTANT: When items array exists, discussions array should NOT be scanned to avoid duplicates
538539
*/
539540
public function testRenderDiscussionsFlattensItemsArray() {
540541
$lessons2 = new class extends \Tsugi\UI\Lessons2 {
@@ -553,19 +554,25 @@ public function testFlattenDiscussions() {
553554

554555
foreach($this->lessons->modules as $module) {
555556
if ( isset($module->hidden) && $module->hidden ) continue;
556-
if ( isset($module->discussions) && is_array($module->discussions) ) {
557-
foreach($module->discussions as $discussion) {
558-
$discussions [] = $discussion;
559-
}
560-
}
561-
// Scan items array for discussion items (Lessons 2 format)
562-
if ( isset($module->items) && is_array($module->items) ) {
557+
558+
// Check if module uses items array (new format)
559+
$has_items = isset($module->items) && is_array($module->items) && count($module->items) > 0;
560+
561+
if ( $has_items ) {
562+
// New format: scan items array for discussion items
563563
foreach($module->items as $item) {
564564
$item_obj = is_array($item) ? (object)$item : $item;
565565
if ( isset($item_obj->type) && $item_obj->type == 'discussion' ) {
566566
$discussions [] = $item_obj;
567567
}
568568
}
569+
} else {
570+
// Legacy format: scan discussions array
571+
if ( isset($module->discussions) && is_array($module->discussions) ) {
572+
foreach($module->discussions as $discussion) {
573+
$discussions [] = $discussion;
574+
}
575+
}
569576
}
570577
}
571578
return $discussions;
@@ -604,20 +611,31 @@ public function testFlattenDiscussions() {
604611
'items' => [
605612
['type' => 'discussion', 'title' => 'Array Discussion', 'resource_link_id' => 'rlid6'] // Array format
606613
]
614+
],
615+
(object)[
616+
'title' => 'Module 4',
617+
'anchor' => 'mod4',
618+
// No items array - should use legacy discussions array
619+
'discussions' => [
620+
(object)['title' => 'Legacy Discussion', 'resource_link_id' => 'rlid7']
621+
]
607622
]
608623
];
609624

610625
$flattened = $lessons2->testFlattenDiscussions();
611626

612-
// Should include top-level discussion
627+
// Should include: top-level (1) + Module 1 items (1) + Module 3 items (1) + Module 4 legacy (1) = 4
613628
$this->assertCount(4, $flattened, 'Should flatten all discussions from various sources');
614629

615630
// Verify all discussions are included
616631
$titles = array_map(function($d) { return $d->title; }, $flattened);
617632
$this->assertContains('Top Level Discussion', $titles, 'Should include top-level discussions');
618-
$this->assertContains('Module Discussion', $titles, 'Should include module discussions');
619633
$this->assertContains('Item Discussion', $titles, 'Should include discussions from items array');
620634
$this->assertContains('Array Discussion', $titles, 'Should handle array format items');
635+
$this->assertContains('Legacy Discussion', $titles, 'Should include legacy discussions when no items array');
636+
637+
// Should NOT include Module Discussion from Module 1 (has items array, so discussions array is skipped)
638+
$this->assertNotContains('Module Discussion', $titles, 'Should NOT scan discussions array when items array exists (avoids duplicates)');
621639

622640
// Should NOT include hidden module discussions
623641
$this->assertNotContains('Hidden Discussion', $titles, 'Should skip discussions from hidden modules');

‎tests/UI/LessonsTest.php‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,7 @@ public function __construct() {
544544
/**
545545
* Test renderDiscussions() flattening logic - extracts discussions from items array
546546
* This tests the new functionality added to support items array format
547+
* IMPORTANT: When items array exists, discussions array should NOT be scanned to avoid duplicates
547548
*/
548549
public function testRenderDiscussionsFlattensItemsArray() {
549550
$lessons = new class extends Lessons {
@@ -562,19 +563,25 @@ public function testFlattenDiscussions() {
562563

563564
foreach($this->lessons->modules as $module) {
564565
if ( isset($module->hidden) && $module->hidden ) continue;
565-
if ( isset($module->discussions) && is_array($module->discussions) ) {
566-
foreach($module->discussions as $discussion) {
567-
$discussions [] = $discussion;
568-
}
569-
}
570-
// Scan items array for discussion items (Lessons 2 format)
571-
if ( isset($module->items) && is_array($module->items) ) {
566+
567+
// Check if module uses items array (new format)
568+
$has_items = isset($module->items) && is_array($module->items) && count($module->items) > 0;
569+
570+
if ( $has_items ) {
571+
// New format: scan items array for discussion items
572572
foreach($module->items as $item) {
573573
$item_obj = is_array($item) ? (object)$item : $item;
574574
if ( isset($item_obj->type) && $item_obj->type == 'discussion' ) {
575575
$discussions [] = $item_obj;
576576
}
577577
}
578+
} else {
579+
// Legacy format: scan discussions array
580+
if ( isset($module->discussions) && is_array($module->discussions) ) {
581+
foreach($module->discussions as $discussion) {
582+
$discussions [] = $discussion;
583+
}
584+
}
578585
}
579586
}
580587
return $discussions;
@@ -613,20 +620,31 @@ public function testFlattenDiscussions() {
613620
'items' => [
614621
['type' => 'discussion', 'title' => 'Array Discussion', 'resource_link_id' => 'rlid6'] // Array format
615622
]
623+
],
624+
(object)[
625+
'title' => 'Module 4',
626+
'anchor' => 'mod4',
627+
// No items array - should use legacy discussions array
628+
'discussions' => [
629+
(object)['title' => 'Legacy Discussion', 'resource_link_id' => 'rlid7']
630+
]
616631
]
617632
];
618633

619634
$flattened = $lessons->testFlattenDiscussions();
620635

621-
// Should include top-level discussion
636+
// Should include: top-level (1) + Module 1 items (1) + Module 3 items (1) + Module 4 legacy (1) = 4
622637
$this->assertCount(4, $flattened, 'Should flatten all discussions from various sources');
623638

624639
// Verify all discussions are included
625640
$titles = array_map(function($d) { return $d->title; }, $flattened);
626641
$this->assertContains('Top Level Discussion', $titles, 'Should include top-level discussions');
627-
$this->assertContains('Module Discussion', $titles, 'Should include module discussions');
628642
$this->assertContains('Item Discussion', $titles, 'Should include discussions from items array');
629643
$this->assertContains('Array Discussion', $titles, 'Should handle array format items');
644+
$this->assertContains('Legacy Discussion', $titles, 'Should include legacy discussions when no items array');
645+
646+
// Should NOT include Module Discussion from Module 1 (has items array, so discussions array is skipped)
647+
$this->assertNotContains('Module Discussion', $titles, 'Should NOT scan discussions array when items array exists (avoids duplicates)');
630648

631649
// Should NOT include hidden module discussions
632650
$this->assertNotContains('Hidden Discussion', $titles, 'Should skip discussions from hidden modules');

0 commit comments

Comments
 (0)