mitodl / mitxonline

BSD 3-Clause "New" or "Revised" License
4 stars 2 forks source link

Letter grade is set to None when grade is populated through a management command #1762

Closed annagav closed 1 year ago

annagav commented 1 year ago

Steps to Reproduce

Example course run grade id 43640.

I noticed that some grades are missing letter_grade value. We think that happens through the management command manage_certificates. If is_grade_override is true then we set letter grade to None: [code here] (https://github.com/mitodl/mitxonline/blob/2f951479e328c2a9f9d21d79f84fc3e04837ff43/courses/api.py#L871-L897)

Expected Behavior

If the grade is passing and enrollment verified we need an actual letter grade.

Actual Behavior

The learner is missing a letter grade.

Screen Shot 2023-07-20 at 8 51 09 AM
pdpinch commented 1 year ago

When we pass a grade to the management command, we should require a letter grade (string) as well.

pdpinch commented 1 year ago

Example course run grade id 43640.

Solving this issue also requires doing a search for other missing letter grades and fixing them up. If the courses are DEDP courses, it's straightforward since they always use the same grading policy. If there are missing letter grades from other courses, we will have to look at their grading policy in Studio.

arslanashraf7 commented 1 year ago

@pdpinch FYI.

Solving this issue also requires doing a search for other missing letter grades and fixing them up. If the courses are DEDP courses, it's straightforward since they always use the same grading policy. If there are missing letter grades from other courses, we will have to look at their grading policy in Studio.

This might be difficult in the sense that, a None letter grade in Django would not mean that it was only set by the override command. So, To reduce the scope, We can check those grades that are set_by_admin=True to backfill the data.

Also, Just to add more thoughts to what's already defined above:

  1. I believe that the Letter grade from edX can have possible values (A, B, C, D, F, Pass, Fail, None)
  2. So, along with the override management command, The grade can come as None from edX as well.
  3. I think we'll need to update validation on letter grade as defined here in the PR
  4. Usually, The Pass/Fail comes when there is a 50/50 grading policy set which is also the default in edX. e.g take a look at https://rc.mitxonline.mit.edu/admin/courses/courserungrade/175/change/?_changelist_filters=q%3Darslan (It was auto sync, has Pass letter grade)

See some screenshots below:

image image
annagav commented 1 year ago

@pdpinch @arslanashraf7 The problem we are having is not about all grades, but about grades that we have to show on the Program Records page. Those grades only belong to a program and those grades are only for certificate track learners. We need to have these letter grades be consistent A-F.

pdpinch commented 1 year ago

I'm OK with omitting the pass/fail option if it's a big lift. I think it's unlikely that we will need it. And I assume we can add it if/when we do.

annagav commented 1 year ago

@arslanashraf7 In what scenario would letter_grade come from edx as None?

annagav commented 1 year ago

Given that we don't have a clear direction with this, I will create a migration to populate only DEDP grades that were set by admin.

arslanashraf7 commented 1 year ago

@arslanashraf7 In what scenario would letter_grade come from edx as None?

I believe that happens in cases when the completion of the course by the user is not done(The user has not attempted all the graded items) e.g. in a Self-paced course and in instructor-paced courses when the grades are not finalized and persisted.

annagav commented 1 year ago

We populated all DEDP grades and we set a restriction on the management command to provide letter_grade