Moodle

Please see linked bug.

This is a very large and important patch, so I am waiting for all issues to be resolved before I submit the code to CVS.

Attached big patch

Just updated the patch with a few bug fixes.

Patch updated again

Updated patch again

security and coding problems:
1/ CSRFs in the new tree/index.php code - we MUST test sesskey before changing anything in db
2/ wordwrap(addslashes_js is not correct, the addslashes_js should be the last step - grader report
3/ text going into overlib should be processed with addslashes_js() too - s($intersection_info) in grader report

I have applied this patch to a local copy of Moodle 1.9 stable for testing.

I received two errors:

\moodle\lang\en_utf8\gradereport_grader.php (Cannot apply hunk @@ 7 )
\moodle\lib\simpletest\testmoodlelib.php (Cannot apply hunk @@ 4 )

Since both seemed to be mostly harmless, I proceed with testing, initially trying to recreate the Moodle Gradebook Tutorial I write for for docs.moodle.org.

I did not see the new tabs in the grader page. There is also no way to get to Categories and items that I can see (the menu item is missing).

I will start testing this and then try it with my school's many courses once this is ready.

I was happy to see that turning off the unlimited grading feature did not change the scores already entered (only the summaries).

--Gary

Gary,

The new patch includes 4 new files, which you seem to be missing. Depending on how you are applying the patch and your file and directory permissions, these files may not be correctly created in your source tree. Please make sure you have the following:

grade/edit/tree/lib.php
grade/edit/tree/functions.js
grade/edit/tree/tree.css
grade/report/styles.css

Note that after chatting with Nicolas, it turns out these items are there. But since the Category and items entry has become a heading and changed its location in the menu structure, it was not where I was expecting it and I missed it.

It is now near the top, where it belongs.

Also, tabs were turned off by default which is not what I expected.

I will add these to my notes as others will probably have similar problems. A fix would probably be to have tabs and the menu on by default.

I will continue testing over the next few days as I have time.

Actually the tabs must be turned off by default, this is what most people will be expecting, since it keeps the previous behaviour. This is documented on http://docs.moodle.org/en/Development:Gradebook_improvements_in_Moodle_1.9.5#Tabs_navigation

I hope other users won't give up looking for "Categories and items" as quickly as you did, though

Should we post our testing notes here too?
We have applied the patch to our test environment with the course data, and everything looks great so far! Most previous calculations seem to be in place.
Several of our faculty tested it as well, and liked it a lot. I am going to post a summary in the forum later.
The only thing that we have got for some reason is the missing indents in the dropdown menu. Here is a screenshot: http://umn.edu/moodle/samples/dropdown.png

indents: Looks like a CSS issue, could you please try purging your browser cache and using standard theme?

I confirm Petr's comments. I had the same issue until I reset my browser's cache (ctrl+F5).

Hi, yes, I completely forgot that our css was not updated there!

I think I found more bugs. Well, some of them are things that need interface improvement. Here are my notes (Please let me know if I should post them somewhere else or create separate tickets).

1. Non-graded Assignment and Quizzes are getting displayed in the gradebook, and they look like being included into aggregation in the Cat&Items interface (Other activities behave definitely. E.g. If Forum is not graded, it does not appear in the gradebook as a column. Which for me is an expected behavior)

• Add an Assignment. Set it to No Grade. Column will still appear in the gradebook. Now go to the Cat&Items interface. It will display Max Grade for that column to be 100 points. (Though it will not be actually included into the calculation)
• Add an Assignment. Set it to be graded on a scale (scales are NOT included in aggregation for us). Maximum Grade in the Cat&Items interface will display some number (e.g. 3, if there are 3 items in a scale). It is confusing, since one would now expect that 3 to be calculated toward course/category total.
• Add a quiz, add some questions, and make the quiz to be worth 0 points. Maximum Grade in the Cat&Items interface will still be 10 points (or 100). Interestingly enough, I can manually delete a gradebook column for this kind of quiz.

I think it MDL-14017 is related

2. Once you add a category and set Aggregation method to Sum, it defaults to be worth 100 points (even though this category can have nothing inside). Those 100 points are included in the calculation of the course total, so students are getting penalized for nothing.
Is it possible to default Max grade to 0 points in such case?

3. Weird "extra credit "check mark, if the Course Total is set to Sum aggregation
When I look on the Cat&Items interface, it looks fine (no extra credits marked anywhere). Then, I click Edit icon near the name of any category, and it has the Extra Credit checkbox marked.

Helena, your items 1 and 2 are valid but not related to this tracker issue. No doubt we will still find small bugs in the gradebook 6 months from now, but we don't need to delay the current patch for them. If you could create a separate tracker issue for each of these, it would be very appreciated.

Item 3 has just been fixed, I'm updating the patch now.

Just looking at the test site, I noticed the immensely useful tooltips for each cell. Nice one!

However, I also noticed from the markup that they are using overlib and that each one is defined individually. I think it would be far better to use YUI panel and attach one event listener to the gradebook table so that all of these panels are defined in one place. Is overlib in the process of being phased out in favour of YUI, or is it being kept in parallell?

Apart from MDL-13433 (sometimes, long tooltips go off the bottom of the page), my major reason for saying this is that it would save effor later in the AJAXifying of the gradebook, when I'm sure we will want to be able to edit cells using YUI dialogues (which are extensions of the panel module) as well as perhaps adding more information to the cells or altering the styles so that the tooltips are sticky or have links etc.

If the data was placed in the cell in standard module format http://developer.yahoo.com/yui/examples/container/module.html
ready to be picked up by the mouseover event handler, then it could be easily extended dynamically for later display, i.e. adding/altering a div after a cell edit, which will be difficult at the moment, using hardcoded function calls.

Thank you, Nicolas.
I have created MDL-18770 and MDL-18772 (I could not choose 1.9.5 as "affected"version though).
Sorry if I will add more non-related stuff here. It is hard to distinguish which bugs should be considered for this release, and which for the next ones.
p.s. I am still struggling with the css stuff in the gradebook (items are non-indented even in the standard theme, plus my editing controls for columns/items are 2 columns off from where they are supposed to be).

Thanks for your idea, Matt. I've followed your advice and replaced overlib with the YUI Tooltip widget. I've improved the look of the tooltip, and fixed the issue of category totals not showing. I've also updated the patch attached to this issue.

Linking to the issue I commented about.

Thanks for the changes Nicolas - it's much better and fixes this issue (almost). I've added huge feedback comments at the start and end of the lowest row of the gradebook in the 1.9 test site gradebook test course. A bit of an issue still with the tooltips vanishing when they are hovered over.

I think the tooltips would be complete with a fix for MDL-13902 as the quiz essay questions are the only place where human inputted feedback will be missing from both the grader report and user report. I have stopped using quizzes for timed essays because of this, as the students don't even realise there are any comments there at all a lot of the time.

I suggest:

<Name>
<Quiz name>

"General feedback"
<General feedback>

"Manually enetered feedback on essay question answers follows (automatic feedback on your other answers available in the quiz itself)"

"Question 1 "
<teacher feedback>

"Question 2"
<teacher feedback>

etc

Hi.

When I apply the last patch (03/Apr/09) and I try to configure the categories, there is a fatal error:

Fatal error: Cannot redeclare class grade_edit_tree_column_category in /var/www/moodle/grade/edit/tree/lib.php on line 1454

Thanks

Students can see hidden grades and categories in the gradebook if the new patch is applied (even if you set viewhidden capability to Prevent as an admin).
Just in case I have tested regular 1.9.4 and it was OK (students could not see hidden grades).

Thanks for the bug report, Elena, I fixed it and updated the patch file.

I wold love to test it, but getting an error at Cat and Items page: "Fatal error: Cannot redeclare class grade_edit_tree_column_category in /var/www/html/test/1.9/grade/edit/tree/lib.php on line 1454"
(May be you are working on it at the moment).

The patch got applied in a funny way, I've fixed it now.

If you login as a real student, User report is getting misaligned.
Studnet view: http://umn.edu/moodle/samples/student1.jpg
Instructor view: http://umn.edu/moodle/samples/instr1.jpg

In the new Category and items view, whether the weight and extra credit columns appear depends on whether certain aggregation types are used someplace in the grading method. This can be confusing, as neither is true in the default case (Simple weighted mean of grades), so it make it difficult to know the options are even available.

I would suggest showing those columns in all cases, even if there is no checkbox of fields for methods where these features are not yet implemented. This would mean that the columns are always consistent, and users would be able to discover which methods have the feature implemented.

Either or both of these columns could be visible in only the "full view" if desired (or keep both in the simple view).

Removing the conditional check at line 62 and 65 of \grade\edit\tree solves this issue.

hmm, I personally think we do not need to show those weight and extra credit columns because not everybody needs/wants them

Petr:

I don't think the criteria for including items on the Category and item page is one's view on whether everyone will want our use them. For example, I am fairly certain that more people will use Extra Credit and Weight than will use multiplicator and offset. In the full view, at least, this page should show any setting that may affect computation.

The more subtle point in my last suggestion is whether a column should appear or be hidden depending on if an aggregation method someplace within a course is able to use the feature. It is a balance between three usability qualities: minimalism, discoverability, and completeness.

Having tested the feature, I am of the view that columns should not be changing around as one tries different settings of aggregation methods. The reason is that, if one does want extra credit or weight (or any other feature), having the column always there means that one knows they are on the right page to determine if a feature is enabled or not. And I think that is more important than the screen space saved by not displaying the column. Since the cells will be empty when not used, I don't think it adds too much to the complexity of the page.

In both cases I think it is fine to put these columns in the full view, which gives a very clean display for the most simple (and the default) gradebook which will be fine and less intimidating for first time teachers.

I disagree with Gary on this one. Something that is not there (weight/extra credit) cannot be confusing. Besides, seeing a column with nothing in it is very likely to confuse people, especially if the column is called "Weight" and the aggregation method is "Simple weighted mean of grades".

I'm happy with the conditional columns as they are.

I agree with Gary.
It is indeed confusing to have those columns to appear/disappear. Empty unused cells (if the aggregation type does not support such calculation method) are much less confusing.

I also agree that more people will use Extra Credit and Weight rather than Multiplicator and Offset (I honestly do not get at the moment how they are supposed to work. Are they supposed to act like extra credits? Since I was actually expecting them to act like calculation mechanisms and increase the individual grade item AND category total).

Please do not speak for the whole world again, some countries may not use extra credit and weights at all

Petr, I was simply wondering how Multiplicator and Offset are supposed to work, plus I was expressing my own opinion on what people would want to use.
And, honestly, I do not understand the whole deal with "you, in canada/us".
Yes, there can be different needs depending on different ways of Moodle use, but they do not depend on what country we are from.

Multiplicator and offset were in the old 1.8.x gradebook because somebody implemented it that way. I hope they will be removed from 2.0 after we complete the transition of grading implementation from activities into gradebook plugins.

All I say is I know many people here that do not use extra credit or weight at all. I understand that it may be needed in some other parts of the world. In this case I think it is a good compromise to show those columns only when appropriate - imagine admin disables all aggregation types with weights or extra credit, why should the column be there at all?

To Elena: Do you know how is grading done in Europe? It sounds like previous gradebook was "perfect" for US/Canada, but for teachers in my country it was totally unusable. During the gradebook development we talked a lot how grading is done in several countries, unfortunately there was nobody from the US/Canada in the development team - all the feedback came much later after the database and UI freeze (very big thanks to Gary Several people from different parts of the world discussed what to do with those weights and extra credits and we all agreed that the autohiding of those columns is a good compromise - indeed it may not be the best solution for everybody (in this case for people that like to use extra credit and weights).

In any case thanks for the all the feedback, it is really welcome. I personally hope we will all learn something new and in the end 1.9.5 or 1.9.6 will be much more suitable/usable for everybody (especially US/Canada)

Thank you, Petr.

And I am from Russia (does it considered to be Europe?) , where we have both concepts of extra credit and weights. Even though they are called and managed differently, they do exist.

sure, it is also in Europe - that makes your votes more important, we did not get much feedback from Russia

Nicolas already fixed the issue when students could see hidden grades/items and categories in the gradebook when "Show hidden items" is set to No. (Though there is a misalignment issue)

I have just noticed some other things.

1. Go to Settings > User Report > Show hidden items > Set to Yes.
Students will now see the hidden grade item AND the grades. I believe they shouldn't see the actual grade, just an item.

2. Go to Settings > User Report > Show hidden items > Set to Hidden until
Hidden grade items will now be hidden completely from students, but items that are set to be Hidden until", will be displayed to a student grayed out with a grade assigned.

3. Even though the item is hidden from the student view, it is still included into the aggregation, and such total is displayed on the User Report for a student.
Regular 1.9.4 behaves differently (User report for a teacher shows total with hidden items being included in aggregation. User report for a student has total that does not include hidden items).

Please let me know if screenshots are needed.
Thank you!

Elena,

Many thanks for the bug reports and useful feedback. I've just fixed items 1 and 2 in your list. Item 3 needs an explanation:

In 1.9.4, we were getting lots of people complaining that the course totals were inconsistent for students and teachers. It is clear that some people prefer to aggregate the hidden items, while others don't. Each approach has merits. Unfortunately we cannot implement both without adding a User Report Setting controlled by teachers/admins for the whole course. This requires a new database field for the user report, so we can't add it in 1.9+ and will have to wait for 2.0.

In the meantime, we are aggregating hidden items for the user report. If you don't want your users to see how hidden items are affecting their course total, you could maybe hide the course total until all hidden items are shown, or put your hidden items in a category of their own, with a weight of 0.

Updated the patch once more

Again getting an error at Categories and Items page: Fatal error: Cannot redeclare class grade_edit_tree_column_category in /var/www/moodle/grade/edit/tree/lib.php on line 1454

I applied the latest patch: 15/Apr/09 09:43 PM

Thanks. J. Martín

If you've already applied the patch on your code, you cannot apply it again, otherwise the new files (such as grade/edit/tree/lib.php) will append to themselves, duplicating most of their code. You must first revert the patch or do a fresh checkout of MOODLE_19_STABLE

Hi again.
First of all, I replaced all the files with the Moodle 1.9.4+ (built weekly) with cp -r * /var/www/moodle

Then I applied the new patch.

This is the code from 1.431 to 1.454 (/var/www/moodle/grade/edit/tree/lib.php):

abstract class grade_edit_tree_column_category extends grade_edit_tree_column {

public $forced;
public $advanced;

public function __construct($name) {
global $CFG;
$this->forced = (int)$CFG->{"grade_$name"."_flag"} & 1;
$this->advanced = (int)$CFG->{"grade_$name"."_flag"} & 2;
}

public function is_hidden($mode='simple') {
global $CFG;
if ($mode == 'simple') { return $this->advanced; } elseif ($mode == 'advanced') {
if ($this->forced && $CFG->grade_hideforcedsettings) { return true; } else { return false; }
}
}
}

Thanks again.

Thank you, Nicolas.
I agree that we want to make the views as close to each other as possible, so it seems like a good temp solution.

Something weird is happening with hidden totals though. I was hiding/unhiding them to check what the student will see, and some totals became "stuck" to be hidden.

1. Go to Cat&Items > Hide Total for a Sub-category (or a course). The whole category will become hidden there, plus from students on the User report. ( I believe that students still should the category with grades). However, if you will check the Grader report, it will still show items in this category as non-hidden=not gray.

2. Now, on Cat&Items unhide the Sub-category back (Not the Total). Total for the category will also become unhidden. Or at least the icon will indicate so.
Go to the Grader report. Notice that Total for that subcategory still looks hidden there.
Click Turn Editing on. You will see that Total itself is indeed unhidden (eye open), but all individual totals below are hidden.
It seems to me that this should not be happening, though I am not sure if there is an easy fix.

I just updated the patch again, because it contained PHP5-only keywords

Jesus,
This file only has 965 lines. In addition to replacing existing files with those from 1.9.4, you must also DELETE manually the 4 files that are NEW in 1.9.5:

• grade/edit/tree/lib.php
• grade/edit/tree/functions.js
• grade/edit/tree/tree.css
• grade/report/styles.css

Elena,

All this is working fine, actually.

A note of explanation: the hide/show icons have additional function than the Hidden setting. They also cascade the Hidden setting down to all children categories and items. For example:

• When you click the hide icon of the course category, Everything becomes hidden in your course
• When you click the hide icon of the course category's total, only the total and its grades become hidden. The category itself remains visible including all its children
• When you change the course category's status to Hidden in the category edit form, no cascading occurs.

Also important: categories do not have a hidden status, only their grade item does. The confusion here is that the eye icon represents a status for grade items and an action for categories. I'm not sure how to resolve that without using two different icons.

I think a good solution would be to change the action of the category total's icon: make it hide all the grades, but not hide the item itself. The problem then becomes: what state do we show for the category total's hidden icon? It could potentially look hidden when in fact it is not. This will be subject to discussion for 2.0, or maybe 1.9.6.

If you want to only hide the category total, but not the category itself:
1. Click the hide icon of the category total
2. Click the show icon of the category

I've committed all these improvements into HEAD

"When you click the hide icon of the course category's total, only the total and its grades become hidden. The category itself remains visible including all its children "

It is not doing that in the current test environment. Please login as a student, and you will see that the whole category is gone from the user report

And, yes, I was indeed expecting that if Cat&Items interface shows Category Total as Unhidden, then the Grader/User reports will reflect that and all total grades will be unhidden too.
Since it is not the case now, it looks like a bug.

I am a little bit lost with the last suggestion

"category total's icon: make it hide all the grades, but not hide the item itself."
Do you mean that we will show words and column "category total", but still hide the grades inside from students? If so, I think the icon should look like a closed eye, and words "Category total" should be grayed out on the User report.
Though, yes, the behavior would be different from regular gradable items. Should we create a separate control: "Show hidden Totals: Only Hidden until"?

We should move this discussion to a different tracker issue. This will NOT get resolved in 1.9.5.

Thanks for your help with the patch, Nicolas.

In the outcomes report, the activity column shows the complete name of the outcome, not the name of the activity.

J. Martin

Hi,
I am sorry to re-post. But since we had a lot of items in this ticket, I was a little worried that my last comment on the bug can be lost. %)
Basically, once you hide category's total, the whole category becomes hidden for a student (not just the total and its grades). As far as I can tell from the note above this is not the intended behavior.
Screenshot (1 part: settings; 2-instructor view of the user report; 3 - student view of the user report with missing Category 1):
http://umn.edu/moodle/samples/hiddenornot.png

Sorry, it if already on the to-do list, or if it is for the next version to fix.

Gradebook patch applied, resolving this issue.

reopening, going to add more fixes over the weekend

Hello! Recently I've updated CVS on my test moodle installation and came upon the new grader report interface - it's nice, but there's a problem: one of our courses has about 60 grade items, some have even more. Currently the page seems to run a JS to create a tooltip for every grade table cell: even if I restrict user per page count to 20, for example, it's still 1200 tooltips in the array. This causes browser to hang up for about a minute to create all tooltips after loading the page. If there's no performance solution for this, I suggest adding a setting to disable generating tooltips in grader report would be reasonable.

hmm, alternative solution could be to attach the tooltips later when page is fully loaded, right?

Yes Petr, I propose to use AJAX to load the tooltip on-the-fly (keeping them cached).

We have the same issue as Vlas. We have some courses with over 800 students and 20+ grade items which kills every browser we've tried it on.

The AJAX solution would probably be the best solution for the problem at hand.

I should add, that some instructors like to view those pages with 100 students per page, or alternatively, all members of a group, which can be large as well. Definitely the AJAX solution is preferred in case someone has 60 items and 800 students and decides to show all of them.

I think this can be fixed by using event bubbling and a single listener on the root element.

Sorry this isn't a proper patch - I've not worked out how to get NetBeans to do one on Windows yet (any tips appreciated)

Replacing index.php and styles.php with these ones switches the tooltips to a single listener on the user-grades table, so there's no looping when the page loads. I've also made it use YUI panel, not YUI tooltip so that very long tooltips can now be read even if they go off the page (MDL-13433, linked). You can also move your mouse over them and they are not fixed width, so long assignment titles are not poking out. This was causing me a problem because tooltips for items with long feedback or with comment inline enabled were disappearing as soon as they appeared and couldn't be read.

I've noticed two remaining issues - with no javascript, the tooltips are messy as they are full of HTML tags, so not very accessible (can't think of an easy fix for that) and also feedback with tags like br ul and li seem to have those stripped out, which is a shame as some of the comments I'm using rely on those heavily and the YUI panel can deal with them fine.

Matt, thank you heaps for this patch. I've tried it and it works wonders. I'm creating a new subtask for these tooltips (MDL-18963) so that we don't pollute this meta-issue with it.

should be done now, please file regressions as separate issues
thanks for testing

btw, gradebook on http://test.moodle.org/1.9 gives the error: "XML Parsing Error: not well-formed"

Thanks Elena, the XML parsing error is reported in MDL-19029.

updating test server

upgrade failed :-D reinstalling ...

I have several sites that are running Moodle on Windows servers (some IIS, some Apache) and they are seeing the gradebook issue where the comment box is full of html code. I don't see it on any Linux based sites. I assume it is fixed for Linux, but what extra needs to be done for Windows based servers? Happy for login access to see this.

HTML code showing in feedback cells only on Windows based sites.

Paul, as this issue has been marked resolved, please could you report the problem of html code in the comment box as a new issue.

Resolved. Closing