Moodle

Gradebook does not show groups

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.8, 1.8.1, 1.8.2
  • Fix Version/s: 1.8.4
  • Component/s: Gradebook, Groups
  • Labels:
    None
  • Database:
    Microsoft SQL
  • Affected Branches:
    MOODLE_18_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE

Description

Same issue as MDL-9081being that groups do not show in the gradebook running with MSsql database but using same code with mysql groups do work.
Any Ideas?

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Warren Gowen added a comment -

I have traced this to /group/lib/legacylib.php at line 115 and removed the DISTINCT keyword it now works. However I am not sure if this is going to cause any other issues.

Show
Warren Gowen added a comment - I have traced this to /group/lib/legacylib.php at line 115 and removed the DISTINCT keyword it now works. However I am not sure if this is going to cause any other issues.
Hide
Permalink
Wen Hao Chuang added a comment -

Hi Anthony this was assigned to Jeff who is no longer maintaining the GBPv2, could you please take a look at this and see if this got fixed already? Thanks!

Show
Wen Hao Chuang added a comment - Hi Anthony this was assigned to Jeff who is no longer maintaining the GBPv2, could you please take a look at this and see if this got fixed already? Thanks!
Hide
Permalink
Anthony Borrow added a comment -

Warren - My apologies for the slow response to this; however, it seems like you were able to get it working. Have you had any troubles? Not being familiar with MSSQL, it seems to me that this is related to the order by clause in the SQL statement. I'm basing this off of what I read at:

http://www.codingforums.com/archive/index.php?t-27947.html

As I looked at the query I'm not sure there is any reason to have the DISTINCT there. AFAIK (actually I justed checked), there is a unique index for groupid, userid so I do not see where the benefit is in making it distinct. The function seems to have been copied from somewhere else and I suspect that it could use one of the main lib functions. In any case, I will check with some of the more experienced developers and see how this might best be resolved. Peace - Anthony

Show
Anthony Borrow added a comment - Warren - My apologies for the slow response to this; however, it seems like you were able to get it working. Have you had any troubles? Not being familiar with MSSQL, it seems to me that this is related to the order by clause in the SQL statement. I'm basing this off of what I read at: http://www.codingforums.com/archive/index.php?t-27947.html As I looked at the query I'm not sure there is any reason to have the DISTINCT there. AFAIK (actually I justed checked), there is a unique index for groupid, userid so I do not see where the benefit is in making it distinct. The function seems to have been copied from somewhere else and I suspect that it could use one of the main lib functions. In any case, I will check with some of the more experienced developers and see how this might best be resolved. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

Nick - I added you as a watcher since you originally wrote this code. I would very much appreciate your opinion/insights on this. Am I overlooking a scenario whereby we are benefiting from having the DISTINCT here? Just to be clear - I'm referring to the SQL query in the function get_group_users in /group/lib/legacylib.php (around line 115). Thanks for any insights you might be able to provide. Peace - Anthony

Show
Anthony Borrow added a comment - Nick - I added you as a watcher since you originally wrote this code. I would very much appreciate your opinion/insights on this. Am I overlooking a scenario whereby we are benefiting from having the DISTINCT here? Just to be clear - I'm referring to the SQL query in the function get_group_users in /group/lib/legacylib.php (around line 115). Thanks for any insights you might be able to provide. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

Yu - I added you as a watcher; however, if you want to take over this issue I would be happy to surrender it to you. Unfortunately, I do not have a MSSQL server to test against but looking back on the code the DISTINCT did not seem to make that much sense. Any thoughts? Peace - Anthony

Show
Anthony Borrow added a comment - Yu - I added you as a watcher; however, if you want to take over this issue I would be happy to surrender it to you. Unfortunately, I do not have a MSSQL server to test against but looking back on the code the DISTINCT did not seem to make that much sense. Any thoughts? Peace - Anthony
Hide
Permalink
Yu Zhang added a comment -

Hi Anthony,

I had a quick look, I think it's ok to deleted the DISTINCT since the first field is always u.id in all calls to this function (either u.id passed in, or u.* as default). get_records_sql() should force the uniqueness of u.id so I think this can be taken out.

Cheers,

Yu

Show
Yu Zhang added a comment - Hi Anthony, I had a quick look, I think it's ok to deleted the DISTINCT since the first field is always u.id in all calls to this function (either u.id passed in, or u.* as default). get_records_sql() should force the uniqueness of u.id so I think this can be taken out. Cheers, Yu
Hide
Permalink
Anthony Borrow added a comment -

Yu - Thanks for taking a look and confirming my suspicion that the DISTINCT is not really doing anything in this particular context. In order to allow for easier MSSQL access I am going to remove it from 18STABLE, 19STABLE, and HEAD. If it produces some sort of unforeseen problem we can put it back in and specifically re-write the query for MSSQL; however, this approach of simply removing the DISTINCT seems to me to be the cleanest. Peace - Anthony

Show
Anthony Borrow added a comment - Yu - Thanks for taking a look and confirming my suspicion that the DISTINCT is not really doing anything in this particular context. In order to allow for easier MSSQL access I am going to remove it from 18STABLE, 19STABLE, and HEAD. If it produces some sort of unforeseen problem we can put it back in and specifically re-write the query for MSSQL; however, this approach of simply removing the DISTINCT seems to me to be the cleanest. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

suggested patch of removing DISTINCT from query

Show
Anthony Borrow added a comment - suggested patch of removing DISTINCT from query
Hide
Permalink
Anthony Borrow added a comment -

OK - I committed the change to 18STABLE; however, the legacylib.php file does not exist in 19STABLE or HEAD. I did check the /lib/grouplib.php file and similar functions were not using the DISTINCT. So I believe this is limited to 18STABLE. I'm not sure how groups were handled in 17STABLE or 16STABLE; however, if a similar issue exists then we can re-open. I'm going to mark this as resolved. Peace - Anthony

Show
Anthony Borrow added a comment - OK - I committed the change to 18STABLE; however, the legacylib.php file does not exist in 19STABLE or HEAD. I did check the /lib/grouplib.php file and similar functions were not using the DISTINCT. So I believe this is limited to 18STABLE. I'm not sure how groups were handled in 17STABLE or 16STABLE; however, if a similar issue exists then we can re-open. I'm going to mark this as resolved. Peace - Anthony

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.