From 3fb659244302d694ea234831dff5ec750ccaba1f Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Mon, 1 Feb 2016 14:01:36 +0500 Subject: [PATCH 1/6] SUST-24 Initial commit Add bokchoy test and changes in html --- .../test/acceptance/pages/lms/courseware.py | 20 + common/test/acceptance/pages/studio/index.py | 24 + .../tests/studio/base_studio_test.py | 1 + .../tests/studio/test_studio_home.py | 78 ++- .../tests/studio/test_studio_outline.py | 8 + lms/djangoapps/certificates/models.py | 1 - lms/djangoapps/courseware/views/views.py | 443 ++++++++++++++++++ lms/djangoapps/instructor/views/api.py | 1 - lms/djangoapps/instructor_task/api.py | 1 + lms/templates/enroll_staff.html | 37 ++ lms/urls.py | 8 + 11 files changed, 619 insertions(+), 3 deletions(-) create mode 100644 lms/templates/enroll_staff.html diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 34629acd99..ca0b7f59e8 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -286,3 +286,23 @@ class CoursewareSequentialTabPage(CoursePage): return the body of the sequential currently selected """ return self.q(css='#seq_content .xblock').text[0] + + +class AboutPage(CoursePage): + """ + Course about. + """ + + url_path = "about/" + + def is_browser_on_page(self): + return self.q(css='.intro').present + + + @property + def is_register_button_present(self): + """ + Returns True if the timed/proctored exam timer bar is visible on the courseware. + """ + from nose.tools import set_trace; set_trace() + return self.q(css=".register").is_present() diff --git a/common/test/acceptance/pages/studio/index.py b/common/test/acceptance/pages/studio/index.py index ed6bd4158a..eedeb60667 100644 --- a/common/test/acceptance/pages/studio/index.py +++ b/common/test/acceptance/pages/studio/index.py @@ -5,6 +5,7 @@ Studio Home page from bok_choy.page_object import PageObject from . import BASE_URL from selenium.webdriver import ActionChains +from ..common.utils import click_css class DashboardPage(PageObject): @@ -12,11 +13,24 @@ class DashboardPage(PageObject): Studio Home page """ + def __init__(self, browser): + """ + Initialize the page. + """ + super(DashboardPage, self).__init__(browser) url = BASE_URL + "/course/" + def is_browser_on_page(self): return self.q(css='.content-primary').visible + def mouse_hover(self, element): + """ + Mouse over on given element. + """ + mouse_hover_action = ActionChains(self.browser).move_to_element(element) + mouse_hover_action.perform() + @property def course_runs(self): """ @@ -48,6 +62,16 @@ class DashboardPage(PageObject): # Clicking on course with run will trigger an ajax event self.wait_for_ajax() + + def view_live(self, element): + """ + Clicks the "View Live" link and switches to the new tab + """ + self.mouse_hover(self.browser.find_element_by_css_selector('.view-button')) + click_css(self, '.view-button', require_notification=False) + self.browser.switch_to_window(self.browser.window_handles[-1]) + click_css(self, element, require_notification=False) + def has_new_library_button(self): """ (bool) is the "New Library" button present? diff --git a/common/test/acceptance/tests/studio/base_studio_test.py b/common/test/acceptance/tests/studio/base_studio_test.py index 96f1fb706e..e7f959fbf3 100644 --- a/common/test/acceptance/tests/studio/base_studio_test.py +++ b/common/test/acceptance/tests/studio/base_studio_test.py @@ -11,6 +11,7 @@ from ...pages.studio.overview import CourseOutlinePage from ...pages.studio.utils import verify_ordering + class StudioCourseTest(UniqueCourseTest): """ Base class for all Studio course tests. diff --git a/common/test/acceptance/tests/studio/test_studio_home.py b/common/test/acceptance/tests/studio/test_studio_home.py index ddd58d4aed..482433d3c9 100644 --- a/common/test/acceptance/tests/studio/test_studio_home.py +++ b/common/test/acceptance/tests/studio/test_studio_home.py @@ -1,9 +1,14 @@ """ Acceptance tests for Home Page (My Courses / My Libraries). """ +import os +import uuid + from bok_choy.web_app_test import WebAppTest +from common.test.acceptance.pages.common.logout import LogoutPage +from common.test.acceptance.pages.lms.courseware import AboutPage, CoursewarePage from flaky import flaky -from opaque_keys.edx.locator import LibraryLocator +from opaque_keys.edx.locator import LibraryLocator, CourseLocator from uuid import uuid4 from ...fixtures import PROGRAMS_STUB_URL @@ -173,3 +178,74 @@ class StudioLanguageTest(WebAppTest): get_selected_option_text(language_selector), u'Dummy Language (Esperanto)' ) + +class CourseNotEnrollTest(WebAppTest): + """ + Test that we can create a new content library on the studio home page. + """ + + def setUp(self): + """ + Load the helper for the home page (dashboard page) + """ + super(CourseNotEnrollTest, self).setUp() + + self.auth_page = AutoAuthPage(self.browser, staff=True) + self.dashboard_page = DashboardPage(self.browser) + self.course_name = "New Course Name" + self.course_org = "orgX" + self.course_number = str(uuid.uuid4().get_hex().upper()[0:6]) + self.course_run = "2015_T2" + + def course_id(self): + """ + Returns the serialized course_key for the test + """ + # TODO - is there a better way to make this agnostic to the underlying default module store? + default_store = os.environ.get('DEFAULT_STORE', 'draft') + course_key = CourseLocator( + self.course_org, + self.course_number, + self.course_run, + deprecated=(default_store == 'draft') + ) + return unicode(course_key) + + def test_unenroll_course(self): + """ + From the home page: + Click "New Course" ,Fill out the form + Submit the form + Return to the home page and logout + Login with the staff user which is not enrolled in the course + click the view live button of the course + Here are two scenario: + First click the continue button + Second click the Enroll button and see the response. + """ + self.auth_page.visit() + self.dashboard_page.visit() + self.assertTrue(self.dashboard_page.new_course_button.present) + + self.dashboard_page.click_new_course_button() + self.assertTrue(self.dashboard_page.is_new_course_form_visible()) + self.dashboard_page.fill_new_course_form( + self.course_name, self.course_org, self.course_number, self.course_run + ) + self.assertTrue(self.dashboard_page.is_new_course_form_valid()) + self.dashboard_page.submit_new_course_form() + + LogoutPage(self.browser).visit() + AutoAuthPage(self.browser, course_id=None, staff=True).visit() + + self.dashboard_page.visit() + self.dashboard_page.view_live('.submit>input:last-child') + about_page = AboutPage(self.browser, self.course_id) + about_page.wait_for_page() + self.assertTrue(about_page.is_register_button_present) + + self.dashboard_page.visit() + self.dashboard_page.view_live('.submit>input:first-child') + courseware = CoursewarePage(self.browser, self.course_id) + courseware.wait_for_page() + self.assertTrue(courseware.is_browser_on_page()) \ No newline at end of file diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index d7cd119d69..eab14b5ea3 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -1449,6 +1449,14 @@ class DefaultStatesContentTest(CourseOutlineTest): self.assertEqual(courseware.xblock_component_type(1), 'html') self.assertEqual(courseware.xblock_component_type(2), 'discussion') + def test_unenroll_course(self): + from nose.tools import set_trace; set_trace() + self.course_outline_page.visit() + self.course_outline_page.view_live() + courseware = CoursewarePage(self.browser, self.course_id) + courseware.wait_for_page() + self.assertEqual(courseware.num_xblock_components, 3) + @attr('shard_3') class UnitNavigationTest(CourseOutlineTest): diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 17031810c9..4e03bfaea6 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -222,7 +222,6 @@ class GeneratedCertificate(models.Model): MODES = Choices('verified', 'honor', 'audit', 'professional', 'no-id-professional') VERIFIED_CERTS_MODES = [CourseMode.VERIFIED, CourseMode.CREDIT_MODE] - user = models.ForeignKey(User) course_id = CourseKeyField(max_length=255, blank=True, default=None) verify_uuid = models.CharField(max_length=32, blank=True, default='', db_index=True) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 9cdeb1d62f..606c9fbd2a 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -30,6 +30,8 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from rest_framework import status +from xblock.fragment import Fragment +from instructor.views.api import require_global_staff import shoppingcart import survey.utils @@ -72,6 +74,10 @@ from openedx.core.djangoapps.theming import helpers as theming_helpers from shoppingcart.utils import is_shopping_cart_enabled from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from student.models import UserTestGroup, CourseEnrollment + +from student.roles import GlobalStaff +from student.views import is_course_blocked + from util.cache import cache, cache_if_anonymous from util.date_utils import strftime_localized from util.db import outer_atomic @@ -195,6 +201,389 @@ def get_current_child(xmodule, min_depth=None, requested_child=None): return child +def redirect_to_course_position(course_module, content_depth): + """ + Return a redirect to the user's current place in the course. + + If this is the user's first time, redirects to COURSE/CHAPTER/SECTION. + If this isn't the users's first time, redirects to COURSE/CHAPTER, + and the view will find the current section and display a message + about reusing the stored position. + + If there is no current position in the course or chapter, then selects + the first child. + + """ + urlargs = {'course_id': course_module.id.to_deprecated_string()} + chapter = get_current_child(course_module, min_depth=content_depth) + if chapter is None: + # oops. Something bad has happened. + raise Http404("No chapter found when loading current position in course") + + urlargs['chapter'] = chapter.url_name + if course_module.position is not None: + return redirect(reverse('courseware_chapter', kwargs=urlargs)) + + # Relying on default of returning first child + section = get_current_child(chapter, min_depth=content_depth - 1) + if section is None: + raise Http404("No section found when loading current position in course") + + urlargs['section'] = section.url_name + return redirect(reverse('courseware_section', kwargs=urlargs)) + + +def save_child_position(seq_module, child_name): + """ + child_name: url_name of the child + """ + for position, c in enumerate(seq_module.get_display_items(), start=1): + if c.location.name == child_name: + # Only save if position changed + if position != seq_module.position: + seq_module.position = position + # Save this new position to the underlying KeyValueStore + seq_module.save() + + +def save_positions_recursively_up(user, request, field_data_cache, xmodule, course=None): + """ + Recurses up the course tree starting from a leaf + Saving the position property based on the previous node as it goes + """ + current_module = xmodule + + while current_module: + parent_location = modulestore().get_parent_location(current_module.location) + parent = None + if parent_location: + parent_descriptor = modulestore().get_item(parent_location) + parent = get_module_for_descriptor( + user, + request, + parent_descriptor, + field_data_cache, + current_module.location.course_key, + course=course + ) + + if parent and hasattr(parent, 'position'): + save_child_position(parent, current_module.location.name) + + current_module = parent + + +@transaction.non_atomic_requests +@login_required +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@ensure_valid_course_key +@outer_atomic(read_committed=True) +def index(request, course_id, chapter=None, section=None, + position=None): + """ + Displays courseware accordion and associated content. If course, chapter, + and section are all specified, renders the page, or returns an error if they + are invalid. + + If section is not specified, displays the accordion opened to the right chapter. + + If neither chapter or section are specified, redirects to user's most recent + chapter, or the first chapter if this is the user's first visit. + + Arguments: + + - request : HTTP request + - course_id : course id (str: ORG/course/URL_NAME) + - chapter : chapter url_name (str) + - section : section url_name (str) + - position : position in module, eg of module (str) + + Returns: + + - HTTPresponse + """ + + course_key = CourseKey.from_string(course_id) + + # Gather metrics for New Relic so we can slice data in New Relic Insights + newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) + newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) + + user = User.objects.prefetch_related("groups").get(id=request.user.id) + + redeemed_registration_codes = CourseRegistrationCode.objects.filter( + course_id=course_key, + registrationcoderedemption__redeemed_by=request.user + ) + + # Redirect to dashboard if the course is blocked due to non-payment. + if is_course_blocked(request, redeemed_registration_codes, course_key): + # registration codes may be generated via Bulk Purchase Scenario + # we have to check only for the invoice generated registration codes + # that their invoice is valid or not + log.warning( + u'User %s cannot access the course %s because payment has not yet been received', + user, + course_key.to_deprecated_string() + ) + return redirect(reverse('dashboard')) + + request.user = user # keep just one instance of User + with modulestore().bulk_operations(course_key): + return _index_bulk_op(request, course_key, chapter, section, position) + + +# pylint: disable=too-many-statements +def _index_bulk_op(request, course_key, chapter, section, position): + """ + Render the index page for the specified course. + """ + # Verify that position a string is in fact an int + if position is not None: + try: + int(position) + except ValueError: + raise Http404(u"Position {} is not an integer!".format(position)) + + course = get_course_with_access(request.user, 'load', course_key, depth=2) + staff_access = has_access(request.user, 'staff', course) + masquerade, user = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True) + + registered = registered_for_course(course, user) + if not registered: + # TODO (vshnayder): do course instructors need to be registered to see course? + log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.to_deprecated_string()) + if bool(staff_access) == False: + return redirect("{url}?{redirect}".format( + url=reverse(enroll_staff, args=[course_key.to_deprecated_string()]), + redirect=request.GET.urlencode() + ) + ) + return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) + + # see if all pre-requisites (as per the milestones app feature) have been fulfilled + # Note that if the pre-requisite feature flag has been turned off (default) then this check will + # always pass + if not has_access(user, 'view_courseware_with_prerequisites', course): + # prerequisites have not been fulfilled therefore redirect to the Dashboard + log.info( + u'User %d tried to view course %s ' + u'without fulfilling prerequisites', + user.id, unicode(course.id)) + return redirect(reverse('dashboard')) + + # Entrance Exam Check + # If the course has an entrance exam and the requested chapter is NOT the entrance exam, and + # the user hasn't yet met the criteria to bypass the entrance exam, redirect them to the exam. + if chapter and course_has_entrance_exam(course): + chapter_descriptor = course.get_child_by(lambda m: m.location.name == chapter) + if chapter_descriptor and not getattr(chapter_descriptor, 'is_entrance_exam', False) \ + and user_must_complete_entrance_exam(request, user, course): + log.info(u'User %d tried to view course %s without passing entrance exam', user.id, unicode(course.id)) + return redirect(reverse('courseware', args=[unicode(course.id)])) + + # Gated Content Check + gated_content = gating_api.get_gated_content(course, user) + if section and gated_content: + for usage_key in gated_content: + if section in usage_key: + raise Http404 + + # check to see if there is a required survey that must be taken before + # the user can access the course. + if survey.utils.must_answer_survey(course, user): + return redirect(reverse('course_survey', args=[unicode(course.id)])) + + bookmarks_api_url = reverse('bookmarks') + + try: + field_data_cache = FieldDataCache.cache_for_descriptor_descendents( + course_key, user, course, depth=2) + + studio_url = get_studio_url(course, 'course') + + language_preference = get_user_preference(request.user, LANGUAGE_KEY) + if not language_preference: + language_preference = settings.LANGUAGE_CODE + + context = { + 'csrf': csrf(request)['csrf_token'], + 'COURSE_TITLE': course.display_name_with_default_escaped, + 'course': course, + 'init': '', + 'fragment': Fragment(), + 'staff_access': staff_access, + 'studio_url': studio_url, + 'masquerade': masquerade, + 'xqa_server': settings.FEATURES.get('XQA_SERVER', "http://your_xqa_server.com"), + 'bookmarks_api_url': bookmarks_api_url, + 'language_preference': language_preference, + 'disable_optimizely': True, + } + table_of_contents, __, __ = toc_for_course(user, request, course, chapter, section, field_data_cache) + context['accordion'] = render_accordion(request, course, table_of_contents) + + now = datetime.now(UTC()) + effective_start = _adjust_start_date_for_beta_testers(user, course, course_key) + if not in_preview_mode() and staff_access and now < effective_start: + # Disable student view button if user is staff and + # course is not yet visible to students. + context['disable_student_access'] = True + + has_content = course.has_children_at_depth(CONTENT_DEPTH) + if not has_content: + # Show empty courseware for a course with no units + return render_to_response('courseware/courseware.html', context) + elif chapter is None: + # Check first to see if we should instead redirect the user to an Entrance Exam + if course_has_entrance_exam(course): + exam_chapter = get_entrance_exam_content(request, course) + if exam_chapter: + if exam_chapter.get_children(): + exam_section = exam_chapter.get_children()[0] + if exam_section: + return redirect('courseware_section', + course_id=unicode(course_key), + chapter=exam_chapter.url_name, + section=exam_section.url_name) + + # passing CONTENT_DEPTH avoids returning 404 for a course with an + # empty first section and a second section with content + return redirect_to_course_position(course, CONTENT_DEPTH) + + chapter_descriptor = course.get_child_by(lambda m: m.location.name == chapter) + if chapter_descriptor is not None: + save_child_position(course, chapter) + else: + # User may be trying to access a chapter that isn't live yet + if masquerade and masquerade.role == 'student': # if staff is masquerading as student be kinder, don't 404 + log.debug('staff masquerading as student: no chapter %s', chapter) + return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) + raise Http404('No chapter descriptor found with name {}'.format(chapter)) + + if course_has_entrance_exam(course): + # Message should not appear outside the context of entrance exam subsection. + # if section is none then we don't need to show message on welcome back screen also. + if getattr(chapter_descriptor, 'is_entrance_exam', False) and section is not None: + context['entrance_exam_current_score'] = get_entrance_exam_score(request, course) + context['entrance_exam_passed'] = user_has_passed_entrance_exam(request, course) + + if section is None: + section_descriptor = get_current_child(chapter_descriptor, requested_child=request.GET.get("child")) + if section_descriptor: + section = section_descriptor.url_name + else: + # Something went wrong -- perhaps this chapter has no sections visible to the user. + # Clearing out the last-visited state and showing "first-time" view by redirecting + # to courseware. + course.position = None + course.save() + return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) + else: + section_descriptor = chapter_descriptor.get_child_by(lambda m: m.location.name == section) + + if section_descriptor is None: + # Specifically asked-for section doesn't exist + if masquerade and masquerade.role == 'student': # don't 404 if staff is masquerading as student + log.debug('staff masquerading as student: no section %s', section) + return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) + raise Http404 + + # Allow chromeless operation + if section_descriptor.chrome: + chrome = [s.strip() for s in section_descriptor.chrome.lower().split(",")] + if 'accordion' not in chrome: + context['disable_accordion'] = True + if 'tabs' not in chrome: + context['disable_tabs'] = True + + if section_descriptor.default_tab: + context['default_tab'] = section_descriptor.default_tab + + # cdodge: this looks silly, but let's refetch the section_descriptor with depth=None + # which will prefetch the children more efficiently than doing a recursive load + section_descriptor = modulestore().get_item(section_descriptor.location, depth=None) + + # Load all descendants of the section, because we're going to display its + # html, which in general will need all of its children + field_data_cache.add_descriptor_descendents( + section_descriptor, depth=None + ) + + section_module = get_module_for_descriptor( + user, + request, + section_descriptor, + field_data_cache, + course_key, + position, + course=course + ) + + # Save where we are in the chapter. + save_child_position(chapter_descriptor, section) + + table_of_contents, prev_section_info, next_section_info = toc_for_course( + user, request, course, chapter, section, field_data_cache + ) + context['accordion'] = render_accordion(request, course, table_of_contents) + + def _compute_section_url(section_info, requested_child): + """ + Returns the section URL for the given section_info with the given child parameter. + """ + return "{url}?child={requested_child}".format( + url=reverse( + 'courseware_section', + args=[unicode(course.id), section_info['chapter_url_name'], section_info['url_name']], + ), + requested_child=requested_child, + ) + + section_render_context = { + 'activate_block_id': request.GET.get('activate_block_id'), + 'requested_child': request.GET.get("child"), + 'prev_url': _compute_section_url(prev_section_info, 'last') if prev_section_info else None, + 'next_url': _compute_section_url(next_section_info, 'first') if next_section_info else None, + } + context['fragment'] = section_module.render(STUDENT_VIEW, section_render_context) + context['section_title'] = section_descriptor.display_name_with_default_escaped + result = render_to_response('courseware/courseware.html', context) + except Exception as e: + + # Doesn't bar Unicode characters from URL, but if Unicode characters do + # cause an error it is a graceful failure. + if isinstance(e, UnicodeEncodeError): + raise Http404("URL contains Unicode characters") + + if isinstance(e, Http404): + # let it propagate + raise + + # In production, don't want to let a 500 out for any reason + if settings.DEBUG: + raise + else: + log.exception( + u"Error in index view: user=%s, effective_user=%s, course=%s, chapter=%s section=%s position=%s", + request.user, user, course, chapter, section, position + ) + try: + result = render_to_response('courseware/courseware-error.html', { + 'staff_access': staff_access, + 'course': course + }) + except: + # Let the exception propagate, relying on global config to at + # at least return a nice error message + log.exception("Error while rendering courseware-error page") + raise + + return result + + + @ensure_csrf_cookie @ensure_valid_course_key def jump_to_id(request, course_id, module_id): @@ -238,7 +627,13 @@ def jump_to(_request, course_id, location): except InvalidKeyError: raise Http404(u"Invalid course_key or usage_key") try: + user = _request.user redirect_url = get_redirect_url(course_key, usage_key) + if GlobalStaff().has_user(user) and not CourseEnrollment.is_enrolled(user, course_key): + redirect_url = "{url}?next={redirect}".format( + url=reverse(enroll_staff, args=[course_key.to_deprecated_string()]), + redirect=redirect_url + ) except ItemNotFoundError: raise Http404(u"No data at this location: {0}".format(usage_key)) except NoPathToItem: @@ -443,6 +838,54 @@ def get_cosmetic_display_price(course, registration_price): # Translators: This refers to the cost of the course. In this case, the course costs nothing so it is free. return _('Free') +@require_global_staff +@require_http_methods(['POST', 'GET']) +def enroll_staff(request, course_id): + ''' + 1. Should be staff + 2. should be a valid course_id + 3. shouldn't be enrolled before + 4. The requested view url to redirect + + URL-ABC-GOTO-HERE- + + 1. You want to register for this course? + Confirm: + 1. User is valid staff user who wants to enroll. + 2. Course is valid course + 2. Yes + 3. Post request, enroll the user and redirect him to the requested view + + :param request: + :param course_id: + :return: + ''' + user = request.user + course_key = CourseKey.from_string(course_id) + _next = urllib.quote_plus(request.GET.get('next', 'info'), safe='/:?=') + + if request.method == 'GET': + with modulestore().bulk_operations(course_key): + course = get_course_with_access(user, 'load', course_key, depth=2) + + # Prompt for enrollment if Globalstaff is not enrolled in the course + if not registered_for_course(course, user): + return render_to_response('enroll_staff.html', { + 'course': course, + 'csrftoken': csrf(request)["csrf_token"] + }) + + elif request.method == 'POST' and 'enroll' in request.POST.dict(): + enrollment = CourseEnrollment.get_or_create_enrollment(request.user, course_key) + enrollment.update_enrollment(is_active=True) + log.info( + u"User %s enrolled in %s via `enroll_staff` view", + user.username, + course_id + ) + + return redirect(_next) + @ensure_csrf_cookie @cache_if_anonymous() diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index a21db446bc..d6be03b833 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3289,7 +3289,6 @@ def validate_request_data_and_get_certificate(certificate_invalidation, course_k ) student = get_student(user, course_key) - certificate = GeneratedCertificate.certificate_for_student(student, course_key) if not certificate: raise ValueError(_( diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 10939226f4..2898a21f50 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -446,6 +446,7 @@ def submit_export_ora2_data(request, course_key): def generate_certificates_for_students(request, course_key, student_set=None, specific_student_id=None): # pylint: disable=invalid-name """ +<<<<<<< HEAD Submits a task to generate certificates for given students enrolled in the course. Arguments: diff --git a/lms/templates/enroll_staff.html b/lms/templates/enroll_staff.html new file mode 100644 index 0000000000..552e45ab8b --- /dev/null +++ b/lms/templates/enroll_staff.html @@ -0,0 +1,37 @@ +<%inherit file="main.html" /> +<%namespace name='static' file='static_content.html'/> +<%! +from django.utils.translation import ugettext as _ +from courseware.courses import get_course_info_section, get_course_date_summary + + +%> + +<%block name="pagetitle">${_("{course_number} Course Info").format(course_number=course.display_number_with_default)} + +<%block name="headextra"> +<%static:css group='style-course-vendor'/> +<%static:css group='style-course'/> + + +<%block name="bodyclass">view-in-course view-course-info ${course.css_class or ''} + + + diff --git a/lms/urls.py b/lms/urls.py index 9848cc0620..2b3cb0b437 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -366,6 +366,14 @@ urlpatterns += ( name='about_course', ), + url( + r'^courses/{}/enroll_staff$'.format( + settings.COURSE_ID_PATTERN, + ), + 'courseware.views.enroll_staff', + name='enroll_staff', + ), + #Inside the course url( r'^courses/{}/$'.format( From 94b58c2df67b79c90328be9a8970b5bc44330e2d Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Tue, 19 Apr 2016 17:39:21 +0500 Subject: [PATCH 2/6] Adding bokchoy test and change in html file. --- .../test/acceptance/pages/lms/courseware.py | 2 -- common/test/acceptance/pages/studio/index.py | 2 -- .../tests/studio/base_studio_test.py | 1 - .../tests/studio/test_studio_home.py | 20 +++++++++++-------- .../tests/studio/test_studio_outline.py | 1 - lms/djangoapps/certificates/models.py | 1 + lms/djangoapps/courseware/views/views.py | 5 +++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index ca0b7f59e8..ba2514dc5f 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -298,11 +298,9 @@ class AboutPage(CoursePage): def is_browser_on_page(self): return self.q(css='.intro').present - @property def is_register_button_present(self): """ Returns True if the timed/proctored exam timer bar is visible on the courseware. """ - from nose.tools import set_trace; set_trace() return self.q(css=".register").is_present() diff --git a/common/test/acceptance/pages/studio/index.py b/common/test/acceptance/pages/studio/index.py index eedeb60667..b9dd8c357d 100644 --- a/common/test/acceptance/pages/studio/index.py +++ b/common/test/acceptance/pages/studio/index.py @@ -20,7 +20,6 @@ class DashboardPage(PageObject): super(DashboardPage, self).__init__(browser) url = BASE_URL + "/course/" - def is_browser_on_page(self): return self.q(css='.content-primary').visible @@ -62,7 +61,6 @@ class DashboardPage(PageObject): # Clicking on course with run will trigger an ajax event self.wait_for_ajax() - def view_live(self, element): """ Clicks the "View Live" link and switches to the new tab diff --git a/common/test/acceptance/tests/studio/base_studio_test.py b/common/test/acceptance/tests/studio/base_studio_test.py index e7f959fbf3..96f1fb706e 100644 --- a/common/test/acceptance/tests/studio/base_studio_test.py +++ b/common/test/acceptance/tests/studio/base_studio_test.py @@ -11,7 +11,6 @@ from ...pages.studio.overview import CourseOutlinePage from ...pages.studio.utils import verify_ordering - class StudioCourseTest(UniqueCourseTest): """ Base class for all Studio course tests. diff --git a/common/test/acceptance/tests/studio/test_studio_home.py b/common/test/acceptance/tests/studio/test_studio_home.py index 482433d3c9..951f6a47a6 100644 --- a/common/test/acceptance/tests/studio/test_studio_home.py +++ b/common/test/acceptance/tests/studio/test_studio_home.py @@ -179,6 +179,7 @@ class StudioLanguageTest(WebAppTest): u'Dummy Language (Esperanto)' ) + class CourseNotEnrollTest(WebAppTest): """ Test that we can create a new content library on the studio home page. @@ -204,11 +205,11 @@ class CourseNotEnrollTest(WebAppTest): # TODO - is there a better way to make this agnostic to the underlying default module store? default_store = os.environ.get('DEFAULT_STORE', 'draft') course_key = CourseLocator( - self.course_org, - self.course_number, - self.course_run, - deprecated=(default_store == 'draft') - ) + self.course_org, + self.course_number, + self.course_run, + deprecated=(default_store == 'draft') + ) return unicode(course_key) def test_unenroll_course(self): @@ -235,17 +236,20 @@ class CourseNotEnrollTest(WebAppTest): self.assertTrue(self.dashboard_page.is_new_course_form_valid()) self.dashboard_page.submit_new_course_form() + self.dashboard_page.visit() LogoutPage(self.browser).visit() AutoAuthPage(self.browser, course_id=None, staff=True).visit() self.dashboard_page.visit() self.dashboard_page.view_live('.submit>input:last-child') + about_page = AboutPage(self.browser, self.course_id) about_page.wait_for_page() + self.assertTrue(about_page.is_browser_on_page()) self.assertTrue(about_page.is_register_button_present) self.dashboard_page.visit() self.dashboard_page.view_live('.submit>input:first-child') - courseware = CoursewarePage(self.browser, self.course_id) - courseware.wait_for_page() - self.assertTrue(courseware.is_browser_on_page()) \ No newline at end of file + course_ware = CoursewarePage(self.browser, self.course_id) + course_ware.wait_for_page() + self.assertTrue(course_ware.is_browser_on_page()) diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index eab14b5ea3..ef65e003c4 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -1450,7 +1450,6 @@ class DefaultStatesContentTest(CourseOutlineTest): self.assertEqual(courseware.xblock_component_type(2), 'discussion') def test_unenroll_course(self): - from nose.tools import set_trace; set_trace() self.course_outline_page.visit() self.course_outline_page.view_live() courseware = CoursewarePage(self.browser, self.course_id) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 4e03bfaea6..17031810c9 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -222,6 +222,7 @@ class GeneratedCertificate(models.Model): MODES = Choices('verified', 'honor', 'audit', 'professional', 'no-id-professional') VERIFIED_CERTS_MODES = [CourseMode.VERIFIED, CourseMode.CREDIT_MODE] + user = models.ForeignKey(User) course_id = CourseKeyField(max_length=255, blank=True, default=None) verify_uuid = models.CharField(max_length=32, blank=True, default='', db_index=True) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 606c9fbd2a..294b816ecf 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -354,11 +354,11 @@ def _index_bulk_op(request, course_key, chapter, section, position): if not registered: # TODO (vshnayder): do course instructors need to be registered to see course? log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.to_deprecated_string()) - if bool(staff_access) == False: + if not bool(staff_access): return redirect("{url}?{redirect}".format( url=reverse(enroll_staff, args=[course_key.to_deprecated_string()]), redirect=request.GET.urlencode() - ) + ) ) return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) @@ -838,6 +838,7 @@ def get_cosmetic_display_price(course, registration_price): # Translators: This refers to the cost of the course. In this case, the course costs nothing so it is free. return _('Free') + @require_global_staff @require_http_methods(['POST', 'GET']) def enroll_staff(request, course_id): From b19264974bc3eb0ef48d6f2367408870af057f8f Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Fri, 22 Apr 2016 17:09:43 +0500 Subject: [PATCH 3/6] Adding unit test --- lms/djangoapps/courseware/tests/test_views.py | 42 +++++++++++++++++++ lms/djangoapps/courseware/views/views.py | 26 ++++++------ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index cf5c9cba88..8327f065c1 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -323,6 +323,48 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertNotIn('Problem 1', response.content) self.assertNotIn('Problem 2', response.content) + def test_enroll_staff(self): + """ + Here we check two methods GET and POST and should be a staff user + """ + self.user.is_staff = True + self.user.save() + self.client.login(username=self.user.username, password=self.password) + + # create the course + course = CourseFactory.create() + chapter = ItemFactory.create(parent=course, category='chapter') + section = ItemFactory.create(parent=chapter, category='sequential', display_name="Sequence") + + # create the _next parameter + courseware_url = reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(course.id), + 'chapter': chapter.url_name, + 'section': section.url_name, + } + ) + '?activate_block_id=test_block_id' + + # create the url for enroll_staff view + url = "{enroll_staff_url}?next={courseware_url}".format( + enroll_staff_url= reverse('enroll_staff', kwargs={'course_id': unicode(course.id)}), + courseware_url= courseware_url + ) + + # Check the GET method + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + response_content = response.content + self.assertIn('Enroll' , response_content) + self.assertIn('Continue', response_content) + + # Check the POST Method + data = {'enroll' : 'Enroll'} + response_post = self.client.post(url, data=data) + # here we check the status code 302 because of the redirect + self.assertEqual(response_post.status_code, 302) + @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) def test_course_about_in_cart(self): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 294b816ecf..53b657ac0f 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -840,27 +840,29 @@ def get_cosmetic_display_price(course, registration_price): @require_global_staff -@require_http_methods(['POST', 'GET']) +@ensure_valid_course_key def enroll_staff(request, course_id): - ''' + """ 1. Should be staff 2. should be a valid course_id 3. shouldn't be enrolled before 4. The requested view url to redirect - URL-ABC-GOTO-HERE- + GET + html: return enroll staff page - 1. You want to register for this course? - Confirm: - 1. User is valid staff user who wants to enroll. - 2. Course is valid course - 2. Yes - 3. Post request, enroll the user and redirect him to the requested view + POST + 1. You want to register for this course? + Confirm: + 1. User is valid staff user who wants to enroll. + 2. Course is valid course + 2. Yes + 3. Post request, enroll the user and redirect him to the requested view :param request: :param course_id: :return: - ''' + """ user = request.user course_key = CourseKey.from_string(course_id) _next = urllib.quote_plus(request.GET.get('next', 'info'), safe='/:?=') @@ -876,8 +878,8 @@ def enroll_staff(request, course_id): 'csrftoken': csrf(request)["csrf_token"] }) - elif request.method == 'POST' and 'enroll' in request.POST.dict(): - enrollment = CourseEnrollment.get_or_create_enrollment(request.user, course_key) + elif request.method == 'POST' and 'enroll' in request.POST: + enrollment = CourseEnrollment.get_or_create_enrollment(user, course_key) enrollment.update_enrollment(is_active=True) log.info( u"User %s enrolled in %s via `enroll_staff` view", From 19ea9ae9a07b395ba82b897daaea9e43f9ebc5d5 Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Mon, 25 Apr 2016 15:57:00 +0500 Subject: [PATCH 4/6] Unit tests added and changes in enroll_staff view. --- .../tests/studio/test_studio_outline.py | 7 --- lms/djangoapps/courseware/tests/test_views.py | 55 ++++++++++-------- lms/djangoapps/courseware/views/views.py | 30 +++++----- lms/djangoapps/instructor/views/api.py | 1 + lms/djangoapps/instructor_task/api.py | 1 - lms/templates/enroll_staff.html | 57 ++++++++++--------- 6 files changed, 78 insertions(+), 73 deletions(-) diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index ef65e003c4..d7cd119d69 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -1449,13 +1449,6 @@ class DefaultStatesContentTest(CourseOutlineTest): self.assertEqual(courseware.xblock_component_type(1), 'html') self.assertEqual(courseware.xblock_component_type(2), 'discussion') - def test_unenroll_course(self): - self.course_outline_page.visit() - self.course_outline_page.view_live() - courseware = CoursewarePage(self.browser, self.course_id) - courseware.wait_for_page() - self.assertEqual(courseware.num_xblock_components, 3) - @attr('shard_3') class UnitNavigationTest(CourseOutlineTest): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 8327f065c1..8f85e86e94 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -323,47 +323,54 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertNotIn('Problem 1', response.content) self.assertNotIn('Problem 2', response.content) - def test_enroll_staff(self): + def _create_url_for_enroll_staff(self): """ - Here we check two methods GET and POST and should be a staff user + User will have satff access and creates the url for enroll_staff view """ self.user.is_staff = True - self.user.save() + self.user.save() # pylint: disable=no-member self.client.login(username=self.user.username, password=self.password) # create the course course = CourseFactory.create() - chapter = ItemFactory.create(parent=course, category='chapter') - section = ItemFactory.create(parent=chapter, category='sequential', display_name="Sequence") # create the _next parameter - courseware_url = reverse( - 'courseware_section', - kwargs={ - 'course_id': unicode(course.id), - 'chapter': chapter.url_name, - 'section': section.url_name, - } - ) + '?activate_block_id=test_block_id' + courseware_url = reverse('courseware', kwargs={ + 'course_id': course.id.to_deprecated_string()}) + '?activate_block_id=test_block_id' # create the url for enroll_staff view - url = "{enroll_staff_url}?next={courseware_url}".format( - enroll_staff_url= reverse('enroll_staff', kwargs={'course_id': unicode(course.id)}), - courseware_url= courseware_url + enroll_staff_url = "{enroll_staff_url}?next={courseware_url}".format( + enroll_staff_url=reverse('enroll_staff', kwargs={'course_id': unicode(course.id)}), + courseware_url=courseware_url ) + return enroll_staff_url + + def test_redirection_unenrolled_staff(self): + """ + Verify unenrolled staff is not redirected to the 'about' section of the chapter + """ + enroll_staff_url = self._create_url_for_enroll_staff() # Check the GET method - response = self.client.get(url) + response = self.client.get(enroll_staff_url) self.assertEqual(response.status_code, 200) response_content = response.content - self.assertIn('Enroll' , response_content) - self.assertIn('Continue', response_content) + self.assertIn('Enroll', response_content) + self.assertIn("Don't enroll", response_content) - # Check the POST Method - data = {'enroll' : 'Enroll'} - response_post = self.client.post(url, data=data) - # here we check the status code 302 because of the redirect - self.assertEqual(response_post.status_code, 302) + @ddt.data( + {'enroll': "Enroll"}, + {'dont_enroll': "Don't enroll"}, + ) + def test_redirection_unenrolled_staff_post_data(self, data): + """ + Verify unenrolled staff is redirected to the page according to data passed. + """ + enroll_staff_url = self._create_url_for_enroll_staff() + response = self.client.post(enroll_staff_url, data=data) + + # Here we check the status code 302 because of the redirect + self.assertEqual(response.status_code, 302) @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 53b657ac0f..1c6b7b0514 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -354,12 +354,12 @@ def _index_bulk_op(request, course_key, chapter, section, position): if not registered: # TODO (vshnayder): do course instructors need to be registered to see course? log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.to_deprecated_string()) - if not bool(staff_access): + if bool(staff_access): + usage_key = UsageKey.from_string(course.location.to_deprecated_string()).replace(course_key=course_key) + redirect_url = get_redirect_url(course_key, usage_key) return redirect("{url}?{redirect}".format( url=reverse(enroll_staff, args=[course_key.to_deprecated_string()]), - redirect=request.GET.urlencode() - ) - ) + redirect=redirect_url)) return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) # see if all pre-requisites (as per the milestones app feature) have been fulfilled @@ -878,16 +878,18 @@ def enroll_staff(request, course_id): 'csrftoken': csrf(request)["csrf_token"] }) - elif request.method == 'POST' and 'enroll' in request.POST: - enrollment = CourseEnrollment.get_or_create_enrollment(user, course_key) - enrollment.update_enrollment(is_active=True) - log.info( - u"User %s enrolled in %s via `enroll_staff` view", - user.username, - course_id - ) - - return redirect(_next) + elif request.method == 'POST': + if 'enroll' in request.POST: + enrollment = CourseEnrollment.get_or_create_enrollment(user, course_key) + enrollment.update_enrollment(is_active=True) + log.info( + u"User %s enrolled in %s via `enroll_staff` view", + user.username, + course_id + ) + return redirect(_next) + else: + return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) @ensure_csrf_cookie diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index d6be03b833..a21db446bc 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3289,6 +3289,7 @@ def validate_request_data_and_get_certificate(certificate_invalidation, course_k ) student = get_student(user, course_key) + certificate = GeneratedCertificate.certificate_for_student(student, course_key) if not certificate: raise ValueError(_( diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 2898a21f50..10939226f4 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -446,7 +446,6 @@ def submit_export_ora2_data(request, course_key): def generate_certificates_for_students(request, course_key, student_set=None, specific_student_id=None): # pylint: disable=invalid-name """ -<<<<<<< HEAD Submits a task to generate certificates for given students enrolled in the course. Arguments: diff --git a/lms/templates/enroll_staff.html b/lms/templates/enroll_staff.html index 552e45ab8b..5730cc6edf 100644 --- a/lms/templates/enroll_staff.html +++ b/lms/templates/enroll_staff.html @@ -1,37 +1,40 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> <%! -from django.utils.translation import ugettext as _ -from courseware.courses import get_course_info_section, get_course_date_summary - - + from django.utils.translation import ugettext as _ + from courseware.courses import get_course_about_section %> -<%block name="pagetitle">${_("{course_number} Course Info").format(course_number=course.display_number_with_default)} - <%block name="headextra"> -<%static:css group='style-course-vendor'/> -<%static:css group='style-course'/> + + -<%block name="bodyclass">view-in-course view-course-info ${course.css_class or ''} - - - + + + From 275a31ec42eb1e8c69bfe1916cc58c4ff2232b74 Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Thu, 28 Apr 2016 11:43:24 +0500 Subject: [PATCH 5/6] Change in Bokchoy test according to new design. --- .../test/acceptance/pages/lms/courseware.py | 4 +- common/test/acceptance/pages/studio/index.py | 2 +- .../tests/studio/test_studio_home.py | 12 +-- lms/djangoapps/courseware/tests/test_views.py | 29 ++++-- lms/djangoapps/courseware/url_helpers.py | 16 ++++ lms/djangoapps/courseware/views/views.py | 96 +++++++++---------- lms/templates/enroll_staff.html | 13 +-- lms/urls.py | 4 +- 8 files changed, 100 insertions(+), 76 deletions(-) diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index ba2514dc5f..9f717a27f5 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -290,7 +290,7 @@ class CoursewareSequentialTabPage(CoursePage): class AboutPage(CoursePage): """ - Course about. + Course about page. """ url_path = "about/" @@ -301,6 +301,6 @@ class AboutPage(CoursePage): @property def is_register_button_present(self): """ - Returns True if the timed/proctored exam timer bar is visible on the courseware. + Returns True if the Enrollment button is visible on the about page. """ return self.q(css=".register").is_present() diff --git a/common/test/acceptance/pages/studio/index.py b/common/test/acceptance/pages/studio/index.py index b9dd8c357d..5427cf2324 100644 --- a/common/test/acceptance/pages/studio/index.py +++ b/common/test/acceptance/pages/studio/index.py @@ -65,7 +65,7 @@ class DashboardPage(PageObject): """ Clicks the "View Live" link and switches to the new tab """ - self.mouse_hover(self.browser.find_element_by_css_selector('.view-button')) + self.mouse_hover(self.browser.find_element_by_css_selector('.course-item')) click_css(self, '.view-button', require_notification=False) self.browser.switch_to_window(self.browser.window_handles[-1]) click_css(self, element, require_notification=False) diff --git a/common/test/acceptance/tests/studio/test_studio_home.py b/common/test/acceptance/tests/studio/test_studio_home.py index 951f6a47a6..6751e9a8c4 100644 --- a/common/test/acceptance/tests/studio/test_studio_home.py +++ b/common/test/acceptance/tests/studio/test_studio_home.py @@ -203,12 +203,12 @@ class CourseNotEnrollTest(WebAppTest): Returns the serialized course_key for the test """ # TODO - is there a better way to make this agnostic to the underlying default module store? - default_store = os.environ.get('DEFAULT_STORE', 'draft') + default_store = os.environ.get('DEFAULT_STORE', 'split') course_key = CourseLocator( self.course_org, self.course_number, self.course_run, - deprecated=(default_store == 'draft') + deprecated=(default_store == 'split') ) return unicode(course_key) @@ -221,8 +221,8 @@ class CourseNotEnrollTest(WebAppTest): Login with the staff user which is not enrolled in the course click the view live button of the course Here are two scenario: - First click the continue button - Second click the Enroll button and see the response. + -First click the Don't Enroll button + -Second click the Enroll button and see the response. """ self.auth_page.visit() self.dashboard_page.visit() @@ -241,7 +241,7 @@ class CourseNotEnrollTest(WebAppTest): AutoAuthPage(self.browser, course_id=None, staff=True).visit() self.dashboard_page.visit() - self.dashboard_page.view_live('.submit>input:last-child') + self.dashboard_page.view_live('input[name= "dont_enroll"]') about_page = AboutPage(self.browser, self.course_id) about_page.wait_for_page() @@ -249,7 +249,7 @@ class CourseNotEnrollTest(WebAppTest): self.assertTrue(about_page.is_register_button_present) self.dashboard_page.visit() - self.dashboard_page.view_live('.submit>input:first-child') + self.dashboard_page.view_live('input[name= "enroll"]') course_ware = CoursewarePage(self.browser, self.course_id) course_ware.wait_for_page() self.assertTrue(course_ware.is_browser_on_page()) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 8f85e86e94..0d2447ee76 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -336,41 +336,50 @@ class ViewsTestCase(ModuleStoreTestCase): # create the _next parameter courseware_url = reverse('courseware', kwargs={ - 'course_id': course.id.to_deprecated_string()}) + '?activate_block_id=test_block_id' + 'course_id': unicode(course.id)}) + '?activate_block_id=test_block_id' # create the url for enroll_staff view enroll_staff_url = "{enroll_staff_url}?next={courseware_url}".format( enroll_staff_url=reverse('enroll_staff', kwargs={'course_id': unicode(course.id)}), courseware_url=courseware_url ) - return enroll_staff_url + return enroll_staff_url, course.id def test_redirection_unenrolled_staff(self): """ - Verify unenrolled staff is not redirected to the 'about' section of the chapter + Test that staff is not redirected to the 'about' page when visiting an unregistered course """ - enroll_staff_url = self._create_url_for_enroll_staff() + enroll_staff_url, __ = self._create_url_for_enroll_staff() # Check the GET method response = self.client.get(enroll_staff_url) self.assertEqual(response.status_code, 200) response_content = response.content self.assertIn('Enroll', response_content) - self.assertIn("Don't enroll", response_content) + self.assertIn("dont_enroll", response_content) @ddt.data( - {'enroll': "Enroll"}, - {'dont_enroll': "Don't enroll"}, - ) - def test_redirection_unenrolled_staff_post_data(self, data): + ({'enroll': "Enroll"}, 'courses/{}/courseware?activate_block_id=test_block_id'), + ({'dont_enroll': "Don't enroll"}, '/courses/{}/about')) + @ddt.unpack + def test_enroll_staff_redirection(self, data, expected_url): """ Verify unenrolled staff is redirected to the page according to data passed. """ - enroll_staff_url = self._create_url_for_enroll_staff() + enroll_staff_url, course_id = self._create_url_for_enroll_staff() response = self.client.post(enroll_staff_url, data=data) # Here we check the status code 302 because of the redirect self.assertEqual(response.status_code, 302) + self.assertRedirects(response, expected_url.format(unicode(course_id))) + + def test_enroll_staff_redirection_invalid_data(self): + """ + Verify unenrolled staff is redirected to the page according to data passed. + """ + enroll_staff_url, __ = self._create_url_for_enroll_staff() + response = self.client.post(enroll_staff_url, data={'test': "test"}) + self.assertEqual(response.status_code, 404) @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) diff --git a/lms/djangoapps/courseware/url_helpers.py b/lms/djangoapps/courseware/url_helpers.py index fcdf567dd2..41acff095d 100644 --- a/lms/djangoapps/courseware/url_helpers.py +++ b/lms/djangoapps/courseware/url_helpers.py @@ -50,3 +50,19 @@ def get_redirect_url(course_key, usage_key): redirect_url += "?{}".format(urlencode({'activate_block_id': unicode(final_target_id)})) return redirect_url + + +def get_redirect_url_for_global_staff(course_key, location): + """ + Returns the redirect url + + Args: + course_key(str): Course key string + location(str): The location id of course component + """ + + _next = get_redirect_url(course_key, location) + redirect_url = "{url}?next={redirect}".format( + url=reverse('enroll_staff', args=[unicode(course_key)]), + redirect=_next) + return redirect_url diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 1c6b7b0514..3847c82a02 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -18,11 +18,13 @@ from django.db import transaction from django.db.models import Q from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden from django.shortcuts import redirect +from django.utils.decorators import method_decorator from django.utils.timezone import UTC from django.utils.translation import ugettext as _ from django.views.decorators.cache import cache_control from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_GET, require_POST, require_http_methods +from django.views.generic import View from eventtracking import tracker from ipware.ip import get_ip from markupsafe import escape @@ -59,7 +61,7 @@ from courseware.courses import ( from courseware.masquerade import setup_masquerade from courseware.model_data import FieldDataCache, ScoresClient from courseware.models import StudentModule, BaseStudentModuleHistory -from courseware.url_helpers import get_redirect_url +from courseware.url_helpers import get_redirect_url, get_redirect_url_for_global_staff from courseware.user_state_client import DjangoXBlockUserStateClient from edxmako.shortcuts import render_to_response, render_to_string, marketing_link from instructor.enrollment import uses_shib @@ -79,6 +81,7 @@ from student.roles import GlobalStaff from student.views import is_course_blocked from util.cache import cache, cache_if_anonymous +from util.course_key_utils import from_string_or_404 from util.date_utils import strftime_localized from util.db import outer_atomic from util.milestones_helpers import get_prerequisite_courses_display @@ -354,13 +357,10 @@ def _index_bulk_op(request, course_key, chapter, section, position): if not registered: # TODO (vshnayder): do course instructors need to be registered to see course? log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.to_deprecated_string()) - if bool(staff_access): - usage_key = UsageKey.from_string(course.location.to_deprecated_string()).replace(course_key=course_key) - redirect_url = get_redirect_url(course_key, usage_key) - return redirect("{url}?{redirect}".format( - url=reverse(enroll_staff, args=[course_key.to_deprecated_string()]), - redirect=redirect_url)) - return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) + if GlobalStaff().has_user(user): + redirect_url = get_redirect_url_for_global_staff(course_key, course.location) + return redirect(redirect_url) + return redirect(reverse('about_course', args=[unicode(course_key)])) # see if all pre-requisites (as per the milestones app feature) have been fulfilled # Note that if the pre-requisite feature flag has been turned off (default) then this check will @@ -630,10 +630,7 @@ def jump_to(_request, course_id, location): user = _request.user redirect_url = get_redirect_url(course_key, usage_key) if GlobalStaff().has_user(user) and not CourseEnrollment.is_enrolled(user, course_key): - redirect_url = "{url}?next={redirect}".format( - url=reverse(enroll_staff, args=[course_key.to_deprecated_string()]), - redirect=redirect_url - ) + redirect_url = get_redirect_url_for_global_staff(course_key, usage_key) except ItemNotFoundError: raise Http404(u"No data at this location: {0}".format(usage_key)) except NoPathToItem: @@ -839,57 +836,58 @@ def get_cosmetic_display_price(course, registration_price): return _('Free') -@require_global_staff -@ensure_valid_course_key -def enroll_staff(request, course_id): +class EnrollStaffView(View): """ - 1. Should be staff - 2. should be a valid course_id - 3. shouldn't be enrolled before - 4. The requested view url to redirect + Determine If user is global staff, it will be redirected to page "enroll_satff.html" + that asks you if you want to register for the course. + This pages has the courseware link you were heading to as a ?next parameter + Click "enroll" and be redirected to the page you wanted to go to + Click "Don't enroll" and go back to the course about page - GET - html: return enroll staff page + Arguments: + - request : HTTP request + - course_id : course id - POST - 1. You want to register for this course? - Confirm: - 1. User is valid staff user who wants to enroll. - 2. Course is valid course - 2. Yes - 3. Post request, enroll the user and redirect him to the requested view - - :param request: - :param course_id: - :return: + Returns: + -RedirectResponse """ - user = request.user - course_key = CourseKey.from_string(course_id) - _next = urllib.quote_plus(request.GET.get('next', 'info'), safe='/:?=') + template_name = 'enroll_staff.html' - if request.method == 'GET': + @method_decorator(require_global_staff) + @method_decorator(ensure_valid_course_key) + def get(self, request, course_id): + """ + Renders Enroll Staff View + """ + user = request.user + course_key = from_string_or_404(course_id) with modulestore().bulk_operations(course_key): - course = get_course_with_access(user, 'load', course_key, depth=2) + course = get_course_with_access(user, 'load', course_key) + if not registered_for_course(course, user): + context = {'course': course, + 'csrftoken': csrf(request)["csrf_token"]} + return render_to_response(self.template_name, context) - # Prompt for enrollment if Globalstaff is not enrolled in the course - if not registered_for_course(course, user): - return render_to_response('enroll_staff.html', { - 'course': course, - 'csrftoken': csrf(request)["csrf_token"] - }) - - elif request.method == 'POST': + @method_decorator(require_global_staff) + @method_decorator(ensure_valid_course_key) + def post(self, request, course_id): + """ + Enroll and returns the response + """ + _next = urllib.quote_plus(request.GET.get('next', 'info'), safe='/:?=') + course_key = from_string_or_404(course_id) if 'enroll' in request.POST: - enrollment = CourseEnrollment.get_or_create_enrollment(user, course_key) - enrollment.update_enrollment(is_active=True) + CourseEnrollment.enroll(request.user, course_key) log.info( u"User %s enrolled in %s via `enroll_staff` view", - user.username, + request.user.username, course_id ) return redirect(_next) + elif 'dont_enroll' in request.POST: + return redirect(reverse('about_course', args=[unicode(course_key)])) else: - return redirect(reverse('about_course', args=[course_key.to_deprecated_string()])) + raise Http404 @ensure_csrf_cookie diff --git a/lms/templates/enroll_staff.html b/lms/templates/enroll_staff.html index 5730cc6edf..b0d3c36bc7 100644 --- a/lms/templates/enroll_staff.html +++ b/lms/templates/enroll_staff.html @@ -1,3 +1,4 @@ +<%page expression_filter="h" /> <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> <%! @@ -6,11 +7,11 @@ %> <%block name="headextra"> - + -<%block name="pagetitle">${course.display_name_with_default_escaped} +<%block name="pagetitle">${course.display_name_with_default}
@@ -22,15 +23,15 @@

- ${course.display_name_with_default_escaped} - ${course.display_org_with_default | h} + ${course.display_name_with_default} + ${course.display_org_with_default}

- - + +
diff --git a/lms/urls.py b/lms/urls.py index 2b3cb0b437..b1b3387435 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -10,7 +10,7 @@ from django.conf.urls.static import static from microsite_configuration import microsite import auth_exchange.views - +from courseware.views import EnrollStaffView from config_models.views import ConfigurationModelCurrentAPIView from courseware.views.index import CoursewareIndex from openedx.core.djangoapps.programs.models import ProgramsApiConfig @@ -370,7 +370,7 @@ urlpatterns += ( r'^courses/{}/enroll_staff$'.format( settings.COURSE_ID_PATTERN, ), - 'courseware.views.enroll_staff', + EnrollStaffView.as_view(), name='enroll_staff', ), From 48e7fc8125ced6f4a6cbf4df27660756ce2b8bda Mon Sep 17 00:00:00 2001 From: attiyaishaque Date: Tue, 3 May 2016 11:30:15 +0500 Subject: [PATCH 6/6] Bokchoy Test Deleted. --- .../test/acceptance/pages/lms/courseware.py | 18 - common/test/acceptance/pages/studio/index.py | 22 - .../tests/studio/test_studio_home.py | 82 +--- lms/djangoapps/courseware/tests/test_views.py | 125 +++-- lms/djangoapps/courseware/url_helpers.py | 14 +- lms/djangoapps/courseware/views/index.py | 10 + lms/djangoapps/courseware/views/views.py | 434 ++---------------- lms/templates/enroll_staff.html | 54 ++- lms/urls.py | 2 +- 9 files changed, 157 insertions(+), 604 deletions(-) diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 9f717a27f5..34629acd99 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -286,21 +286,3 @@ class CoursewareSequentialTabPage(CoursePage): return the body of the sequential currently selected """ return self.q(css='#seq_content .xblock').text[0] - - -class AboutPage(CoursePage): - """ - Course about page. - """ - - url_path = "about/" - - def is_browser_on_page(self): - return self.q(css='.intro').present - - @property - def is_register_button_present(self): - """ - Returns True if the Enrollment button is visible on the about page. - """ - return self.q(css=".register").is_present() diff --git a/common/test/acceptance/pages/studio/index.py b/common/test/acceptance/pages/studio/index.py index 5427cf2324..ed6bd4158a 100644 --- a/common/test/acceptance/pages/studio/index.py +++ b/common/test/acceptance/pages/studio/index.py @@ -5,7 +5,6 @@ Studio Home page from bok_choy.page_object import PageObject from . import BASE_URL from selenium.webdriver import ActionChains -from ..common.utils import click_css class DashboardPage(PageObject): @@ -13,23 +12,11 @@ class DashboardPage(PageObject): Studio Home page """ - def __init__(self, browser): - """ - Initialize the page. - """ - super(DashboardPage, self).__init__(browser) url = BASE_URL + "/course/" def is_browser_on_page(self): return self.q(css='.content-primary').visible - def mouse_hover(self, element): - """ - Mouse over on given element. - """ - mouse_hover_action = ActionChains(self.browser).move_to_element(element) - mouse_hover_action.perform() - @property def course_runs(self): """ @@ -61,15 +48,6 @@ class DashboardPage(PageObject): # Clicking on course with run will trigger an ajax event self.wait_for_ajax() - def view_live(self, element): - """ - Clicks the "View Live" link and switches to the new tab - """ - self.mouse_hover(self.browser.find_element_by_css_selector('.course-item')) - click_css(self, '.view-button', require_notification=False) - self.browser.switch_to_window(self.browser.window_handles[-1]) - click_css(self, element, require_notification=False) - def has_new_library_button(self): """ (bool) is the "New Library" button present? diff --git a/common/test/acceptance/tests/studio/test_studio_home.py b/common/test/acceptance/tests/studio/test_studio_home.py index 6751e9a8c4..ddd58d4aed 100644 --- a/common/test/acceptance/tests/studio/test_studio_home.py +++ b/common/test/acceptance/tests/studio/test_studio_home.py @@ -1,14 +1,9 @@ """ Acceptance tests for Home Page (My Courses / My Libraries). """ -import os -import uuid - from bok_choy.web_app_test import WebAppTest -from common.test.acceptance.pages.common.logout import LogoutPage -from common.test.acceptance.pages.lms.courseware import AboutPage, CoursewarePage from flaky import flaky -from opaque_keys.edx.locator import LibraryLocator, CourseLocator +from opaque_keys.edx.locator import LibraryLocator from uuid import uuid4 from ...fixtures import PROGRAMS_STUB_URL @@ -178,78 +173,3 @@ class StudioLanguageTest(WebAppTest): get_selected_option_text(language_selector), u'Dummy Language (Esperanto)' ) - - -class CourseNotEnrollTest(WebAppTest): - """ - Test that we can create a new content library on the studio home page. - """ - - def setUp(self): - """ - Load the helper for the home page (dashboard page) - """ - super(CourseNotEnrollTest, self).setUp() - - self.auth_page = AutoAuthPage(self.browser, staff=True) - self.dashboard_page = DashboardPage(self.browser) - self.course_name = "New Course Name" - self.course_org = "orgX" - self.course_number = str(uuid.uuid4().get_hex().upper()[0:6]) - self.course_run = "2015_T2" - - def course_id(self): - """ - Returns the serialized course_key for the test - """ - # TODO - is there a better way to make this agnostic to the underlying default module store? - default_store = os.environ.get('DEFAULT_STORE', 'split') - course_key = CourseLocator( - self.course_org, - self.course_number, - self.course_run, - deprecated=(default_store == 'split') - ) - return unicode(course_key) - - def test_unenroll_course(self): - """ - From the home page: - Click "New Course" ,Fill out the form - Submit the form - Return to the home page and logout - Login with the staff user which is not enrolled in the course - click the view live button of the course - Here are two scenario: - -First click the Don't Enroll button - -Second click the Enroll button and see the response. - """ - self.auth_page.visit() - self.dashboard_page.visit() - self.assertTrue(self.dashboard_page.new_course_button.present) - - self.dashboard_page.click_new_course_button() - self.assertTrue(self.dashboard_page.is_new_course_form_visible()) - self.dashboard_page.fill_new_course_form( - self.course_name, self.course_org, self.course_number, self.course_run - ) - self.assertTrue(self.dashboard_page.is_new_course_form_valid()) - self.dashboard_page.submit_new_course_form() - - self.dashboard_page.visit() - LogoutPage(self.browser).visit() - AutoAuthPage(self.browser, course_id=None, staff=True).visit() - - self.dashboard_page.visit() - self.dashboard_page.view_live('input[name= "dont_enroll"]') - - about_page = AboutPage(self.browser, self.course_id) - about_page.wait_for_page() - self.assertTrue(about_page.is_browser_on_page()) - self.assertTrue(about_page.is_register_button_present) - - self.dashboard_page.visit() - self.dashboard_page.view_live('input[name= "enroll"]') - course_ware = CoursewarePage(self.browser, self.course_id) - course_ware.wait_for_page() - self.assertTrue(course_ware.is_browser_on_page()) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 0d2447ee76..bbfd5ba898 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -39,7 +39,7 @@ from course_modes.tests.factories import CourseModeFactory from courseware.model_data import set_score from courseware.module_render import toc_for_course from courseware.testutils import RenderXBlockTestMixin -from courseware.tests.factories import StudentModuleFactory +from courseware.tests.factories import StudentModuleFactory, GlobalStaffFactory from courseware.url_helpers import get_redirect_url from courseware.user_state_client import DjangoXBlockUserStateClient from courseware.views.index import render_accordion, CoursewareIndex @@ -196,7 +196,7 @@ class ViewsTestCase(ModuleStoreTestCase): def setUp(self): super(ViewsTestCase, self).setUp() - self.course = CourseFactory.create(display_name=u'teꜱᴛ course') + self.course = CourseFactory.create(display_name=u'teꜱᴛ course', run="Testing_course") self.chapter = ItemFactory.create( category='chapter', parent_location=self.course.location, @@ -323,63 +323,104 @@ class ViewsTestCase(ModuleStoreTestCase): self.assertNotIn('Problem 1', response.content) self.assertNotIn('Problem 2', response.content) + def _create_global_staff_user(self): + """ + Create global staff user and log them in + """ + self.global_staff = GlobalStaffFactory.create() # pylint: disable=attribute-defined-outside-init + self.client.login(username=self.global_staff.username, password='test') + def _create_url_for_enroll_staff(self): """ - User will have satff access and creates the url for enroll_staff view + creates the courseware url and enroll staff url """ - self.user.is_staff = True - self.user.save() # pylint: disable=no-member - self.client.login(username=self.user.username, password=self.password) - - # create the course - course = CourseFactory.create() - # create the _next parameter - courseware_url = reverse('courseware', kwargs={ - 'course_id': unicode(course.id)}) + '?activate_block_id=test_block_id' - + courseware_url = reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(self.course_key), + 'chapter': unicode(self.chapter.location.name), + 'section': unicode(self.section.location.name), + } + ) # create the url for enroll_staff view - enroll_staff_url = "{enroll_staff_url}?next={courseware_url}".format( - enroll_staff_url=reverse('enroll_staff', kwargs={'course_id': unicode(course.id)}), + enroll_url = "{enroll_url}?next={courseware_url}".format( + enroll_url=reverse('enroll_staff', kwargs={'course_id': unicode(self.course.id)}), courseware_url=courseware_url ) - return enroll_staff_url, course.id + return courseware_url, enroll_url - def test_redirection_unenrolled_staff(self): + @ddt.data( + ({'enroll': "Enroll"}, True), + ({'dont_enroll': "Don't enroll"}, False)) + @ddt.unpack + def test_enroll_staff_redirection(self, data, enrollment): """ - Test that staff is not redirected to the 'about' page when visiting an unregistered course + Verify unenrolled staff is redirected to correct url. """ - enroll_staff_url, __ = self._create_url_for_enroll_staff() + self._create_global_staff_user() + courseware_url, enroll_url = self._create_url_for_enroll_staff() + response = self.client.post(enroll_url, data=data, follow=True) + self.assertEqual(response.status_code, 200) - # Check the GET method - response = self.client.get(enroll_staff_url) + # we were redirected to our current location + self.assertIn(302, response.redirect_chain[0]) + self.assertEqual(len(response.redirect_chain), 1) + if enrollment: + self.assertRedirects(response, courseware_url) + else: + self.assertRedirects(response, '/courses/{}/about'.format(unicode(self.course_key))) + + def test_enroll_staff_with_invalid_data(self): + """ + If we try to post with an invalid data pattern, then we'll redirected to + course about page. + """ + self._create_global_staff_user() + __, enroll_url = self._create_url_for_enroll_staff() + response = self.client.post(enroll_url, data={'test': "test"}) + self.assertEqual(response.status_code, 302) + self.assertRedirects(response, '/courses/{}/about'.format(unicode(self.course_key))) + + def test_courseware_redirection(self): + """ + Tests that a global staff member is redirected to the staff enrollment page. + + Un-enrolled Staff user should be redirected to the staff enrollment page accessing courseware, + user chooses to enroll in the course. User is enrolled and redirected to the requested url. + + Scenario: + 1. Un-enrolled staff tries to access any course vertical (courseware url). + 2. User is redirected to the staff enrollment page. + 3. User chooses to enroll in the course. + 4. User is enrolled in the course and redirected to the requested courseware url. + """ + self._create_global_staff_user() + courseware_url, enroll_url = self._create_url_for_enroll_staff() + + # Accessing the courseware url in which not enrolled & redirected to staff enrollment page + response = self.client.get(courseware_url, follow=True) + self.assertEqual(response.status_code, 200) + self.assertIn(302, response.redirect_chain[0]) + self.assertEqual(len(response.redirect_chain), 1) + self.assertRedirects(response, enroll_url) + + # Accessing the enroll staff url and verify the correct url + response = self.client.get(enroll_url) self.assertEqual(response.status_code, 200) response_content = response.content self.assertIn('Enroll', response_content) self.assertIn("dont_enroll", response_content) - @ddt.data( - ({'enroll': "Enroll"}, 'courses/{}/courseware?activate_block_id=test_block_id'), - ({'dont_enroll': "Don't enroll"}, '/courses/{}/about')) - @ddt.unpack - def test_enroll_staff_redirection(self, data, expected_url): - """ - Verify unenrolled staff is redirected to the page according to data passed. - """ - enroll_staff_url, course_id = self._create_url_for_enroll_staff() - response = self.client.post(enroll_staff_url, data=data) + # Post the valid data to enroll the staff in the course + response = self.client.post(enroll_url, data={'enroll': "Enroll"}, follow=True) + self.assertEqual(response.status_code, 200) + self.assertIn(302, response.redirect_chain[0]) + self.assertEqual(len(response.redirect_chain), 1) + self.assertRedirects(response, courseware_url) - # Here we check the status code 302 because of the redirect - self.assertEqual(response.status_code, 302) - self.assertRedirects(response, expected_url.format(unicode(course_id))) - - def test_enroll_staff_redirection_invalid_data(self): - """ - Verify unenrolled staff is redirected to the page according to data passed. - """ - enroll_staff_url, __ = self._create_url_for_enroll_staff() - response = self.client.post(enroll_staff_url, data={'test': "test"}) - self.assertEqual(response.status_code, 404) + # Verify staff has been enrolled to the given course + self.assertTrue(CourseEnrollment.is_enrolled(self.global_staff, self.course.id)) @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings") @patch.dict(settings.FEATURES, {'ENABLE_PAID_COURSE_REGISTRATION': True}) diff --git a/lms/djangoapps/courseware/url_helpers.py b/lms/djangoapps/courseware/url_helpers.py index 41acff095d..9acd931a40 100644 --- a/lms/djangoapps/courseware/url_helpers.py +++ b/lms/djangoapps/courseware/url_helpers.py @@ -42,27 +42,23 @@ def get_redirect_url(course_key, usage_key): # Here we use the navigation_index from the position returned from # path_to_location - we can only navigate to the topmost vertical at the # moment - redirect_url = reverse( 'courseware_position', args=(unicode(course_key), chapter, section, navigation_index(position)) ) - redirect_url += "?{}".format(urlencode({'activate_block_id': unicode(final_target_id)})) return redirect_url -def get_redirect_url_for_global_staff(course_key, location): +def get_redirect_url_for_global_staff(course_key, _next): """ - Returns the redirect url + Returns the redirect url for staff enrollment Args: course_key(str): Course key string - location(str): The location id of course component + _next(str): Redirect url of course component """ - - _next = get_redirect_url(course_key, location) - redirect_url = "{url}?next={redirect}".format( + redirect_url = ("{url}?next={redirect}".format( url=reverse('enroll_staff', args=[unicode(course_key)]), - redirect=_next) + redirect=_next)) return redirect_url diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 49d5f49200..88de50285f 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -15,6 +15,8 @@ from django.views.decorators.cache import cache_control from django.views.decorators.csrf import ensure_csrf_cookie from django.views.generic import View from django.shortcuts import redirect + +from courseware.url_helpers import get_redirect_url_for_global_staff from edxmako.shortcuts import render_to_response, render_to_string import logging import newrelic.agent @@ -26,7 +28,9 @@ from opaque_keys.edx.keys import CourseKey from openedx.core.lib.gating import api as gating_api from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from shoppingcart.models import CourseRegistrationCode +from student.models import CourseEnrollment from student.views import is_course_blocked +from student.roles import GlobalStaff from util.views import ensure_valid_course_key from xmodule.modulestore.django import modulestore from xmodule.x_module import STUDENT_VIEW @@ -89,6 +93,7 @@ class CoursewareIndex(View): self.section_url_name = section self.position = position self.chapter, self.section = None, None + self.url = request.path try: self._init_new_relic() @@ -221,6 +226,11 @@ class CoursewareIndex(View): self.effective_user, unicode(self.course.id) ) + user_is_global_staff = GlobalStaff().has_user(self.effective_user) + user_is_enrolled = CourseEnrollment.is_enrolled(self.effective_user, self.course_key) + if user_is_global_staff and not user_is_enrolled: + redirect_url = get_redirect_url_for_global_staff(self.course_key, _next=self.url) + raise Redirect(redirect_url) raise Redirect(reverse('about_course', args=[unicode(self.course.id)])) def _redirect_if_needed_for_prereqs(self): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 3847c82a02..501b816257 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -14,6 +14,7 @@ from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User, AnonymousUser from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse +from django.core.context_processors import csrf from django.db import transaction from django.db.models import Q from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden @@ -32,7 +33,6 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from rest_framework import status -from xblock.fragment import Fragment from instructor.views.api import require_global_staff import shoppingcart @@ -41,6 +41,7 @@ import survey.views from certificates import api as certs_api from openedx.core.djangoapps.models.course_details import CourseDetails from commerce.utils import EcommerceService +from enrollment.api import add_enrollment from course_modes.models import CourseMode from courseware import grades from courseware.access import has_access, has_ccx_coach_role, _adjust_start_date_for_beta_testers @@ -76,12 +77,8 @@ from openedx.core.djangoapps.theming import helpers as theming_helpers from shoppingcart.utils import is_shopping_cart_enabled from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from student.models import UserTestGroup, CourseEnrollment - from student.roles import GlobalStaff -from student.views import is_course_blocked - from util.cache import cache, cache_if_anonymous -from util.course_key_utils import from_string_or_404 from util.date_utils import strftime_localized from util.db import outer_atomic from util.milestones_helpers import get_prerequisite_courses_display @@ -204,386 +201,6 @@ def get_current_child(xmodule, min_depth=None, requested_child=None): return child -def redirect_to_course_position(course_module, content_depth): - """ - Return a redirect to the user's current place in the course. - - If this is the user's first time, redirects to COURSE/CHAPTER/SECTION. - If this isn't the users's first time, redirects to COURSE/CHAPTER, - and the view will find the current section and display a message - about reusing the stored position. - - If there is no current position in the course or chapter, then selects - the first child. - - """ - urlargs = {'course_id': course_module.id.to_deprecated_string()} - chapter = get_current_child(course_module, min_depth=content_depth) - if chapter is None: - # oops. Something bad has happened. - raise Http404("No chapter found when loading current position in course") - - urlargs['chapter'] = chapter.url_name - if course_module.position is not None: - return redirect(reverse('courseware_chapter', kwargs=urlargs)) - - # Relying on default of returning first child - section = get_current_child(chapter, min_depth=content_depth - 1) - if section is None: - raise Http404("No section found when loading current position in course") - - urlargs['section'] = section.url_name - return redirect(reverse('courseware_section', kwargs=urlargs)) - - -def save_child_position(seq_module, child_name): - """ - child_name: url_name of the child - """ - for position, c in enumerate(seq_module.get_display_items(), start=1): - if c.location.name == child_name: - # Only save if position changed - if position != seq_module.position: - seq_module.position = position - # Save this new position to the underlying KeyValueStore - seq_module.save() - - -def save_positions_recursively_up(user, request, field_data_cache, xmodule, course=None): - """ - Recurses up the course tree starting from a leaf - Saving the position property based on the previous node as it goes - """ - current_module = xmodule - - while current_module: - parent_location = modulestore().get_parent_location(current_module.location) - parent = None - if parent_location: - parent_descriptor = modulestore().get_item(parent_location) - parent = get_module_for_descriptor( - user, - request, - parent_descriptor, - field_data_cache, - current_module.location.course_key, - course=course - ) - - if parent and hasattr(parent, 'position'): - save_child_position(parent, current_module.location.name) - - current_module = parent - - -@transaction.non_atomic_requests -@login_required -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@ensure_valid_course_key -@outer_atomic(read_committed=True) -def index(request, course_id, chapter=None, section=None, - position=None): - """ - Displays courseware accordion and associated content. If course, chapter, - and section are all specified, renders the page, or returns an error if they - are invalid. - - If section is not specified, displays the accordion opened to the right chapter. - - If neither chapter or section are specified, redirects to user's most recent - chapter, or the first chapter if this is the user's first visit. - - Arguments: - - - request : HTTP request - - course_id : course id (str: ORG/course/URL_NAME) - - chapter : chapter url_name (str) - - section : section url_name (str) - - position : position in module, eg of module (str) - - Returns: - - - HTTPresponse - """ - - course_key = CourseKey.from_string(course_id) - - # Gather metrics for New Relic so we can slice data in New Relic Insights - newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) - newrelic.agent.add_custom_parameter('org', unicode(course_key.org)) - - user = User.objects.prefetch_related("groups").get(id=request.user.id) - - redeemed_registration_codes = CourseRegistrationCode.objects.filter( - course_id=course_key, - registrationcoderedemption__redeemed_by=request.user - ) - - # Redirect to dashboard if the course is blocked due to non-payment. - if is_course_blocked(request, redeemed_registration_codes, course_key): - # registration codes may be generated via Bulk Purchase Scenario - # we have to check only for the invoice generated registration codes - # that their invoice is valid or not - log.warning( - u'User %s cannot access the course %s because payment has not yet been received', - user, - course_key.to_deprecated_string() - ) - return redirect(reverse('dashboard')) - - request.user = user # keep just one instance of User - with modulestore().bulk_operations(course_key): - return _index_bulk_op(request, course_key, chapter, section, position) - - -# pylint: disable=too-many-statements -def _index_bulk_op(request, course_key, chapter, section, position): - """ - Render the index page for the specified course. - """ - # Verify that position a string is in fact an int - if position is not None: - try: - int(position) - except ValueError: - raise Http404(u"Position {} is not an integer!".format(position)) - - course = get_course_with_access(request.user, 'load', course_key, depth=2) - staff_access = has_access(request.user, 'staff', course) - masquerade, user = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True) - - registered = registered_for_course(course, user) - if not registered: - # TODO (vshnayder): do course instructors need to be registered to see course? - log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.to_deprecated_string()) - if GlobalStaff().has_user(user): - redirect_url = get_redirect_url_for_global_staff(course_key, course.location) - return redirect(redirect_url) - return redirect(reverse('about_course', args=[unicode(course_key)])) - - # see if all pre-requisites (as per the milestones app feature) have been fulfilled - # Note that if the pre-requisite feature flag has been turned off (default) then this check will - # always pass - if not has_access(user, 'view_courseware_with_prerequisites', course): - # prerequisites have not been fulfilled therefore redirect to the Dashboard - log.info( - u'User %d tried to view course %s ' - u'without fulfilling prerequisites', - user.id, unicode(course.id)) - return redirect(reverse('dashboard')) - - # Entrance Exam Check - # If the course has an entrance exam and the requested chapter is NOT the entrance exam, and - # the user hasn't yet met the criteria to bypass the entrance exam, redirect them to the exam. - if chapter and course_has_entrance_exam(course): - chapter_descriptor = course.get_child_by(lambda m: m.location.name == chapter) - if chapter_descriptor and not getattr(chapter_descriptor, 'is_entrance_exam', False) \ - and user_must_complete_entrance_exam(request, user, course): - log.info(u'User %d tried to view course %s without passing entrance exam', user.id, unicode(course.id)) - return redirect(reverse('courseware', args=[unicode(course.id)])) - - # Gated Content Check - gated_content = gating_api.get_gated_content(course, user) - if section and gated_content: - for usage_key in gated_content: - if section in usage_key: - raise Http404 - - # check to see if there is a required survey that must be taken before - # the user can access the course. - if survey.utils.must_answer_survey(course, user): - return redirect(reverse('course_survey', args=[unicode(course.id)])) - - bookmarks_api_url = reverse('bookmarks') - - try: - field_data_cache = FieldDataCache.cache_for_descriptor_descendents( - course_key, user, course, depth=2) - - studio_url = get_studio_url(course, 'course') - - language_preference = get_user_preference(request.user, LANGUAGE_KEY) - if not language_preference: - language_preference = settings.LANGUAGE_CODE - - context = { - 'csrf': csrf(request)['csrf_token'], - 'COURSE_TITLE': course.display_name_with_default_escaped, - 'course': course, - 'init': '', - 'fragment': Fragment(), - 'staff_access': staff_access, - 'studio_url': studio_url, - 'masquerade': masquerade, - 'xqa_server': settings.FEATURES.get('XQA_SERVER', "http://your_xqa_server.com"), - 'bookmarks_api_url': bookmarks_api_url, - 'language_preference': language_preference, - 'disable_optimizely': True, - } - table_of_contents, __, __ = toc_for_course(user, request, course, chapter, section, field_data_cache) - context['accordion'] = render_accordion(request, course, table_of_contents) - - now = datetime.now(UTC()) - effective_start = _adjust_start_date_for_beta_testers(user, course, course_key) - if not in_preview_mode() and staff_access and now < effective_start: - # Disable student view button if user is staff and - # course is not yet visible to students. - context['disable_student_access'] = True - - has_content = course.has_children_at_depth(CONTENT_DEPTH) - if not has_content: - # Show empty courseware for a course with no units - return render_to_response('courseware/courseware.html', context) - elif chapter is None: - # Check first to see if we should instead redirect the user to an Entrance Exam - if course_has_entrance_exam(course): - exam_chapter = get_entrance_exam_content(request, course) - if exam_chapter: - if exam_chapter.get_children(): - exam_section = exam_chapter.get_children()[0] - if exam_section: - return redirect('courseware_section', - course_id=unicode(course_key), - chapter=exam_chapter.url_name, - section=exam_section.url_name) - - # passing CONTENT_DEPTH avoids returning 404 for a course with an - # empty first section and a second section with content - return redirect_to_course_position(course, CONTENT_DEPTH) - - chapter_descriptor = course.get_child_by(lambda m: m.location.name == chapter) - if chapter_descriptor is not None: - save_child_position(course, chapter) - else: - # User may be trying to access a chapter that isn't live yet - if masquerade and masquerade.role == 'student': # if staff is masquerading as student be kinder, don't 404 - log.debug('staff masquerading as student: no chapter %s', chapter) - return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) - raise Http404('No chapter descriptor found with name {}'.format(chapter)) - - if course_has_entrance_exam(course): - # Message should not appear outside the context of entrance exam subsection. - # if section is none then we don't need to show message on welcome back screen also. - if getattr(chapter_descriptor, 'is_entrance_exam', False) and section is not None: - context['entrance_exam_current_score'] = get_entrance_exam_score(request, course) - context['entrance_exam_passed'] = user_has_passed_entrance_exam(request, course) - - if section is None: - section_descriptor = get_current_child(chapter_descriptor, requested_child=request.GET.get("child")) - if section_descriptor: - section = section_descriptor.url_name - else: - # Something went wrong -- perhaps this chapter has no sections visible to the user. - # Clearing out the last-visited state and showing "first-time" view by redirecting - # to courseware. - course.position = None - course.save() - return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) - else: - section_descriptor = chapter_descriptor.get_child_by(lambda m: m.location.name == section) - - if section_descriptor is None: - # Specifically asked-for section doesn't exist - if masquerade and masquerade.role == 'student': # don't 404 if staff is masquerading as student - log.debug('staff masquerading as student: no section %s', section) - return redirect(reverse('courseware', args=[course.id.to_deprecated_string()])) - raise Http404 - - # Allow chromeless operation - if section_descriptor.chrome: - chrome = [s.strip() for s in section_descriptor.chrome.lower().split(",")] - if 'accordion' not in chrome: - context['disable_accordion'] = True - if 'tabs' not in chrome: - context['disable_tabs'] = True - - if section_descriptor.default_tab: - context['default_tab'] = section_descriptor.default_tab - - # cdodge: this looks silly, but let's refetch the section_descriptor with depth=None - # which will prefetch the children more efficiently than doing a recursive load - section_descriptor = modulestore().get_item(section_descriptor.location, depth=None) - - # Load all descendants of the section, because we're going to display its - # html, which in general will need all of its children - field_data_cache.add_descriptor_descendents( - section_descriptor, depth=None - ) - - section_module = get_module_for_descriptor( - user, - request, - section_descriptor, - field_data_cache, - course_key, - position, - course=course - ) - - # Save where we are in the chapter. - save_child_position(chapter_descriptor, section) - - table_of_contents, prev_section_info, next_section_info = toc_for_course( - user, request, course, chapter, section, field_data_cache - ) - context['accordion'] = render_accordion(request, course, table_of_contents) - - def _compute_section_url(section_info, requested_child): - """ - Returns the section URL for the given section_info with the given child parameter. - """ - return "{url}?child={requested_child}".format( - url=reverse( - 'courseware_section', - args=[unicode(course.id), section_info['chapter_url_name'], section_info['url_name']], - ), - requested_child=requested_child, - ) - - section_render_context = { - 'activate_block_id': request.GET.get('activate_block_id'), - 'requested_child': request.GET.get("child"), - 'prev_url': _compute_section_url(prev_section_info, 'last') if prev_section_info else None, - 'next_url': _compute_section_url(next_section_info, 'first') if next_section_info else None, - } - context['fragment'] = section_module.render(STUDENT_VIEW, section_render_context) - context['section_title'] = section_descriptor.display_name_with_default_escaped - result = render_to_response('courseware/courseware.html', context) - except Exception as e: - - # Doesn't bar Unicode characters from URL, but if Unicode characters do - # cause an error it is a graceful failure. - if isinstance(e, UnicodeEncodeError): - raise Http404("URL contains Unicode characters") - - if isinstance(e, Http404): - # let it propagate - raise - - # In production, don't want to let a 500 out for any reason - if settings.DEBUG: - raise - else: - log.exception( - u"Error in index view: user=%s, effective_user=%s, course=%s, chapter=%s section=%s position=%s", - request.user, user, course, chapter, section, position - ) - try: - result = render_to_response('courseware/courseware-error.html', { - 'staff_access': staff_access, - 'course': course - }) - except: - # Let the exception propagate, relying on global config to at - # at least return a nice error message - log.exception("Error while rendering courseware-error page") - raise - - return result - - - @ensure_csrf_cookie @ensure_valid_course_key def jump_to_id(request, course_id, module_id): @@ -627,10 +244,12 @@ def jump_to(_request, course_id, location): except InvalidKeyError: raise Http404(u"Invalid course_key or usage_key") try: - user = _request.user redirect_url = get_redirect_url(course_key, usage_key) - if GlobalStaff().has_user(user) and not CourseEnrollment.is_enrolled(user, course_key): - redirect_url = get_redirect_url_for_global_staff(course_key, usage_key) + user = _request.user + user_is_global_staff = GlobalStaff().has_user(user) + user_is_enrolled = CourseEnrollment.is_enrolled(user, course_key) + if user_is_global_staff and not user_is_enrolled: + redirect_url = get_redirect_url_for_global_staff(course_key, _next=redirect_url) except ItemNotFoundError: raise Http404(u"No data at this location: {0}".format(usage_key)) except NoPathToItem: @@ -838,18 +457,18 @@ def get_cosmetic_display_price(course, registration_price): class EnrollStaffView(View): """ - Determine If user is global staff, it will be redirected to page "enroll_satff.html" - that asks you if you want to register for the course. - This pages has the courseware link you were heading to as a ?next parameter - Click "enroll" and be redirected to the page you wanted to go to - Click "Don't enroll" and go back to the course about page + Displays view for registering in the course to a global staff user. + + User can either choose to 'Enroll' or 'Don't Enroll' in the course. + Enroll: Enrolls user in course and redirects to the courseware. + Don't Enroll: Redirects user to course about page. Arguments: - request : HTTP request - course_id : course id Returns: - -RedirectResponse + - RedirectResponse """ template_name = 'enroll_staff.html' @@ -857,37 +476,40 @@ class EnrollStaffView(View): @method_decorator(ensure_valid_course_key) def get(self, request, course_id): """ - Renders Enroll Staff View + Display enroll staff view to global staff user with `Enroll` and `Don't Enroll` options. """ user = request.user - course_key = from_string_or_404(course_id) + course_key = CourseKey.from_string(course_id) with modulestore().bulk_operations(course_key): course = get_course_with_access(user, 'load', course_key) if not registered_for_course(course, user): - context = {'course': course, - 'csrftoken': csrf(request)["csrf_token"]} + context = { + 'course': course, + 'csrftoken': csrf(request)["csrf_token"] + } return render_to_response(self.template_name, context) @method_decorator(require_global_staff) @method_decorator(ensure_valid_course_key) def post(self, request, course_id): """ - Enroll and returns the response + Either enrolls the user in course or redirects user to course about page + depending upon the option (Enroll, Don't Enroll) chosen by the user. """ _next = urllib.quote_plus(request.GET.get('next', 'info'), safe='/:?=') - course_key = from_string_or_404(course_id) - if 'enroll' in request.POST: - CourseEnrollment.enroll(request.user, course_key) + course_key = CourseKey.from_string(course_id) + enroll = 'enroll' in request.POST + if enroll: + add_enrollment(request.user.username, course_id) log.info( u"User %s enrolled in %s via `enroll_staff` view", request.user.username, course_id ) return redirect(_next) - elif 'dont_enroll' in request.POST: - return redirect(reverse('about_course', args=[unicode(course_key)])) - else: - raise Http404 + + # In any other case redirect to the course about page. + return redirect(reverse('about_course', args=[unicode(course_key)])) @ensure_csrf_cookie diff --git a/lms/templates/enroll_staff.html b/lms/templates/enroll_staff.html index b0d3c36bc7..c088de9846 100644 --- a/lms/templates/enroll_staff.html +++ b/lms/templates/enroll_staff.html @@ -12,30 +12,34 @@ <%block name="pagetitle">${course.display_name_with_default} - -
-
-
-
-
-
-

${_("You should Register before trying to access the Unit")}

-
-
-

- ${course.display_name_with_default} - ${course.display_org_with_default} -

-
-
- -
- - +
+
+
+
+
+
+
+

${_("You should Register before trying to access the Unit")}

- -
+
+

+ ${course.display_name_with_default} +

+
+
+ +
+ + +
+
+
+
- -
-
+ + + diff --git a/lms/urls.py b/lms/urls.py index b1b3387435..dfcb5ed2d5 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -10,7 +10,7 @@ from django.conf.urls.static import static from microsite_configuration import microsite import auth_exchange.views -from courseware.views import EnrollStaffView +from courseware.views.views import EnrollStaffView from config_models.views import ConfigurationModelCurrentAPIView from courseware.views.index import CoursewareIndex from openedx.core.djangoapps.programs.models import ProgramsApiConfig