Moodle
  1. Moodle
  2. MDL-17807

Make changes in core so that LSU gradebook does not need hacks in core

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Cannot Reproduce
    • Affects Version/s: 1.9.3
    • Fix Version/s: None
    • Component/s: Gradebook
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Rank:
      4918

      Description

      The LSU gradebook (CONTRIB-738) looks very promising. We are trying to merge it into core, removing the need for some of the patching of files in lib/

      Once this is done I will create a patch file including all the changes, and we will put it on test.moodle.org for the community to test.

      See http://moodle.org/mod/forum/discuss.php?d=101449 for description of LSU gradebook.

        Issue Links

          Activity

          Hide
          Kenneth Newquist added a comment -

          Just to clarify, is the goal here to actually incorporate the LSU gradebook changes into core itself, or to create a series of patches that allow it to be more easily used with core?

          Show
          Kenneth Newquist added a comment - Just to clarify, is the goal here to actually incorporate the LSU gradebook changes into core itself, or to create a series of patches that allow it to be more easily used with core?
          Hide
          Nicolas Connault added a comment -

          well this is still subject to testing and review. I think that by tomorrow we'll have a live test install with the LSU gradebook running, so that people can test and give feedback.

          There are some obvious concerns regarding the compatibility of this "patch" with existing code in core. Ideally we would like to take the best stuff from LSU gradebook and merge into core (extra weights, curving, sticky tabs are the main features), but we're not sure if this is feasible for 1.9 yet.

          Show
          Nicolas Connault added a comment - well this is still subject to testing and review. I think that by tomorrow we'll have a live test install with the LSU gradebook running, so that people can test and give feedback. There are some obvious concerns regarding the compatibility of this "patch" with existing code in core. Ideally we would like to take the best stuff from LSU gradebook and merge into core (extra weights, curving, sticky tabs are the main features), but we're not sure if this is feasible for 1.9 yet.
          Hide
          Kenneth Newquist added a comment -

          [bump]

          Any progress on this? Is there a test install available anywhere?

          Show
          Kenneth Newquist added a comment - [bump] Any progress on this? Is there a test install available anywhere?
          Hide
          Nicolas Connault added a comment -

          Hi Kenneth. I've just applied the LSU gradebook patch to a copy of the 1.9 moodle test site (it uses the same database). You can use it by reaching http://test.moodle.org/lsugradebook and login as teacher / testm00dle

          Show
          Nicolas Connault added a comment - Hi Kenneth. I've just applied the LSU gradebook patch to a copy of the 1.9 moodle test site (it uses the same database). You can use it by reaching http://test.moodle.org/lsugradebook and login as teacher / testm00dle
          Hide
          Barry Oosthuizen added a comment -

          Hi Nicolas, I get 'Sorry, but you do not currently have permissions to do that (View the gradebook)' when trying to view Quick Edit Items or Quick edit Categories. Which parts of the LSU gradebook are available on the http://test.moodle.org/lsugradebook test site?

          Show
          Barry Oosthuizen added a comment - Hi Nicolas, I get 'Sorry, but you do not currently have permissions to do that (View the gradebook)' when trying to view Quick Edit Items or Quick edit Categories. Which parts of the LSU gradebook are available on the http://test.moodle.org/lsugradebook test site?
          Hide
          Nicolas Connault added a comment -

          Barry, you need a special "Tester" role to be able to test everything. Please send me an email with your desired username and email address, and I will create an account for you which will work on this test site as well as the 1.9 site (shared database).

          Show
          Nicolas Connault added a comment - Barry, you need a special "Tester" role to be able to test everything. Please send me an email with your desired username and email address, and I will create an account for you which will work on this test site as well as the 1.9 site (shared database).
          Hide
          Barry Oosthuizen added a comment -

          Thanks for the profile Nicolas. I can't see any difference now that I log in with my own profile. What am I missing?

          Show
          Barry Oosthuizen added a comment - Thanks for the profile Nicolas. I can't see any difference now that I log in with my own profile. What am I missing?
          Hide
          Helen Foster added a comment -

          Hi Barry, I've just given you teacher rights to http://test.moodle.org/lsugradebook/course/view.php?id=5

          Thanks for helping with testing.

          Show
          Helen Foster added a comment - Hi Barry, I've just given you teacher rights to http://test.moodle.org/lsugradebook/course/view.php?id=5 Thanks for helping with testing.
          Hide
          Barry Oosthuizen added a comment -

          Hi Helen, thanks for that. I can't see anything new even with my new powers. What should I be looking for? I still get the same error I mentioned before.

          Show
          Barry Oosthuizen added a comment - Hi Helen, thanks for that. I can't see anything new even with my new powers. What should I be looking for? I still get the same error I mentioned before.
          Hide
          Nicolas Connault added a comment - - edited

          I just realised that I needed to bump the version.php file in the simple_grader report. This triggered the proper installation of the LSU gradebook. You should now be able to see the patch in action.

          Show
          Nicolas Connault added a comment - - edited I just realised that I needed to bump the version.php file in the simple_grader report. This triggered the proper installation of the LSU gradebook. You should now be able to see the patch in action.
          Hide
          Barry Oosthuizen added a comment -

          Thanks, everything seems to be working now.

          Show
          Barry Oosthuizen added a comment - Thanks, everything seems to be working now.
          Hide
          Elena Ivanova added a comment -

          it is just me? I do not see the horizontal ruler in LSU gradebook via View > Gradebook (any browser)

          Show
          Elena Ivanova added a comment - it is just me? I do not see the horizontal ruler in LSU gradebook via View > Gradebook (any browser)
          Hide
          Elena Ivanova added a comment -

          hm, disregard my previous comment

          Show
          Elena Ivanova added a comment - hm, disregard my previous comment
          Hide
          Nicolas Connault added a comment -

          On http://test.moodle.org/1.9/grade/report/grader/index.php?id=2 you can see the Fixed column in action. I just merged it into the core gradebook code.

          Show
          Nicolas Connault added a comment - On http://test.moodle.org/1.9/grade/report/grader/index.php?id=2 you can see the Fixed column in action. I just merged it into the core gradebook code.
          Hide
          Barry Oosthuizen added a comment -

          Have you thought of any way to do sticky headers?

          Show
          Barry Oosthuizen added a comment - Have you thought of any way to do sticky headers?
          Hide
          Nicolas Connault added a comment -

          Making sticky headers would be a major usability issue if sticky columns were also enabled. This would put a horizontal scroll bar as well as a vertical one, meaning that the table content could end up at any position between left, right, top and bottom boundaries. I really can't see any advantage to this idea.

          Besides, it's not possible without a heap of JS, and probably a week of work. Not worth it IMO.

          Show
          Nicolas Connault added a comment - Making sticky headers would be a major usability issue if sticky columns were also enabled. This would put a horizontal scroll bar as well as a vertical one, meaning that the table content could end up at any position between left, right, top and bottom boundaries. I really can't see any advantage to this idea. Besides, it's not possible without a heap of JS, and probably a week of work. Not worth it IMO.
          Hide
          Barry Oosthuizen added a comment -

          How about mirroring the headers at the bottom of the page? Otherwise we can't see which column we're looking at when we scroll left and right with lots of students.

          Or is the only solution to tell people to remember to limit the number of students per page? That brings it's own set of challenges like how do you navigate to the students who are on the other pages? You have to click through page numbers to find them.

          We need a workaround and I guess I'll keep on thinking of ideas. Hope it's helping rather than hindering

          Show
          Barry Oosthuizen added a comment - How about mirroring the headers at the bottom of the page? Otherwise we can't see which column we're looking at when we scroll left and right with lots of students. Or is the only solution to tell people to remember to limit the number of students per page? That brings it's own set of challenges like how do you navigate to the students who are on the other pages? You have to click through page numbers to find them. We need a workaround and I guess I'll keep on thinking of ideas. Hope it's helping rather than hindering
          Hide
          Nicolas Connault added a comment -

          Barry,

          Of course you're not hindering! What a silly idea

          Someone in the forum suggested an alphabetical filter like the one used in the admin user search page. This would make it easier to find students.

          And of course we could put the headers at the bottom, but it would look rather awkward I think, if you have 4 levels of categories... Plus you would need to mirror them completely, so that the top-level categories would be output at the bottom. Maybe you could put together a mock (on paper or html) to see what it would look like. I'm not convinced that this is the best solution.

          Show
          Nicolas Connault added a comment - Barry, Of course you're not hindering! What a silly idea Someone in the forum suggested an alphabetical filter like the one used in the admin user search page. This would make it easier to find students. And of course we could put the headers at the bottom, but it would look rather awkward I think, if you have 4 levels of categories... Plus you would need to mirror them completely, so that the top-level categories would be output at the bottom. Maybe you could put together a mock (on paper or html) to see what it would look like. I'm not convinced that this is the best solution.
          Hide
          Robert Puffer added a comment -

          GBv2 had a setting that allowed "repeat headers every x rows" which covered every eventuality.

          Show
          Robert Puffer added a comment - GBv2 had a setting that allowed "repeat headers every x rows" which covered every eventuality.
          Hide
          Barry Oosthuizen added a comment -

          "repeat headers every x rows" would mean less clicking and less having to limit students by page etc so looks like a win-win situation. Was 'x' a set number or was it dependent on other factors?

          Show
          Barry Oosthuizen added a comment - "repeat headers every x rows" would mean less clicking and less having to limit students by page etc so looks like a win-win situation. Was 'x' a set number or was it dependent on other factors?
          Hide
          Robert Puffer added a comment -

          'x' was defaulted to zero (but configurable sitewide by admin) and settable by instructor per course.

          Show
          Robert Puffer added a comment - 'x' was defaulted to zero (but configurable sitewide by admin) and settable by instructor per course.
          Hide
          Elena Ivanova added a comment -

          I am transferring this from the forum, so it will not get lost.

          I have to say that not having the horizontal stroller right inside the browser window is cumbersome.
          It is indeed really nice to have student names "stuck" on the left, but this is true for small gradebooks only. Instructors with complicated gradebooks and a lot of columns will be in trouble. Also, previous view had narrower columns/rows, which helped visually.
          I think we should un-stuck this column, and repeat not headers, but student names every X columns.
          Unless we can make the horizontal stroller to appear in browser.

          Show
          Elena Ivanova added a comment - I am transferring this from the forum, so it will not get lost. I have to say that not having the horizontal stroller right inside the browser window is cumbersome. It is indeed really nice to have student names "stuck" on the left, but this is true for small gradebooks only. Instructors with complicated gradebooks and a lot of columns will be in trouble. Also, previous view had narrower columns/rows, which helped visually. I think we should un-stuck this column, and repeat not headers, but student names every X columns. Unless we can make the horizontal stroller to appear in browser.
          Hide
          Barry Oosthuizen added a comment -

          Another idea: Have two options:

          1. If the number of students per page is set to a small number like 10 then, enable sticky column and add A-Z navigation to find students

          2. If more students per page:
          a) Switch off the Sticky Column
          b) Repeat BOTH rows and headers every x number of rows and columns.

          For option 2 you'll have these benefits:
          1. You'll always have the students names and column headers handy.
          2. You get 2 scroll bars which are always visible on large gradebooks.

          Show
          Barry Oosthuizen added a comment - Another idea: Have two options: 1. If the number of students per page is set to a small number like 10 then, enable sticky column and add A-Z navigation to find students 2. If more students per page: a) Switch off the Sticky Column b) Repeat BOTH rows and headers every x number of rows and columns. For option 2 you'll have these benefits: 1. You'll always have the students names and column headers handy. 2. You get 2 scroll bars which are always visible on large gradebooks.
          Hide
          Elena Ivanova added a comment -

          I agree with Barry, and I think that those options should go into the (my viewing) Preferences or Course (gradebook) Settings.

          Do it similar to 1.8 Preferences:

          • Reprint Headers: select # (note: do not repeat Categories structure every single time; just simple Items and Totals would be enough)
          • Reprint Student names : select #
          • Stick Student names on the left?: Yes/No (note: should be No by default )
          Show
          Elena Ivanova added a comment - I agree with Barry, and I think that those options should go into the (my viewing) Preferences or Course (gradebook) Settings. Do it similar to 1.8 Preferences: Reprint Headers: select # (note: do not repeat Categories structure every single time; just simple Items and Totals would be enough) Reprint Student names : select # Stick Student names on the left?: Yes/No (note: should be No by default )
          Hide
          Barry Oosthuizen added a comment -

          To have only the grade items repeated from the headers might be a problem for people who's grade items have the same name e.g.

          Exam Exam Exam Exam Exam Exam Assignment Assignment Assignment

          The categories would help tell them apart.

          Show
          Barry Oosthuizen added a comment - To have only the grade items repeated from the headers might be a problem for people who's grade items have the same name e.g. Exam Exam Exam Exam Exam Exam Assignment Assignment Assignment The categories would help tell them apart.
          Hide
          Elena Ivanova added a comment -

          I see the point.. so goes saving space...
          I personally think that people should give good descriptive names to the items anyway (but who would listen..)

          Show
          Elena Ivanova added a comment - I see the point.. so goes saving space... I personally think that people should give good descriptive names to the items anyway (but who would listen..)
          Hide
          Elena Ivanova added a comment -

          p.s.

          • Reprint Headers: select #
          • Reprint Headers with Categories: Yes/No
          Show
          Elena Ivanova added a comment - p.s. Reprint Headers: select # Reprint Headers with Categories: Yes/No
          Hide
          Barry Oosthuizen added a comment -

          Elena,

          • Reprint Headers: select #
          • Reprint Headers with Categories: Yes/No
            Nice one, Good idea! Keeps it flexible for everyone.

          Nicolas, I guess, we'd need new database fields for settings options like these in the interface. Does that affect anything?

          Show
          Barry Oosthuizen added a comment - Elena, Reprint Headers: select # Reprint Headers with Categories: Yes/No Nice one, Good idea! Keeps it flexible for everyone. Nicolas, I guess, we'd need new database fields for settings options like these in the interface. Does that affect anything?
          Hide
          Robert Russo added a comment -

          grade/report/simple_grader/lib/grade_sql.php:
          Fixed bug responsible for displaying items instead of categories in the quick edit categories page on test.moodle.org.

          Please update test.moodle.org.

          No other files need to be updated besides the following 2:

          grade/report/simple_grader/quick_edit.php has been updated in CVS
          grade/report/simple_grader/lib/grade_sql.php has been updated in CVS

          Show
          Robert Russo added a comment - grade/report/simple_grader/lib/grade_sql.php: Fixed bug responsible for displaying items instead of categories in the quick edit categories page on test.moodle.org. Please update test.moodle.org. No other files need to be updated besides the following 2: grade/report/simple_grader/quick_edit.php has been updated in CVS grade/report/simple_grader/lib/grade_sql.php has been updated in CVS
          Hide
          Matt Gibson added a comment -

          Love the improvements, but I second Elena's comment about the missing scrollbar - it's a real pain. I suggest a small bit of js using yui's event and animation utilities so that as the mouse is rolled over the left or right of the screen, the gradebook automatically scrolls in that direction. Alternatively, click to drag could work, with a clause added to the listener that it should not operate if one of the editing icons is clicked.

          Show
          Matt Gibson added a comment - Love the improvements, but I second Elena's comment about the missing scrollbar - it's a real pain. I suggest a small bit of js using yui's event and animation utilities so that as the mouse is rolled over the left or right of the screen, the gradebook automatically scrolls in that direction. Alternatively, click to drag could work, with a clause added to the listener that it should not operate if one of the editing icons is clicked.
          Hide
          Matt Gibson added a comment -

          I've also just noticed that the click-to-highlight function ignores the first column and that there are missing CSS bits for cells that are both manually altered and highlighted.

          Show
          Matt Gibson added a comment - I've also just noticed that the click-to-highlight function ignores the first column and that there are missing CSS bits for cells that are both manually altered and highlighted.
          Hide
          Nicolas Connault added a comment -

          This is implemented in HEAD. Thanks for your feedback, please keep it coming.

          Show
          Nicolas Connault added a comment - This is implemented in HEAD. Thanks for your feedback, please keep it coming.
          Hide
          Matt Gibson added a comment -

          Just went to http://test.moodle.org/1.9/grade/report/grader/index.php?id=2

          and got

          Fatal error: Call to a member function is_excluded() on a non-object in /var/www/html/test/1.9/lib/grade/grade_grade.php on line 631

          Show
          Matt Gibson added a comment - Just went to http://test.moodle.org/1.9/grade/report/grader/index.php?id=2 and got Fatal error: Call to a member function is_excluded() on a non-object in /var/www/html/test/1.9/lib/grade/grade_grade.php on line 631
          Hide
          Helen Foster added a comment -

          Thanks everyone for your comments.

          To return to Kenneth's initial comment about whether the goal is to incorporate the LSU gradebook changes into core, or to create a series of patches that allow it to be more easily used with core, as the LSU gradebook includes a number of features, it seems easiest to track these as separate issues.

          Thanks to everyone who has voted for this issue. Please consider voting for individual features of the LSU gradebook which are listed as related issues - MDL-18228 and MDL-18229 so far.

          If there are additional features of the LSU gradebook which you'd like included in core, please create separate issues for them.

          I'm removing 1.9.5 as the fix version for this issue, since to make changes in core so that LSU gradebook does not need hacks in core (the title of this issue) would require database changes which can only happen in Moodle 2.0.

          Show
          Helen Foster added a comment - Thanks everyone for your comments. To return to Kenneth's initial comment about whether the goal is to incorporate the LSU gradebook changes into core, or to create a series of patches that allow it to be more easily used with core, as the LSU gradebook includes a number of features, it seems easiest to track these as separate issues. Thanks to everyone who has voted for this issue. Please consider voting for individual features of the LSU gradebook which are listed as related issues - MDL-18228 and MDL-18229 so far. If there are additional features of the LSU gradebook which you'd like included in core, please create separate issues for them. I'm removing 1.9.5 as the fix version for this issue, since to make changes in core so that LSU gradebook does not need hacks in core (the title of this issue) would require database changes which can only happen in Moodle 2.0.
          Hide
          Helen Foster added a comment -

          Just to clarify Nicolas' comment "This is implemented in HEAD", Nicolas is referring to the horizontal scrollbar - MDL-18228.

          Show
          Helen Foster added a comment - Just to clarify Nicolas' comment "This is implemented in HEAD", Nicolas is referring to the horizontal scrollbar - MDL-18228 .
          Hide
          Helen Foster added a comment -

          Removing 1.9.6 fix version. Apologies for the spam.

          Show
          Helen Foster added a comment - Removing 1.9.6 fix version. Apologies for the spam.
          Hide
          Wen Hao Chuang added a comment -

          Just curious, what's the status of this ticket? By the way I was trying to test it out on http://test.moodle.net/lsugradebook/, but got a "Error: Database connection failed" error message. Could anyone please help fixing it? Thanks!

          Show
          Wen Hao Chuang added a comment - Just curious, what's the status of this ticket? By the way I was trying to test it out on http://test.moodle.net/lsugradebook/ , but got a "Error: Database connection failed" error message. Could anyone please help fixing it? Thanks!
          Hide
          Helen Foster added a comment -

          Closing as cannot reproduce.

          Show
          Helen Foster added a comment - Closing as cannot reproduce.

            People

            • Votes:
              36 Vote for this issue
              Watchers:
              24 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: