PROD-9833 by sajib-bb · Pull Request #4931 · buddyboss/buddyboss-platform

sajib-bb · 2026-04-10T04:54:11Z

Jira Issue:

https://buddyboss.atlassian.net/browse/PROD-9833

General Note

Keep all conversations related to this PR in the associated Jira issue(s). Do NOT add comment on this PR or edit this PR’s description.

Notes to Developer

Ensure the IDs (i.e. PROD-1) of all associated Jira issues are reference in this PR’s title
Ensure that you have achieved the Definition of Done before submitting for review
When this PR is ready for review, move the associate Jira issue(s) to “Needs Review” (or “Code Review” for Dev Tasks)

Notes to Reviewer

Ensure that the Definition of Done have been achieved before approving a PR
When this PR is approved, move the associated Jira issue(s) to “Needs QA” (or “Approved” for Dev Tasks)

…ry to ensure proper compatibility with Tutor LMS when ReadyLaunch is active

The paginate_links_output filter hook is now conditionally loaded to resolve a pagination conflict. Since Tutor LMS ships with its own pagination that operates via AJAX, it is incompatible with ReadyLaunch's pagination implementation. To address this, the ReadyLaunch pagination is excluded only when all of the following conditions are true: - ReadyLaunch is active - Tutor LMS is active - The user is on the member profile page Courses tab or any of its sub-tabs Outside of this specific context, ReadyLaunch pagination continues to function normally across the rest of the website.

welcome · 2026-04-10T04:54:15Z

Thanks for your pull request. It looks like this may be your first contribution to the BuddyBoss Platform open source project. Please note that this project and all contributions to it are public and bounded by the GPL v2.0 license, and that a record of the contribution (including all personal information you submit with it, including your full name and email address) is maintained indefinitely and may be redistributed with this project. If you are not okay with these terms, please close this pull request. Alternatively, you can let us know about your concerns by adding a comment to this pull request.

claude · 2026-04-10T04:54:35Z

Claude finished @sajib-bb's task —— View job

PR Review Complete

I've thoroughly reviewed the changes in this PR. Here's my comprehensive assessment:

Review Checklist

Read repository guidelines and coding standards
Analyze modified file: class-bb-readylaunch.php
Review new file: groups/single/courses.php
Review new file: members/single/tutor/courses.php
Check for code quality and best practices
Identify potential bugs or issues
Assess performance considerations
Evaluate security implications
Provide constructive feedback and suggestions

Summary

This PR adds ReadyLaunch template support for TutorLMS courses and conditionally excludes the pagination filter from course pages to fix pagination issues. The changes are minimal but targeted.

Code Quality Assessment

✅ Strengths

Proper conditional logic in class-bb-readylaunch.php:431-433 - Correctly excludes pagination filter when TutorLMS is active on user profile course pages
Consistent template structure - Both new templates follow established patterns
Proper internationalization - Uses correct buddyboss text domain
Security practices - Uses esc_html(), wp_kses(), and intval() appropriately
Template documentation - Includes proper PHPDoc headers with version info

⚠️ Issues Identified

1. Missing ReadyLaunch Template Wrapper (HIGH PRIORITY)

File: Both template files
Issue: ReadyLaunch templates require .bb-rl-screen-content wrapper for modal functionality (per CLAUDE.md:166-171)

// Both templates should be wrapped like this:
<div class="bb-rl-screen-content">
    
</div>

2. Incorrect Text Domain (MEDIUM PRIORITY)

File: members/single/tutor/courses.php:54
Line: 'buddyboss-pro' should be 'buddyboss'

// Current (incorrect):
printf( wp_kses( _n( '<span class="bb-count">%d</span> Course', '<span class="bb-count">%d</span> Courses', $count, 'buddyboss-pro' ), /* ... */ )

// Should be:
printf( wp_kses( _n( '<span class="bb-count">%d</span> Course', '<span class="bb-count">%d</span> Courses', $count, 'buddyboss' ), /* ... */ )

3. Potential XSS Security Issue (LOW PRIORITY)

File: groups/single/courses.php:30
Issue: Direct echo of TutorLMS shortcode output without additional escaping

// Current:
echo tutor_lms()->shortcode->tutor_course( array( 'id' => $course_ids_string, 'show_pagination' => 'on', 'post_status' => array('publish', 'private') ) );

// Consider wrapping with wp_kses_post() if the shortcode output is not already escaped:
echo wp_kses_post( tutor_lms()->shortcode->tutor_course( /* ... */ ) );

4. Input Validation Improvement (LOW PRIORITY)

File: groups/single/courses.php:29 & members/single/tutor/courses.php:64
Issue: implode() on potentially non-array data

// Add validation before implode():
if ( ! empty( $course_ids ) && is_array( $course_ids ) ) {
    $course_ids_string = implode( ',', $course_ids );
    // ... rest of code
}

Performance Considerations

✅ Good Practices

Efficient early returns (bp-get-group-id() check)
Proper use of per_page => false for complete data retrieval
Array filtering and deduplication in member template

⚠️ Potential Issues

No caching implemented for course data retrieval
Multiple function calls in conditional logic could be optimized

Security Assessment

✅ Secure Practices

Uses intval() for ID sanitization
Proper array validation in member template
Uses wp_kses() for safe HTML output

⚠️ Minor Concerns

Direct shortcode output echoing (mentioned above)
Text domain inconsistency

Recommendations

Critical (Must Fix)

Add ReadyLaunch wrapper to both template files
Fix text domain in members template

Suggested Improvements

Add input validation before implode() calls
Consider caching course data for better performance
Review shortcode output escaping

Code Style

Follows WordPress coding standards ✅
Proper indentation and spacing ✅
Consistent with existing codebase ✅

Conclusion

The core functionality appears sound and addresses the pagination issue correctly. The main concerns are the missing ReadyLaunch wrapper (which could break modal functionality) and the text domain inconsistency. These should be addressed before merging.

The PR successfully implements TutorLMS course display in ReadyLaunch mode while maintaining compatibility with existing pagination filters.

claude · 2026-04-10T05:00:39Z

Claude finished @sajib-bb's task —— View job

PR Review Complete

I've thoroughly reviewed the changes in this PR. Here's my comprehensive assessment:

Review Checklist

Read repository guidelines and coding standards
Analyze modified file: class-bb-readylaunch.php
Review new file: groups/single/courses.php
Review new file: members/single/tutor/courses.php
Check for code quality and best practices
Identify potential bugs or issues
Assess performance considerations
Evaluate security implications
Provide constructive feedback and suggestions

Summary

This PR adds ReadyLaunch template support for TutorLMS courses and conditionally excludes the pagination filter from course pages to fix pagination issues. The changes are minimal but targeted.

Code Quality Assessment

✅ Strengths

Proper conditional logic in class-bb-readylaunch.php:431-433 - Correctly excludes pagination filter when TutorLMS is active on user profile course pages
Consistent template structure - Both new templates follow established patterns
Proper internationalization - Uses correct buddyboss text domain
Security practices - Uses esc_html(), wp_kses(), and intval() appropriately
Template documentation - Includes proper PHPDoc headers with version info

⚠️ Issues Identified

1. Missing ReadyLaunch Template Wrapper (HIGH PRIORITY)

File: Both template files
Issue: ReadyLaunch templates require .bb-rl-screen-content wrapper for modal functionality (per CLAUDE.md:166-171)

// Both templates should be wrapped like this:
<div class="bb-rl-screen-content">
    
</div>

2. Incorrect Text Domain (MEDIUM PRIORITY)

File: members/single/tutor/courses.php:54
Line: 'buddyboss-pro' should be 'buddyboss'

// Current (incorrect):
printf( wp_kses( _n( '<span class="bb-count">%d</span> Course', '<span class="bb-count">%d</span> Courses', $count, 'buddyboss-pro' ), /* ... */ )

// Should be:
printf( wp_kses( _n( '<span class="bb-count">%d</span> Course', '<span class="bb-count">%d</span> Courses', $count, 'buddyboss' ), /* ... */ )

3. Potential XSS Security Issue (LOW PRIORITY)

File: groups/single/courses.php:30
Issue: Direct echo of TutorLMS shortcode output without additional escaping

// Current:
echo tutor_lms()->shortcode->tutor_course( array( 'id' => $course_ids_string, 'show_pagination' => 'on', 'post_status' => array('publish', 'private') ) );

// Consider wrapping with wp_kses_post() if the shortcode output is not already escaped:
echo wp_kses_post( tutor_lms()->shortcode->tutor_course( /* ... */ ) );

4. Input Validation Improvement (LOW PRIORITY)

File: groups/single/courses.php:29 & members/single/tutor/courses.php:64
Issue: implode() on potentially non-array data

// Add validation before implode():
if ( ! empty( $course_ids ) && is_array( $course_ids ) ) {
    $course_ids_string = implode( ',', $course_ids );
    // ... rest of code
}

Performance Considerations

✅ Good Practices

Efficient early returns (bp-get-group-id() check)
Proper use of per_page => false for complete data retrieval
Array filtering and deduplication in member template

⚠️ Potential Issues

No caching implemented for course data retrieval
Multiple function calls in conditional logic could be optimized

Security Assessment

✅ Secure Practices

Uses intval() for ID sanitization
Proper array validation in member template
Uses wp_kses() for safe HTML output

⚠️ Minor Concerns

Direct shortcode output echoing (mentioned above)
Text domain inconsistency

Recommendations

Critical (Must Fix)

Add ReadyLaunch wrapper to both template files
Fix text domain in members template

Suggested Improvements

Add input validation before implode() calls
Consider caching course data for better performance
Review shortcode output escaping

Code Style

Follows WordPress coding standards ✅
Proper indentation and spacing ✅
Consistent with existing codebase ✅

Conclusion

The core functionality appears sound and addresses the pagination issue correctly. The main concerns are the missing ReadyLaunch wrapper (which could break modal functionality) and the text domain inconsistency. These should be addressed before merging.

The PR successfully implements TutorLMS course display in ReadyLaunch mode while maintaining compatibility with existing pagination filters.

reza-bb

Please make the necessary changes to meet the requirements.

reza-bb · 2026-04-13T12:49:06Z


-			add_filter( 'paginate_links_output', array( $this, 'bb_rl_filter_paginate_links_output' ), 10, 2 );
+			// Apply pagination filter to all pages except BuddyBoss user profile's (courses tab or any of its sub-tabs) when Tutor LMS is active.
+			if ( ! ( function_exists( 'tutor_utils' ) && bp_is_user() && preg_match( '/courses/i', bp_current_action() ) ) ) {


Avoid using preg_match, use any alternate for the validation.

Add more validation for bp_current_action.

Add proper validation for checking if tutor_lms is active or not.

reza-bb · 2026-04-13T12:53:48Z

+
+if ( ! empty( $bb_tutorlms_groups['courses'] ) && tutor_utils()->count( $bb_tutorlms_groups['courses'] ) ) {
+	$course_ids_string = implode( ',', $bb_tutorlms_groups['courses'] );
+	echo tutor_lms()->shortcode->tutor_course( array( 'id' => $course_ids_string, 'show_pagination' => 'on', 'post_status' => array('publish', 'private') ) );


Missing output escaping.

Missing validation.

Use the long array syntax with a trailing comma.

This is a generic file that is used for all the LMSs. Making strict changes for only one LMS may make the system fail for other LMSs. Test properly with all the LMSs.

reza-bb · 2026-04-13T13:07:07Z

+ *
+ * @since   2.4.40
+ *
+ * @package BuddyBoss\TutorLMS


This is a generic course page. TutorLMS cannot be mentioned strictly here.

reza-bb · 2026-04-13T13:08:29Z

+ *
+ * This template can be overridden by copying it to yourtheme/buddypress/groups/single/courses.php.
+ *
+ * @since   2.4.40


Never use version numbers hardcoded. Use @since [BBVERSION] to add the version number dynamically during the release build process.

reza-bb · 2026-04-13T13:48:50Z

+	return;
+}
+
+$bb_tutorlms_groups = bb_load_tutorlms_group()->get(


Missing validation for other LMSs

reza-bb · 2026-04-13T14:14:27Z

+
+			// Check if the item is a numeric ID (integer or string representation of an integer).
+			$course_ids[] = intval( $course );
+		} elseif ( is_object( $course ) ) {


missing validation

reza-bb · 2026-04-13T14:16:05Z

+if ( ! empty( $courses_list ) && is_array( $courses_list ) ) {
+	foreach ( $courses_list as $course ) {
+		if ( is_numeric( $course ) ) {
+


Indentation issue

reza-bb · 2026-04-13T14:16:10Z

+
+			// If the item is an object, you can access its ID property.
+			$course_ids[] = $course->ID;
+		}


Fix indentation issue

reza-bb · 2026-04-13T14:17:25Z

+$course_ids = array_unique( $course_ids );
+
+if ( ! function_exists( 'bb_enable_content_counts' ) || bb_enable_content_counts() ) {
+	$count = count( $course_ids );


count is a reserved word for PHP. Use something self-explaining

reza-bb · 2026-04-13T14:18:20Z

+		<?php
+		/* translators: %d is the courses count */
+		printf(
+			wp_kses( _n( '<span class="bb-count">%d</span> Course', '<span class="bb-count">%d</span> Courses', $count, 'buddyboss-pro' ), array( 'span' => array( 'class' => true ) ) ),


Wrong text domain.

sajib-bb added 2 commits April 10, 2026 10:44

Added the missing courses template files into the ReadyLaunch directo…

6649b25

…ry to ensure proper compatibility with Tutor LMS when ReadyLaunch is active

typo in commetns

31dd94d

reza-bb reviewed Apr 14, 2026

View reviewed changes

Conversation

sajib-bb commented Apr 10, 2026

Jira Issue:

General Note

Notes to Developer

Notes to Reviewer

Uh oh!

welcome bot commented Apr 10, 2026

Uh oh!

claude bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Complete

Review Checklist

Summary

Code Quality Assessment

✅ Strengths

⚠️ Issues Identified

1. Missing ReadyLaunch Template Wrapper (HIGH PRIORITY)

2. Incorrect Text Domain (MEDIUM PRIORITY)

3. Potential XSS Security Issue (LOW PRIORITY)

4. Input Validation Improvement (LOW PRIORITY)

Performance Considerations

✅ Good Practices

⚠️ Potential Issues

Security Assessment

✅ Secure Practices

⚠️ Minor Concerns

Recommendations

Critical (Must Fix)

Suggested Improvements

Code Style

Conclusion

Uh oh!

claude bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Complete

Review Checklist

Summary

Code Quality Assessment

✅ Strengths

⚠️ Issues Identified

1. Missing ReadyLaunch Template Wrapper (HIGH PRIORITY)

2. Incorrect Text Domain (MEDIUM PRIORITY)

3. Potential XSS Security Issue (LOW PRIORITY)

4. Input Validation Improvement (LOW PRIORITY)

Performance Considerations

✅ Good Practices

⚠️ Potential Issues

Security Assessment

✅ Secure Practices

⚠️ Minor Concerns

Recommendations

Critical (Must Fix)

Suggested Improvements

Code Style

Conclusion

Uh oh!

reza-bb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reza-bb Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude bot commented Apr 10, 2026 •

edited

Loading

claude bot commented Apr 10, 2026 •

edited

Loading

reza-bb Apr 13, 2026 •

edited

Loading