Moodle

Get more informative grading link for a teacher

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.9.3
  • Fix Version/s: None
  • Component/s: Assignment, Usability
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE

Description

The current link with the number of submissions isn't very informative. Adding a number of ungraded submissions to it would an improvment.

Patch should be available shortly.

Attachments

  1. Text File
    assignment_submittedlink19.patch
    12/Jan/09 2:01 AM
    4 kB
    Oleg Sychev
  2. Text File
    assignment_submittedlink20.patch
    12/Jan/09 2:01 AM
    4 kB
    Oleg Sychev
  3. Text File
    submittedlink.patch
    12/Dec/08 7:27 AM
    4 kB
    dmitri dubinin

Issue Links

This issue has a non-specific relationship to:
MDL-15356 Add optional to block access of students to own assignment submissions after finalisation (or grading) Minor Open
MDL-14238 Filter out submitted only, or ungraded only items on assignment grading table Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Oleg Sychev added a comment -

Dmitry is my student. I reviewed the patch already.

Show
Oleg Sychev added a comment - Dmitry is my student. I reviewed the patch already.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Linking the 3 related 1.9 assignment improvements

Show
Eloy Lafuente (stronk7) added a comment - Linking the 3 related 1.9 assignment improvements
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Hi Oleg, about the patch... it looks fine, but there is one detail I don't really like and it's the "duality" introduced by the new second parameter in the count_real_submissions() function. I don't like functions returning integers/objects based in parameter.

IMO I'd create a new count_pending_submissions() or something like that instead.

Also... I haven't tested that but... are you sure that this query returns properly the users needing grading:

$count->needgrade = count_records_sql("
SELECT COUNT('x')
FROM {$CFG->prefix}assignment_submissions
WHERE assignment = $cm->instance AND
timemarked < timemodified AND
userid IN ($userlists)");

Is enough to check for timemarked? Shouldn't we check for grade too? Just to be sure...

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Hi Oleg, about the patch... it looks fine, but there is one detail I don't really like and it's the "duality" introduced by the new second parameter in the count_real_submissions() function. I don't like functions returning integers/objects based in parameter. IMO I'd create a new count_pending_submissions() or something like that instead. Also... I haven't tested that but... are you sure that this query returns properly the users needing grading: $count->needgrade = count_records_sql(" SELECT COUNT('x') FROM {$CFG->prefix}assignment_submissions WHERE assignment = $cm->instance AND timemarked < timemodified AND userid IN ($userlists)"); Is enough to check for timemarked? Shouldn't we check for grade too? Just to be sure... Ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

P.S.: Nice to meet you Dimitri

Show
Eloy Lafuente (stronk7) added a comment - P.S.: Nice to meet you Dimitri
Hide
Permalink
Oleg Sychev added a comment -

Hi, Eloy
I don't like it too, but you must consider two things:
1) performance (index.php call submittedlink repeatedly on every assignment in the course)
2) code duplication
The new function must repeat all steps to get $userlists.

Actually, another, maybe better, way is to use referenced argument to return additional count, something like
function assignment_count_real_submissions($cm, $groupid=0,&$countungraded = null)
>>Dmitry - could you check if this'll work? (filling countungraded if non-null only)

On timemarked - you must consider situation with resubmissions, that should be regraded. Only timemarked can tell us that student resubmitted after grading, grade can't help there. And without grade timemarked should be 0 (without submission timemodified will be also). See no way to get more useful information from grade there.

Show
Oleg Sychev added a comment - Hi, Eloy I don't like it too, but you must consider two things: 1) performance (index.php call submittedlink repeatedly on every assignment in the course) 2) code duplication The new function must repeat all steps to get $userlists. Actually, another, maybe better, way is to use referenced argument to return additional count, something like function assignment_count_real_submissions($cm, $groupid=0,&$countungraded = null) >>Dmitry - could you check if this'll work? (filling countungraded if non-null only) On timemarked - you must consider situation with resubmissions, that should be regraded. Only timemarked can tell us that student resubmitted after grading, grade can't help there. And without grade timemarked should be 0 (without submission timemodified will be also). See no way to get more useful information from grade there.
Hide
Permalink
Oleg Sychev added a comment -

Oh, Eloy, it seems that PHP doesn't support default values on referenced arguments. And making an argument not-optional results in interface changing, so that all places using it should be edited. So there are three choices:

a - leave things as they are in the patch, with different returning object
b - get perfomance/code duplication isssues with additional function
c - use a global variable to return additional number

For me the first is the less evil, but I can do any one you want to apply.

The other two patches are staled also

Show
Oleg Sychev added a comment - Oh, Eloy, it seems that PHP doesn't support default values on referenced arguments. And making an argument not-optional results in interface changing, so that all places using it should be edited. So there are three choices: a - leave things as they are in the patch, with different returning object b - get perfomance/code duplication isssues with additional function c - use a global variable to return additional number For me the first is the less evil, but I can do any one you want to apply. The other two patches are staled also
Hide
Permalink
Oleg Sychev added a comment -

Hi, Eloy

It seems that PHP 5 supports default referenced argument. So the best solution I can suggest is to create a two patches: clean one for Moodle 2.0 (which requires php 5) and current one for 1.9.

Show
Oleg Sychev added a comment - Hi, Eloy It seems that PHP 5 supports default referenced argument. So the best solution I can suggest is to create a two patches: clean one for Moodle 2.0 (which requires php 5) and current one for 1.9.
Hide
Permalink
Oleg Sychev added a comment -

Eloy,
I correct the issue with returning value, thought 1.9 implementation will be somewhat slower than before. A separate patch for 2.0 gives an efficient implementation, but it'll work only in php 5; in php 4 it gives a syntax error.

Show
Oleg Sychev added a comment - Eloy, I correct the issue with returning value, thought 1.9 implementation will be somewhat slower than before. A separate patch for 2.0 gives an efficient implementation, but it'll work only in php 5; in php 4 it gives a syntax error.
Hide
Permalink
Oleg Sychev added a comment -

Hmm, Eloy, could we do something with these 3 patches at last? Or at least some of them?

Show
Oleg Sychev added a comment - Hmm, Eloy, could we do something with these 3 patches at last? Or at least some of them?

People

Vote (1)
Watch (5)

Dates

  • Created:
    Updated:
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.