Skip to content

[FIX] LTI: Provide outcome status lists for LP collections#11641

Merged
Saaweel merged 1 commit into
ILIAS-eLearning:release_11from
surlabs:release_11
Jun 3, 2026
Merged

[FIX] LTI: Provide outcome status lists for LP collections#11641
Saaweel merged 1 commit into
ILIAS-eLearning:release_11from
surlabs:release_11

Conversation

@Saaweel

@Saaweel Saaweel commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Certain missing and/or inherited functions have been added to ensure that learning progress is correctly transferred to the parent object in all cases.

Mantis ticket: https://mantis.ilias.de/view.php?id=41919

@Saaweel Saaweel self-assigned this Jun 3, 2026
@Saaweel Saaweel added bugfix php Pull requests that update Php code labels Jun 3, 2026
@Saaweel Saaweel merged commit 583fa0a into ILIAS-eLearning:release_11 Jun 3, 2026
5 checks passed
Comment on lines +37 to +47
$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;

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants