Infinite redirect loop with finished but not finalized learning path

Created on 25 August 2023, about 1 year ago
Updated 4 June 2024, 6 months ago

Problem/Motivation

We have a user with a finished but not finalized and a new learning path:

```
MariaDB [db]> select id, uid, gid, score, status, started, finished, finalized from user_lp_status where uid = 8;
+----+-----+-----+-------+-------------+------------+------------+-----------+
| id | uid | gid | score | status | started | finished | finalized |
+----+-----+-----+-------+-------------+------------+------------+-----------+
| 8 | 8 | 6 | 100 | passed | 1692101358 | 1692101871 | 1 |
| 9 | 8 | 6 | 100 | passed | 1692103335 | 1692105548 | 0 |
| 10 | 8 | 6 | NULL | in progress | 1692105548 | 0 | 0 |
+----+-----+-----+-------+-------------+------------+------------+-----------+
```

This results in one place creating module statues for lp_status 10, but \Drupal\opigno_module\Entity\OpignoModule::getTrainingActiveAttempt() finds 9 and then can't find a module status for it and redirects back, which creates another new module status for 10.

Steps to reproduce

Not 100% clear, but happens frequently in testing when going through modules multiple times in non-guided mode for us.

Proposed resolution

My workaround was to add a ->sort('started', 'DESC') so we get lp status 10, but maybe it should look at finished instead and not finalized? I don't fully understand the difference between the two and how you finalize a finished lp_status, because 9 has no unfinished module attempts.

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

RTBC

Version

3.0

Component

Code

Created by

πŸ‡¨πŸ‡­Switzerland berdir Switzerland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @berdir
  • Status changed to Needs review about 1 year ago
  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland
  • Status changed to RTBC 6 months ago
  • πŸ‡ΈπŸ‡ͺSweden johnzzon Gothenburg πŸ‡ΈπŸ‡ͺ

    We encountered this and I spent a couple of hours debugging it today.

    I came to the same conclusions as you, I just wish I found this issue earlier. I only looked in the opigno_lms issue queue.

    My fix was a condition on finished, as you discuss. But a sort will effectively do the same.

    The original issue seems to stem from that when you restart a learning path, it finishes the unfinished LP statuses first, and then finishes unfinished module statuses. However, the finalize logic in LPStatus::setFinished will not finalize if there are unfinished modules.

    I know too little of the code base to determine whether it would be better to finish unfinished modules before finishing unfinished LP statuses, but I think that would also solve the issue, because the previous LP status would be finalized.

Production build 0.71.5 2024