Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-19430

user-determined order and number of courses on myMoodle

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 1.9.4, 1.9.5, 2.2.1
    • Fix Version/s: 2.4
    • Component/s: Dashboard (My home)
    • Environment:
      Vista SP2, Apache 2.2.11, PHP 5.2.8 , MySQL 5.1.30
    • Database:
      MySQL
    • Testing Instructions:
      Hide
      1. Install patch
      2. Log in as user who is enrolled in 10 courses
      3. Go to "My home" page
      4. Click "Customise this page" and limit courses and make sure only those courses are visible
      5. Re-order courses and make sure order is saved
      6. Enable ajax and try drag-n-drop to reorder course on my home page and make sure it works
      7. Change configuration of "course_overview" block and make sure it works.
      Show
      Install patch Log in as user who is enrolled in 10 courses Go to "My home" page Click "Customise this page" and limit courses and make sure only those courses are visible Re-order courses and make sure order is saved Enable ajax and try drag-n-drop to reorder course on my home page and make sure it works Change configuration of "course_overview" block and make sure it works.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      wip-mdl-19430

      Description

      The current myMoodle centre column lists up to 21 courses a user has access to, and if there are more there is no way to change which ones are listed. Courses are listed in the order in which they are retrieved from the database.

      This patch will give users the option to:

      • choose the order in which courses are listed in the centre column of the myMoodle page,
      • set how many courses shall be displayed when loading myMoodle.

        Gliffy Diagrams

        1. hook-for-a-local-my-moodle-195.patch
          0.8 kB
          Mark Pearson
        2. mymoodle-styles-layout-css.patch
          1 kB
          Mark Pearson
        1. myMoodle (editing mode).jpg
          133 kB
        2. myMoodle (normal mode).jpg
          110 kB

          Issue Links

            Activity

            minhtam Minh-Tam Nguyen created issue -
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            parts of this sort-of go in the same direction as shown in the myMoodle-myCourses mockup in MDL-19124

            Show
            minhtam Minh-Tam Nguyen added a comment - parts of this sort-of go in the same direction as shown in the myMoodle-myCourses mockup in MDL-19124
            minhtam Minh-Tam Nguyen made changes -
            Field Original Value New Value
            Link This issue has a non-specific relationship to MDL-19124 [ MDL-19124 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            only a certain number of courses is displayed in normal mode

            Show
            minhtam Minh-Tam Nguyen added a comment - only a certain number of courses is displayed in normal mode
            minhtam Minh-Tam Nguyen made changes -
            Attachment myMoodle (normal mode).jpg [ 17589 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            all courses are displayed in editing mode. THe ser can move courses up and down and choose how many should be displayed when editing is turned off

            Show
            minhtam Minh-Tam Nguyen added a comment - all courses are displayed in editing mode. THe ser can move courses up and down and choose how many should be displayed when editing is turned off
            minhtam Minh-Tam Nguyen made changes -
            Attachment myMoodle (editing mode).jpg [ 17590 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            Attached file contains the local hack code so far. Effects demonstrated in screen shots.
            Mind you, it's still not done. The code is quite ugly and will need some cleaning up, and more importantly, some comments.

            The code in the zip needs to go to /moodle/local/. in addition to this, my code relies on
            http://tracker.moodle.org/browse/MDL-17446 ("LOCAL: my moodle centre column override") which Penny Leach wrote in December.

            Another thing that's not quite ready for prime time yet, is that I haven't created a database xml file in /moodle/local/db/ yet. It will come, though.
            As it is at the moment, the attached code requires the manual (by admin) creation of two new additional profile fields:
            "myNumCourses"
            "myorder"
            both are straight text fields (not text input), and it's clearly visible that I need to fix the names to be firstly consistent, and secondly in agreement with the coding guidelines.

            If you risk installing this code as it is right now, users will get two and a half new features on myMoodle:
            1: users have the option to change the order of courses on myMoodle by clicking up and down arrows while in editing mode.
            2: user can choose how many courses are displayed on myMoodle by changing the number in the drop-down box while in editing mode.
            2 1/2: course details such as due assignments are initially collapsed when the myMoodle page is loaded. they can be unfolded by clicking an icon.

            Show
            minhtam Minh-Tam Nguyen added a comment - Attached file contains the local hack code so far. Effects demonstrated in screen shots. Mind you, it's still not done. The code is quite ugly and will need some cleaning up, and more importantly, some comments. The code in the zip needs to go to /moodle/local/. in addition to this, my code relies on http://tracker.moodle.org/browse/MDL-17446 ("LOCAL: my moodle centre column override") which Penny Leach wrote in December. Another thing that's not quite ready for prime time yet, is that I haven't created a database xml file in /moodle/local/db/ yet. It will come, though. As it is at the moment, the attached code requires the manual (by admin) creation of two new additional profile fields: "myNumCourses" "myorder" both are straight text fields (not text input), and it's clearly visible that I need to fix the names to be firstly consistent, and secondly in agreement with the coding guidelines. If you risk installing this code as it is right now, users will get two and a half new features on myMoodle: 1: users have the option to change the order of courses on myMoodle by clicking up and down arrows while in editing mode. 2: user can choose how many courses are displayed on myMoodle by changing the number in the drop-down box while in editing mode. 2 1/2: course details such as due assignments are initially collapsed when the myMoodle page is loaded. they can be unfolded by clicking an icon.
            minhtam Minh-Tam Nguyen made changes -
            Attachment local.zip [ 17593 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            as this is a local hack, it requires MDL-17446

            Show
            minhtam Minh-Tam Nguyen added a comment - as this is a local hack, it requires MDL-17446
            minhtam Minh-Tam Nguyen made changes -
            Link This issue has a non-specific relationship to MDL-17446 [ MDL-17446 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            the patch at the moment also needs an addition to styles_layout.css in your favourite (or standard) theme. THis will need to be moved to local as well, I suppose.

            Show
            minhtam Minh-Tam Nguyen added a comment - the patch at the moment also needs an addition to styles_layout.css in your favourite (or standard) theme. THis will need to be moved to local as well, I suppose.
            minhtam Minh-Tam Nguyen made changes -
            Attachment addition for styles_layout.css [ 17594 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            discussion about this is here: http://moodle.org/mod/forum/discuss.php?d=122236

            Show
            minhtam Minh-Tam Nguyen added a comment - discussion about this is here: http://moodle.org/mod/forum/discuss.php?d=122236
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            I have uploaded a new package of files. The code has some better comments now and the style looks nicer.
            This still requires manual creation of profile fields and an edit to the standard style sheet, unfortunately.

            Show
            minhtam Minh-Tam Nguyen added a comment - I have uploaded a new package of files. The code has some better comments now and the style looks nicer. This still requires manual creation of profile fields and an edit to the standard style sheet, unfortunately.
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090618).zip [ 17712 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment local.zip [ 17593 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment addition for styles_layout.css [ 17594 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            I reformulated the install.txt to be a little bit clearer and changed a few words and links.

            Show
            minhtam Minh-Tam Nguyen added a comment - I reformulated the install.txt to be a little bit clearer and changed a few words and links.
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090721).zip [ 17956 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090618).zip [ 17712 ]
            minhtam Minh-Tam Nguyen made changes -
            Affects Version/s 1.9.5 [ 10320 ]
            Hide
            markpearson Mark Pearson added a comment -

            Minh-Tam,
            I got rather confused attempting to apply this patch to a 1.9.5 installation. The 'Affected Branches:" field above indicates MOODLE_19_STABLE so I was surprised to find reference in the patch file to a root level directory called 'local' which is not in the current 1.9.5 distribution. Moreover, Penny Leach's patch at MDL-17446 implies MOODLE_20_STABLE and I'm assuming that this directory is present in 2.0. However, I am not going to attempt to bring up version 2.0 as a production server at this point in development, so I'm wondering how to proceed. Do you have this 'backported' (I think that's the phrase) to 1.9.5? Do you have a moodle/index.php and moodle/local/lib.php file which is 1.9.5 compatible to use with your patch? Thanks for any help.
            Mark

            Show
            markpearson Mark Pearson added a comment - Minh-Tam, I got rather confused attempting to apply this patch to a 1.9.5 installation. The 'Affected Branches:" field above indicates MOODLE_19_STABLE so I was surprised to find reference in the patch file to a root level directory called 'local' which is not in the current 1.9.5 distribution. Moreover, Penny Leach's patch at MDL-17446 implies MOODLE_20_STABLE and I'm assuming that this directory is present in 2.0. However, I am not going to attempt to bring up version 2.0 as a production server at this point in development, so I'm wondering how to proceed. Do you have this 'backported' (I think that's the phrase) to 1.9.5? Do you have a moodle/index.php and moodle/local/lib.php file which is 1.9.5 compatible to use with your patch? Thanks for any help. Mark
            Hide
            markpearson Mark Pearson added a comment -

            This is the patch file referenced below from Penny Leach which implements custom handler code for display by MyMoodle. It's fixed for 1.9.5

            Show
            markpearson Mark Pearson added a comment - This is the patch file referenced below from Penny Leach which implements custom handler code for display by MyMoodle. It's fixed for 1.9.5
            markpearson Mark Pearson made changes -
            Hide
            markpearson Mark Pearson added a comment -

            So, yes, I did get rather confused but I think I've sussed it out:
            1. If you're on Moodle v1.9.5 (like what I am) use the patch file above like this:
            a. get the file into your moodle system and move it into the moodle/my directory
            b. apply with $ patch <hook-for-a-local-my-moodle-195.patch
            2. Now download and unpack the above ZIP file and follow instructions in the install.txt. This will provide the handler referred to by the patch file above (which is what got me rather confused)
            Hope this helps.

            Show
            markpearson Mark Pearson added a comment - So, yes, I did get rather confused but I think I've sussed it out: 1. If you're on Moodle v1.9.5 (like what I am) use the patch file above like this: a. get the file into your moodle system and move it into the moodle/my directory b. apply with $ patch <hook-for-a-local-my-moodle-195.patch 2. Now download and unpack the above ZIP file and follow instructions in the install.txt. This will provide the handler referred to by the patch file above (which is what got me rather confused) Hope this helps.
            Hide
            markpearson Mark Pearson added a comment -

            To make life easier I've created a patch for the styles_layout.css. To apply:
            Download to your moodle site with ftp or scp
            $ cd moodle/theme/standard
            $ mv ../../mymoodle-styles-layout-css.patch .
            $ patch <mymoodle-styles-layout-css.patch
            That should do it! Nice system Minh-Tam! I like it.

            Show
            markpearson Mark Pearson added a comment - To make life easier I've created a patch for the styles_layout.css. To apply: Download to your moodle site with ftp or scp $ cd moodle/theme/standard $ mv ../../mymoodle-styles-layout-css.patch . $ patch <mymoodle-styles-layout-css.patch That should do it! Nice system Minh-Tam! I like it.
            markpearson Mark Pearson made changes -
            Attachment mymoodle-styles-layout-css.patch [ 18101 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090721).zip [ 17956 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            I uploaded a new package of files: MDL-19430 (20090818).zip

            I have made a number of changes to improve security, maintainability and performance, as suggested by Ashley Holman.

            I have also made changes to the install.txt to hopefully avoid the confusion Mark went through.

            This package is straight from of my installation of 1.9.5, and Penny's patch also works fine on 1.9.5, once you create the 'local' folder.

            Show
            minhtam Minh-Tam Nguyen added a comment - I uploaded a new package of files: MDL-19430 (20090818).zip I have made a number of changes to improve security, maintainability and performance, as suggested by Ashley Holman. I have also made changes to the install.txt to hopefully avoid the confusion Mark went through. This package is straight from of my installation of 1.9.5, and Penny's patch also works fine on 1.9.5, once you create the 'local' folder.
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090818).zip [ 18190 ]
            Hide
            markpearson Mark Pearson added a comment -

            I downloaded the MDL-19430 (20090818).zip and substituted it into a working 1.9.5 system where the previous version had worked fine. I found two errors:
            1) The pulldown menu for number of courses always has "10" at the bottom (not there before)
            2) Switch on debug in administration and get the following notices:

            Notice: Undefined variable: site in /usr/home/markp/public_html/moo-195/local/lib.php on line 169
            Notice: Trying to get property of non-object in /usr/home/markp/public_html/moo-195/local/lib.php on line 169

            I thought you should know.

            Show
            markpearson Mark Pearson added a comment - I downloaded the MDL-19430 (20090818).zip and substituted it into a working 1.9.5 system where the previous version had worked fine. I found two errors: 1) The pulldown menu for number of courses always has "10" at the bottom (not there before) 2) Switch on debug in administration and get the following notices: Notice: Undefined variable: site in /usr/home/markp/public_html/moo-195/local/lib.php on line 169 Notice: Trying to get property of non-object in /usr/home/markp/public_html/moo-195/local/lib.php on line 169 I thought you should know.
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            Thank you Mark for those two points.
            The extra 10 (if the user has access to less than 10 courses) was mainly to allow people to set a higher number than they currently have, in anticipation of soon getting more.
            However your comment made me realise that this wasn't the right solution. Instead, I have now changed it to be "Always show all", 1, 2, 3, etc. up to the number of courses. THis shoudl allow users to set it to 'Always all' and know that new courses will show.
            I have also changed the code to avoid that error message. I guess I was a little too keen in removing unused variables.
            CHeers,
            Minh-Tam

            Show
            minhtam Minh-Tam Nguyen added a comment - Thank you Mark for those two points. The extra 10 (if the user has access to less than 10 courses) was mainly to allow people to set a higher number than they currently have, in anticipation of soon getting more. However your comment made me realise that this wasn't the right solution. Instead, I have now changed it to be "Always show all", 1, 2, 3, etc. up to the number of courses. THis shoudl allow users to set it to 'Always all' and know that new courses will show. I have also changed the code to avoid that error message. I guess I was a little too keen in removing unused variables. CHeers, Minh-Tam
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090818-2).zip [ 18194 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090818).zip [ 18190 ]
            Hide
            markpearson Mark Pearson added a comment -

            Yep. That fixed it! This is a brilliant modification to the MyMoodle interface and changes the whole way that we use Moodle. No longer do I have to 'hide' courses from teachers – they can organise the display any way they want. I hope that this feature gets included in any MyMoodle improvements (certainly for 2.0).
            Cheers!
            Mark

            Show
            markpearson Mark Pearson added a comment - Yep. That fixed it! This is a brilliant modification to the MyMoodle interface and changes the whole way that we use Moodle. No longer do I have to 'hide' courses from teachers – they can organise the display any way they want. I hope that this feature gets included in any MyMoodle improvements (certainly for 2.0). Cheers! Mark
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            I have uploaded a new version of the files, with changes addressing most of the points raised in the discussion forum: http://moodle.org/mod/forum/discuss.php?d=122236
            When in edit mode, there now is a greater delineation to show which courses will be shown and which ones won't, and the language files have been changed a little. The logic relating to the display of up and down arrows was changed a little, too.
            Cheers,
            Minh-Tam

            Show
            minhtam Minh-Tam Nguyen added a comment - I have uploaded a new version of the files, with changes addressing most of the points raised in the discussion forum: http://moodle.org/mod/forum/discuss.php?d=122236 When in edit mode, there now is a greater delineation to show which courses will be shown and which ones won't, and the language files have been changed a little. The logic relating to the display of up and down arrows was changed a little, too. Cheers, Minh-Tam
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090824).zip [ 18246 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090818-2).zip [ 18194 ]
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            This is the version we have now running on production.
            I also made a very quick video of how it works here: http://www.minhtam.info/a/screens/MDL-19430_%2820090904%29.swf

            Show
            minhtam Minh-Tam Nguyen added a comment - This is the version we have now running on production. I also made a very quick video of how it works here: http://www.minhtam.info/a/screens/MDL-19430_%2820090904%29.swf
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090904).zip [ 18329 ]
            minhtam Minh-Tam Nguyen made changes -
            Attachment MDL-19430 (20090824).zip [ 18246 ]
            minhtam Minh-Tam Nguyen made changes -
            Issue Type New Feature [ 2 ] Improvement [ 4 ]
            dougiamas Martin Dougiamas made changes -
            Link This issue has been marked as being related by MDL-19124 [ MDL-19124 ]
            danielneis Daniel Neis made changes -
            Link This issue has a clone MDL-20141 [ MDL-20141 ]
            Hide
            tlock Tim Lock added a comment -

            I have found that if a student blocks a contact, the unread messages are still counted in the MyMoodle page when it should only counting non-blocked contact messages with something like this :-

            select m.id,useridfrom,useridto,blocked from mdl_message m left join
            mdl_message_contacts mc on m.useridfrom=mc.contactid where m.useridto=XXXX
            and mc.userid=XXXX and mc.blocked=0;

            Current query is only checking for unread messages by :-

            if ($countmessages = count_records('message', 'useridto', $USER->id)) {

            Show
            tlock Tim Lock added a comment - I have found that if a student blocks a contact, the unread messages are still counted in the MyMoodle page when it should only counting non-blocked contact messages with something like this :- select m.id,useridfrom,useridto,blocked from mdl_message m left join mdl_message_contacts mc on m.useridfrom=mc.contactid where m.useridto=XXXX and mc.userid=XXXX and mc.blocked=0; Current query is only checking for unread messages by :- if ($countmessages = count_records('message', 'useridto', $USER->id)) {
            bluenovember Clinton Graham made changes -
            Link This issue will be resolved by MDL-11613 [ MDL-11613 ]
            dougiamas Martin Dougiamas made changes -
            Workflow jira [ 32272 ] MDL Workflow [ 45012 ]
            dougiamas Martin Dougiamas made changes -
            Workflow MDL Workflow [ 45012 ] MDL Full Workflow [ 73370 ]
            Hide
            salvetore Michael de Raadt added a comment -

            This appears to still be a problem and people have duplicated this issue.

            I'm bumping the priority for this issue and linking a few issues to it.

            Show
            salvetore Michael de Raadt added a comment - This appears to still be a problem and people have duplicated this issue. I'm bumping the priority for this issue and linking a few issues to it.
            salvetore Michael de Raadt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Priority Minor [ 4 ] Critical [ 2 ]
            Labels partner triaged
            Affects Version/s 2.2.1 [ 11456 ]
            salvetore Michael de Raadt made changes -
            Link This issue is duplicated by MDL-23957 [ MDL-23957 ]
            salvetore Michael de Raadt made changes -
            Link This issue will help resolve MDL-31670 [ MDL-31670 ]
            Hide
            nmagill Neill Magill added a comment -

            The code above does not seem to be quickly useful for 2.x.

            We have been having issues with some of the limits of the course overview block as well, so I did some development that allows the block to be more useful.

            https://github.com/NeillM/moodle/compare/master...course_overview_fix

            The changes allow the user to set how many courses they with to displayy at one time, change the order that courses are displayed in, based on one of several fields.

            If the user is enrolled on more courses than the display limit it will let them page back and forth through their courses.

            Show
            nmagill Neill Magill added a comment - The code above does not seem to be quickly useful for 2.x. We have been having issues with some of the limits of the course overview block as well, so I did some development that allows the block to be more useful. https://github.com/NeillM/moodle/compare/master...course_overview_fix The changes allow the user to set how many courses they with to displayy at one time, change the order that courses are displayed in, based on one of several fields. If the user is enrolled on more courses than the display limit it will let them page back and forth through their courses.
            Hide
            minhtam Minh-Tam Nguyen added a comment -

            Moodle 2 version of this patch by Netspot for University of Canberra and Flinders University:
            https://github.com/netspotau/moodle-block_my_course_overview

            Show
            minhtam Minh-Tam Nguyen added a comment - Moodle 2 version of this patch by Netspot for University of Canberra and Flinders University: https://github.com/netspotau/moodle-block_my_course_overview
            rajeshtaneja Rajesh Taneja made changes -
            Assignee moodle.com [ moodle.com ] Rajesh Taneja [ rajeshtaneja ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Adding this to next sprint

            Show
            rajeshtaneja Rajesh Taneja added a comment - Adding this to next sprint
            rajeshtaneja Rajesh Taneja made changes -
            Fix Version/s STABLE Sprint 20 [ 12152 ]
            Fix Version/s STABLE backlog [ 10463 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for the update Minh

            Hi Rajesh,
            it's great to see that this has been added to the next sprint. Thanks.
            When you're working on it, please be aware that the Moodle 2.x version of this moduls is on github at https://github.com/netspotau/moodle-block_my_course_overview
            I have observed one performance issue in both this myMoodle block but also in the core myMoodle, relating to users with more than 20 courses showing, due to Moodle querying so any courses.
            Cheers,
            Minh-Tam

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for the update Minh Hi Rajesh, it's great to see that this has been added to the next sprint. Thanks. When you're working on it, please be aware that the Moodle 2.x version of this moduls is on github at https://github.com/netspotau/moodle-block_my_course_overview I have observed one performance issue in both this myMoodle block but also in the core myMoodle, relating to users with more than 20 courses showing, due to Moodle querying so any courses. Cheers, Minh-Tam
            Hide
            markpearson Mark Pearson added a comment -

            It's great to see this being fixed in 2.x. Thanks.
            Mark

            Show
            markpearson Mark Pearson added a comment - It's great to see this being fixed in 2.x. Thanks. Mark
            Hide
            aolley Adam Olley added a comment -

            Hi All,
            https://github.com/netspotau/moodle-block_my_course_overview has been updated with some fixes:

            • Add note about hidden courses when editing
            • Fix icon positioning in some browsers
            • Switch icon to match correct moving style (move_2d for ajax moving)
            Show
            aolley Adam Olley added a comment - Hi All, https://github.com/netspotau/moodle-block_my_course_overview has been updated with some fixes: Add note about hidden courses when editing Fix icon positioning in some browsers Switch icon to match correct moving style (move_2d for ajax moving)
            aolley Adam Olley made changes -
            Labels partner triaged netspot partner triaged
            Hide
            drex Mark Drechsler added a comment -

            Woohoo!

            Great news Rajesh

            Mark.

            Show
            drex Mark Drechsler added a comment - Woohoo! Great news Rajesh Mark.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Adam and Mark

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Adam and Mark
            rajeshtaneja Rajesh Taneja made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Looking at this in 2.x functionality has changed a bit.
            Not sure if this improvement will raise bug. In 2.x $CFG->mycoursesperpage has been added which allow admin to set max courses for user. Also, Admin can change the order in which user should view the courses (Settings -> Site administration -> courses -> Add/edit courses)

            I think we need more discussion, before we plan to implement this. As this will affect category sorting as well.
            Currently admin can set the max visible courses on my moodle page, and set the order in which courses are displayed to user (sorted by category and then courses within category). Sorting is set from admin section.

            FYI:
            Unfortunately we are only supporting 1.9 for security patches, so I am not considering 1.9 implementation.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Looking at this in 2.x functionality has changed a bit. Not sure if this improvement will raise bug. In 2.x $CFG->mycoursesperpage has been added which allow admin to set max courses for user. Also, Admin can change the order in which user should view the courses (Settings -> Site administration -> courses -> Add/edit courses) I think we need more discussion, before we plan to implement this. As this will affect category sorting as well. Currently admin can set the max visible courses on my moodle page, and set the order in which courses are displayed to user (sorted by category and then courses within category). Sorting is set from admin section. FYI: Unfortunately we are only supporting 1.9 for security patches, so I am not considering 1.9 implementation.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Sorry, didn't get much time to look at this. Moving it to next sprint.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Sorry, didn't get much time to look at this. Moving it to next sprint.
            rajeshtaneja Rajesh Taneja made changes -
            Fix Version/s STABLE Sprint 21 [ 12155 ]
            Fix Version/s STABLE Sprint 20 [ 12152 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Estimate effort for working on this: 14hr.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Estimate effort for working on this: 14hr.
            rajeshtaneja Rajesh Taneja made changes -
            Pull Master Diff URL https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-19430
            Pull Master Branch wip-mdl-19430
            Pull from Repository git://github.com/rajeshtaneja/moodle.git
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Adam for the awesome code.
            I have done few changes and will push it for review now.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Adam for the awesome code. I have done few changes and will push it for review now.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            rajeshtaneja Rajesh Taneja made changes -
            Testing Instructions # Install patch
            # Log in as user who is enrolled in 10 courses
            # Go to "My home" page
            # Click "Customise this page" and limit courses and make sure only those courses are visible
            # Re-order courses and make sure order is saved
            # Enable ajax and try drag-n-drop to reorder course on my home page and make sure it works
            # Change configuration of "course_overview" block and make sure it works.
            dmonllao David Monllaó made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Peer reviewer davmon
            Hide
            dmonllao David Monllaó added a comment -

            Hi,

            The first commit gets rid of the remote courses https://github.com/rajeshtaneja/moodle/commit/a7da3061144e6c67f67f196a17b8ca45c0e3732b#L1L71 so it seems to be a loss of funcionality; this will also affect the JS methods, which are based only in local courses ids without taking into account the mnethostid

            Show
            dmonllao David Monllaó added a comment - Hi, The first commit gets rid of the remote courses https://github.com/rajeshtaneja/moodle/commit/a7da3061144e6c67f67f196a17b8ca45c0e3732b#L1L71 so it seems to be a loss of funcionality; this will also affect the JS methods, which are based only in local courses ids without taking into account the mnethostid
            dmonllao David Monllaó made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for pointing that David,

            I have updated the patch to show remote courses.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for pointing that David, I have updated the patch to show remote courses.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            dmonllao David Monllaó made changes -
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Hide
            dmonllao David Monllaó added a comment -

            All ok Rajesh, the only thing I see according to http://docs.moodle.org/dev/Peer_reviewing_checklist is that there is no sesskey checking in the move.php action

            Thanks

            Show
            dmonllao David Monllaó added a comment - All ok Rajesh, the only thing I see according to http://docs.moodle.org/dev/Peer_reviewing_checklist is that there is no sesskey checking in the move.php action Thanks
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks David,

            Session check added in move.php

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks David, Session check added in move.php
            Hide
            dmonllao David Monllaó added a comment -

            Thanks Raj, all looks good for me

            Show
            dmonllao David Monllaó added a comment - Thanks Raj, all looks good for me
            dmonllao David Monllaó made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi guys,

            I've been having a good look at this and have made the following notes.
            There are some cool things happening here but this still needs more polishing sorry.

            1. README.php - looks like its a copy of the docs page for this block? Would be great to simplfy that to what is needed or convert it to a docs page and remove it entirely. This would be the only block with a readme so perhaps best to remove it.
            2. @package should be block_course_overview.
            3. A few vars with underscores in their names. Just general coding style guff.
            4. block_course_overview.php
              1. + why set instance_can_be_hidden to true?
              2. + remove closing ?>
            5. locallib.php
              1. block_course_overview_get_overviews unused global vars
              2. + Should probably be using get_plugin_list_with_function rather than manually finding modules with callbacks.
              3. block_course_overview_get_child_shortnames unused globals
              4. + that is function is manually constructing a string (is complex but needs to be translatable).
              5. + looks like it could be using get_records_sql_menu rather than iterating to build $shortnames.
              6. + those course shortnames need to be run through format_string.
            6. renderer.php
              1. inclusion of message/lib.php should be moved to welcome_area method (looks like that is the only use) or much better yet included when needed in block_course_overview.php and the message count passed in as an argument (renderers should contain little to no logic).
              2. Please don't use globals within a renderer. $OUTPUT is available through $this->output and $PAGE is available through $this->page.
              3. course_overview moving the following calls out of the foreach will be good for performance. $this->pix_url, ajaxenabled and perhaps get_string if you can be bothered (not nearly as important as the other two).
              4. + there is a <br> that should be a <br /> (I'm just guessing xhtml)
              5. + course->fullname needs to be run through format_string (filters etc) please ensure you provide the context as an arg (required for new format calls)
              6. collapsible_region_start unused globals
              7. activity_display activity_display.$module strings need to be coming from the modules and not from the block_course_overview lang file. I also think they should be passed in rather than fetched within the render as renderers should contain minimal logic.
            7. save.php
              1. use require_sesskey instead of confirm_sesskey.
              2. MUST require_login there
            8. move.php
              1. use require_sesskey instead of confirm_sesskey.
              2. MUST require_login there
              3. The two debugging calls look like they should be exceptions. Assuming you don't hit that path through normal operation.
            9. settings.php
              1. Rather than iterating over $configs to set the plugin change the name of each setting to: block_course_overview/settingname. That way you don't need to manually set the plugin.
            10. styles.css
              1. Use of font-size property is undesireable. Especially h2.title which uses !important. We need to test this with all core themes if we are going to set that.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi guys, I've been having a good look at this and have made the following notes. There are some cool things happening here but this still needs more polishing sorry. README.php - looks like its a copy of the docs page for this block? Would be great to simplfy that to what is needed or convert it to a docs page and remove it entirely. This would be the only block with a readme so perhaps best to remove it. @package should be block_course_overview. A few vars with underscores in their names. Just general coding style guff. block_course_overview.php + why set instance_can_be_hidden to true? + remove closing ?> locallib.php block_course_overview_get_overviews unused global vars + Should probably be using get_plugin_list_with_function rather than manually finding modules with callbacks. block_course_overview_get_child_shortnames unused globals + that is function is manually constructing a string (is complex but needs to be translatable). + looks like it could be using get_records_sql_menu rather than iterating to build $shortnames. + those course shortnames need to be run through format_string. renderer.php inclusion of message/lib.php should be moved to welcome_area method (looks like that is the only use) or much better yet included when needed in block_course_overview.php and the message count passed in as an argument (renderers should contain little to no logic). Please don't use globals within a renderer. $OUTPUT is available through $this->output and $PAGE is available through $this->page. course_overview moving the following calls out of the foreach will be good for performance. $this->pix_url, ajaxenabled and perhaps get_string if you can be bothered (not nearly as important as the other two). + there is a <br> that should be a <br /> (I'm just guessing xhtml) + course->fullname needs to be run through format_string (filters etc) please ensure you provide the context as an arg (required for new format calls) collapsible_region_start unused globals activity_display activity_display.$module strings need to be coming from the modules and not from the block_course_overview lang file. I also think they should be passed in rather than fetched within the render as renderers should contain minimal logic. save.php use require_sesskey instead of confirm_sesskey. MUST require_login there move.php use require_sesskey instead of confirm_sesskey. MUST require_login there The two debugging calls look like they should be exceptions. Assuming you don't hit that path through normal operation. settings.php Rather than iterating over $configs to set the plugin change the name of each setting to: block_course_overview/settingname. That way you don't need to manually set the plugin. styles.css Use of font-size property is undesireable. Especially h2.title which uses !important. We need to test this with all core themes if we are going to set that. Cheers Sam
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Reopened [ 4 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Sam,

            I will update the code.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Sam, I will update the code.
            rajeshtaneja Rajesh Taneja made changes -
            Status Reopened [ 4 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Carrying it over to next sprint.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Carrying it over to next sprint.
            rajeshtaneja Rajesh Taneja made changes -
            Fix Version/s STABLE Sprint 22 [ 12156 ]
            Fix Version/s STABLE Sprint 21 [ 12155 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for the comments Sam,

            I have updated code with most of your feedback. Few questions:
            4-1 Not sure, currently it is set to false.
            5-6 It is going through format_string, do I need something else ?
            6-7 It is done for custom message for different modules. If not found then custom message is shown. Do you want me to move these strings to corresponding modules?

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for the comments Sam, I have updated code with most of your feedback. Few questions: 4-1 Not sure, currently it is set to false. 5-6 It is going through format_string, do I need something else ? 6-7 It is done for custom message for different modules. If not found then custom message is shown. Do you want me to move these strings to corresponding modules?
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Raj,

            4.1. Perhaps best to look into that and find out why its now being changed. If there is a good reason no probs at all, but if its just a random change then perhaps best to leave it as false.

            5.6. https://github.com/rajeshtaneja/moodle/blob/3877daee439a0d0a8de08bc5484823bfe730da7a/blocks/course_overview/locallib.php#L76 Each of those shortnames would need to be formatted by itself using the context of the course they belong to. Probably best to preload contexts in the SQL statement
            The string as a whole needs to be converted to a lang string, and should not be run through format_string once that is done.

            6.7. Yip, those strings need to be coming from the modules themselves, so that if someone writes their own module they can define a string that will be used by the course overview block. Of course it should still check the string exists and use the default string if not.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Raj, 4.1. Perhaps best to look into that and find out why its now being changed. If there is a good reason no probs at all, but if its just a random change then perhaps best to leave it as false. 5.6. https://github.com/rajeshtaneja/moodle/blob/3877daee439a0d0a8de08bc5484823bfe730da7a/blocks/course_overview/locallib.php#L76 Each of those shortnames would need to be formatted by itself using the context of the course they belong to. Probably best to preload contexts in the SQL statement The string as a whole needs to be converted to a lang string, and should not be run through format_string once that is done. 6.7. Yip, those strings need to be coming from the modules themselves, so that if someone writes their own module they can define a string that will be used by the course overview block. Of course it should still check the string exists and use the default string if not. Cheers Sam
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            David, Can you please review this again.

            I have integrated all comments, suggested by Sam.

            Show
            rajeshtaneja Rajesh Taneja added a comment - David, Can you please review this again. I have integrated all comments, suggested by Sam.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            dmonllao David Monllaó made changes -
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Hide
            dmonllao David Monllaó added a comment -

            Hi Raj, it looks ok to me

            Show
            dmonllao David Monllaó added a comment - Hi Raj, it looks ok to me
            dmonllao David Monllaó made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks David,

            Pushing it back for integration review.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks David, Pushing it back for integration review.
            rajeshtaneja Rajesh Taneja made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Branch re-based.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Branch re-based.
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Raj + everyone!
            This has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Raj + everyone! This has been integrated now
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Fix Version/s 2.4 [ 12255 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Sam,

            It's nice to see this patch in moodle

            Thanks everyone, especially Adam for sharing this patch and thoughts.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Sam, It's nice to see this patch in moodle Thanks everyone, especially Adam for sharing this patch and thoughts.
            trogdor Julian Ridden made changes -
            Comment [ If it is not too late I would like to add a comment re the GUI. :P

            Would be nice to see something a bit more substantial in place of the small + icon. I fear it will be easily missed.

            How about a folder image. A bit larger perhaps too...40x40 as a guestimate. A folder of courses would make sense from a user perspective and be less easily overlooked.

            Just my 2c

            JR ]
            timb Tim Barker made changes -
            Tester fred
            fred Frédéric Massart made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            fred Frédéric Massart added a comment -

            The test passes but there is a minor error with the spacer image.

            blocks/course_overview/renderer.php:90:
            $html .= html_writer::empty_tag('img', array('src' => $this->pix_url('t/spacer'),
             
            should be
             
            $html .= html_writer::empty_tag('img', array('src' => $this->pix_url('spacer'),

            If someone could push that in, I'll pass the test.

            Show
            fred Frédéric Massart added a comment - The test passes but there is a minor error with the spacer image. blocks/course_overview/renderer.php:90: $html .= html_writer::empty_tag('img', array('src' => $this->pix_url('t/spacer'),   should be   $html .= html_writer::empty_tag('img', array('src' => $this->pix_url('spacer'), If someone could push that in, I'll pass the test.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Fred for spotting that.

            @Sam: I have added an other commit, can you please pull that.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Fred for spotting that. @Sam: I have added an other commit, can you please pull that.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for spotting Fred, and thanks for creating a fix Raj.
            I've merged the fix in now and its all ready to be pulled and tested again.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for spotting Fred, and thanks for creating a fix Raj. I've merged the fix in now and its all ready to be pulled and tested again.
            Hide
            fred Frédéric Massart added a comment -

            Test successful on master. Yeepee!

            Show
            fred Frédéric Massart added a comment - Test successful on master. Yeepee!
            fred Frédéric Massart made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            nebgor Aparup Banerjee added a comment -

            yay, it works!

            This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

            Thank you all for taking the time to get us here.

            cheers!

            Show
            nebgor Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!
            nebgor Aparup Banerjee made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 26/Jul/12
            Hide
            gordonmc Gordon McLeod added a comment -

            Hi,
            Just wanted to check - is there any step-by-step documentation on installing and using this? Our users are very keen to customise the My Moodle page, but I'm not clear if this will run in 2.3 or if we'll need to wait until next year when we plan to upgrade to 2.4, whether I also need to install the MDL-17446 patch mentioned or if that will come as part of the git clone, and where the files should be installed/cloned to?
            Thanks, Gordon.

            Show
            gordonmc Gordon McLeod added a comment - Hi, Just wanted to check - is there any step-by-step documentation on installing and using this? Our users are very keen to customise the My Moodle page, but I'm not clear if this will run in 2.3 or if we'll need to wait until next year when we plan to upgrade to 2.4, whether I also need to install the MDL-17446 patch mentioned or if that will come as part of the git clone, and where the files should be installed/cloned to? Thanks, Gordon.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Gordon,

            This should run on 2.3. No you don't have to install MDL-17446.
            You can use git cherry-pick and pick two commits from git.moodle.org (http://git.moodle.org/gw?p=moodle.git&a=search&h=HEAD&st=commit&s=mdl-19430)

            Let us know if you need more help on this.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Gordon, This should run on 2.3. No you don't have to install MDL-17446 . You can use git cherry-pick and pick two commits from git.moodle.org ( http://git.moodle.org/gw?p=moodle.git&a=search&h=HEAD&st=commit&s=mdl-19430 ) Let us know if you need more help on this.
            tsala Helen Foster made changes -
            Labels netspot partner triaged docs_required netspot partner triaged
            tsala Helen Foster made changes -
            Labels docs_required netspot partner triaged docs_required netspot partner qa_test_required triaged
            marycooch Mary Cooch made changes -
            Labels docs_required netspot partner qa_test_required triaged netspot partner qa_test_required triaged
            Hide
            marycooch Mary Cooch added a comment -

            Removing docs_required label as this is documented now in http://docs.moodle.org/24/en/My_Moodle

            Show
            marycooch Mary Cooch added a comment - Removing docs_required label as this is documented now in http://docs.moodle.org/24/en/My_Moodle
            damyon Damyon Wiese made changes -
            Link This issue caused a regression MDL-37017 [ MDL-37017 ]
            tsala Helen Foster made changes -
            Labels netspot partner qa_test_required triaged docs_required netspot partner qa_test_required triaged
            tsala Helen Foster made changes -
            Labels docs_required netspot partner qa_test_required triaged netspot partner qa_test_required triaged
            Hide
            tsala Helen Foster added a comment -

            Just noting that the documentation about this new feature has been moved to http://docs.moodle.org/en/Course_overview_block

            Show
            tsala Helen Foster added a comment - Just noting that the documentation about this new feature has been moved to http://docs.moodle.org/en/Course_overview_block
            tsala Helen Foster made changes -
            mr-russ Russell Smith made changes -
            Link This issue has been marked as being related by MDL-34273 [ MDL-34273 ]
            rajeshtaneja Rajesh Taneja made changes -
            Link This issue duplicates MDL-34273 [ MDL-34273 ]
            rajeshtaneja Rajesh Taneja made changes -
            Link This issue has been marked as being related by MDL-34273 [ MDL-34273 ]
            danmarsden Dan Marsden made changes -
            Link This issue is duplicated by MDL-16404 [ MDL-16404 ]
            Hide
            dkaelin Daniel Kaelin added a comment -

            Customizing the course order on a My Moodle page does not work if a user is displaying all of their courses.

            The course order feature only works if they specify a certain number of courses to display. (10, 20, etc)

            I saw this issue was closed and I don't know if I'm running into a functionality bug or if anyone else has experienced this.

            Show
            dkaelin Daniel Kaelin added a comment - Customizing the course order on a My Moodle page does not work if a user is displaying all of their courses. The course order feature only works if they specify a certain number of courses to display. (10, 20, etc) I saw this issue was closed and I don't know if I'm running into a functionality bug or if anyone else has experienced this.
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 22 [ 12156 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Daniel,

            I tried re-ordering courses on My home page, and it works fine.
            Can you please provide more information on how to reproduce this issue.
            Please make sure you are not getting an JS error while viewing all courses.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Daniel, I tried re-ordering courses on My home page, and it works fine. Can you please provide more information on how to reproduce this issue. Please make sure you are not getting an JS error while viewing all courses.
            Hide
            dkaelin Daniel Kaelin added a comment - - edited

            The only error that is in the console for the page is this:

            event.returnValue is deprecated. Please use the standard event.preventDefault() instead.

            Version: 2012120307.00
            Release: 2.4.7 (Build: 20131111)

            How many courses are you reordering?

            The instructor / administrator in question has roles in 114 courses and has them all displayed on his my moodle page.

            The problem occurring is when you move courses using ajax editing they do not stay in the custom order. When you turn editing off (customize this page) the courses just return to their original order.

            The courses are a mix of visible and hidden courses but it doesn't appear that has anything to do with the issue. I've also looked at how the categories are affecting the course order and it doesn't seem to matter either. It isn't sorting them alphabetically either so I'm not entirely sure how the courses are even sorted. The original issue states it orders them based upon retrieval from the database but what determines how quickly one course is retrieved versus another?

            It just seems to me Moodle is not properly registering and updating the "course_overview_course_order" field in the mdl_user_preferences.

            Show
            dkaelin Daniel Kaelin added a comment - - edited The only error that is in the console for the page is this: event.returnValue is deprecated. Please use the standard event.preventDefault() instead. Version: 2012120307.00 Release: 2.4.7 (Build: 20131111) How many courses are you reordering? The instructor / administrator in question has roles in 114 courses and has them all displayed on his my moodle page. The problem occurring is when you move courses using ajax editing they do not stay in the custom order. When you turn editing off (customize this page) the courses just return to their original order. The courses are a mix of visible and hidden courses but it doesn't appear that has anything to do with the issue. I've also looked at how the categories are affecting the course order and it doesn't seem to matter either. It isn't sorting them alphabetically either so I'm not entirely sure how the courses are even sorted. The original issue states it orders them based upon retrieval from the database but what determines how quickly one course is retrieved versus another? It just seems to me Moodle is not properly registering and updating the "course_overview_course_order" field in the mdl_user_preferences.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Hello Daniel,

            Can you please confirm which browser you are using. it will be helpful if you check this works on other browser.

            I checked with 15 courses and it seems to work fine. Visibility should not be a problem. Problem seems to be coming from JS error, which could be because of custom block as well.

            Can you track from where "event.returnValue is deprecated. Please use the standard event.preventDefault() instead." gets trigged.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Hello Daniel, Can you please confirm which browser you are using. it will be helpful if you check this works on other browser. I checked with 15 courses and it seems to work fine. Visibility should not be a problem. Problem seems to be coming from JS error, which could be because of custom block as well. Can you track from where "event.returnValue is deprecated. Please use the standard event.preventDefault() instead." gets trigged.
            Hide
            aruethe2 Andrew Ruether added a comment -

            I have the seen the same behavior as Daniel – when a user has many courses, changing the order on the "Course Overview" block in the My Home page doesn't work. I checked the Apache error log and this is what shows up:

            Default exception handler: Coding error detected, it must be fixed by a programmer: 
            Invalid value in set_user_preference() call, value is is too long for the value column 
            Debug: 
            Error code: codingerror 
            line 1744 of /lib/moodlelib.php: coding_exception thrown
            line 65 of /blocks/course_overview/locallib.php: call to set_user_preference() 
            line 34 of /blocks/course_overview/save.php: call to block_course_overview_update_myorder(), 
            referer: https://moodle.xxxxxx.edu/my/index.php?mynumber=125

            It seems that the amount of data sent to the set_user_preference() call is too large.

            Show
            aruethe2 Andrew Ruether added a comment - I have the seen the same behavior as Daniel – when a user has many courses, changing the order on the "Course Overview" block in the My Home page doesn't work. I checked the Apache error log and this is what shows up: Default exception handler: Coding error detected, it must be fixed by a programmer: Invalid value in set_user_preference() call, value is is too long for the value column Debug: Error code: codingerror line 1744 of /lib/moodlelib.php: coding_exception thrown line 65 of /blocks/course_overview/locallib.php: call to set_user_preference() line 34 of /blocks/course_overview/save.php: call to block_course_overview_update_myorder(), referer: https://moodle.xxxxxx.edu/my/index.php?mynumber=125 It seems that the amount of data sent to the set_user_preference() call is too large.
            rajeshtaneja Rajesh Taneja made changes -
            Link This issue caused a regression MDL-43739 [ MDL-43739 ]
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Aha, Thanks for spotting that Andrew,

            user_preferences can be of max 1333 chars and seems it's exceeding in this case.
            I have created MDL-43739 to handle this.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Aha, Thanks for spotting that Andrew, user_preferences can be of max 1333 chars and seems it's exceeding in this case. I have created MDL-43739 to handle this.
            marycooch Mary Cooch made changes -
            Labels netspot partner qa_test_required triaged netspot partner triaged
            Hide
            marycooch Mary Cooch added a comment -

            Removing qa_test_required label as there is a test here MDLQA-7162

            Show
            marycooch Mary Cooch added a comment - Removing qa_test_required label as there is a test here MDLQA-7162

              People

              • Votes:
                18 Vote for this issue
                Watchers:
                15 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/12