Commit d0ca5228 by Anthony Mangano

split audit and credit into two noop options on course entitlement form

parent 0917535b
......@@ -475,20 +475,39 @@ class SeatForm(BaseForm):
class CourseEntitlementForm(BaseForm):
AUDIT_MODE = 'audit'
VERIFIED_MODE = CourseEntitlement.VERIFIED
PROFESSIONAL_MODE = CourseEntitlement.PROFESSIONAL
CREDIT_MODE = 'credit'
# Modes for which we should not create entitlements.
NOOP_MODES = [AUDIT_MODE, CREDIT_MODE]
# Modes for which we should create an entitlement.
PAID_MODES = [VERIFIED_MODE, PROFESSIONAL_MODE]
MODE_CHOICES = [
('', _('Audit only or credit')),
(CourseEntitlement.VERIFIED, _('Verified')),
(CourseEntitlement.PROFESSIONAL, _('Professional education')),
(AUDIT_MODE, _('Audit only')),
(VERIFIED_MODE, _('Verified')),
(PROFESSIONAL_MODE, _('Professional education')),
(CREDIT_MODE, _('Credit')),
]
mode = forms.ChoiceField(choices=MODE_CHOICES, required=False, label=_('Enrollment Track'),
initial=CourseEntitlement.VERIFIED)
initial=VERIFIED_MODE)
price = forms.DecimalField(max_digits=6, decimal_places=2, required=False, initial=0.00)
class Meta:
fields = ('mode', 'price')
model = CourseEntitlement
def __init__(self, *args, **kwargs):
include_blank_mode = kwargs.pop('include_blank_mode', False)
super(CourseEntitlementForm, self).__init__(*args, **kwargs)
if include_blank_mode:
self.fields['mode'].choices = [('', '')] + self.MODE_CHOICES
def save(self, commit=True, course=None): # pylint: disable=arguments-differ
entitlement = super(CourseEntitlementForm, self).save(commit=False)
......@@ -500,6 +519,14 @@ class CourseEntitlementForm(BaseForm):
return entitlement
def clean_mode(self):
mode = self.cleaned_data['mode']
if mode in self.NOOP_MODES:
# Allow 'audit'/'credit' to be submitted as valid modes, but don't save an entitlement for them.
return None
else:
return mode
def clean(self):
cleaned_data = super().clean()
mode = cleaned_data.get('mode')
......@@ -509,10 +536,10 @@ class CourseEntitlementForm(BaseForm):
if not mode:
cleaned_data['price'] = None
if mode in [CourseEntitlement.VERIFIED, CourseEntitlement.PROFESSIONAL] and price and price < 0.01:
if mode in self.PAID_MODES and price and price < 0.01:
self.add_error('price', _('Price must be greater than or equal to 0.01'))
if mode in [CourseEntitlement.VERIFIED, CourseEntitlement.PROFESSIONAL] and not price:
self.add_error('price', _('Only audit seat can be without price.'))
if mode in self.PAID_MODES and not price:
self.add_error('price', _('Price is required.'))
return cleaned_data
......
......@@ -141,7 +141,7 @@
<label class="field-label ">{{ entitlement_form.mode.label_tag }}</label>
{{ entitlement_form.mode }}
</div>
<div id="seatPriceBlock" class="col col-6 {% if not entitlement_form.mode.value %}hidden{% endif %}">
<div id="modePriceBlock" class="col col-6 {% if entitlement_form.mode.value not in entitlement_form.PAID_MODES %}hidden{% endif %}">
<label class="field-label ">{{ entitlement_form.price.label_tag }} <span class="required">*</span></label>
{{ entitlement_form.price }}
</div>
......@@ -188,7 +188,7 @@
<script src="{% static 'js/publisher/course-tabs.js' %}"></script>
<script src="{% static 'js/publisher/organizations.js' %}"></script>
<script src="{% static 'js/publisher/instructors.js' %}"></script>
<script src="{% static 'js/publisher/seat-type-change.js' %}"></script>
<script src="{% static 'js/publisher/entitlement-type-change.js' %}"></script>
<script src="{% static 'js/publisher/image-validation.js' %}"></script>
<script src="{% static 'js/publisher/program-types.js' %}"></script>
<script src="{% static 'js/publisher/modal-screen.js' %}"></script>
......
......@@ -618,7 +618,7 @@
<label class="field-label ">{{ entitlement_form.mode.label_tag }}</label>
{{ entitlement_form.mode }}
</div>
<div id="seatPriceBlock" class="col col-6 {% if not entitlement_form.mode.value %}hidden{% endif %}">
<div id="modePriceBlock" class="col col-6 {% if entitlement_form.mode.value not in entitlement_form.PAID_MODES %}hidden{% endif %}">
<label class="field-label ">{{ entitlement_form.price.label_tag }} <span class="required">*</span></label>
{{ entitlement_form.price }}
</div>
......@@ -650,7 +650,7 @@
<script src="{% static 'js/publisher/course-tabs.js' %}"></script>
<script src="{% static 'js/publisher/organizations.js' %}"></script>
<script src="{% static 'js/publisher/seat-type-change.js' %}"></script>
<script src="{% static 'js/publisher/entitlement-type-change.js' %}"></script>
<script src="{% static 'js/publisher/image-validation.js' %}"></script>
<script src="{% static 'js/publisher/course-image.js' %}"></script>
<script src="{% static 'bower_components/tinymce/tinymce.min.js' %}"></script>
......
......@@ -17,7 +17,7 @@ from course_discovery.apps.publisher.forms import (
CourseEntitlementForm, CourseForm, CourseRunForm, CourseRunStateAdminForm, CourseSearchForm, CourseStateAdminForm,
PublisherUserCreationForm, SeatForm
)
from course_discovery.apps.publisher.models import CourseEntitlement, Group, OrganizationExtension, Seat
from course_discovery.apps.publisher.models import Group, OrganizationExtension, Seat
from course_discovery.apps.publisher.tests.factories import (
CourseFactory, CourseUserRoleFactory, OrganizationExtensionFactory, SeatFactory
)
......@@ -349,14 +349,14 @@ class PublisherCustomCourseFormTests(TestCase):
@ddt.ddt
class PublisherCourseEntitlementFormTests(TestCase):
without_price_error = 'Only audit seat can be without price.'
without_price_error = 'Price is required.'
negative_price_error = 'Price must be greater than or equal to 0.01'
@ddt.data(
(CourseEntitlement.VERIFIED, None, without_price_error),
(CourseEntitlement.PROFESSIONAL, None, without_price_error),
(CourseEntitlement.VERIFIED, -0.05, negative_price_error),
(CourseEntitlement.PROFESSIONAL, -0.05, negative_price_error),
(CourseEntitlementForm.VERIFIED_MODE, None, without_price_error),
(CourseEntitlementForm.PROFESSIONAL_MODE, None, without_price_error),
(CourseEntitlementForm.VERIFIED_MODE, -0.05, negative_price_error),
(CourseEntitlementForm.PROFESSIONAL_MODE, -0.05, negative_price_error),
)
@ddt.unpack
def test_invalid_price(self, mode, price, error_message):
......@@ -371,22 +371,43 @@ class PublisherCourseEntitlementFormTests(TestCase):
@ddt.data(
(None, None),
(None, 0),
(CourseEntitlement.VERIFIED, 50),
(CourseEntitlement.PROFESSIONAL, 50),
(CourseEntitlementForm.AUDIT_MODE, None),
(CourseEntitlementForm.AUDIT_MODE, 0),
(CourseEntitlementForm.VERIFIED_MODE, 50),
(CourseEntitlementForm.PROFESSIONAL_MODE, 50),
(CourseEntitlementForm.CREDIT_MODE, None),
(CourseEntitlementForm.CREDIT_MODE, 0),
)
@ddt.unpack
def test_valid_price(self, mode, price):
def test_valid_data(self, mode, price):
"""
Verify that clean works fine for valid price/type combos
Verify that is_valid returns True for valid mode/price combos
"""
entitlement_form = CourseEntitlementForm()
entitlement_form.cleaned_data = {}
if mode is not None:
entitlement_form.cleaned_data['mode'] = mode
if price is not None:
entitlement_form.cleaned_data['price'] = price
entitlement_form = CourseEntitlementForm({'mode': mode, 'price': price})
self.assertTrue(entitlement_form.is_valid())
self.assertEqual(entitlement_form.clean(), entitlement_form.cleaned_data)
@ddt.data(
(CourseEntitlementForm.AUDIT_MODE, 0, None),
(CourseEntitlementForm.VERIFIED_MODE, 50, CourseEntitlementForm.VERIFIED_MODE),
(CourseEntitlementForm.PROFESSIONAL_MODE, 50, CourseEntitlementForm.PROFESSIONAL_MODE),
(CourseEntitlementForm.CREDIT_MODE, 0, None),
)
@ddt.unpack
def test_clean_mode(self, raw_mode, raw_price, cleaned_mode):
"""
Verify that mode is cleaned properly and that NOOP_MODES are set to None.
"""
entitlement_form = CourseEntitlementForm({'mode': raw_mode, 'price': raw_price})
self.assertTrue(entitlement_form.is_valid())
self.assertEqual(entitlement_form.cleaned_data['mode'], cleaned_mode)
def test_include_blank_mode(self):
"""
Verify that when the include_blank_mode option is passed to the constructor, the mode field includes
a blank option.
"""
entitlement_form = CourseEntitlementForm(include_blank_mode=True)
self.assertEqual([('', '')] + CourseEntitlementForm.MODE_CHOICES, entitlement_form.fields['mode'].choices)
@pytest.mark.django_db
......
......@@ -37,6 +37,7 @@ from course_discovery.apps.publisher.constants import (
ADMIN_GROUP_NAME, INTERNAL_USER_GROUP_NAME, PROJECT_COORDINATOR_GROUP_NAME,
PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS, REVIEWER_GROUP_NAME
)
from course_discovery.apps.publisher.forms import CourseEntitlementForm
from course_discovery.apps.publisher.models import (
Course, CourseEntitlement, CourseRun, CourseRunState, CourseState, OrganizationExtension, Seat
)
......@@ -297,7 +298,7 @@ class CreateCourseViewTests(SiteMixin, TestCase):
)
return Course.objects.get(number=course_dict['number'])
@ddt.data(CourseEntitlement.VERIFIED, CourseEntitlement.PROFESSIONAL)
@ddt.data(CourseEntitlementForm.VERIFIED_MODE, CourseEntitlementForm.PROFESSIONAL_MODE)
def test_create_entitlement(self, mode):
"""
Verify that we create an entitlement for appropriate types (happy path)
......@@ -314,11 +315,17 @@ class CreateCourseViewTests(SiteMixin, TestCase):
self.assertEqual(mode, entitlement.mode)
self.assertEqual(50, entitlement.price)
def test_seat_version(self):
@ddt.data(
{},
{'mode': CourseEntitlementForm.AUDIT_MODE, 'price': 0},
{'mode': CourseEntitlementForm.CREDIT_MODE, 'price': 0}
)
def test_seat_version(self, entitlement_form_data):
"""
Verify that when we create a seat product without an entitlement, we set version correctly
Verify that when we create a course without a mode, or with a mode that doesn't support entitlements,
we set version correctly
"""
course = self._create_course_with_post()
course = self._create_course_with_post(entitlement_form_data)
self.assertEqual(0, CourseEntitlement.objects.all().count())
self.assertEqual(Course.SEAT_VERSION, course.version)
......@@ -327,7 +334,7 @@ class CreateCourseViewTests(SiteMixin, TestCase):
"""
Verify that we check price validity when making an entitlement
"""
data = {'title': 'Test2', 'number': 'testX234', 'mode': CourseEntitlement.VERIFIED}
data = {'title': 'Test2', 'number': 'testX234', 'mode': CourseEntitlementForm.VERIFIED_MODE}
if price is not None:
data['price'] = price
course_dict = self._post_data(data, self.course)
......@@ -3000,7 +3007,7 @@ class CourseEditViewTests(SiteMixin, TestCase):
# Test that this saves without seats after resetting this to Seat version
self.course.version = Course.SEAT_VERSION
self.course.save()
post_data['mode'] = Seat.PROFESSIONAL
post_data['mode'] = CourseEntitlementForm.PROFESSIONAL_MODE
post_data['price'] = 1
response = self.client.post(self.edit_page_url, data=post_data)
self.assertRedirects(
......@@ -3015,26 +3022,28 @@ class CourseEditViewTests(SiteMixin, TestCase):
self.course.version = Course.SEAT_VERSION
self.course.save()
post_data['mode'] = ''
post_data['price'] = ''
response = self.client.post(self.edit_page_url, data=post_data)
# Assert that this saves for a SEAT_VERSION
self.assertRedirects(
response,
expected_url=reverse('publisher:publisher_course_detail', kwargs={'pk': self.course.id}),
status_code=302,
target_status_code=200
)
# Verify that we can switch between NOOP_MODES
for noop_mode in [''] + CourseEntitlementForm.NOOP_MODES:
post_data['mode'] = noop_mode
post_data['price'] = 0
response = self.client.post(self.edit_page_url, data=post_data)
self.assertRedirects(
response,
expected_url=reverse('publisher:publisher_course_detail', kwargs={'pk': self.course.id}),
status_code=302,
target_status_code=200
)
# Modify the Course to try and create CourseEntitlements differing from the CourseRun and Seat type
post_data['mode'] = Seat.PROFESSIONAL
post_data['mode'] = CourseEntitlementForm.PROFESSIONAL_MODE
post_data['price'] = 2 # just a number different than what is used in the SeatFactory constructor
response = self.client.post(self.edit_page_url, data=post_data)
self.assertEqual(response.status_code, 400)
# Modify the Course to try and create CourseEntitlements differing from the CourseRun and Seat price
post_data['mode'] = Seat.VERIFIED
post_data['mode'] = CourseEntitlementForm.VERIFIED_MODE
post_data['price'] = 1 # just a number different than what is used in the SeatFactory constructor
response = self.client.post(self.edit_page_url, data=post_data)
......@@ -3060,7 +3069,7 @@ class CourseEditViewTests(SiteMixin, TestCase):
self.course.save()
post_data = self._post_data(self.organization_extension)
post_data['team_admin'] = self.course_team_role.user.id
post_data['mode'] = CourseEntitlement.VERIFIED
post_data['mode'] = CourseEntitlementForm.VERIFIED_MODE
post_data['price'] = 150
response = self.client.post(self.edit_page_url, data=post_data)
......@@ -3073,13 +3082,14 @@ class CourseEditViewTests(SiteMixin, TestCase):
)
# Assert that trying to switch to Audit/Credit Course (and thus allowing Course Run Seat configuration) fails
post_data['mode'] = ''
post_data['price'] = ''
response = self.client.post(self.edit_page_url, data=post_data)
self.assertEqual(response.status_code, 400)
for noop_mode in [''] + CourseEntitlementForm.NOOP_MODES:
post_data['mode'] = noop_mode
post_data['price'] = 0
response = self.client.post(self.edit_page_url, data=post_data)
self.assertEqual(response.status_code, 400)
# Assert that not setting a price for a verified course fails
post_data['mode'] = CourseEntitlement.VERIFIED
post_data['mode'] = CourseEntitlementForm.VERIFIED_MODE
post_data['price'] = ''
response = self.client.post(self.edit_page_url, data=post_data)
self.assertEqual(response.status_code, 400)
......@@ -3087,7 +3097,7 @@ class CourseEditViewTests(SiteMixin, TestCase):
# Assert that changing the price for a course with a Verified Entitlement is allowed
new_course = factories.CourseFactory()
factories.CourseEntitlementFactory(course=new_course, mode=CourseEntitlement.VERIFIED)
post_data['mode'] = CourseEntitlement.VERIFIED
post_data['mode'] = CourseEntitlementForm.VERIFIED_MODE
post_data['price'] = 1
response = self.client.post(self.edit_page_url, data=post_data)
self.assertRedirects(
......
......@@ -282,7 +282,9 @@ class CreateCourseView(mixins.LoginRequiredMixin, mixins.PublisherUserRequiredMi
try:
with transaction.atomic():
course = course_form.save(commit=False)
if entitlement_form['mode'].value():
# Check for mode in cleaned_data, as mode will be set to None during validation for modes that
# do not support entitlements.
if entitlement_form.cleaned_data.get('mode', None):
course.version = Course.ENTITLEMENT_VERSION
else:
course.version = Course.SEAT_VERSION
......@@ -383,9 +385,12 @@ class CourseEditView(mixins.PublisherPermissionMixin, UpdateView):
)
if self.object.uses_entitlements:
context['entitlement_form'] = self.entitlement_form(instance=self.object.entitlements.first())
context['entitlement_form'] = self.entitlement_form(
instance=self.object.entitlements.first(),
include_blank_mode=True
)
else:
context['entitlement_form'] = self.entitlement_form({'mode': ''})
context['entitlement_form'] = self.entitlement_form({'mode': ''}, include_blank_mode=True)
return context
......@@ -461,7 +466,7 @@ class CourseEditView(mixins.PublisherPermissionMixin, UpdateView):
entitlement.mode = entitlement_form.cleaned_data.get('mode')
entitlement.save()
else:
entitlement_form.save(course=course)
entitlement = entitlement_form.save(course=course)
return entitlement
@transaction.atomic
......@@ -495,8 +500,10 @@ class CourseEditView(mixins.PublisherPermissionMixin, UpdateView):
return course
def _handle_entitlement_update(self, user, request, course_form):
entitlement_form = self.entitlement_form(request.POST)
entitlement_form = self.entitlement_form(request.POST, include_blank_mode=True)
# Make sure to extract these values from cleaned_data, as the validation process will set mode to None
# if a mode that doesn't support entitlements was selected.
entitlement_mode = entitlement_form.cleaned_data.get('mode')
entitlement_price = entitlement_form.cleaned_data.get('price')
......@@ -543,8 +550,7 @@ class CourseEditView(mixins.PublisherPermissionMixin, UpdateView):
})
elif self.object.uses_entitlements and not entitlement_mode:
messages.error(request, _(
"Courses with a previously set Type and Price cannot be "
"changed to an Audit/Credit Course"
"Enrollment track cannot be unset or changed from verified or professional to audit or credit."
))
return self._render_post_error(request, ctx_overrides={
'course_form': course_form,
......
......@@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2018-02-21 20:02+0000\n"
"POT-Creation-Date: 2018-03-06 21:05+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
......@@ -836,7 +836,7 @@ msgid "Only audit seat can be without price."
msgstr ""
#: apps/publisher/forms.py
msgid "Audit only or credit"
msgid "Price is required."
msgstr ""
#: apps/publisher/forms.py
......@@ -3332,8 +3332,8 @@ msgstr ""
#: apps/publisher/views.py
msgid ""
"Courses with a previously set Type and Price cannot be changed to an "
"Audit/Credit Course"
"Enrollment track cannot be unset or changed from verified or professional to"
" audit or credit."
msgstr ""
#: apps/publisher/views.py
......
......@@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2018-02-21 20:02+0000\n"
"POT-Creation-Date: 2018-03-06 21:05+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
......
......@@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2018-02-21 20:02+0000\n"
"POT-Creation-Date: 2018-03-06 21:05+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
......@@ -1007,8 +1007,8 @@ msgstr ""
"¢σηѕє¢тєтυ#"
#: apps/publisher/forms.py
msgid "Audit only or credit"
msgstr "Àüdït önlý ör çrédït Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, #"
msgid "Price is required."
msgstr "Prïçé ïs réqüïréd. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт#"
#: apps/publisher/forms.py
msgid "This field is required."
......@@ -4059,11 +4059,11 @@ msgstr ""
#: apps/publisher/views.py
msgid ""
"Courses with a previously set Type and Price cannot be changed to an "
"Audit/Credit Course"
"Enrollment track cannot be unset or changed from verified or professional to"
" audit or credit."
msgstr ""
"Çöürsés wïth ä prévïöüslý sét Týpé änd Prïçé çännöt ßé çhängéd tö än "
"Àüdït/Çrédït Çöürsé Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σηѕє#"
"Énröllmént träçk çännöt ßé ünsét ör çhängéd fröm vérïfïéd ör pröféssïönäl tö"
" äüdït ör çrédït. Ⱡ'σяєм ιρѕυм ∂σłσя ѕιт αмєт, ¢σ#"
#: apps/publisher/views.py
msgid "Course updated successfully."
......
......@@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2018-02-21 20:02+0000\n"
"POT-Creation-Date: 2018-03-06 21:05+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
......
$(document).on('change', '#id_mode', function (e) {
var $modeBlock = $("#modePriceBlock"),
selectedMode = this.value;
if (selectedMode === 'verified' || selectedMode === 'professional') {
$modeBlock.show();
} else{
$modeBlock.hide();
}
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment