Skip to content
Closed
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2516,6 +2516,9 @@ def _list_instructor_tasks(request, course_id, serialize_data=None):
else:
# Specifying for single problem's history
tasks = task_api.get_instructor_task_history(course_id, module_state_key)
elif request.GET.get('include_canvas') is not None:
from ol_openedx_canvas_integration.task_helpers import get_filtered_instructor_tasks # pylint: disable=import-error
tasks = get_filtered_instructor_tasks(course_id, request.user)
else:
# If no problem or student, just get currently running tasks
tasks = task_api.get_running_instructor_tasks(course_id)
Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/instructor_task/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ def get_task_completion_info(instructor_task): # lint-amnesty, pylint: disable=

student = None
problem_url = None
entrance_exam_url = None
email_id = None
task_input = None
try:
task_input = json.loads(instructor_task.task_input)
except ValueError:
Expand Down Expand Up @@ -192,6 +194,10 @@ def get_task_completion_info(instructor_task): # lint-amnesty, pylint: disable=
else: # num_succeeded < num_attempted
# Translators: {action} is a past-tense verb that is localized separately. {succeeded} and {attempted} are counts. # lint-amnesty, pylint: disable=line-too-long
msg_format = _("Problem {action} for {succeeded} of {attempted} students")
elif task_input and task_input.get('course_key'):
from ol_openedx_canvas_integration.utils import get_task_output_formatted_message # pylint: disable=import-error
msg_format = get_task_output_formatted_message(task_output)
succeeded = True
elif email_id is not None:
# this reports on actions on bulk emails
if num_attempted == 0:
Expand Down
20 changes: 18 additions & 2 deletions lms/djangoapps/lti_provider/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
"""
Tests for the LTI provider views
"""


from unittest.mock import MagicMock, patch

from django.contrib.auth.models import AnonymousUser
from django.test import TestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
from django.urls import reverse
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator

Expand Down Expand Up @@ -121,6 +120,23 @@ def test_valid_launch_with_optional_params(self, _authenticate, store_params, _r
self.consumer
)

@patch('lms.djangoapps.lti_provider.views.render_courseware')
@patch('lms.djangoapps.lti_provider.views.store_outcome_parameters')
@patch('lms.djangoapps.lti_provider.views.authenticate_lti_user')
@override_settings(LTI_CUSTOM_PARAMS=["extra_param1", "extra_param2"])
def test_valid_launch_with_extra_params(self, _authenticate, store_params, _render):
"""
Verifies that the LTI launch succeeds when passed a valid request.
"""
extra_params = {'extra_param1': 'extra_value1', 'extra_param2': "extra_value2"}
request = build_launch_request(extra_post_data=LTI_OPTIONAL_PARAMS | extra_params)
views.lti_launch(request, str(COURSE_KEY), str(USAGE_KEY))
store_params.assert_called_with(
dict(list(ALL_PARAMS.items()) + list(LTI_OPTIONAL_PARAMS.items()) + list(extra_params.items())),
request.user,
self.consumer
)
Comment on lines +134 to +138

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The dictionary construction for the assertion is a bit verbose. Since you're using the | operator for dictionary union on line 132, you could use it here as well to make the code more concise and readable.

Suggested change
store_params.assert_called_with(
dict(list(ALL_PARAMS.items()) + list(LTI_OPTIONAL_PARAMS.items()) + list(extra_params.items())),
request.user,
self.consumer
)
store_params.assert_called_with(
ALL_PARAMS | LTI_OPTIONAL_PARAMS | extra_params,
request.user,
self.consumer
)


@patch('lms.djangoapps.courseware.views.views.render_xblock')
@patch('lms.djangoapps.lti_provider.views.authenticate_lti_user')
def test_render_xblock_params(self, _authenticate, render):
Expand Down
18 changes: 18 additions & 0 deletions lms/djangoapps/lti_provider/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from lms.djangoapps.lti_provider.outcomes import store_outcome_parameters
from lms.djangoapps.lti_provider.signature_validator import SignatureValidator
from lms.djangoapps.lti_provider.users import authenticate_lti_user
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.lib.url_utils import unquote_slashes

log = logging.getLogger("edx.lti_provider")
Expand Down Expand Up @@ -59,6 +60,7 @@ def lti_launch(request, course_id, usage_id):
if not params:
return HttpResponseBadRequest()
params.update(get_optional_parameters(request.POST))
params.update(get_custom_parameters(request.POST))

# Get the consumer information from either the instance GUID or the consumer
# key
Expand Down Expand Up @@ -145,6 +147,22 @@ def get_optional_parameters(dictionary):
return {key: dictionary[key] for key in OPTIONAL_PARAMETERS if key in dictionary}


def get_custom_parameters(params: dict[str]) -> dict[str]:
"""
Extract all optional LTI parameters from a dictionary. This method does not
fail if any parameters are missing.

:param params: A dictionary containing zero or more parameters.
:return: A new dictionary containing all optional parameters from the
original dictionary, or an empty dictionary if no optional parameters
were present.
"""
Comment on lines +150 to +159

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The type hint for params is invalid. It should be dict[str, str] since request.POST values are strings. Also, the docstring seems to be a copy-paste from get_optional_parameters and is misleading. It should refer to "custom" LTI parameters, not "optional" ones, to avoid confusion.

Suggested change
def get_custom_parameters(params: dict[str]) -> dict[str]:
"""
Extract all optional LTI parameters from a dictionary. This method does not
fail if any parameters are missing.
:param params: A dictionary containing zero or more parameters.
:return: A new dictionary containing all optional parameters from the
original dictionary, or an empty dictionary if no optional parameters
were present.
"""
def get_custom_parameters(params: dict[str, str]) -> dict[str, str]:
"""
Extract all custom LTI parameters from a dictionary. This method does not
fail if any parameters are missing.
:param params: A dictionary containing zero or more parameters.
:return: A new dictionary containing all custom parameters from the
original dictionary, or an empty dictionary if no custom parameters
were present.
"""

custom_params = configuration_helpers.get_value("LTI_CUSTOM_PARAMS", settings.LTI_CUSTOM_PARAMS)
if not custom_params:
return {}
return {key: params[key] for key in custom_params if key in params}


def render_courseware(request, usage_key):
"""
Render the content requested for the LTI launch.
Expand Down
10 changes: 10 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -4450,6 +4450,16 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
# The time value is in seconds.
LTI_AGGREGATE_SCORE_PASSBACK_DELAY = 15 * 60


# .. setting_name: LTI_CUSTOM_PARAMS
# .. setting_default: []
# .. setting_description: This expands the list of optional LTI parameters that the
# platform accepts. These parameters are not used by the platform, but can then
# be used by other plugins.
# .. setting_creation_date: 2025-08-22
# .. setting_tickets:
LTI_CUSTOM_PARAMS = []

# Credit notifications settings
NOTIFICATION_EMAIL_CSS = "templates/credit_notifications/credit_notification.css"
NOTIFICATION_EMAIL_EDX_LOGO = "templates/credit_notifications/edx-logo-header.png"
Expand Down
3 changes: 3 additions & 0 deletions lms/static/js/instructor_dashboard/instructor_dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ such that the value can be defined later than this assignment (file load order).
}, {
constructor: window.InstructorDashboard.sections.ECommerce,
$element: idashContent.find('.' + CSS_IDASH_SECTION + '#e-commerce')
}, {
constructor: window.InstructorDashboard.sections.CanvasIntegration,
$element: idashContent.find('.' + CSS_IDASH_SECTION + '#canvas_integration')
}, {
constructor: window.InstructorDashboard.sections.Membership,
$element: idashContent.find('.' + CSS_IDASH_SECTION + '#membership')
Expand Down
6 changes: 4 additions & 2 deletions lms/static/js/instructor_dashboard/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
enableColumnReorder: false,
autoHeight: true,
rowHeight: 100,
forceFitColumns: true
forceFitColumns: true,
enableTextSelectionOnCells: true
};
columns = [
{
Expand Down Expand Up @@ -492,7 +493,8 @@
enableCellNavigation: true,
enableColumnReorder: false,
rowHeight: 30,
forceFitColumns: true
forceFitColumns: true,
enableTextSelectionOnCells: true
};
columns = [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<p>
<h2><%- title %></h2>
<table class="stat_table"><tr>
<%_.forEach(header, function (h) {%>
<th><%- h %></th>
<%})%>
</tr>
<%_.forEach(data, function (row) {%>
<tr>
<%_.forEach(row, function (value) {%>
<td><%- value %></td>
<%})%>
</tr>
<%})%>
</table>
</p>
Comment on lines +1 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The HTML structure is invalid because an <h2> element cannot be a direct child of a <p> element. This can lead to unpredictable rendering in some browsers. You should use a <div> as the container instead.

<div>
    <h2><%- title %></h2>
    <table class="stat_table"><tr>
        <%_.forEach(header, function (h) {%>
            <th><%- h %></th>
        <%})%>
        </tr>
        <%_.forEach(data, function (row) {%>
            <tr>
                <%_.forEach(row, function (value) {%>
                    <td><%- value %></td>
                <%})%>
            </tr>
        <%})%>
    </table>
</div>

Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@

## Include Underscore templates
<%block name="header_extras">
% for template_name in ["cohorts", "discussions", "enrollment-code-lookup-links", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state", "divided-discussions-inline", "divided-discussions-course-wide", "cohort-discussions-category", "cohort-discussions-subcategory", "certificate-allowlist", "certificate-allowlist-editor", "certificate-bulk-allowlist", "certificate-invalidation", "membership-list-widget"]:
% for template_name in ["cohorts", "discussions", "enrollment-code-lookup-links", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state", "divided-discussions-inline", "divided-discussions-course-wide", "cohort-discussions-category", "cohort-discussions-subcategory", "certificate-allowlist", "certificate-allowlist-editor", "certificate-bulk-allowlist", "certificate-invalidation", "membership-list-widget", "html-datatable"]:
<script type="text/template" id="${template_name}-tpl">
<%static:include path="instructor/instructor_dashboard_2/${template_name}.underscore" />
</script>
Expand Down
22 changes: 21 additions & 1 deletion lms/templates/seq_block.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@
from django.conf import settings
%>

<script type="text/template" id="dropdown-button-tpl">
<li id="dropdown-container" class="h-100">
<button
id="dropdown-sequence-list-button"
class="dropdown-toggle"
type="button"
>
<span class="icon fa fa-chevron-down" aria-hidden="true"></span>
</button>
<div id="dropdown-sequence-list" style="display: none; position: absolute; width: 240px; right: 0;">
<ol class="d-block dropdown-menu bg-white py-0 shadow-sm border"
aria-labelledby="dropdown-sequence-list-button">
</ol>
</div>
</li>
</script>

<div id="sequence_${element_id}" class="sequence" data-id="${item_id}"
data-position="${position}"
data-next-url="${next_url}" data-prev-url="${prev_url}"
Expand Down Expand Up @@ -46,7 +63,7 @@
</li>
% else:
% for idx, item in enumerate(items):
<li role="presentation">
<li role="presentation" class="sequence-list-item">
<button class="seq_${item['type']} inactive nav-item tab"
role="tab"
tabindex="-1"
Expand Down Expand Up @@ -98,6 +115,9 @@
</ul>
</li>
% endif
% if show_dropdown:
## <%include file='seq_dropdown.html' args="items=items[15:], start_index=15"/>
% endif
</ol>
</nav>
</div>
Expand Down
76 changes: 76 additions & 0 deletions xmodule/js/src/sequence/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
(function() {
'use strict';

const { HtmlUtils } = window.edx;

this.Sequence = (function() {
function Sequence(element, runtime) {
var self = this;
Expand All @@ -26,6 +28,9 @@
this.goto = function(event) {
return Sequence.prototype.goto.apply(self, [event]);
};
this.toggleDropdown = function(event) {
return Sequence.prototype.toggleDropdown.apply(self, [event]);
};
this.toggleArrows = function() {
return Sequence.prototype.toggleArrows.apply(self);
};
Expand All @@ -38,6 +43,12 @@
this.displayTabTooltip = function(event) {
return Sequence.prototype.displayTabTooltip.apply(self, [event]);
};
this.renderDropdown = function() {
return Sequence.prototype.renderDropdown.apply(self);
}
this.handleClickOutsideDropdown = function(event) {
return Sequence.prototype.handleClickOutsideDropdown.apply(self, [event]);
}
this.arrowKeys = {
LEFT: 37,
UP: 38,
Expand All @@ -62,23 +73,87 @@
this.showCompletion = this.el.data('show-completion');
this.keydownHandler($(element).find('#sequence-list .tab'));
this.base_page_title = ($('title').data('base-title') || '').trim();
this.dropdownButtonTpl = _.template($('#dropdown-button-tpl').text())({});
this.renderDropdown();
this.bind();
this.render(parseInt(this.el.data('position'), 10));
}

Sequence.prototype.renderDropdown = function() {
// Renders the dropdown when there isn't enough space for all units in the bar
// Hide the dropdown by default and only show if needed.
this.$('#sequence-list > #dropdown-container').hide();
this.$(`#sequence-list > li.sequence-list-item`).show();
// Calculate the number of tabs that can fit comfortably and if the
// number of units is greater we show the dropdown.
const tabListWidth = this.$('#sequence-list').width();
const singleTabWidth = this.$('#sequence-list > li:first').width();
const tabCount = this.$('#sequence-list > li.sequence-list-item').length;
const overFlowCount = Math.floor(tabListWidth / singleTabWidth);
// Reduce 1 to offsets index and another one to accommodate the button
const overFlowIdx = overFlowCount - 2;
const showDropdown = overFlowCount < tabCount;
if (!showDropdown) {
return;
}
// If the dropdown button doesn't exist add it, otherwise move the
// existing button to the correct place.
if (this.$('#sequence-list > #dropdown-container').length === 0) {
// xss-lint: disable=javascript-jquery-insertion
this.$('#sequence-list > li.sequence-list-item').eq(overFlowIdx).after(this.dropdownButtonTpl);
} else {
this.$('#sequence-list > li.sequence-list-item').eq(overFlowIdx)
// xss-lint: disable=javascript-jquery-insertion
.after(this.$('#sequence-list > #dropdown-container'));
}
// Show the dropdown UX and hide all the overflowing unit buttons.
this.$('#sequence-list > #dropdown-container').show();
this.$(`#sequence-list > li.sequence-list-item:lt(${overFlowIdx + 1})`).show();
this.$(`#sequence-list > li.sequence-list-item:gt(${overFlowIdx})`).hide();
const dropdownList = this.$('#dropdown-sequence-list > ol');
// The dropdown buttons are modified copies of the unit nav buttons.
dropdownList.empty();
this.$(`#sequence-list > li.sequence-list-item:gt(${overFlowIdx})`).each((idx, el) => {
const cloneEl = $(el).clone();
const navButton = cloneEl.find("button");
const unitTitle = navButton.data('page-title');
navButton.click(self.goto);
navButton.find(".sequence-tooltip").remove();
navButton.find("span.icon").after(
HtmlUtils.joinHtml(HtmlUtils.HTML('<span class="unit-title">'), unitTitle, HtmlUtils.HTML('</span>')).toString()
);
//xss-lint: disable=javascript-jquery-insert-into-target
cloneEl.show().appendTo(dropdownList);
});
}

Sequence.prototype.$ = function(selector) {
return $(selector, this.el);
};

Sequence.prototype.bind = function() {
this.$('#sequence-list .nav-item').click(this.goto);
$(document).click(this.handleClickOutsideDropdown);
this.$('#dropdown-sequence-list .dropdown-item').click(this.goto);
this.$('#dropdown-sequence-list-button').click(this.toggleDropdown);
this.$('#sequence-list .nav-item').keypress(this.keyDownHandler);
this.el.on('bookmark:add', this.addBookmarkIconToActiveNavItem);
this.el.on('bookmark:remove', this.removeBookmarkIconFromActiveNavItem);
this.$('#sequence-list .nav-item').on('focus mouseenter', this.displayTabTooltip);
this.$('#sequence-list .nav-item').on('blur mouseleave', this.hideTabTooltip);
$(window).on('resize', _.debounce(this.renderDropdown.bind(this), 200));
};

Sequence.prototype.handleClickOutsideDropdown = function(event) {
if(!this.$('#dropdown-container')?.[0]?.contains(event.target)) {
this.$('#dropdown-sequence-list').hide();
}
}

Sequence.prototype.toggleDropdown = function() {
$('#dropdown-sequence-list').toggle();
}

Sequence.prototype.previousNav = function(focused, index) {
var $navItemList,
$sequenceList = $(focused).parent().parent();
Expand Down Expand Up @@ -289,6 +364,7 @@
Sequence.prototype.goto = function(event) {
var alertTemplate, alertText, isBottomNav, newPosition, widgetPlacement;
event.preventDefault();
this.$('#dropdown-sequence-list').hide();

// Links from courseware <a class='seqnav' href='n'>...</a>, was .target_tab
if ($(event.currentTarget).hasClass('seqnav')) {
Expand Down
Loading
Loading