Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add script to count unique learnersin Sensei #2172

Closed
wants to merge 3 commits into from
Closed

Add script to count unique learnersin Sensei #2172

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
456da99
Add script to count unique learnersin Sensei
Jan 23, 2024
29dbaf0
Fixing lint errors
Jan 23, 2024
ac6c837
Apply suggestions from code review
kaitohm Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions wp-content/plugins/wporg-learn/inc/sensei.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,60 @@ function wporg_fix_learning_mode_header_space() {
wp_add_inline_style( 'learning-mode-header-fix', $custom_styles );
}
add_action( 'sensei_course_learning_mode_load_theme', __NAMESPACE__ . '\wporg_fix_learning_mode_header_space' );

/**
* Add script to count unique learners
*/
function wporg_learn_add_student_count_to_reports( $type ) {
if ( 'users' !== $type ) {
return; // Only show the count on the students report screen.
}

$from_date = \DateTime::createFromFormat( 'Y-m-d', $_GET['from_date'] ?? '', new \DateTimeZone( 'UTC' ) );
$to_date = \DateTime::createFromFormat( 'Y-m-d', $_GET['to_date'] ?? '', new \DateTimeZone( 'UTC' ) );

global $wpdb;
$query = "SELECT COUNT(DISTINCT user_id)
FROM $wpdb->comments
WHERE comment_type = 'sensei_course_status'";

if ( $from_date ) {
$from_date->setTime( 0, 0, 0 );

$query .= " AND comment_date_gmt >= '" . $from_date->format( 'Y-m-d H:i:s' ) . "'";
}

if ( $to_date ) {
$to_date->setTime( 23, 59, 59 );

$query .= " AND comment_date_gmt <= '" . $to_date->format( 'Y-m-d H:i:s' ) . "'";
}

$student_count = $wpdb->get_var( $query ); //phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pkevan Ben and I discussed this, and we feel ignoring the PHPCS rule here is ok, but we'd appreciate your opinion. I personally can't see how this could be attacked via SQL injection, but maybe I'm missing something.

Copy link
Contributor

Choose a reason for hiding this comment

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

While it's true that the current code isn't vulnerable, that doesn't mean in the future it wouldn't be, so it's preferable to built in elements which would force it to not be.

For example, the $from_date variable assignment logic could be changed to grab from the $_GET variable directly or the appended $query concatenation logic changed in a way which did make it.

By adding some prepare statements, it would help avoid those scenarios, particularly when dealing with user input.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@pkevan Thanks, that's a fair point, I will work on adding sanitization to the date variables, and change the SQL concatenation to use a prepare statement.


?>
<div class="actions bulkactions">
<label><?php esc_html_e( 'Total number of students', 'wporg-learn' ); ?></label>
<input
class="sensei-date-picker"
name="from_date"
type="text"
autocomplete="off"
placeholder="<?php echo esc_attr( __( 'From Date', 'wporg-learn' ) ); ?>"
value="<?php echo esc_attr( $from_date ? $from_date->format( 'Y-m-d' ) : '' ); ?>"
/>
<input
class="sensei-date-picker"
name="to_date"
type="text"
autocomplete="off"
placeholder="<?php echo esc_attr( __( 'To Date', 'wporg-learn' ) ); ?>"
value="<?php echo esc_attr( $to_date ? $to_date->format( 'Y-m-d' ) : '' ); ?>"
/>
<label>: <?php echo (int) $student_count; ?></label>
</div>
<br>
<?php
}
add_action( 'sensei_reports_overview_before_top_filters', __NAMESPACE__ . '\wporg_learn_add_student_count_to_reports' );

Loading