diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 847720102a40..f783ddb64739 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -1558,6 +1558,9 @@ def extras_update_lti_grades(request): #SA || letter_grade changes letter_grade = request.POST.get("letter_grade", "") grade = 0 if letter_grade else request.POST.get("user_grade", "") + + #AK || feedback/comment changes + comment = request.POST.get("comment", "") try: block_data = get_course_blocks(User.objects.get(id = user_id), modulestore().make_course_usage_key(CourseKey.from_string(str(course_id))), allow_start_dates_in_future=True, include_completion=True) @@ -1568,6 +1571,7 @@ def extras_update_lti_grades(request): #Update Grades studentmodule.grade = grade studentmodule.letter_grade = letter_grade + studentmodule.comment = comment student_state = json.loads(studentmodule.state) student_state["module_score"] = grade studentmodule.state = json.dumps(student_state) @@ -1585,9 +1589,10 @@ def extras_update_lti_grades(request): only_if_higher=False, modified=datetime.datetime.now().replace(tzinfo=pytz.UTC), score_db_table=grades_constants.ScoreDatabaseTableEnum.courseware_student_module, + comment=comment, ) except StudentModule.DoesNotExist: - studentmodule = StudentModule.objects.create(student_id=user_id,course_id=request.POST.get("course_id"),module_state_key=usage_id,state=json.dumps({"module_score" : grade, "score_comment" : ""}), max_grade= 0 if letter_grade else block_data.get_xblock_field(usage_id, 'weight'), letter_grade=letter_grade) + studentmodule = StudentModule.objects.create(student_id=user_id,course_id=request.POST.get("course_id"),module_state_key=usage_id,state=json.dumps({"module_score" : grade, "score_comment" : ""}), max_grade= 0 if letter_grade else block_data.get_xblock_field(usage_id, 'weight'), letter_grade=letter_grade, comment=comment) log.info("Student module created {0}".format(studentmodule)) diff --git a/lms/djangoapps/course_home_api/progress/serializers.py b/lms/djangoapps/course_home_api/progress/serializers.py index f0eebc703b2b..d4d6f9cbe531 100644 --- a/lms/djangoapps/course_home_api/progress/serializers.py +++ b/lms/djangoapps/course_home_api/progress/serializers.py @@ -39,6 +39,9 @@ class SubsectionScoresSerializer(ReadOnlySerializer): #SA || letter_grade changes letter_grade = serializers.CharField() + + #AK || feedback/comment changes + comment = serializers.CharField() def get_override(self, subsection): """Proctoring or grading score override""" diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index b755d30fec43..4b8131d28390 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -916,7 +916,8 @@ class ScoresClient: handles the read side of things. """ #SA || letter_grade changes - Score = namedtuple('Score', 'correct total created letter_grade') + #AK || feedback/comment changes + Score = namedtuple('Score', 'correct total created letter_grade comment') def __init__(self, course_key, user_id): self.course_key = course_key @@ -939,9 +940,9 @@ def fetch_scores(self, locations): # attached to them (since old mongo identifiers don't include runs). # So we have to add that info back in before we put it into our lookup. self._locations_to_scores.update({ - location.map_into_course(self.course_key): self.Score(correct, total, created, letter_grade) - for location, correct, total, created, letter_grade - in scores_qset.values_list('module_state_key', 'grade', 'max_grade', 'created', 'letter_grade') + location.map_into_course(self.course_key): self.Score(correct, total, created, letter_grade, comment) + for location, correct, total, created, letter_grade, comment + in scores_qset.values_list('module_state_key', 'grade', 'max_grade', 'created', 'letter_grade', 'comment') }) self._has_fetched = True diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index c531def779ed..e6acab68a9ab 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -123,6 +123,9 @@ class Meta: #SA || letter_grade changes letter_grade = models.TextField(null=True, blank=True) + + #AK || feedback/comment changes + comment = models.TextField(null=True, blank=True) @classmethod def all_submitted_problems_read_only(cls, course_id): @@ -154,6 +157,7 @@ def __repr__(self): 'module_state_key': self.module_state_key, 'state': str(self.state)[:20], 'letter_grade': self.letter_grade, + 'comment': self.comment }) def __str__(self): diff --git a/lms/djangoapps/grades/events.py b/lms/djangoapps/grades/events.py index c7e4172f52a1..0315efae04dd 100644 --- a/lms/djangoapps/grades/events.py +++ b/lms/djangoapps/grades/events.py @@ -51,6 +51,7 @@ def grade_updated(**kwargs): root_id = create_new_event_transaction_id() set_event_transaction_type(PROBLEM_SUBMITTED_EVENT_TYPE) #SA || letter_grade changes + #AK || feedback/comment changes tracker.emit( str(PROBLEM_SUBMITTED_EVENT_TYPE), { @@ -62,6 +63,7 @@ def grade_updated(**kwargs): 'weighted_earned': kwargs.get('weighted_earned'), 'weighted_possible': kwargs.get('weighted_possible'), 'letter_grade': kwargs.get('letter_grade'), + 'comment': kwargs.get('comment'), } ) diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 78f04155b518..bfaa1c24abdd 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -344,6 +344,9 @@ class Meta: #SA || letter_grade changes letter_grade = models.CharField(max_length=255, blank=True, null=True) + + #AK || feedback/comment changes + comment = models.TextField(blank=True, null=True) # timestamp for the learner's first attempt at content in # this subsection. If null, indicates no attempt @@ -373,7 +376,7 @@ def __str__(self): """ #SA || letter_grade changes return ( - "{} user: {}, course version: {}, subsection: {} ({}). {}/{} graded, {}/{} all, first_attempted: {}, letter_grade: {}" + "{} user: {}, course version: {}, subsection: {} ({}). {}/{} graded, {}/{} all, first_attempted: {}, letter_grade: {}, comment: {}" ).format( type(self).__name__, self.user_id, @@ -386,6 +389,7 @@ def __str__(self): self.possible_all, self.first_attempted, self.letter_grade, + self.comment ) @classmethod diff --git a/lms/djangoapps/grades/rest_api/serializers.py b/lms/djangoapps/grades/rest_api/serializers.py index 5e6db42e29d4..59d848f26888 100644 --- a/lms/djangoapps/grades/rest_api/serializers.py +++ b/lms/djangoapps/grades/rest_api/serializers.py @@ -47,6 +47,9 @@ class SectionBreakdownSerializer(serializers.Serializer): #SA || letter_grade changes letter_grade = serializers.CharField() + + #AK || feedback/comment changes + comment = serializers.CharField() class StudentGradebookEntrySerializer(serializers.Serializer): diff --git a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py index 0e86badbda65..7841ac384916 100644 --- a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py @@ -476,6 +476,7 @@ def _section_breakdown(self, course, graded_subsections, course_grade): # 'displayed_value' should maybe be 'description_percent' # 'grade_description' should be 'description_ratio' #SA || letter_grade changes + #AK || feedback/comment changes breakdown.append({ 'attempted': attempted, 'category': subsection_grade.format, @@ -486,6 +487,7 @@ def _section_breakdown(self, course, graded_subsections, course_grade): 'score_possible': score_possible, 'subsection_name': subsection_grade.display_name, 'letter_grade': subsection_grade.letter_grade, + 'comment': subsection_grade.comment, }) return breakdown diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index 907b3c52894e..57a976420ab6 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -110,7 +110,8 @@ def get_score(submissions_scores, csm_scores, persisted_block, block): # Priority order for retrieving the scores: # submissions API -> CSM -> grades persisted block -> latest block content #SA || letter_grade changes - raw_earned, raw_possible, weighted_earned, weighted_possible, first_attempted, letter_grade = ( + #AK || feedback/comment changes + raw_earned, raw_possible, weighted_earned, weighted_possible, first_attempted, letter_grade, comment = ( _get_score_from_submissions(submissions_scores, block) or _get_score_from_csm(csm_scores, block, weight) or _get_score_from_persisted_or_latest_block(persisted_block, block, weight) @@ -131,6 +132,7 @@ def get_score(submissions_scores, csm_scores, persisted_block, block): graded = _get_graded_from_block(persisted_block, block) if has_valid_denominator else False #SA || letter_grade changes + #AK || feedback/comment changes return ProblemScore( raw_earned, raw_possible, @@ -140,6 +142,7 @@ def get_score(submissions_scores, csm_scores, persisted_block, block): graded, first_attempted=first_attempted, letter_grade=letter_grade, + comment=comment, ) @@ -178,13 +181,15 @@ def _get_score_from_submissions(submissions_scores, block): if submissions_scores: submission_value = submissions_scores.get(str(block.location)) #SA || letter_grade changes + #AK || feedback/comment changes if submission_value: first_attempted = submission_value['created_at'] weighted_earned = submission_value['points_earned'] weighted_possible = submission_value['points_possible'] letter_grade = submission_value.get('letter_grade', '') + comment = submission_value.get('comment', '') assert weighted_earned >= 0.0 and weighted_possible > 0.0 # per contract from submissions API - return (None, None) + (weighted_earned, weighted_possible) + (first_attempted, letter_grade,) + return (None, None) + (weighted_earned, weighted_possible) + (first_attempted, letter_grade, comment,) def _get_score_from_csm(csm_scores, block, weight): @@ -215,7 +220,8 @@ def _get_score_from_csm(csm_scores, block, weight): raw_possible = score.total #SA || letter_grade changes - return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) + (first_attempted, score.letter_grade,) + #AK || feedback/comment changes + return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) + (first_attempted, score.letter_grade, score.comment,) def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): @@ -235,6 +241,9 @@ def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): #SA || letter_grade changes letter_grade = None + + #AK || feedback/comment changes + comment = None if persisted_block: raw_possible = persisted_block.raw_possible @@ -252,7 +261,8 @@ def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): weighted_scores = weighted_score(raw_earned, raw_possible, weight) #SA || letter_grade changes - return (raw_earned, raw_possible) + weighted_scores + (first_attempted, letter_grade,) + #AK || feedback/comment changes + return (raw_earned, raw_possible) + weighted_scores + (first_attempted, letter_grade, comment,) def _get_weight_from_block(persisted_block, block): diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 361c509021b7..3855e2ad8c38 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -208,6 +208,7 @@ def problem_raw_score_changed_handler(sender, **kwargs): # pylint: disable=unus weighted_earned, weighted_possible = kwargs['raw_earned'], kwargs['raw_possible'] #SA || letter_grade changes + #AK || feedback/comment changes PROBLEM_WEIGHTED_SCORE_CHANGED.send( sender=None, weighted_earned=weighted_earned, @@ -220,7 +221,8 @@ def problem_raw_score_changed_handler(sender, **kwargs): # pylint: disable=unus modified=kwargs['modified'], score_db_table=kwargs['score_db_table'], grader_response=kwargs.get('grader_response', False), - letter_grade=kwargs.get('letter_grade', '') + letter_grade=kwargs.get('letter_grade', ''), + comment = kwargs.get('comment', '') ) @@ -236,6 +238,7 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum if not context_key.is_course: return # If it's not a course, it has no subsections, so skip the subsection grading update #SA || letter_grade changes + #AK || feedback/comment changes recalculate_subsection_grade_v3.apply_async( kwargs=dict( user_id=kwargs['user_id'], @@ -250,6 +253,7 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum score_db_table=kwargs['score_db_table'], force_update_subsections=kwargs.get('force_update_subsections', False), letter_grade=kwargs.get('letter_grade', ''), + comment = kwargs.get('comment', ''), ), countdown=RECALCULATE_GRADE_DELAY_SECONDS, ) diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 13d048f4c1b3..31cd34cc3acb 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -167,7 +167,33 @@ def letter_grade(self): letter_grades[block_key] = problem_score.letter_grade letter_grade = problem_score.letter_grade return letter_grade - + + #AK || feedback/comment changes + @property + def comment(self): + """ + Overrides the problem_scores member variable in order + to return empty scores for all scorable problems in the + course. + NOTE: The use of `course_data.structure` here is very intentional. + It means we look through the user-specific subtree of this subsection, + taking into account which problems are visible to the user. + """ + comments = OrderedDict() # dict of problem locations to ProblemScore + comment = '' + for block_key in self.course_data.structure.post_order_traversal( + filter_func=possibly_scored, + start_node=self.location, + ): + block = self.course_data.structure[block_key] + if getattr(block, 'has_score', False): + problem_score = get_score( + submissions_scores={}, csm_scores={}, persisted_block=None, block=block, + ) + if problem_score is not None: + comments[block_key] = problem_score.comment + comment = problem_score.comment + return comment class NonZeroSubsectionGrade(SubsectionGradeBase, metaclass=ABCMeta): """ @@ -317,6 +343,26 @@ def letter_grade(self): if problem_score: letter_grade = problem_score.letter_grade return letter_grade + + #AK || feedback/comment changes + @property + def comment(self): + """ + Returns the letter grade from model + """ + # pylint: disable=protected-access + comment = '' + for block in self.model.visible_blocks.blocks: + problem_score = self._compute_block_score( + block.locator, + self.factory.course_data.structure, + self.factory._submissions_scores, + self.factory._csm_scores, + block, + ) + if problem_score: + comment = problem_score.comment + return comment class CreateSubsectionGrade(NonZeroSubsectionGrade): @@ -328,6 +374,9 @@ def __init__(self, subsection, course_structure, submissions_scores, csm_scores) #SA || letter_grade changes self.letter_grade = '' + + #AK || feedback/comment changes + self.comment = '' for block_key in course_structure.post_order_traversal( filter_func=possibly_scored, start_node=subsection.location, @@ -342,6 +391,7 @@ def __init__(self, subsection, course_structure, submissions_scores, csm_scores) if problem_score: self.problem_scores[block_key] = problem_score self.letter_grade = problem_score.letter_grade + self.comment = problem_score.comment all_total, graded_total = graders.aggregate_scores(list(self.problem_scores.values())) @@ -413,6 +463,7 @@ def _persisted_model_params(self, student): persisted model for this subsection grade. """ #SA || letter_grade changes + #AK || feedback/comment changes return dict( user_id=student.id, usage_key=self.location, @@ -425,6 +476,7 @@ def _persisted_model_params(self, student): visible_blocks=self._get_visible_blocks, first_attempted=self.all_total.first_attempted, letter_grade=self.letter_grade, + comment=self.comment, ) @property diff --git a/xmodule/graders.py b/xmodule/graders.py index 0312f49c9f9f..a7984c21b91a 100644 --- a/xmodule/graders.py +++ b/xmodule/graders.py @@ -26,7 +26,8 @@ class ScoreBase(metaclass=abc.ABCMeta): """ #SA || letter_grade changes - def __init__(self, graded, first_attempted, letter_grade=None): + #AK || feedback/comment changes + def __init__(self, graded, first_attempted, letter_grade=None, comment=None): """ Fields common to all scores include: @@ -39,6 +40,7 @@ def __init__(self, graded, first_attempted, letter_grade=None): self.graded = graded self.first_attempted = first_attempted self.letter_grade = letter_grade + self.comment = comment def __eq__(self, other): if type(other) is type(self):