Non-core contributed modules

Here is the jist of the problem with GBV2.

First, the problem is only with graded items put in through GBV2.

Second, the graded items (put in through the GBV2 module) go in correctly to the grade_exceptions table correctly.

The problem appears to be when those items are pulled they are not included in the array of exceptions.

I originally thought it was in the grade_get_exceptions_user function. I can followed the code logic to there, but now I think it is in how the items are added in the first place. As it does not have a corresponding module to match up with in the modules table, it is not being pulled with the other graded events.

The GBPV2 graded items add needs to be changed slightly so that it is an actual module that matches up in the other table. It currently simply says it is modid 0 (which isn't in the table) instead of actually adding an activity type in the module table when it is initially created. There may be a simpler workaround than changing how it is added in the table but I think changing the addition to match other modules would bring it more in line with existing moodle code and prevent future errors.

I have already fixed the groups error and will be sumbitting those changes to be implemented in the GBPV2. We still need the module changed that adds the events.

Brian

These three files will fix the errors in GBPV2 for the exceptions error for grade_events and for the groups dropdown not working in the exceptions page.

Brian - I have looked at your proposed changes and am OK with them except for the /grade/exceptions.php file. You have a line change from:

$nonmembers[$grade_item->id][$student->id] = fullname($student, true);

to:

$nonmembers[$grade_item->id][$student->id] = lfname($student, true);

Where is the lfname function defined? For now I am going to leave it as fullname and check these others in and ask for testing. I agree that these should take care of the issue; however, as always it is easy to overlook things. I'll ask Wen to test and confirm and then we can resolve this.

Also I did not see any difference between /grade/exceptions.html version 1.1.8.3 (in CVS) and the file you attached. Is there something I missed or did we already patch it?

Peace - Anthony

Wen - I've checked in Brian's patches except as commented. Could you verify that this corrects the problem. If so, please close this bug report and if it causes other problems or does not fix it just let me know and we can continue working on it. Thanks - Anthony

Brian - Thanks for a great bug report and for the patches. I greatly appreciate it and you helped to speed up the resolution of this 100 fold with the patch files as I could look at your proposed solution, see what you were doing and then copy it in. Peace - Anthony

I am re-opening this issue because I am not satisfied with how groups are being handled. I will attach a screenshot detailing my concerns. I've begun working on this. Brian's solution gets the groups just fine; however, if there is an exception it does not later test that the excepted member is part of the group. Here is what I would like to see happen:

If there are 2 excepted members in group 1 and we select group one the middle column should show 2 as the count of the number of exceptions for that group. Only non-excepted group 1 members should appear on the left. Only the 2 excepted members of group 1 should appear on the right.

If there is 1 excepted member from group 2 likewise it should indicate that and display that one entry in the excepted box to the right.

If we are viewing all participants then we should see 3 excepted members (2 from group 1 and 1 from group 2), with all non-excepted members on the left, and the 3 excepted members on the right.

I think we simply need to do a little more testing on things. We were close but I was seeing blank entries on the right where the excepted student was not in the group and the count was indicating all of the students.

Please note that the screenshot is from a patch I am working on and not the current code; however, the behavior should be reproducible with the current code.

Peace - Anthony

Here is the behavior that I am seeing with my patched file. Peace - Anthony

Here is my suggested patch for testing. Looking at Brian's patch, I wanted to make sure we were getting the current group in a consistent way as defined in lib.php to improve consistency. Also, I felt that Brian's approach of getting the current group for each grade item was inefficient and doing more work than necessary. The group will not change on the current page so I get the list of students in the current group and use that (no need to do the unset($students) AFAIK. Also, I added a check later on to make sure that we do not display excepted students from other groups. I think this behaves the way that I would like it to but would appreciate confirmation and testing. Peace - Anthony

Since I made minor changes to how the lib.php file gets the groups, it is probably best of the gbpv2_exception.php.diff file is used along with the gbpv2_lib.php.diff file in CONTRIB-105. I will link these two issues together to show the connection. Peace - Anthony

Both CONTRIB-105 and CONTRIB-109 deal with how groups are handled in the GBPv2 patch. CONTRIB-105 determines how groups are used to generate the view grades page (index.php) and CONTRIB-109 determines how groups are used to generate the list of assignment/student exceptions.

Brian reported:

I tried the fixes you made but they didn't completely work for me. They worked for the GBPV2 items only, not the other ones. I did get it to work they way you wanted though. Here is my version. Basically it is your change (removing the UNSET on line 104) and changing the display. I commented them.

In response, I checked on my production site that is running the code that I have in this tracker issue and was not able to reproduce any errors. I'm not 100% sure what Brian means by "they worked for GBPv2 items only, not the other ones".

The course I looked at on the production site has GBPv2 graded events and uploaded file assignments. The teacher has exceptions and groups for both the grade events and assignments so I do not see a
problem with the code or the logic. So if there is a mistake in the logic or an error please give me some
information on how I might reproduce those errors.

Note that the biggest change I am trying to introduce is to move the code about getting the group to outside of the foreach loop (around line 99 - the one right after the comment:

// we need to create a multidimensional array keyed by grade_itemid with all_students at each level

I see no reason to create the group list and call the group related functions multiple times. I'm going with the logic of get the group and then process the items. By default, all students of the group are listed as nonmembers - that is they are not excluded. Then later when we go through and process the grade items we add them to list of excluded listmembers and then remove them from the nonmembers list for that grade
item (see the unset on line 127).

Again, thanks for your help with testing and thinking this through. I appreciate the support.

Because this error seems to be impacting folks and that it seems to be working on at least one production site. I am planning on committing this tomorrow unless I hear that it breaks more than it fixes but at this point I feel reasonably confident that it will help. Peace - Anthony

I believe this is now working as anticipated. The fix better handles groups and I think is a little more efficient. Peace - Anthony

Closing all of my resolved issues. Peace - Anthony