Skip to content

C
course-discovery
Commit b99d4b5c by Awais Jibran
Options

Fix & cleanup course run seats in publisher

parent 1eaf7a1c
Showing with 151 additions and 50 deletions
course_discovery/apps/publisher/constants.py
......@@ -17,3 +17,4 @@ PARTNER_COORDINATOR_GROUP_NAME = 'Partner Coordinators'
# Waffle flags
PUBLISHER_ENTITLEMENTS_WAFFLE_SWITCH = 'publisher_entitlements'
PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS = 'publisher_create_audit_seats_for_verified_course_runs'
course_discovery/apps/publisher/forms.py
......@@ -15,6 +15,7 @@ from course_discovery.apps.course_metadata.choices import CourseRunPacing
from course_discovery.apps.course_metadata.models import LevelType, Organization, Person, Subject
from course_discovery.apps.ietf_language_tags.models import LanguageTag
from course_discovery.apps.publisher.choices import CourseRunStateChoices, PublisherUserRole
from course_discovery.apps.publisher.constants import PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS
from course_discovery.apps.publisher.mixins import LanguageModelSelect2Multiple, get_user_organizations
from course_discovery.apps.publisher.models import (
Course, CourseEntitlement, CourseRun, CourseRunState, CourseState, CourseUserRole, OrganizationExtension,
......@@ -392,6 +393,10 @@ class SeatForm(BaseForm):
model = Seat
def save(self, commit=True, course_run=None, changed_by=None): # pylint: disable=arguments-differ
"""Saves the seat for a particular course run."""
# Background -- EDUCATOR-2337 (Should be removed once the data is consistent)
self.validate_and_remove_seats(course_run)
# When seat is save make sure its prices and others fields updated accordingly.
seat = super(SeatForm, self).save(commit=False)
if seat.type in [Seat.HONOR, Seat.AUDIT]:
......@@ -413,12 +418,12 @@ class SeatForm(BaseForm):
if commit:
seat.save()
if waffle.switch_is_active('publisher_create_audit_seats_for_verified_course_runs'):
if waffle.switch_is_active(PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS):
course_run = seat.course_run
audit_seats = course_run.seats.filter(type=Seat.AUDIT)
# Ensure that course runs with a verified seat always have an audit seat
if seat.type in (Seat.CREDIT, Seat.VERIFIED,):
if seat.type in Seat.PAID_AND_AUDIT_APPLICABLE_SEATS:
if not audit_seats.exists():
course_run.seats.create(type=Seat.AUDIT, price=0, upgrade_deadline=None)
logger.info('Created audit seat for course run [%d]', course_run.id)
......@@ -448,6 +453,22 @@ class SeatForm(BaseForm):
seat.credit_hours = None
seat.credit_price = 0.00
def validate_and_remove_seats(self, course_run):
"""
Remove course run seats if the data is bogus
Temporary call to remove the duplicate course run seats -- EDUCATOR-2337
Background: There was a bug in the system, where course run seats being duplicated,
in order to cleanup, remove all the existing seats.
"""
if not course_run:
return
all_course_run_seats = course_run.seats.all()
seats_data_is_bogus = all_course_run_seats.count() > 2
if seats_data_is_bogus:
logger.info('Removing bogus course run [%d] seats [%d]', course_run.id, all_course_run_seats.count())
all_course_run_seats.delete()
class CourseEntitlementForm(BaseForm):
MODE_CHOICES = [
......
course_discovery/apps/publisher/models.py
......@@ -25,15 +25,15 @@ from course_discovery.apps.course_metadata.models import LevelType, Organization
from course_discovery.apps.course_metadata.utils import UploadToFieldNamePath
from course_discovery.apps.ietf_language_tags.models import LanguageTag
from course_discovery.apps.publisher import emails
from course_discovery.apps.publisher.choices import (CourseRunStateChoices, CourseStateChoices, InternalUserRole,
PublisherUserRole)
from course_discovery.apps.publisher.choices import (
CourseRunStateChoices, CourseStateChoices, InternalUserRole, PublisherUserRole
)
from course_discovery.apps.publisher.utils import is_email_notification_enabled, is_internal_user, is_publisher_admin
from course_discovery.apps.publisher.validators import ImageMultiSizeValidator
logger = logging.getLogger(__name__)
PAID_SEATS = ['verified', 'professional']
class ChangedByMixin(models.Model):
changed_by = models.ForeignKey(User, null=True, blank=True)
......@@ -430,6 +430,11 @@ class CourseRun(TimeStampedModel, ChangedByMixin):
seats = self.seats.filter(type__in=[Seat.AUDIT, Seat.VERIFIED, Seat.PROFESSIONAL, Seat.CREDIT])
return all([seat.is_valid_seat for seat in seats]) if seats else False
@property
def paid_seats(self):
""" Return course run paid seats """
return self.seats.filter(type__in=Seat.PAID_SEATS)
def get_absolute_url(self):
return reverse('publisher:publisher_course_run_detail', kwargs={'pk': self.id})
......@@ -441,6 +446,8 @@ class Seat(TimeStampedModel, ChangedByMixin):
PROFESSIONAL = 'professional'
NO_ID_PROFESSIONAL = 'no-id-professional'
CREDIT = 'credit'
PAID_SEATS = [VERIFIED, PROFESSIONAL, CREDIT]
PAID_AND_AUDIT_APPLICABLE_SEATS = [CREDIT, VERIFIED]
SEAT_TYPE_CHOICES = (
(HONOR, _('Honor')),
......
course_discovery/apps/publisher/tests/test_views.py
......@@ -34,8 +34,8 @@ from course_discovery.apps.publisher.choices import (
CourseRunStateChoices, CourseStateChoices, InternalUserRole, PublisherUserRole
)
from course_discovery.apps.publisher.constants import (
ADMIN_GROUP_NAME, INTERNAL_USER_GROUP_NAME, PROJECT_COORDINATOR_GROUP_NAME, PUBLISHER_ENTITLEMENTS_WAFFLE_SWITCH,
REVIEWER_GROUP_NAME
ADMIN_GROUP_NAME, INTERNAL_USER_GROUP_NAME, PROJECT_COORDINATOR_GROUP_NAME,
PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS, PUBLISHER_ENTITLEMENTS_WAFFLE_SWITCH, REVIEWER_GROUP_NAME
)
from course_discovery.apps.publisher.models import (
Course, CourseEntitlement, CourseRun, CourseRunState, CourseState, OrganizationExtension, Seat
......@@ -554,25 +554,36 @@ class CreateCourseRunViewTests(SiteMixin, TestCase):
def test_existing_run_and_seat_data_auto_populated(self):
"""
Verify that existing course run and seat data auto populated on new course run form.
Create a course run with a verified seat (for a verified and credit seat, audit seat
is automatically created), then try to create a new run of the same course and verify
the previous course run's seats and pacing is automatically populated.
"""
verified_seat_price = 550.0
latest_run = self.course.course_runs.latest('created')
factories.SeatFactory(course_run=latest_run, type=Seat.VERIFIED, price=550.0)
latest_seat = latest_run.seats.latest('created')
factories.SeatFactory(course_run=latest_run, type=Seat.VERIFIED, price=verified_seat_price)
factories.SeatFactory(course_run=latest_run, type=Seat.AUDIT)
# Verified seat should be preselected
verified_seat = latest_run.seats.get(type=Seat.VERIFIED)
response = self.client.get(self.create_course_run_url_new)
response_content = BeautifulSoup(response.content)
pacing_type_attribute = response_content.find(
"input", {"value": latest_run.pacing_type, "type": "radio", "name": "pacing_type"}
)
seat_type_attribute = response_content.find("option", {"value": latest_seat.type})
seat_type_attribute = response_content.find("option", {"value": verified_seat.type})
price_attribute = response_content.find(
"input", {"value": latest_seat.price, "id": "id_price", "step": "0.01", "type": "number"}
"input", {"value": verified_seat.price, "id": "id_price", "step": "0.01", "type": "number"}
)
# Verify that existing course run and seat values auto populated on form.
assert pacing_type_attribute is not None
assert seat_type_attribute is not None
assert price_attribute is not None
self.assertIsNotNone(pacing_type_attribute)
self.assertIn('checked=""', str(pacing_type_attribute))
self.assertIsNotNone(seat_type_attribute)
self.assertIn('selected=""', str(seat_type_attribute))
self.assertIsNotNone(price_attribute)
self.assertIn(str(verified_seat_price), str(price_attribute))
def test_credit_type_without_price(self):
""" Verify that without credit price course-run cannot be created with credit seat type. """
......@@ -664,7 +675,7 @@ class CreateCourseRunViewTests(SiteMixin, TestCase):
assign_perm(
OrganizationExtension.VIEW_COURSE_RUN, self.organization_extension.group, self.organization_extension
)
toggle_switch('publisher_create_audit_seats_for_verified_course_runs', True)
toggle_switch(PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS, True)
self.course.entitlements.create(mode=entitlement_mode, price=entitlement_price)
post_data = {'start': '2018-02-01 00:00:00', 'end': '2018-02-28 00:00:00', 'pacing_type': 'instructor_paced'}
......@@ -3072,7 +3083,7 @@ class CourseRunEditViewTests(SiteMixin, TestCase):
assign_perm(OrganizationExtension.EDIT_COURSE_RUN, self.group, self.organization_extension)
assign_perm(OrganizationExtension.VIEW_COURSE_RUN, self.group, self.organization_extension)
# assert edit page is loading sucesfully.
# assert edit page is loading successfully.
self.edit_page_url = reverse('publisher:publisher_course_runs_edit', kwargs={'pk': self.new_course_run.id})
response = self.client.get(self.edit_page_url)
self.assertEqual(response.status_code, 200)
......@@ -3105,6 +3116,19 @@ class CourseRunEditViewTests(SiteMixin, TestCase):
response = self.client.get(url)
self.assertContains(response, 'CERTIFICATE TYPE AND PRICE', status_code=200)
def login_with_course_user_role(self, course_run, role=PublisherUserRole.CourseTeam):
self.client.logout()
user = course_run.course.course_user_roles.get(role=role).user
self.client.login(username=user.username, password=USER_PASSWORD)
return user
def assert_seats(self, course_run, expected_seat_count, expected_seats):
"""Helper method to assert expected course run seats"""
course_run_seats = Seat.objects.filter(course_run=course_run).order_by('type')
self.assertEqual(course_run_seats.count(), expected_seat_count)
seat_types = [seat.type for seat in course_run_seats]
self.assertEqual(seat_types, expected_seats)
def test_edit_page_with_two_seats(self):
"""
Verify that if a course run has both audit and verified seats, Verified seat is displayed
......@@ -3220,16 +3244,12 @@ class CourseRunEditViewTests(SiteMixin, TestCase):
def test_update_course_run_with_seat(self):
""" Verify that course run edit page create seat object also if not exists previously."""
self.client.logout()
user = self.new_course_run.course.course_user_roles.get(role=PublisherUserRole.CourseTeam).user
self.client.login(username=user.username, password=USER_PASSWORD)
user = self.login_with_course_user_role(self.new_course_run)
self.assertNotEqual(self.course_run.changed_by, user)
# post data without seat
data = {'image': ''}
updated_dict = self._post_data(data, self.new_course, self.new_course_run)
updated_dict['type'] = Seat.PROFESSIONAL
updated_dict['price'] = 10.00
......@@ -3242,12 +3262,52 @@ class CourseRunEditViewTests(SiteMixin, TestCase):
target_status_code=200
)
course_run = CourseRun.objects.get(id=self.new_course_run.id)
self.assert_seats(self.new_course_run, 1, [Seat.PROFESSIONAL])
self.assertEqual(self.new_course_run.seats.first().price, 10)
self.assertEqual(self.new_course_run.seats.first().history.all().count(), 1)
def test_update_course_run_create_duplicate_seats(self):
"""
Tests that course run seats are not duplicated when editing.
Course run seats should remain consistent when updating seats.
"""
self.login_with_course_user_role(self.new_course_run)
data = {'image': '', 'type': Seat.VERIFIED, 'price': 10.00}
toggle_switch(PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS, True)
post_data = self._post_data(data, self.new_course, self.new_course_run)
self.client.post(self.edit_page_url, post_data)
self.assert_seats(self.new_course_run, 2, [Seat.AUDIT, Seat.VERIFIED])
post_data['price'] = 20.00
self.client.post(self.edit_page_url, post_data)
self.assert_seats(self.new_course_run, 2, [Seat.AUDIT, Seat.VERIFIED])
self.assertEqual(self.new_course_run.paid_seats.first().price, 20.00)
@ddt.data(Seat.VERIFIED, Seat.CREDIT)
def test_cleanup_course_run_seats_bogus_data(self, seat_type):
"""
Test that bogus course run seat data is corrected upon saving the
course run.
Some data in publisher has duplicate course seats, this tests that we are able
to correct the data upon course run save.
"""
# Login and verify the course run has no seats.
self.login_with_course_user_role(self.new_course_run)
self.assert_seats(self.new_course_run, 0, [])
self.assertEqual(course_run.seats.first().type, Seat.PROFESSIONAL)
self.assertEqual(course_run.seats.first().price, 10)
# create some bogus seats for the course run & verify the seats exists
__ = [self.new_course_run.seats.create(type=seat_type) for _ in range(10)]
self.assert_seats(self.new_course_run, 10, [seat_type] * 10)
self.assertEqual(course_run.seats.first().history.all().count(), 1)
# Make course run save post request and verify the correct course seats.
toggle_switch(PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS, True)
data = {'image': '', 'type': Seat.VERIFIED, 'price': 100.00}
post_data = self._post_data(data, self.new_course, self.new_course_run)
self.client.post(self.edit_page_url, post_data)
self.assert_seats(self.new_course_run, 2, [Seat.AUDIT, Seat.VERIFIED])
self.assertEqual(self.new_course_run.paid_seats.first().price, 100.00)
def test_update_course_run_for_course_that_uses_entitlements(self):
""" Verify that a user cannot change Seat data when editing courseruns for courses that use entitlements. """
......@@ -3840,7 +3900,7 @@ class CreateRunFromDashboardViewTests(SiteMixin, TestCase):
assign_perm(
OrganizationExtension.VIEW_COURSE_RUN, self.organization_extension.group, self.organization_extension
)
toggle_switch('publisher_create_audit_seats_for_verified_course_runs', True)
toggle_switch(PUBLISHER_CREATE_AUDIT_SEATS_FOR_VERIFIED_COURSE_RUNS, True)
self.course.entitlements.create(mode=entitlement_mode, price=entitlement_price)
post_data = {
......
course_discovery/apps/publisher/views.py
......@@ -30,13 +30,17 @@ from course_discovery.apps.publisher.choices import CourseRunStateChoices, Cours
from course_discovery.apps.publisher.constants import PUBLISHER_ENTITLEMENTS_WAFFLE_SWITCH
from course_discovery.apps.publisher.dataloader.create_courses import process_course
from course_discovery.apps.publisher.emails import send_email_for_published_course_run_editing
from course_discovery.apps.publisher.forms import (AdminImportCourseForm, CourseEntitlementForm, CourseForm,
CourseRunForm, CourseSearchForm, SeatForm)
from course_discovery.apps.publisher.models import (PAID_SEATS, Course, CourseEntitlement, CourseRun, CourseRunState,
CourseState, CourseUserRole, OrganizationExtension, PublisherUser,
Seat, UserAttributes)
from course_discovery.apps.publisher.utils import (get_internal_users, has_role_for_course, is_internal_user,
is_project_coordinator_user, is_publisher_admin, make_bread_crumbs)
from course_discovery.apps.publisher.forms import (
AdminImportCourseForm, CourseEntitlementForm, CourseForm, CourseRunForm, CourseSearchForm, SeatForm
)
from course_discovery.apps.publisher.models import (
Course, CourseEntitlement, CourseRun, CourseRunState, CourseState, CourseUserRole, OrganizationExtension,
PublisherUser, Seat, UserAttributes
)
from course_discovery.apps.publisher.utils import (
get_internal_users, has_role_for_course, is_internal_user, is_project_coordinator_user, is_publisher_admin,
make_bread_crumbs
)
from course_discovery.apps.publisher.wrappers import CourseRunWrapper
logger = logging.getLogger(__name__)
......@@ -582,15 +586,23 @@ class CreateCourseRunView(mixins.LoginRequiredMixin, mixins.PublisherUserRequire
new_run.save()
def _initialize_seat_form(self, last_run=None):
def _initialize_seat_form(self, last_run):
initial_seat_data = {}
if last_run:
try:
latest_seat = last_run.seats.latest('created')
initial_seat_data = model_to_dict(latest_seat)
del initial_seat_data['id'], initial_seat_data['course_run'], initial_seat_data['changed_by']
except Seat.DoesNotExist:
pass
if not last_run:
return self.seat_form(initial=initial_seat_data)
def _get_latest_seat():
"""Returns latest course run seat. Paid seats are Preferred"""
if last_run.paid_seats:
return last_run.paid_seats.latest()
return last_run.seats.latest()
try:
latest_seat = _get_latest_seat()
initial_seat_data = model_to_dict(latest_seat)
del initial_seat_data['id'], initial_seat_data['course_run'], initial_seat_data['changed_by']
except Seat.DoesNotExist:
pass
return self.seat_form(initial=initial_seat_data)
......@@ -787,6 +799,11 @@ class CourseRunEditView(mixins.LoginRequiredMixin, mixins.PublisherPermissionMix
'organizations': mixins.get_user_organizations(user)
}
def get_latest_course_run_seat(self, course_run):
if course_run.paid_seats:
return course_run.paid_seats.first()
return course_run.seats.first()
def get(self, request, *args, **kwargs):
context = self.get_context_data()
course_run = context.get('course_run')
......@@ -804,11 +821,8 @@ class CourseRunEditView(mixins.LoginRequiredMixin, mixins.PublisherPermissionMix
)
if not course.uses_entitlements:
course_run_paid_seat = course_run.seats.filter(type__in=PAID_SEATS).first()
if course_run_paid_seat:
context['seat_form'] = self.seat_form(instance=course_run_paid_seat)
else:
context['seat_form'] = self.seat_form(instance=course_run.seats.first())
course_run_seat = self.get_latest_course_run_seat(course_run)
context['seat_form'] = self.seat_form(instance=course_run_seat)
start_date = course_run.start.strftime("%B %d, %Y") if course_run.start else None
context['breadcrumbs'] = make_bread_crumbs(
......@@ -841,16 +855,14 @@ class CourseRunEditView(mixins.LoginRequiredMixin, mixins.PublisherPermissionMix
form_data_is_valid = form_data_is_valid and not seat_data_in_form
seat_form = None
else:
seat_form = self.seat_form(request.POST, instance=course_run.seats.first())
seat_form = self.seat_form(request.POST, instance=self.get_latest_course_run_seat(course_run))
form_data_is_valid = form_data_is_valid and seat_form.is_valid()
context['seat_form'] = seat_form
if form_data_is_valid:
try:
with transaction.atomic():
course_run = run_form.save(changed_by=user)
run_form.save_m2m()
# If price-type comes with request then save the seat object.
......
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