diff --git a/src/illumidesk/apis/nbgrader_service.py b/src/illumidesk/apis/nbgrader_service.py new file mode 100644 index 00000000..db54c5ff --- /dev/null +++ b/src/illumidesk/apis/nbgrader_service.py @@ -0,0 +1,70 @@ +import logging +from pathlib import Path +import os +import shutil +import sys + +from nbgrader.api import Assignment, Course, Gradebook +from nbgrader.api import InvalidEntry + +logging.basicConfig(stream=sys.stdout, level=logging.DEBUG) +logger = logging.getLogger(__name__) + + +class NbGraderServiceHelper: + def __init__(self, course_id: str): + if not course_id: + raise ValueError('course_id missing') + self.course_id = course_id + self.uid = int(os.environ.get('NB_UID') or '10001') + self.gid = int(os.environ.get('NB_GID') or '100') + grader_name = f'grader-{course_id}' + self.course_dir = f'/home/{grader_name}/{course_id}' + + self.gradebook_path = Path(self.course_dir, 'gradebook.db') + # make sure the gradebook path exists + self.gradebook_path.parent.mkdir(exist_ok=True, parents=True) + logger.debug('Gradebook path is %s' % self.gradebook_path) + logger.debug("creating gradebook instance") + self.gb = Gradebook(f'sqlite:///{self.gradebook_path}', course_id=self.course_id) + logger.debug( + 'Changing or making sure the gradebook directory permissions (with path %s) to %s:%s ' + % (self.gradebook_path, self.uid, self.gid) + ) + shutil.chown(str(self.gradebook_path), user=self.uid, group=self.gid) + + def update_course(self, **kwargs) -> None: + with Gradebook(f'sqlite:///{self.gradebook_path}', course_id=self.course_id) as gb: + gb.update_course(self.course_id, **kwargs) + + def get_course(self) -> Course: + with Gradebook(f'sqlite:///{self.gradebook_path}', course_id=self.course_id) as gb: + course = gb.check_course(self.course_id) + logger.debug(f'course got from db:{course}') + return course + + def create_assignment_in_nbgrader(self, assignment_name: str, **kwargs: dict) -> Assignment: + """ + Adds an assignment to nbgrader database + + Args: + assignment_name: The assingment's name + Raises: + InvalidEntry: when there was an error adding the assignment to the database + """ + if not assignment_name: + raise ValueError('assignment_name missing') + assignment = None + with Gradebook(f'sqlite:///{self.gradebook_path}', course_id=self.course_id) as gb: + try: + assignment = gb.update_or_create_assignment(assignment_name, **kwargs) + logger.debug('Added assignment %s to gradebook' % assignment_name) + sourcedir = os.path.abspath(Path(self.course_dir, 'source', assignment_name)) + if not os.path.isdir(sourcedir): + logger.debug('Creating source dir %s for the assignment %s' % (sourcedir, assignment_name)) + os.makedirs(sourcedir) + logger.debug('Fixing folder permissions for %s' % sourcedir) + shutil.chown(str(sourcedir), user=self.uid, group=self.gid) + except InvalidEntry as e: + logger.debug('Error during adding assignment to gradebook: %s' % e) + return assignment diff --git a/src/illumidesk/authenticators/authenticator.py b/src/illumidesk/authenticators/authenticator.py index ff9674c0..0b54c031 100644 --- a/src/illumidesk/authenticators/authenticator.py +++ b/src/illumidesk/authenticators/authenticator.py @@ -18,6 +18,7 @@ from illumidesk.apis.jupyterhub_api import JupyterHubAPI from illumidesk.apis.announcement_service import AnnouncementService +from illumidesk.apis.nbgrader_service import NbGraderServiceHelper from illumidesk.apis.setup_course_service import make_rolling_update from illumidesk.apis.setup_course_service import register_new_service @@ -68,11 +69,12 @@ async def setup_course_hook( username = lti_utils.normalize_string(authentication['name']) lms_user_id = authentication['auth_state']['lms_user_id'] user_role = authentication['auth_state']['user_role'] + # register the user (it doesn't matter if is a student or instructor) with her/his lms_user_id in nbgrader + await jupyterhub_api.add_user_to_nbgrader_gradebook(course_id, username, lms_user_id) # TODO: verify the logic to simplify groups creation and membership if user_role == 'Student' or user_role == 'Learner': # assign the user to 'nbgrader-' group in jupyterhub and gradebook await jupyterhub_api.add_student_to_jupyterhub_group(course_id, username) - await jupyterhub_api.add_user_to_nbgrader_gradebook(course_id, username, lms_user_id) elif user_role == 'Instructor': # assign the user in 'formgrade-' group await jupyterhub_api.add_instructor_to_jupyterhub_group(course_id, username) @@ -244,7 +246,6 @@ async def authenticate(self, handler: BaseHandler, data: Dict[str, str] = None) control_file.register_data( assignment_name, lis_outcome_service_url, lms_user_id, lis_result_sourcedid ) - # ensure the user name is normalized username_normalized = lti_utils.normalize_string(username) self.log.debug('Assigned username is: %s' % username_normalized) @@ -380,12 +381,22 @@ async def authenticate( # noqa: C901 self.log.debug('user_role is %s' % user_role) lms_user_id = jwt_decoded['sub'] if 'sub' in jwt_decoded else username - # Values for the send-grades functionality + # Values for send-grades functionality + resource_link = jwt_decoded['https://purl.imsglobal.org/spec/lti/claim/resource_link'] + resource_link_title = resource_link['title'] or '' course_lineitems = '' - if 'https://purl.imsglobal.org/spec/lti-ags/claim/endpoint' in jwt_decoded: - course_lineitems = jwt_decoded['https://purl.imsglobal.org/spec/lti-ags/claim/endpoint'].get( - 'lineitems' + if ( + 'https://purl.imsglobal.org/spec/lti-ags/claim/endpoint' in jwt_decoded + and 'lineitems' in jwt_decoded['https://purl.imsglobal.org/spec/lti-ags/claim/endpoint'] + ): + course_lineitems = jwt_decoded['https://purl.imsglobal.org/spec/lti-ags/claim/endpoint']['lineitems'] + nbgrader_service = NbGraderServiceHelper(course_id) + nbgrader_service.update_course(lms_lineitems_endpoint=course_lineitems) + if resource_link_title: + self.log.debug( + 'Creating a new assignment from the Authentication flow with title %s' % resource_link_title ) + nbgrader_service.create_assignment_in_nbgrader(resource_link_title) # ensure the user name is normalized username_normalized = lti_utils.normalize_string(username) @@ -397,7 +408,6 @@ async def authenticate( # noqa: C901 'course_id': course_id, 'user_role': user_role, 'workspace_type': workspace_type, - 'course_lineitems': course_lineitems, 'lms_user_id': lms_user_id, }, # noqa: E231 } diff --git a/src/illumidesk/grades/handlers.py b/src/illumidesk/grades/handlers.py index 6c96fb3a..ea9e902d 100644 --- a/src/illumidesk/grades/handlers.py +++ b/src/illumidesk/grades/handlers.py @@ -1,5 +1,6 @@ import json + from illumidesk.authenticators.authenticator import LTI11Authenticator from illumidesk.grades import exceptions from illumidesk.grades.senders import LTI13GradeSender @@ -40,15 +41,14 @@ async def post(self, course_id: str, assignment_name: str) -> None: if isinstance(self.authenticator, LTI11Authenticator) or self.authenticator is LTI11Authenticator: lti_grade_sender = LTIGradeSender(course_id, assignment_name) else: - auth_state = await self.current_user.get_auth_state() - self.log.debug(f'auth_state from current_user:{auth_state}') - lti_grade_sender = LTI13GradeSender(course_id, assignment_name, auth_state) + lti_grade_sender = LTI13GradeSender(course_id, assignment_name) try: await lti_grade_sender.send_grades() except exceptions.GradesSenderCriticalError: raise web.HTTPError(400, 'There was an critical error, please check logs.') except exceptions.AssignmentWithoutGradesError: raise web.HTTPError(400, 'There are no grades yet to submit') - except exceptions.GradesSenderMissingInfoError: - raise web.HTTPError(400, 'Impossible to send grades. There are missing values, please check logs.') + except exceptions.GradesSenderMissingInfoError as e: + self.log.error(f'There are missing values.{e}') + raise web.HTTPError(400, f'Impossible to send grades. There are missing values, please check logs.{e}') self.write(json.dumps({"success": True})) diff --git a/src/illumidesk/grades/senders.py b/src/illumidesk/grades/senders.py index 7dbad819..21823dbb 100644 --- a/src/illumidesk/grades/senders.py +++ b/src/illumidesk/grades/senders.py @@ -13,6 +13,7 @@ from nbgrader.api import Gradebook, MissingEntry from tornado.httpclient import AsyncHTTPClient +from illumidesk.apis.nbgrader_service import NbGraderServiceHelper from illumidesk.lti13.auth import get_lms_access_token @@ -143,33 +144,31 @@ async def send_grades(self) -> None: class LTI13GradeSender(GradesBaseSender): - def __init__(self, course_id: str, assignment_name: str, auth_state: dict): + def __init__(self, course_id: str, assignment_name: str): """ Creates a new class to help us to send grades saved in the nbgrader gradebook (sqlite) back to the LMS + For simplify the submission we're using the lineitem_id (that is a url) obtained in authentication flow and it indicates us where send the scores + So the assignment item in the database should contains the 'lms_lineitem_id' with something like /api/lti/courses/:course_id/line_items/:line_item_id Args: course_id: It's the course label obtained from lti claims assignment_name: the asignment name used on the nbgrader console - auth_state: It's a dictionary with the auth state of the user. Saved when user logged in. - The required key is 'course_lineitems' (obtained from the https://purl.imsglobal.org/spec/lti-ags/claim/endpoint claim) - and its value is something like 'http://canvas.instructure.com/api/lti/courses/1/line_items' """ super(LTI13GradeSender, self).__init__(course_id, assignment_name) - if auth_state is None or 'course_lineitems' not in auth_state: - logger.info('The key "course_lineitems" is missing in the user auth_state and it is required') - raise GradesSenderMissingInfoError() - logger.info(f'User auth_state received from SenderHandler: {auth_state}') - self.lineitems_url = auth_state['course_lineitems'] self.private_key_path = os.environ.get('LTI13_PRIVATE_KEY') self.lms_token_url = os.environ['LTI13_TOKEN_URL'] self.lms_client_id = os.environ['LTI13_CLIENT_ID'] + # retrieve the course entity from nbgrader-gradebook + nbgrader_service = NbGraderServiceHelper(course_id) + course = nbgrader_service.get_course() + self.course = course async def _get_line_item_info_by_assignment_name(self) -> str: client = AsyncHTTPClient() - resp = await client.fetch(self.lineitems_url, headers=self.headers) + resp = await client.fetch(self.course.lms_lineitems_endpoint, headers=self.headers) items = json.loads(resp.body) - logger.debug(f'LineItems got from {self.lineitems_url} -> {items}') + logger.debug(f'LineItems retrieved: {items}') if not items or isinstance(items, list) is False: raise GradesSenderMissingInfoError(f'No line-items were detected for this course: {self.course_id}') lineitem_matched = None @@ -212,18 +211,21 @@ async def send_grades(self): client = AsyncHTTPClient() self.headers.update({'Content-Type': 'application/vnd.ims.lis.v1.score+json'}) for grade in nbgrader_grades: - score = float(grade['score']) - data = { - 'timestamp': datetime.now().isoformat(), - 'userId': grade['lms_user_id'], - 'scoreGiven': score, - 'scoreMaximum': score_maximum, - 'gradingProgress': 'FullyGraded', - 'activityProgress': 'Completed', - 'comment': '', - } - logger.info(f'data used to sent scores: {data}') - - url = lineitem_info['id'] + '/scores' - logger.debug(f'URL for lineitem grades submission {url}') - await client.fetch(url, body=json.dumps(data), method='POST', headers=self.headers) + try: + score = float(grade['score']) + data = { + 'timestamp': datetime.now().isoformat(), + 'userId': grade['lms_user_id'], + 'scoreGiven': score, + 'scoreMaximum': score_maximum, + 'gradingProgress': 'FullyGraded', + 'activityProgress': 'Completed', + 'comment': '', + } + logger.info(f'data used to sent scores: {data}') + + url = lineitem_info['id'] + '/scores' + logger.debug(f'URL for grades submission {url}') + await client.fetch(url, body=json.dumps(data), method='POST', headers=self.headers) + except Exception as e: + logger.error(f"Something went wrong by sending grader for {grade['lms_user_id']}.{e}") diff --git a/src/tests/illumidesk/authenticators/test_lti13_authenticator.py b/src/tests/illumidesk/authenticators/test_lti13_authenticator.py index 53b2c84f..73328557 100644 --- a/src/tests/illumidesk/authenticators/test_lti13_authenticator.py +++ b/src/tests/illumidesk/authenticators/test_lti13_authenticator.py @@ -11,12 +11,13 @@ @pytest.mark.asyncio async def test_authenticator_invokes_lti13validator_handler_get_argument( - build_lti13_jwt_id_token, make_lti13_resource_link_request, make_mock_request_handler + build_lti13_jwt_id_token, make_lti13_resource_link_request, make_mock_request_handler, mock_nbhelper ): """ Does the authenticator invoke the RequestHandler get_argument method? """ authenticator = LTI13Authenticator() + request_handler = make_mock_request_handler(RequestHandler, authenticator=authenticator) with patch.object( request_handler, 'get_argument', return_value=build_lti13_jwt_id_token(make_lti13_resource_link_request) @@ -27,7 +28,7 @@ async def test_authenticator_invokes_lti13validator_handler_get_argument( @pytest.mark.asyncio async def test_authenticator_invokes_lti13validator_jwt_verify_and_decode( - make_lti13_resource_link_request, make_mock_request_handler + make_lti13_resource_link_request, make_mock_request_handler, mock_nbhelper ): """ Does the authenticator invoke the LTI13Validator jwt_verify_and_decode method? @@ -44,7 +45,7 @@ async def test_authenticator_invokes_lti13validator_jwt_verify_and_decode( @pytest.mark.asyncio async def test_authenticator_invokes_lti13validator_validate_launch_request( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Does the authenticator invoke the LTI13Validator validate_launch_request method? @@ -63,7 +64,7 @@ async def test_authenticator_invokes_lti13validator_validate_launch_request( @pytest.mark.asyncio async def test_authenticator_invokes_lti_utils_normalize_string( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Does the authenticator invoke the LTIUtils normalize_string method? @@ -81,7 +82,11 @@ async def test_authenticator_invokes_lti_utils_normalize_string( @pytest.mark.asyncio async def test_authenticator_returns_course_id_in_auth_state_with_valid_resource_link_request( - auth_state_dict, make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + auth_state_dict, + make_lti13_resource_link_request, + build_lti13_jwt_id_token, + make_mock_request_handler, + mock_nbhelper, ): """ Do we get a valid course_id when receiving a valid resource link request? @@ -98,7 +103,7 @@ async def test_authenticator_returns_course_id_in_auth_state_with_valid_resource @pytest.mark.asyncio async def test_authenticator_returns_auth_state_name_from_lti13_email_claim( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we get a valid username when only including an email to the resource link request? @@ -118,7 +123,7 @@ async def test_authenticator_returns_auth_state_name_from_lti13_email_claim( @pytest.mark.asyncio async def test_authenticator_returns_username_in_auth_state_with_with_name( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we get a valid username when only including the name in the resource link request? @@ -139,7 +144,7 @@ async def test_authenticator_returns_username_in_auth_state_with_with_name( @pytest.mark.asyncio async def test_authenticator_returns_username_in_auth_state_with_with_given_name( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we get a valid username when only including the given name in the resource link request? @@ -160,7 +165,7 @@ async def test_authenticator_returns_username_in_auth_state_with_with_given_name @pytest.mark.asyncio async def test_authenticator_returns_username_in_auth_state_with_family_name( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we get a valid username when only including the family name in the resource link request? @@ -182,7 +187,7 @@ async def test_authenticator_returns_username_in_auth_state_with_family_name( @pytest.mark.asyncio async def test_authenticator_returns_username_in_auth_state_with_person_sourcedid( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we get a valid username when only including lis person sourcedid resource link request? @@ -206,7 +211,10 @@ async def test_authenticator_returns_username_in_auth_state_with_person_sourcedi @pytest.mark.asyncio async def test_authenticator_returns_username_in_auth_state_with_privacy_enabled( - make_lti13_resource_link_request_privacy_enabled, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request_privacy_enabled, + build_lti13_jwt_id_token, + make_mock_request_handler, + mock_nbhelper, ): """ Do we get a valid username when initiating the login flow with privacy enabled? @@ -227,7 +235,7 @@ async def test_authenticator_returns_username_in_auth_state_with_privacy_enabled @pytest.mark.asyncio async def test_authenticator_returns_workspace_type_in_auth_state( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we get a valid lms_user_id in the auth_state when receiving a valid resource link request? @@ -245,7 +253,7 @@ async def test_authenticator_returns_workspace_type_in_auth_state( @pytest.mark.asyncio async def test_authenticator_returns_learner_role_in_auth_state( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the learner role in the auth_state when receiving a valid resource link request? @@ -266,7 +274,7 @@ async def test_authenticator_returns_learner_role_in_auth_state( @pytest.mark.asyncio async def test_authenticator_returns_instructor_role_in_auth_state_with_instructor_role( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the instructor role in the auth_state when receiving a valid resource link request? @@ -288,7 +296,7 @@ async def test_authenticator_returns_instructor_role_in_auth_state_with_instruct @pytest.mark.asyncio async def test_authenticator_returns_student_role_in_auth_state_with_learner_role( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the student role in the auth_state when receiving a valid resource link request with the Learner role? @@ -309,7 +317,7 @@ async def test_authenticator_returns_student_role_in_auth_state_with_learner_rol @pytest.mark.asyncio async def test_authenticator_returns_student_role_in_auth_state_with_student_role( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the student role in the auth_state when receiving a valid resource link request with the Student role? @@ -331,7 +339,7 @@ async def test_authenticator_returns_student_role_in_auth_state_with_student_rol @pytest.mark.asyncio async def test_authenticator_returns_learner_role_in_auth_state_with_empty_roles( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the learner role in the auth_state when receiving resource link request @@ -350,7 +358,7 @@ async def test_authenticator_returns_learner_role_in_auth_state_with_empty_roles @pytest.mark.asyncio async def test_authenticator_returns_default_workspace_type_with_unrecognized_workspace_type_in_custom_claim( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the default notebook image when the workspace type is unrecognized? @@ -369,7 +377,7 @@ async def test_authenticator_returns_default_workspace_type_with_unrecognized_wo @pytest.mark.asyncio async def test_authenticator_returns_default_workspace_type_with_missing_workspace_type_in_custom_claim( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the default notebook image when the workspace type is missing? @@ -388,7 +396,7 @@ async def test_authenticator_returns_default_workspace_type_with_missing_workspa @pytest.mark.asyncio async def test_authenticator_returns_notebook_as_workspace_type_in_auth_state_if_the_custom_claim_contains_this_value( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the workspace image to the notebook image when setting the workspace type to notebook? @@ -406,7 +414,7 @@ async def test_authenticator_returns_notebook_as_workspace_type_in_auth_state_if @pytest.mark.asyncio async def test_authenticator_returns_rstudio_workspace_image_with_rstudio_workspace_type_in_auth_state( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the workspace image to the rstudio image when setting the workspace type to rstudio? @@ -425,7 +433,7 @@ async def test_authenticator_returns_rstudio_workspace_image_with_rstudio_worksp @pytest.mark.asyncio async def test_authenticator_returns_theia_workspace_image_with_theia_workspace_type_in_auth_state( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the workspace image to the theia image when setting the workspace type to tbeia? @@ -444,7 +452,7 @@ async def test_authenticator_returns_theia_workspace_image_with_theia_workspace_ @pytest.mark.asyncio async def test_authenticator_returns_vscode_workspace_image_with_vscode_workspace_type_in_auth_state( - make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler + make_lti13_resource_link_request, build_lti13_jwt_id_token, make_mock_request_handler, mock_nbhelper ): """ Do we set the workspace image to the vscode image when setting the workspace type to vscode? diff --git a/src/tests/illumidesk/authenticators/test_setup_course_hook.py b/src/tests/illumidesk/authenticators/test_setup_course_hook.py index 895360c0..790d99f6 100644 --- a/src/tests/illumidesk/authenticators/test_setup_course_hook.py +++ b/src/tests/illumidesk/authenticators/test_setup_course_hook.py @@ -175,34 +175,7 @@ async def test_setup_course_hook_calls_add_instructor_to_jupyterhub_group_when_r local_handler = make_mock_request_handler(RequestHandler, authenticator=local_authenticator) local_authentication = make_auth_state_dict(user_role='Instructor') - with patch.object( - JupyterHubAPI, 'add_instructor_to_jupyterhub_group', return_value=None - ) as mock_add_instructor_to_jupyterhub_group: - with patch.object(AsyncHTTPClient, 'fetch', return_value=make_http_response(handler=local_handler.request)): - await setup_course_hook(local_authenticator, local_handler, local_authentication) - assert mock_add_instructor_to_jupyterhub_group.called - - -@pytest.mark.asyncio() -async def test_setup_course_hook_does_not_call_add_student_to_jupyterhub_group_when_role_is_instructor( - setup_course_environ, - setup_course_hook_environ, - make_auth_state_dict, - make_http_response, - make_mock_request_handler, -): - """ - Is the jupyterhub_api add student to jupyterhub group function called when the user role is - the instructor role? - """ - local_authenticator = Authenticator(post_auth_hook=setup_course_hook) - local_handler = make_mock_request_handler(RequestHandler, authenticator=local_authenticator) - local_authentication = make_auth_state_dict(user_role='Instructor') - response_args = {'handler': local_handler.request} - - with patch.object( - JupyterHubAPI, 'add_student_to_jupyterhub_group', return_value=None - ) as mock_add_student_to_jupyterhub_group: + with patch.object(JupyterHubAPI, 'add_user_to_nbgrader_gradebook', return_value=None): with patch.object( JupyterHubAPI, 'add_instructor_to_jupyterhub_group', return_value=None ) as mock_add_instructor_to_jupyterhub_group: @@ -210,8 +183,7 @@ async def test_setup_course_hook_does_not_call_add_student_to_jupyterhub_group_w AsyncHTTPClient, 'fetch', return_value=make_http_response(handler=local_handler.request) ): await setup_course_hook(local_authenticator, local_handler, local_authentication) - assert not mock_add_student_to_jupyterhub_group.called - assert not mock_add_instructor_to_jupyterhub_group.called + assert mock_add_instructor_to_jupyterhub_group.called @pytest.mark.asyncio() @@ -223,22 +195,26 @@ async def test_setup_course_hook_does_not_call_add_student_to_jupyterhub_group_w make_mock_request_handler, ): """ - Is the jupyterhub_api add student gradebook function called when the user role is + Is the jupyterhub_api add student to jupyterhub group function called when the user role is the instructor role? """ local_authenticator = Authenticator(post_auth_hook=setup_course_hook) local_handler = make_mock_request_handler(RequestHandler, authenticator=local_authenticator) local_authentication = make_auth_state_dict(user_role='Instructor') - with patch.object( - JupyterHubAPI, 'add_user_to_nbgrader_gradebook', return_value=None - ) as mock_add_user_to_nbgrader_gradebook: - with patch.object(JupyterHubAPI, 'add_instructor_to_jupyterhub_group', return_value=None): + with patch.object(JupyterHubAPI, 'add_user_to_nbgrader_gradebook', return_value=None): + with patch.object( + JupyterHubAPI, 'add_student_to_jupyterhub_group', return_value=None + ) as mock_add_student_to_jupyterhub_group: with patch.object( - AsyncHTTPClient, 'fetch', return_value=make_http_response(handler=local_handler.request) - ): - await setup_course_hook(local_authenticator, local_handler, local_authentication) - assert not mock_add_user_to_nbgrader_gradebook.called + JupyterHubAPI, 'add_instructor_to_jupyterhub_group', return_value=None + ) as mock_add_instructor_to_jupyterhub_group: + with patch.object( + AsyncHTTPClient, 'fetch', return_value=make_http_response(handler=local_handler.request) + ): + await setup_course_hook(local_authenticator, local_handler, local_authentication) + assert not mock_add_student_to_jupyterhub_group.called + assert mock_add_instructor_to_jupyterhub_group.called @pytest.mark.asyncio() diff --git a/src/tests/illumidesk/conftest.py b/src/tests/illumidesk/conftest.py index ff7c0fad..0bd8c8df 100644 --- a/src/tests/illumidesk/conftest.py +++ b/src/tests/illumidesk/conftest.py @@ -1,6 +1,7 @@ from io import StringIO import json import jwt +from nbgrader.api import Course import pytest import os import secrets @@ -57,6 +58,19 @@ def post(self): return application +@pytest.fixture(scope='function') +def mock_nbhelper(): + with patch.multiple( + 'illumidesk.apis.nbgrader_service.NbGraderServiceHelper', + __init__=lambda x, y: None, + update_course=Mock(return_value=None), + get_course=Mock( + return_value=Course(id='123', lms_lineitems_endpoint='canvas.docker.com/api/lti/courses/1/line_items') + ), + ) as mock_nb: + yield mock_nb + + @pytest.fixture(scope='function') def jupyterhub_api_environ(monkeypatch): """ diff --git a/src/tests/illumidesk/grades/test_senders.py b/src/tests/illumidesk/grades/test_senders.py index f43d1193..439cc596 100644 --- a/src/tests/illumidesk/grades/test_senders.py +++ b/src/tests/illumidesk/grades/test_senders.py @@ -50,32 +50,24 @@ async def test_grades_sender_raises_an_error_if_assignment_not_found_in_control_ class TestLTI13GradesSender: - def test_sender_raises_an_error_without_auth_state_information(self): - with pytest.raises(GradesSenderMissingInfoError): - LTI13GradeSender('course-id', 'lab', None) - - def test_sender_sets_lineitems_url_with_the_value_in_auth_state_dict(self, lti_config_environ): - sut = LTI13GradeSender( - 'course-id', 'lab', {'course_lineitems': 'canvas.docker.com/api/lti/courses/1/line_items'} - ) - assert sut.lineitems_url == 'canvas.docker.com/api/lti/courses/1/line_items' + def test_sender_sets_lineitems_url_with_the_value_in_auth_state_dict(self, lti_config_environ, mock_nbhelper): + sut = LTI13GradeSender('course-id', 'lab') + assert sut.course.lms_lineitems_endpoint == 'canvas.docker.com/api/lti/courses/1/line_items' @pytest.mark.asyncio - async def test_sender_raises_AssignmentWithoutGradesError_if_there_are_not_grades(self, lti_config_environ): - sut = LTI13GradeSender( - 'course-id', 'lab', {'course_lineitems': 'canvas.docker.com/api/lti/courses/1/line_items'} - ) + async def test_sender_raises_AssignmentWithoutGradesError_if_there_are_not_grades( + self, lti_config_environ, mock_nbhelper + ): + sut = LTI13GradeSender('course-id', 'lab') with patch.object(LTI13GradeSender, '_retrieve_grades_from_db', return_value=(lambda: 10, [])): with pytest.raises(AssignmentWithoutGradesError): await sut.send_grades() @pytest.mark.asyncio async def test_sender_calls__set_access_token_header_before_to_send_grades( - self, lti_config_environ, make_http_response, make_mock_request_handler + self, lti_config_environ, make_http_response, make_mock_request_handler, mock_nbhelper ): - sut = LTI13GradeSender( - 'course-id', 'lab', {'course_lineitems': 'canvas.docker.com/api/lti/courses/1/line_items'} - ) + sut = LTI13GradeSender('course-id', 'lab') local_handler = make_mock_request_handler(RequestHandler) access_token_result = {'token_type': '', 'access_token': ''} line_item_result = {'label': 'lab', 'id': 'line_item_url', 'scoreMaximum': 40} @@ -101,11 +93,9 @@ async def test_sender_calls__set_access_token_header_before_to_send_grades( @pytest.mark.asyncio @pytest.mark.parametrize("http_async_httpclient_with_simple_response", [[]], indirect=True) async def test_sender_raises_an_error_if_no_line_items_were_found( - self, lti_config_environ, http_async_httpclient_with_simple_response + self, lti_config_environ, http_async_httpclient_with_simple_response, mock_nbhelper ): - sut = LTI13GradeSender( - 'course-id', 'lab', {'course_lineitems': 'canvas.docker.com/api/lti/courses/1/line_items'} - ) + sut = LTI13GradeSender('course-id', 'lab') access_token_result = {'token_type': '', 'access_token': ''} with patch('illumidesk.grades.senders.get_lms_access_token', return_value=access_token_result) as mock_method: