Skip to content

Commit

Permalink
feat: [ACADEMIC-16209] Unit summary settings (openedx#32855)
Browse files Browse the repository at this point in the history
* feat: [ACADEMIC-16209] Unit summary settings

[https://jira.2u.com/browse/ACADEMIC-16209](https://jira.2u.com/browse/ACADEMIC-16209)

1. Add unit xpert unit summaries settings behind flag `summaryhook_summaries_configuration` added [here](https://github.com/edx/ai-aside/pull/45/files)
2. Only show the checkbox when the value is a `boolean` otherwise the feature is considerer `disabled` by the flag.
3. Update block handler to update this value via `api` exposed [here](edx/ai-aside#43)
4. Create `AiAsideSummary` configuration class to provide access to the `ai_aside.api` endpoints.
  • Loading branch information
germanolleunlp authored Aug 21, 2023
1 parent d60cdc2 commit 3f20c75
Show file tree
Hide file tree
Showing 10 changed files with 237 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG
from cms.djangoapps.contentstore.toggles import ENABLE_COPY_PASTE_UNITS
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig
from common.djangoapps.edxmako.services import MakoService
from common.djangoapps.static_replace import replace_static_urls
from common.djangoapps.student.auth import (
Expand Down Expand Up @@ -280,6 +281,7 @@ def modify_xblock(usage_key, request):
prereq_min_completion=request_data.get("prereqMinCompletion"),
publish=request_data.get("publish"),
fields=request_data.get("fields"),
summary_configuration_enabled=request_data.get("summary_configuration_enabled"),
)


Expand Down Expand Up @@ -354,6 +356,7 @@ def _save_xblock( # lint-amnesty, pylint: disable=too-many-statements
prereq_min_completion=None,
publish=None,
fields=None,
summary_configuration_enabled=None,
):
"""
Saves xblock w/ its fields. Has special processing for grader_type, publish, and nullout and Nones in metadata.
Expand Down Expand Up @@ -530,6 +533,12 @@ def _save_xblock( # lint-amnesty, pylint: disable=too-many-statements
if publish == "make_public":
modulestore().publish(xblock.location, user.id)

# If summary_configuration_enabled is not None, use AIAsideSummary to update it.
if xblock.category == "vertical" and summary_configuration_enabled is not None:
AiAsideSummaryConfig(course.id).set_summary_settings(xblock.location, {
'enabled': summary_configuration_enabled
})

# Note that children aren't being returned until we have a use case.
return JsonResponse(result, encoder=EdxJSONEncoder)

Expand Down Expand Up @@ -1049,6 +1058,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
user=None,
course=None,
is_concise=False,
summary_configuration=None,
):
"""
Creates the information needed for client-side XBlockInfo.
Expand Down Expand Up @@ -1098,6 +1108,10 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
should_visit_children = include_child_info and (
course_outline and not is_xblock_unit or not course_outline
)

if summary_configuration is None:
summary_configuration = AiAsideSummaryConfig(xblock.location.course_key)

if should_visit_children and xblock.has_children:
child_info = _create_xblock_child_info(
xblock,
Expand All @@ -1107,6 +1121,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
user=user,
course=course,
is_concise=is_concise,
summary_configuration=summary_configuration,
)
else:
child_info = None
Expand Down Expand Up @@ -1357,6 +1372,9 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
xblock, course=course
)

if is_xblock_unit and summary_configuration.is_enabled():
xblock_info["summary_configuration_enabled"] = summary_configuration.is_summary_enabled(xblock_info['id'])

return xblock_info


Expand Down Expand Up @@ -1550,6 +1568,7 @@ def _create_xblock_child_info(
user=None,
course=None,
is_concise=False,
summary_configuration=None,
):
"""
Returns information about the children of an xblock, as well as about the primary category
Expand All @@ -1576,6 +1595,7 @@ def _create_xblock_child_info(
user=user,
course=course,
is_concise=is_concise,
summary_configuration=summary_configuration,
)
for child in xblock.get_children()
]
Expand Down
56 changes: 56 additions & 0 deletions cms/lib/ai_aside_summary_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
"""
This file contains AiAsideSummaryConfig class that take a `course_key` and return if:
* the waffle flag is enabled in ai_aside
* is the summary is enabled for a given unit_key
* change the settings for a given unit_key
"""


class AiAsideSummaryConfig:
"""
Configuration for the AI Aside summary configuration.
"""

def __init__(self, course_key):
self.course_key = course_key

def __str__(self):
"""
Return user-friendly string.
"""
return f"AIAside summary configuration for {self.course_key} course"

def is_enabled(self):
"""
Define if the waffle flag is enabled for the current course_key
"""
try:
from ai_aside.config_api.api import is_summary_config_enabled
return is_summary_config_enabled(self.course_key)
except (ModuleNotFoundError, ImportError):
return False

def is_summary_enabled(self, unit_key=None):
"""
Define if the summary configuration is enabled in ai_aside
"""
try:
from ai_aside.config_api.api import is_course_settings_present, is_summary_enabled
if not is_course_settings_present(self.course_key):
return None
return is_summary_enabled(self.course_key, unit_key)
except (ModuleNotFoundError, ImportError):
return None

def set_summary_settings(self, unit_key, settings=None):
"""
Define the settings for a given unit_key in ai_aside
"""
if settings is None:
return None

try:
from ai_aside.config_api.api import set_unit_settings
return set_unit_settings(self.course_key, unit_key, settings)
except (ModuleNotFoundError, ImportError):
return None
Empty file added cms/lib/test/__init__.py
Empty file.
62 changes: 62 additions & 0 deletions cms/lib/test/test_ai_aside_summary_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
"""
Tests for AiAsideSummaryConfig class.
"""


import sys
from unittest import TestCase
from unittest.mock import Mock

from opaque_keys.edx.keys import CourseKey, UsageKey

from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig

ai_aside = Mock()
sys.modules['ai_aside.config_api.api'] = ai_aside


class AiAsideSummaryConfigTestCase(TestCase):
""" Tests for AiAsideSummaryConfig. """
COURSE_KEY = CourseKey.from_string("course-v1:test+Test+AiAsideSummaryConfigTestCase")
UNIT_KEY = UsageKey.from_string("block-v1:test+Test+AiAsideSummaryConfigTestCase+type@vertical+block@vertical_test")

def test_is_enabled(self):
"""
Check if summary configuration is enabled using the ai_aside lib.
"""
ai_aside_summary_config = AiAsideSummaryConfig(self.COURSE_KEY)
ai_aside.is_summary_config_enabled.return_value = True
self.assertTrue(ai_aside_summary_config.is_enabled())

ai_aside.is_summary_config_enabled.return_value = False
self.assertFalse(ai_aside_summary_config.is_enabled())

def test_is_summary_enabled(self):
"""
Check the summary configuration value for a particular course and an optional unit using the ai_aside lib.
"""
ai_aside_summary_config = AiAsideSummaryConfig(self.COURSE_KEY)
ai_aside.is_course_settings_present.return_value = True
ai_aside.is_summary_enabled.return_value = True
self.assertTrue(ai_aside_summary_config.is_summary_enabled())

ai_aside.is_course_settings_present.return_value = True
ai_aside.is_summary_enabled.return_value = False
self.assertFalse(ai_aside_summary_config.is_summary_enabled(self.UNIT_KEY))

ai_aside.is_course_settings_present.return_value = False
ai_aside.is_summary_enabled.return_value = True
self.assertIsNone(ai_aside_summary_config.is_summary_enabled())

ai_aside.is_course_settings_present.return_value = False
ai_aside.is_summary_enabled.return_value = False
self.assertIsNone(ai_aside_summary_config.is_summary_enabled(self.UNIT_KEY))

def test_set_summary_settings(self):
"""
Set the summary configuration settings for a particular unit using the ai_aside lib.
"""
ai_aside_summary_config = AiAsideSummaryConfig(self.COURSE_KEY)
ai_aside.set_unit_settings.return_value = True
self.assertTrue(ai_aside_summary_config.set_summary_settings(self.UNIT_KEY, {}))
self.assertIsNone(ai_aside_summary_config.set_summary_settings(self.UNIT_KEY))
6 changes: 5 additions & 1 deletion cms/static/js/models/xblock_info.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,11 @@ define(
highlights_enabled: false,
highlights_enabled_for_messaging: false,
highlights_preview_only: true,
highlights_doc_url: ''
highlights_doc_url: '',
/**
* True if summary configuration is enabled.
*/
summary_configuration_enabled: null,
},

initialize: function() {
Expand Down
37 changes: 36 additions & 1 deletion cms/static/js/spec/views/pages/course_outline_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ describe('CourseOutlinePage', function() {
'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor',
'settings-modal-tabs', 'timed-examination-preference-editor', 'access-editor',
'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor',
'course-highlights-enable', 'course-video-sharing-enable'
'course-highlights-enable', 'course-video-sharing-enable', 'summary-configuration-editor'
]);
appendSetFixtures(mockOutlinePage);
mockCourseJSON = createMockCourseJSON({}, [
Expand Down Expand Up @@ -2491,6 +2491,41 @@ describe('CourseOutlinePage', function() {
});
});

describe('summary configuration', function() {
it('hides summary configuration settings if summary_configuration_enabled is not a boolean', function() {
getUnitStatus({summary_configuration_enabled: null});
outlinePage.$('.outline-unit .configure-button').click();
expect($('.modal-section .summary-configuration')).not.toExist();
});

it('shows summary configuration settings if summary_configuration_enabled is true', function() {
getUnitStatus({summary_configuration_enabled: true});
outlinePage.$('.outline-unit .configure-button').click();
expect($('.modal-section .summary-configuration')).toExist();
});

it('shows summary configuration settings if summary_configuration_enabled is false', function() {
getUnitStatus({summary_configuration_enabled: false});
outlinePage.$('.outline-unit .configure-button').click();
expect($('.modal-section .summary-configuration')).toExist();
});

it('can be updated', function() {
getUnitStatus({summary_configuration_enabled: false});
outlinePage.$('.outline-unit .configure-button').click();
expect($('#summary_configuration_enabled').is(':checked')).toBe(false);
$('#summary_configuration_enabled').prop('checked', true).trigger('change');
$('.wrapper-modal-window .action-save').click();
AjaxHelpers.expectJsonRequest(requests, 'POST', '/xblock/mock-unit', {
summary_configuration_enabled: true,
publish: 'republish',
metadata: {
visible_to_staff_only: null,
}
});
})
});

verifyTypePublishable('unit', function(options) {
return createMockCourseJSON({}, [
createMockSectionJSON({}, [
Expand Down
41 changes: 40 additions & 1 deletion cms/static/js/views/modals/course_outline_modals.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview',
ReleaseDateEditor, DueDateEditor, SelfPacedDueDateEditor, GradingEditor, PublishEditor, AbstractVisibilityEditor,
StaffLockEditor, UnitAccessEditor, ContentVisibilityEditor, TimedExaminationPreferenceEditor,
AccessEditor, ShowCorrectnessEditor, HighlightsEditor, HighlightsEnableXBlockModal, HighlightsEnableEditor,
DiscussionEditor;
DiscussionEditor, SummaryConfigurationEditor;

CourseOutlineXBlockModal = BaseModal.extend({
events: _.extend({}, BaseModal.prototype.events, {
Expand Down Expand Up @@ -1203,6 +1203,42 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview',
}
});

SummaryConfigurationEditor = AbstractEditor.extend({
templateName: 'summary-configuration-editor',
className: 'summary-configuration',

afterRender: function() {
AbstractEditor.prototype.afterRender.call(this);
this.setEnabled(this.isModelEnabled());
},

isModelEnabled: function() {
return this.model.get('summary_configuration_enabled');
},

setEnabled: function(value) {
this.$('#summary_configuration_enabled').prop('checked', value);
},

isEnabled: function() {
return this.$('#summary_configuration_enabled').is(':checked');
},

hasChanges: function() {
return this.isModelEnabled() !== this.isEnabled();
},

getRequestData: function() {
if (this.hasChanges()) {
return {
summary_configuration_enabled: this.isEnabled()
};
} else {
return {};
}
}
});

return {
getModal: function(type, xblockInfo, options) {
if (type === 'edit') {
Expand All @@ -1228,6 +1264,9 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview',
};
if (xblockInfo.isVertical()) {
editors = [StaffLockEditor, UnitAccessEditor, DiscussionEditor];
if (typeof xblockInfo.get('summary_configuration_enabled') === 'boolean') {
editors.push(SummaryConfigurationEditor);
}
} else {
tabs = [
{
Expand Down
8 changes: 6 additions & 2 deletions cms/static/sass/elements/_modal-window.scss
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,7 @@

.edit-discussion,
.edit-staff-lock,
.summary-configuration,
.edit-content-visibility,
.edit-unit-access {
margin-bottom: $baseline;
Expand All @@ -760,6 +761,7 @@
// UI: staff lock and discussion
.edit-discussion,
.edit-staff-lock,
.summary-configuration,
.edit-settings-timed-examination,
.edit-unit-access {
.checkbox-cosmetic .input-checkbox {
Expand Down Expand Up @@ -832,7 +834,8 @@

.edit-discussion,
.edit-unit-access,
.edit-staff-lock {
.edit-staff-lock,
.summary-configuration {
.modal-section-content {
@include font-size(16);

Expand Down Expand Up @@ -874,7 +877,8 @@

.edit-discussion,
.edit-unit-access,
.edit-staff-lock {
.edit-staff-lock,
.summary-configuration {
.modal-section-content {
@include font-size(16);

Expand Down
2 changes: 1 addition & 1 deletion cms/templates/course_outline.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

<%block name="header_extras">
<link rel="stylesheet" type="text/css" href="${static.url('js/vendor/timepicker/jquery.timepicker.css')}" />
% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'course-video-sharing-enable']:
% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'self-paced-due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'unit-access-editor', 'discussion-editor', 'content-visibility-editor', 'verification-access-editor', 'timed-examination-preference-editor', 'access-editor', 'settings-modal-tabs', 'show-correctness-editor', 'highlights-editor', 'highlights-enable-editor', 'course-highlights-enable', 'course-video-sharing-enable', 'summary-configuration-editor']:
<script type="text/template" id="${template_name}-tpl">
<%static:include path="js/${template_name}.underscore" />
</script>
Expand Down
11 changes: 11 additions & 0 deletions cms/templates/js/summary-configuration-editor.underscore
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<form>
<h3 class="modal-section-title">
<%- gettext('Xpert unit summaries') %>
</h3>
<div class="modal-section-content summary-configuration">
<label class="label">
<input type="checkbox" id="summary_configuration_enabled" name="summary_configuration_enabled" class="input input-checkbox" />
<%- gettext('Enable summaries') %>
</label>
</div>
</form>

0 comments on commit 3f20c75

Please sign in to comment.