Skip to content
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,54 @@ class ilLPStatusLtiOutcome extends ilLPStatus
{
private static array $userResultCache = array();

private static function getUsersByStatus(int $a_obj_id, int $a_status): array
{
global $DIC;

$ilDB = $DIC['ilDB'];

$usr_ids = array();
$query = 'SELECT DISTINCT usr_id FROM ut_lp_marks'
. ' WHERE obj_id = ' . $ilDB->quote($a_obj_id, 'integer')
. ' AND status = ' . $ilDB->quote($a_status, 'integer');

$res = $ilDB->query($query);
while ($row = $ilDB->fetchAssoc($res)) {
$usr_ids[] = (int) $row['usr_id'];
}

return $usr_ids;
Comment on lines +37 to +47

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @Saaweel ,

I just stumbled across these changes and am not sure whether the overridden methods have been implemented as intended. From my understanding _getInProgress, _getCompleted, _getFailed and _getNotAttempeted are expected to return lists of user IDs for the respective status based on the underlying learning progress data, rather than on the aggregated status values stored in ut_lp_marks.

Let's assume that \ilLPStatusWrapper::_refreshStatus() is invoked for the consuming object. This eventually calls \ilLPStatus::refreshStatus(), which in turn invokes the methods overridden here.

The returned user ID lists are then used to persist the corresponding learning progress status in ut_lp_marks.

Kind regards,
Michael

@Saaweel Saaweel Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @mjansenDatabay ,

thanks for the feedback, and sorry for the late response. I had not seen your comment earlier because, for some reason, GitHub did not notify me about it.

I reviewed this again and agree that these methods should not derive their user lists from ut_lp_marks, since they can also be used during ilLPStatusWrapper::_refreshStatus() to recalculate and persist that table.

I prepared a follow-up PR that derives the LTI outcome status lists from the underlying outcome data instead:
#11667

The lists now use lti_consumer_results and lti_consumer_grades, if available, and classify users through the existing determineStatus() logic. This keeps parent object / LP collection aggregation working without depending on the aggregated LP table.

Kind regards,
Saúl

}

public static function _getInProgress(int $a_obj_id): array
{
return self::getUsersByStatus($a_obj_id, self::LP_STATUS_IN_PROGRESS_NUM);
}

public static function _getCompleted(int $a_obj_id): array
{
return self::getUsersByStatus($a_obj_id, self::LP_STATUS_COMPLETED_NUM);
}

public static function _getFailed(int $a_obj_id): array
{
return self::getUsersByStatus($a_obj_id, self::LP_STATUS_FAILED_NUM);
}

public static function _getNotAttempted(int $a_obj_id): array
{
$members = ilObjectLP::getInstance($a_obj_id)->getMembers();
if (!$members) {
return array();
}

$users = array_diff((array) $members, self::_getInProgress($a_obj_id));
$users = array_diff($users, self::_getCompleted($a_obj_id));
$users = array_diff($users, self::_getFailed($a_obj_id));

return $users;
}

private function getLtiUserResult(
int $objId,
int $usrId
Expand Down
Loading