Disenrolled courses still appear in "my-courses" page

alaa-alshamy commented 3 years ago

Reproduction Steps

Login as a student
Enroll in any course
As an admin disenroll that student from the course
Back to student and open "my-courses" page, and will see the disenrolled course there

Expected Behavior

Should disappear because it's not belong to that student anymore

Actual Behavior

Disenrolled courses still appear

Note

If i open the course page i will see the Enroll button, So that course from my-courses is really not mine

and actually it's more obvious in my case, because i made a plugin that provide the Disenroll functionality for the students (i would like to share it with you if you want ), so it's not expected for student to disenroll from a course and he find it in "my-courses" page

Error Messages / Logs

Include any relevant error messages or log files


<!-- Paste error logs / backtraces below this line -->


### System and Environment Information

<details>
<summary>System Report</summary>

<!-- Paste your System Report between the three backticks below this line -->



</details>

This issue has be recreated:
+ [ ] Locally
+ [ ] On a staging site
+ [ ] On a production website
+ [x] With only LifterLMS and a default theme

### Browser, Device, and Operating System Information

+ Browser name and version
+ Operating System name and version
+ Device name and version (if applicable)

alaa-alshamy commented 3 years ago

@thomasplevy What do you think ?

thomasplevy commented 3 years ago

The list of courses on the dashboard is not intended to be a list of courses a user is currently enrolled in. It's a list of courses the user has ever been enrolled in.

An import scenario to consider is if a student's payment fails their enrollment status changes to unenrolled and they can update their payment information to gain access again. They still have progress in that course and they can continue where they left off when they're re-enrolled.

Perhaps what you're looking to do is not unenroll a student but delete their enrollment. You can read how to delete enrollment here and some more information on enrollment statuses here

As this is the intended functionality I cannot justify changing the behavior as you did in the PR #1430 you opened about this. This would create a regression in the eyes of some users.

However, I cannot say for sure that your the only one considering the current behavior to be a bug. In this scenario since there's already a filter on the query (see below) you can easily adjust this behavior on your site right now.

https://github.com/gocodebox/lifterlms/blob/4793ff7477435c529c56cb512e9919d4b987321a/includes/functions/llms.functions.te mplates.dashboard.php#L167

If this is a problem because of an add-on you've developed, then I'd suggest adding additional functionality into your add-on, possibly using your filter.

Alternatively, I'm not opposed to adding an option somewhere to allow customization of this functionality on the dashboard page.

I think a better solution would be to build an option that allows site owners to determine the behavior of this query. We already have options to determine the sort order of the list (LifterLMS -> Settings -> Accounts -> "Student Dashboard: Courses Sorting") and I could see an additional multi-select option being added here to allow admins to select the enrollment statuses of the courses displayed in this list.

In this way we can ensure we don't create a regression for anyone appreciating the current functionality while simultaneously allowing for alternate interpretations of the way in which this list should function.

alaa-alshamy commented 3 years ago

Mmmm i got your point, i think you are right and your solution you mentioned is the best option :+1:

gocodebox / lifterlms