Fix all existing ccx tests

[cewing]

tests will need to updated to account for the new id usage.

[cguardia]

As usual, fixing tests is taking a good deal of time. So far, I got them down from 30 to 8 errors and failures. @cewing, the only way I found to be able to get non-deprecated course ids, was to set the MODULESTORE in the test cases. Please take a look.

[cewing]

We are now down to three failures, all in the grading system.

[pdpinch]

Can you point me to the failing tests, so I can see if edX can help out?

[cguardia]

https://github.com/jazkarta/edx-platform/blob/ccx-custom-ids/lms/djangoapps/ccx/tests/test_views.py#L445

[cguardia]

The overrides in those tests work fine, but we are not getting the correct student grade summary back from the view.

[cewing]

@pdpinch help with this would be very very welcome.

[ormsbee]

What do the grading test failures look like (the UndefinedContext stuff was already fixed, right?)

[cewing]

The failures are in data comparison. There is no exception raised. We are expecting a certain value, and finding 0 instead. It appears that student data is not being correctly retrieved.

[ormsbee]

Are the tests definitely storing their scores in CSM using the correct course_id?

[cewing]

They are storing the scores by using the course_id for the one course that exists in the tests.

I think what we have here is a situation where the key we have when we look up student results is a CCXLocator or a CCXBlockUsageLocator, rather than the CourseLocator that was used for storing the values. We are having a hard time digging out the location where that lookup happens. We have api for making a conversion of the key but need to know where to look for the place to use it.

[ormsbee]

Okay. So the solution is going to be to change the part that records the scores to use the CCXLocators, so that those are what we're storing in CSM, right? The recording of scores happens in module_render.py in handle_grade_event(), but as long as the XBlock itself is seeing CCX IDs and passing those through, the rest of the system should just use them.

[ormsbee]

I guess I'm concerned if we're doing conversions back and forth in too many places.

[cewing]

@ormsbee, @pdpinch Here is the PR that shows the work done to-date in implementing ccx ids.

https://github.com/jazkarta/edx-platform/pull/99

[cewing]

@ormsbee: I don't know whether updating the storage of grades to use CCX keys or updating the retrieval point to use Course keys is the right way to go. I'm not even 100% certain that this is the cause of the problem. All our other problems have been due to this type of conversion, but I'd love to have pointers into the grade retrieval system as well to be able to verify the actual cause of the test failures.

[ormsbee]

If the course_id that an XBlock saves state to and emits events with are CCXLocators, the rest should just flow through the rest of the grading system. One of my major hopes is that the grading system machinery as a whole can be insulated from having to know anything about CCX, and the only interaction it would have with it is reading the overridden values for course policy and dates (which are overridden at the XBlock field level and so are opaque to the grading system).

I feel very strongly that we should be writing CCXLocator keys to the database in CSM.

Thank you for the PR link.

[ormsbee]

Oh, but as far as pointers go, if you're using devstack, go to the database shell and look at courseware_studentmodule:

python manage.py lms dbshell  --settings=devstack`

mysql> select id, course_id, module_id, grade, max_grade from courseware_studentmodule where student_id=4;

4 is the staff user, but whichever user you're testing with. You should also check the state field to make sure that's getting put in the right place.

[ormsbee]

In terms of reading the scores back, that happens in grades.py, in get_score()

[cewing]

Thanks, @ormsbee, this will help. If you believe storing the grade information with CCX keys is the way to go, we'll work that direction.

[cewing]

Okay @pdpinch. There are definitely a couple of things to report here.

@ormsbee, to start with all entries that I see in the courseware_studentmodule table in the live database used by my devstack instance are using the CCXLocator form of the course_id, so I think we are good there. At least for the writing portion we are good to go.

However, when it comes to reading scores here's what I'm seeing.

We are apparently hitting a codepath that is different from what is expected in our tests because when I dropped a breakpoint into lms/djangoapps/courseware/grades.py in the get_score method, it was never hit. I poked around some more in that file, and finally got a break when I tried the _grade method from that same module. However, that kicks out to submissions/api.py:get_scores to retrieve score information it is using. That method appears to be using a different way of calculating scores because it uses submissions/models.py:ScoreSummary to look up information about the student who's grades have been requested:

def get_scores(course_id, student_id):
    # ...
    try:
        score_summaries = ScoreSummary.objects.filter(
            student_item__course_id=course_id,
            student_item__student_id=student_id,
        ).select_related('latest', 'student_item')
    #...

When I drop a breakpoint into this submissions.api.get_scores method, I can see that the value bound to score_summaries is an empty list. This is clearly why we are not getting any data for our students. I have checked and if I run ScoreSummary.objects.all() that returns an empty list as well.

Checking in my devstack database, there are no entries in any of the tables from the edx-summaries app either.

Since our tests hit this code path, it suggests that our tests are wrong (or that the views we've written to get the grades are not correct). Given that they were modeled after views to retrieve the same information in the instructor app from quite some time ago, I suspect that we have views that are out of date.

The student's "progress" view in the devstack app works correctly, which would be the case if it had been updated to use a different approach to calculating data than our tests are using. I will follow up to this by looking for the views that get grades for the instructor of a course, and see if I can't re-wire our coach dashboard to use the standard views instead. That would eliminate our custom views as a possible failure point, and have the side benefit of allowing us to delete more code from ccx.

[pdpinch]

Interesting.

It’s really helpful to know that the progress page works correctly. I believe Dave speculated that we were using a legacy grade export (part of me is worry that it’s been broken).

Before you go exploring too much Cris, I’d be curious to know what you get from a regular grade export, done on the CCX by someone who is set to be an instructor (not just a coach). I.e. go to data download in the instructor dashboard and click on the “Generate Grade Report” button.

It may be that now that we have a CCX key, that feature may be more appropriate?

I don’t want to get you off track, but if you had a sandbox available, I could give it a try myself.

On Jun 2, 2015, at 7:36 PM, Cris Ewing notifications@github.com wrote:

Okay @pdpinch https://github.com/pdpinch. There are definitely a couple of things to report here.

@ormsbee https://github.com/ormsbee, to start with all entries that I see in the courseware_studentmodule table in the live database used by my devstack instance are using the CCXLocator form of the course_id, so I think we are good there. At least for the writing portion we are good to go.

However, when it comes to reading scores here's what I'm seeing.

We are apparently hitting a codepath that is different from what is expected in our tests because when I dropped a breakpoint into lms/djangoapps/courseware/grades.py in the get_score method, it was never hit. I poked around some more in that file, and finally got a break when I tried the _grade method from that same module. However, that kicks out to submissions/api.py:get_scores to retrieve score information it is using. That method appears to be using a different way of calculating scores because it uses submissions/models.py:ScoreSummary to look up information https://github.com/edx/edx-submissions/blob/master/submissions/api.py#L555 about the student who's grades have been requested:

def get_scores(course_id, student_id):

...

try:
    score_summaries = ScoreSummary.objects.filter(
        student_item__course_id=course_id,
        student_item__student_id=student_id,
    ).select_related('latest', 'student_item')
#...

When I drop a breakpoint into this submissions.api.get_scores method, I can see that the value bound to score_summaries is an empty list. This is clearly why we are not getting any data for our students. I have checked and if I run ScoreSummary.objects.all() that returns an empty list as well.

Checking in my devstack database, there are no entries in any of the tables from the edx-summaries app either.

Since our tests hit this code path, it suggests that our tests are wrong (or that the views we've written to get the grades are not correct). Given that they were modeled after views to retrieve the same information in the instructor app from quite some time ago, I suspect that we have views that are out of date.

The student's "progress" view in the devstack app works correctly, which would be the case if it had been updated to use a different approach to calculating data than our tests are using. I will follow up to this by looking for the views that get grades for the instructor of a course, and see if I can't re-wire our coach dashboard to use the standard views instead. That would eliminate our custom views as a possible failure point, and have the side benefit of allowing us to delete more code from ccx.

— Reply to this email directly or view it on GitHub https://github.com/jazkarta/edx-platform/issues/96#issuecomment-108131877.

[cewing]

@pdpinch I am working on a local box, so no sandbox. Sorry :disappointed:.

I have discovered that the instructor views and our views are incompatible in that the instructor views use the CourseEnrollment objects for users to filter to a list of users enrolled in a specific course. Users are given memebership in a CCX via a different model, and so there are no users listed when I view a standard instructor grading view like courses/<course_id>/instructor/api/gradebook with a ccx id.

We've spoken in the past about trying to eliminate the need for a separate enrollment system for ccx members. I suspect that doing that would solve this problem, as the grades would be stored for the CCXLocator, the students would have enrollment records with the CCXLocator as the course_key and everything would start to line up nicely. Unfortunately, we aren't there quite yet.

[cewing]

Oh, @pdpinch (and @ormsbee), I am also wrong about the different code paths. Both the existing instructor views and our views go through the same logical path. I suspect that the reference to grades.get_scores above may have been inadvertent. That method is also not used by the instructor gradebook view.

[cewing]

@ormsbee, in order for me to make any further progress into this, I'm going to need to find out how the edx-submissions api is tied into the grading process. The module_render.handle_grade_event() method is clearly writing StudentModule objects to the database with grade records. That is working, and is writing those records with the correct CCXLocator ids in place (and with CCXBlockUsageLocator ids for the module_id). However all the code paths that lead to getting a gradebook appear to go through the edx-submissions api, which appears to be relying on ScoreSummary objects to caculcate grades. I have not yet been able to find any code in edx-platform that seems to use this api to set up submissions or summaries. I'm sure it must be somewhere, but I have not yet been able to find it.

Can you help me figure out how a ScoreSummary is intended to be created by the system?

[ormsbee]

@cewing: The grading system uses both the StudentModule and the Submissions API. However, very few things write to the Submissions API at the moment (ORA2 does). A description of the grading process and where the data lives is at:

https://openedx.atlassian.net/wiki/display/TNL/edX+Grading+Guide+for+Developers#edXGradingGuideforDevelopers-TheCurrentApproach

If you haven't seen it before, I would recommend skimming the whole page. It's not that long, and it's the most complete dev picture of edX grading that I know of.

[cewing]

Thanks for the pointer, @ormsbee, it was quite helpful.

@pdpinch the tests are all fixed. There is one more error I am seeing that I will track down and fix, and then we should be ready to open a PR for this.

[cewing]

@pdpinch the final error appears, as far as I can tell, to be a non-issue, though I'd appreciate some feedback on that. I'm opening a new issue for it so we can close this.