8000 Course pinning, search and filter feature in user's course list by AlexandreDoneux · Pull Request #1084 · INGInious/INGInious · GitHub
[go: up one dir, main page]

Skip to content

Course pinning, search and filter feature in user's course list#1084

Open
AlexandreDoneux wants to merge 18 commits intoINGInious:mainfrom
AlexandreDoneux:UI_course_list
Open

Course pinning, search and filter feature in user's course list#1084
AlexandreDoneux wants to merge 18 commits intoINGInious:mainfrom
AlexandreDoneux:UI_course_list

Conversation

@AlexandreDoneux
Copy link
Contributor

This PR aims to improve the user experience regarding the user's course list. Currently displayed as a basic list, it can get difficult to find a specific course if you manage a lot of them. This PR brings multiple ways to improve this list :

  • the possibility to pin certain courses to find them in a designated list
  • a basic search input to limit the courses displayed in the user's courses list
  • filters to limit the displayed courses in that same list. For each filter (archived, LTI, hidden, pinned), you can choose whether to show courses that match or don’t match the filter.

This PR is based on the #1077 PR. Therefore it will stay as draft until those changes are accepted.

10BC0
@anthonygego
Copy link
Member
anthonygego commented Dec 15, 2025

BTW take a look at the Codacy report, there are many JS issues reported such as the usage of eval for instance, which is discouraged.

@AlexandreDoneux AlexandreDoneux marked this pull request as ready for review December 19, 2025 10:58
Copy link
Member
@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

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

I would not include the descriptions in the pinned cards, and rather use the internal link for the course as this is the case for the full list in this view.

Otherwise just cosmetic and more idiomatic suggestions.

$(document).find("#no_pin_message").addClass("d-none");
}

} else { // unpinning
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can provide an upin icon from the pinned card too. This would allow to remove a pinned course without going to the possibly long list of courses.

Comment on lines +106 to +111
if (filter_value === "true" && !$item.hasClass(filter_id)) {
visible = false;
}
if (filter_value === "false" && $item.hasClass(filter_id)) {
visible = false;
}
Copy link
Member

Choose a reason for hiding this comment

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

Would this be equivalent ?

Suggested change
if (filter_value === "true" && !$item.hasClass(filter_id)) {
visible = false;
}
if (filter_value === "false" && $item.hasClass(filter_id)) {
visible = false;
}
if (!$item.hasClass(filter_id)) {
visible = !(filter_value === "true" || filter_value === "false");
}

Comment on lines +123 to +125



Copy link
Member

Choose a reason for hiding this comment

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

You can remove those spaces :-)

Comment on lines +659 to +666
data = User.objects(username=username).first()
if data:
pinned_courses = data.pinned_courses if data else []
if courseid not in pinned_courses:
pinned_courses.append(courseid)
User.objects(username=username).update(pinned_courses=pinned_courses)
return True
return False
Copy link
Member

Choose a reason for hiding this comment

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

I think you can do this more effenciently using the $addToSet MongoDB operator, as you did for the unpin_course method and the $pull operator.

Suggested change
data = User.objects(username=username).first()
if data:
pinned_courses = data.pinned_courses if data else []
if courseid not in pinned_courses:
pinned_courses.append(courseid)
User.objects(username=username).update(pinned_courses=pinned_courses)
return True
return False
modified = User.objects(username=username).update(add_to_set__pinned_courses=courseid)
return modified is not None

'../../frontend/static/js/groups.js',
'../../frontend/static/js/checked-list-group.js',
'../../frontend/static/js/task_dispensers.js',
'../../frontend/static/js/admin.js',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'../../frontend/static/js/admin.js',
'../../frontend/static/js/mycourses.js'

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0