Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: integrate cohort assignment filter definition to cohort model

Browse Source
This commit is contained in:
Maria Grimaldi
2022-05-18 14:34:07 -04:00
parent 116431cb81
commit ec69659253
5 changed files with 145 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
lms/static/js/groups/views/cohort_editor.js
View File

@@ -260,16 +260,17 @@
// Show error messages.
this.undelegateViewEvents(this.errorNotifications);
numErrors = modifiedUsers.unknown.length + modifiedUsers.invalid.length;
numErrors = modifiedUsers.unknown.length + modifiedUsers.invalid.length + modifiedUsers.not_allowed.length;
if (numErrors > 0) {
createErrorDetails = function(unknownUsers, invalidEmails, showAllErrors) {
createErrorDetails = function(unknownUsers, invalidEmails, notAllowed, showAllErrors) {
var unknownErrorsShown = showAllErrors ? unknownUsers.length :
Math.min(errorLimit, unknownUsers.length);
var invalidErrorsShown = showAllErrors ? invalidEmails.length :
Math.min(errorLimit - unknownUsers.length, invalidEmails.length);
var notAllowedErrorsShown = showAllErrors ? notAllowed.length :
Math.min(errorLimit - notAllowed.length, notAllowed.length);
details = [];
for (i = 0; i < unknownErrorsShown; i++) {
details.push(interpolate_text(gettext('Unknown username: {user}'),
{user: unknownUsers[i]}));
@@ -278,6 +279,10 @@
details.push(interpolate_text(gettext('Invalid email address: {email}'),
{email: invalidEmails[i]}));
}
for (i = 0; i < notAllowedErrorsShown; i++) {
details.push(interpolate_text(gettext('Cohort assignment not allowed: {email_or_username}'),
{email_or_username: notAllowed[i]}));
}
return details;
};
@@ -286,12 +291,12 @@
'{numErrors} learners could not be added to this cohort:', numErrors),
{numErrors: numErrors}
);
details = createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, false);
details = createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, modifiedUsers.not_allowed, false);
errorActionCallback = function(view) {
view.model.set('actionText', null);
view.model.set('details',
createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, true));
createErrorDetails(modifiedUsers.unknown, modifiedUsers.invalid, modifiedUsers.not_allowed, true));
view.render();
};

20
lms/static/js/spec/groups/views/cohorts_spec.js
View File

@@ -14,6 +14,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
var catLoversInitialCount = 123,
dogLoversInitialCount = 456,
unknownUserMessage,
notAllowedUserMessage,
invalidEmailMessage, createMockCohort, createMockCohorts, createMockContentGroups,
createMockCohortSettingsJson,
createCohortsView, cohortsView, requests, respondToRefresh, verifyMessage, verifyNoMessage,
@@ -210,6 +211,10 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
return 'Invalid email address: ' + name;
};
notAllowedUserMessage = function(email) {
return 'Cohort assignment not allowed: ' + email;
};
beforeEach(function() {
setFixtures('<ul class="instructor-nav">' +
'<li class="nav-item"><button type="button" data-section="cohort_management" ' +
@@ -602,7 +607,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
respondToAdd = function(result) {
AjaxHelpers.respondWithJson(
requests,
_.extend({unknown: [], added: [], present: [], changed: [],
_.extend({unknown: [], added: [], present: [], changed: [], not_allowed: [],
success: true, preassigned: [], invalid: []}, result)
);
};
@@ -670,6 +675,19 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
);
});
it('shows an error when user assignment not allowed', function() {
createCohortsView(this, {selectCohort: 1});
addStudents('not_allowed');
AjaxHelpers.expectRequest(
requests, 'POST', '/mock_service/cohorts/1/add', 'users=not_allowed'
);
respondToAdd({not_allowed: ['not_allowed']});
respondToRefresh(catLoversInitialCount, dogLoversInitialCount);
verifyHeader(1, 'Cat Lovers', catLoversInitialCount);
verifyDetailedMessage('There was an error when trying to add learners:', 'error',
[notAllowedUserMessage('not_allowed')]
);
});
it('shows a "view all" button when more than 5 students do not exist', function() {
var sixUsers = 'unknown1, unknown2, unknown3, unknown4, unknown5, unknown6';

13
openedx/core/djangoapps/course_groups/models.py
View File

@@ -13,7 +13,7 @@ from django.db.models.signals import pre_delete
from django.dispatch import receiver
from opaque_keys.edx.django.models import CourseKeyField
from openedx_filters.learning.filters import CohortChangeRequested
from openedx_filters.learning.filters import CohortAssignmentRequested, CohortChangeRequested
from openedx.core.djangolib.model_mixins import DeletableByUserValue
@@ -31,6 +31,10 @@ class CohortChangeNotAllowed(CohortMembershipException):
pass
class CohortAssignmentNotAllowed(CohortMembershipException):
pass
class CourseUserGroup(models.Model):
"""
This model represents groups of users in a course. Groups may have different types,
@@ -113,6 +117,13 @@ class CohortMembership(models.Model):
cohort
Returns CohortMembership, previous_cohort (if any)
"""
try:
# .. filter_implemented_name: CohortAssignmentRequested
# .. filter_type: org.openedx.learning.cohort.assignment.requested.v1
user, cohort = CohortAssignmentRequested.run_filter(user=user, target_cohort=cohort)
except CohortAssignmentRequested.PreventCohortAssignment as exc:
raise CohortAssignmentNotAllowed(str(exc)) from exc
with transaction.atomic():
membership, created = cls.objects.select_for_update().get_or_create(
user__id=user.id,

96
openedx/core/djangoapps/course_groups/tests/test_filters.py
View File

@@ -3,12 +3,16 @@ Test that various filters are executed for models in the course_groups app.
"""
from django.test import override_settings
from openedx_filters import PipelineStep
from openedx_filters.learning.filters import CohortChangeRequested
from openedx_filters.learning.filters import CohortAssignmentRequested, CohortChangeRequested
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.course_groups.models import CohortChangeNotAllowed, CohortMembership
from openedx.core.djangoapps.course_groups.models import (
CohortAssignmentNotAllowed,
CohortChangeNotAllowed,
CohortMembership,
)
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
@@ -31,6 +35,23 @@ class TestCohortChangeStep(PipelineStep):
return {}
class TestCohortAssignmentStep(PipelineStep):
"""
Utility function used when getting steps for pipeline.
"""
def run_filter(self, user, target_cohort): # pylint: disable=arguments-differ
"""Pipeline step that adds cohort info to the users profile."""
user.profile.set_meta(
{
"cohort_info":
f"User assigned to Cohort {str(target_cohort)}",
}
)
user.profile.save()
return {}
class TestStopCohortChangeStep(PipelineStep):
"""
Utility function used when getting steps for pipeline.
@@ -41,6 +62,16 @@ class TestStopCohortChangeStep(PipelineStep):
raise CohortChangeRequested.PreventCohortChange("You can't change cohorts.")
class TestStopAssignmentChangeStep(PipelineStep):
"""
Utility function used when getting steps for pipeline.
"""
def run_filter(self, user, target_cohort, *args, **kwargs): # pylint: disable=arguments-differ
"""Pipeline step that stops the cohort change process."""
raise CohortAssignmentRequested.PreventCohortAssignment("You can't be assign to this cohort.")
@skip_unless_lms
class CohortFiltersTest(SharedModuleStoreTestCase):
"""
@@ -92,6 +123,33 @@ class CohortFiltersTest(SharedModuleStoreTestCase):
cohort_membership.user.profile.get_meta(),
)
@override_settings(
OPEN_EDX_FILTERS_CONFIG={
"org.openedx.learning.cohort.assignment.requested.v1": {
"pipeline": [
"openedx.core.djangoapps.course_groups.tests.test_filters.TestCohortAssignmentStep",
],
"fail_silently": False,
},
},
)
def test_cohort_assignment_filter_executed(self):
"""
Test whether the student cohort assignment filter is triggered before the user's
assignment.
Expected result:
- CohortAssignmentRequested is triggered and executes TestCohortAssignmentStep.
- The user's profile meta contains cohort_info.
"""
cohort_membership, _ = CohortMembership.assign(user=self.user, cohort=self.second_cohort, )
self.assertEqual(
{"cohort_info": "User assigned to Cohort SecondCohort"},
cohort_membership.user.profile.get_meta(),
)
@override_settings(
OPEN_EDX_FILTERS_CONFIG={
"org.openedx.learning.cohort.change.requested.v1": {
@@ -115,6 +173,27 @@ class CohortFiltersTest(SharedModuleStoreTestCase):
with self.assertRaises(CohortChangeNotAllowed):
CohortMembership.assign(cohort=self.second_cohort, user=self.user)
@override_settings(
OPEN_EDX_FILTERS_CONFIG={
"org.openedx.learning.cohort.assignment.requested.v1": {
"pipeline": [
"openedx.core.djangoapps.course_groups.tests.test_filters.TestStopAssignmentChangeStep",
],
"fail_silently": False,
},
},
)
def test_cohort_assignment_filter_prevent_move(self):
"""
Test prevent the user's cohort assignment through a pipeline step.
Expected result:
- CohortAssignmentRequested is triggered and executes TestStopAssignmentChangeStep.
- The user can't be assigned to the cohort.
"""
with self.assertRaises(CohortAssignmentNotAllowed):
CohortMembership.assign(cohort=self.second_cohort, user=self.user)
@override_settings(OPEN_EDX_FILTERS_CONFIG={})
def test_cohort_change_without_filter_configuration(self):
"""
@@ -129,3 +208,16 @@ class CohortFiltersTest(SharedModuleStoreTestCase):
cohort_membership, _ = CohortMembership.assign(cohort=self.second_cohort, user=self.user)
self.assertEqual({}, cohort_membership.user.profile.get_meta())
@override_settings(OPEN_EDX_FILTERS_CONFIG={})
def test_cohort_assignment_without_filter_configuration(self):
"""
Test usual cohort assignment process, without filter's intervention.
Expected result:
- CohortAssignmentRequested does not have any effect on the cohort change process.
- The cohort assignment process ends successfully.
"""
cohort_membership, _ = CohortMembership.assign(cohort=self.second_cohort, user=self.user)
self.assertEqual({}, cohort_membership.user.profile.get_meta())

12
openedx/core/djangoapps/course_groups/views.py
View File

@@ -26,7 +26,11 @@ from rest_framework.serializers import Serializer
from lms.djangoapps.courseware.courses import get_course, get_course_with_access
from common.djangoapps.edxmako.shortcuts import render_to_response
from openedx.core.djangoapps.course_groups.models import CohortMembership
from openedx.core.djangoapps.course_groups.models import (
CohortAssignmentNotAllowed,
CohortChangeNotAllowed,
CohortMembership,
)
from openedx.core.djangoapps.course_groups.permissions import IsStaffOrAdmin
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
@@ -321,6 +325,7 @@ def add_users_to_cohort(request, course_key_string, cohort_id):
unknown = []
preassigned = []
invalid = []
not_allowed = []
for username_or_email in split_by_comma_and_whitespace(users):
if not username_or_email:
continue
@@ -346,6 +351,8 @@ def add_users_to_cohort(request, course_key_string, cohort_id):
invalid.append(username_or_email)
except ValueError:
present.append(username_or_email)
except (CohortAssignmentNotAllowed, CohortChangeNotAllowed):
not_allowed.append(username_or_email)
return json_http_response({'success': True,
'added': added,
@@ -353,7 +360,8 @@ def add_users_to_cohort(request, course_key_string, cohort_id):
'present': present,
'unknown': unknown,
'preassigned': preassigned,
'invalid': invalid})
'invalid': invalid,
'not_allowed': not_allowed})
@ensure_csrf_cookie