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

Selecting "Choose..." option from My Courses dropdown on Participants page generates error: Can not find data record in database table course

    Details

    • Story Points (Obsolete):
      8
    • Sprint:
      FRONTEND Sprint 7

      Description

      On the Participants page, selecting selecting 'Choose...' from the My Courses dropdown results in an error.

      Pre-condition:
      User used for testing is enrolled in at least course

      Steps to reproduce:
      1. As a user with at least one course enrollment, access a course.
      2. In Navigation block, click Participants.
      3. From My Courses dropdown, select the 'Choose...' option.

      Expected results: The form should not submit.
      Actual results: The form submits, but 'Choose...' isn't a valid course and an error results.

      Can not find data record in database table course

      Debug info: SELECT * FROM course WHERE id = ?
      [array (
      0 => 0,
      )]
      Error code: invalidrecord
      Stack trace:
      • line 1357 of /lib/dml/moodle_database.php: dml_missing_record_exception thrown
      • line 1333 of /lib/dml/moodle_database.php: call to moodle_database->get_record_select()
      • line 43 of /user/index.php: call to moodle_database->get_record()

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting that, Chris.

            I was able to reproduce the error.

            Feel free to work with us on fixing it.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting that, Chris. I was able to reproduce the error. Feel free to work with us on fixing it.
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            Fixed as described in the issue and...

            In order to check if a similar error if somewhere I checked other single_select in Moodle (78 occurences). Most of them have their "Choose..." (or whatever label it is for this default) set to null or array(), which mean them don't have a "Choose..." .
            The rest of them are designed in a way that as soon as you switch from the "Choose...", then an action is triggered. So you can't select "Choose...".

            The ones with a "Choose..." problem were:

            • workshop - Accumulative grading page: "Best possible grade / Scale" option can be saved with the default and the view page can be saved with "Choose...". Both don't make too much sense and no errors are displayed. But for the user point of view it make no sense to select them, so won't fix but wrote an issue for the purpose to have a trace in the tracker: MDL-43359.
            • wiki - map menu: "Choose..." displays the "page list", the default. Fixed.
            • wiki - administration: "Choose..." displays "Remove pages", the default. Fixed.
            • wiki - view (individual - no group): "Choose..." makes no sense, it should be default to the current user. Fixed.
            • wiki - view (individual - seperated group): design needs investigation - Selecting "Choose..." causes a SQL error. From the code there will always be a selected option, so removing the "Choose...". Fixed.
            • wiki - view (individual - visible group): design needs investigation - Selecting "Choose..." causes a SQL error. From the code there will always be a selected option, so removing the "Choose...". Fixed.
            • wiki - page_index(): the only call of this function is commented. Apparently it's an undone TODO - wrote an issue for the wiki: MDL-43358. // @TODO: Fix call to wiki_get_subwiki_by_group
            • grade/report/overview/index.php (a) and grade/report/user/index.php (b) - both call the same single_select: (a) "Choose..." displays empty main content. I think from looking at the code it is supposed to select the current user but it's not working. (b) no need "Choose..." here. I think the correct solution is to remove the "Choose...", there is some code for grade report overview for "Choose..." but it makes no sense to me, removing it. Fixed.
            • course/management.php: the "Choose..." are quite not useful but nothing breaks when you select "Choose...". Imo I think it would be better removed, but not mandatory. Won't fix.
            • badge/award.php: The "Choose..." can still be selected once a role is choosen. Fixed.

            Sent for peer-review.

            Show
            jerome Jérôme Mouneyrac added a comment - - edited Fixed as described in the issue and... In order to check if a similar error if somewhere I checked other single_select in Moodle (78 occurences). Most of them have their "Choose..." (or whatever label it is for this default) set to null or array(), which mean them don't have a "Choose..." . The rest of them are designed in a way that as soon as you switch from the "Choose...", then an action is triggered. So you can't select "Choose...". The ones with a "Choose..." problem were: workshop - Accumulative grading page: "Best possible grade / Scale" option can be saved with the default and the view page can be saved with "Choose...". Both don't make too much sense and no errors are displayed. But for the user point of view it make no sense to select them, so won't fix but wrote an issue for the purpose to have a trace in the tracker: MDL-43359 . wiki - map menu: "Choose..." displays the "page list", the default. Fixed . wiki - administration: "Choose..." displays "Remove pages", the default. Fixed . wiki - view (individual - no group): "Choose..." makes no sense, it should be default to the current user. Fixed . wiki - view (individual - seperated group): design needs investigation - Selecting "Choose..." causes a SQL error. From the code there will always be a selected option, so removing the "Choose...". Fixed . wiki - view (individual - visible group): design needs investigation - Selecting "Choose..." causes a SQL error. From the code there will always be a selected option, so removing the "Choose...". Fixed . wiki - page_index(): the only call of this function is commented. Apparently it's an undone TODO - wrote an issue for the wiki: MDL-43358 . // @TODO: Fix call to wiki_get_subwiki_by_group grade/report/overview/index.php (a) and grade/report/user/index.php (b) - both call the same single_select: (a) "Choose..." displays empty main content. I think from looking at the code it is supposed to select the current user but it's not working. (b) no need "Choose..." here. I think the correct solution is to remove the "Choose...", there is some code for grade report overview for "Choose..." but it makes no sense to me, removing it. Fixed . course/management.php: the "Choose..." are quite not useful but nothing breaks when you select "Choose...". Imo I think it would be better removed, but not mandatory. Won't fix . badge/award.php: The "Choose..." can still be selected once a role is choosen. Fixed . Sent for peer-review.
            Hide
            fred Frédéric Massart added a comment -

            Hi Jerome,

            your patch look good to me, just some trivial comments:

            1. Your commit message is a bit long, and it's missing the component
            2. You have a coding style error in the wiki, missing space between if and bracket
            3. I think you could fix the call to menu_map, passing the right argument rather than hardcoding 5 there. If we change the default, this would break. The calling code has a switch which where the option 5 and default could be put together.
            4. Can you comment on the issue why you are adding an exception to the gradebook? It makes sense, but why wasn't it done before? (Also I think it's a moodle_exception rather than a coding_exception).

            Apart from that, all good!

            Cheers,
            Fred

            Show
            fred Frédéric Massart added a comment - Hi Jerome, your patch look good to me, just some trivial comments: Your commit message is a bit long, and it's missing the component You have a coding style error in the wiki, missing space between if and bracket I think you could fix the call to menu_map, passing the right argument rather than hardcoding 5 there. If we change the default, this would break. The calling code has a switch which where the option 5 and default could be put together. Can you comment on the issue why you are adding an exception to the gradebook? It makes sense, but why wasn't it done before? (Also I think it's a moodle_exception rather than a coding_exception). Apart from that, all good! Cheers, Fred
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            Thanks Fred for your peer-review.
            3. I do understand that you suggest that, I thought about it. The value is been hardcoded in the switch that you mentioned and it's due to the fact the code logic considers the order of the options in init array. The effort necessary to refactor this part of code (which would require peer-review, testing...) seems not reasonable to me, it also could be a different issue, and actually I heard wiki could be replaced. However if integrators consider we should spend some resource to refactor this wiki part of the code, I'll be happy to do it
            4. I'll add a comment in the commit message but basically, I guess you understood the same way as me, I could not find any use of it and looking at what the code did it is not going to display much. I hesitated to keep this part of the code. Update: finally I decided to keep the code so it doesn't change any behavior if someone bookmarked the url.

            Show
            jerome Jérôme Mouneyrac added a comment - - edited Thanks Fred for your peer-review. 3. I do understand that you suggest that, I thought about it. The value is been hardcoded in the switch that you mentioned and it's due to the fact the code logic considers the order of the options in init array. The effort necessary to refactor this part of code (which would require peer-review, testing...) seems not reasonable to me, it also could be a different issue, and actually I heard wiki could be replaced. However if integrators consider we should spend some resource to refactor this wiki part of the code, I'll be happy to do it 4. I'll add a comment in the commit message but basically, I guess you understood the same way as me, I could not find any use of it and looking at what the code did it is not going to display much. I hesitated to keep this part of the code. Update: finally I decided to keep the code so it doesn't change any behavior if someone bookmarked the url.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Sent to integration.

            Show
            jerome Jérôme Mouneyrac added a comment - Sent to integration.
            Hide
            fred Frédéric Massart added a comment -

            Thanks for the feedback Jerome. I was not suggesting to refactor the code, but to reuse the hardcoded 5 at the same place, to prevent confusion. But it's up to you. Cheers!

            Show
            fred Frédéric Massart added a comment - Thanks for the feedback Jerome. I was not suggesting to refactor the code, but to reuse the hardcoded 5 at the same place, to prevent confusion. But it's up to you. Cheers!
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I misinterpreted what you said, sorry. I'll make a quick change because it's you

            Show
            jerome Jérôme Mouneyrac added a comment - I misinterpreted what you said, sorry. I'll make a quick change because it's you
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Submitted to integration.

            Show
            jerome Jérôme Mouneyrac added a comment - Submitted to integration.
            Hide
            marina Marina Glancy added a comment -

            Hi Jerome,
            thanks for fixing the initially reported bug. But unfortunately your commit contains much more code.

            For example, your change in overview report is actually creating a bug. I logged in as admin user, without "Choose" in dropdown the first "Student1" is selected which means that I have no possibility to view the Student1's report.
            I did not check other places that you have changed but if you believe that you are fixing bugs that need to be backported as far back as 2.4 please create separate issues or at least separate commits and prove that these are the bugs that need fixing and not just UI improvements.

            Show
            marina Marina Glancy added a comment - Hi Jerome, thanks for fixing the initially reported bug. But unfortunately your commit contains much more code. For example, your change in overview report is actually creating a bug. I logged in as admin user, without "Choose" in dropdown the first "Student1" is selected which means that I have no possibility to view the Student1's report. I did not check other places that you have changed but if you believe that you are fixing bugs that need to be backported as far back as 2.4 please create separate issues or at least separate commits and prove that these are the bugs that need fixing and not just UI improvements.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            No problem Marina, it was mentioned to fix them all during the sprint. I'll make a fix just for this one (easy) and create some issues for the rest.

            Show
            jerome Jérôme Mouneyrac added a comment - No problem Marina, it was mentioned to fix them all during the sprint. I'll make a fix just for this one (easy) and create some issues for the rest.
            Hide
            jerome Jérôme Mouneyrac added a comment - - edited

            I created issues for the rest of the "Choose..." options MDL-43396, MDL-43398 and MDL-43399. Sending this issue back to integration.

            Show
            jerome Jérôme Mouneyrac added a comment - - edited I created issues for the rest of the "Choose..." options MDL-43396 , MDL-43398 and MDL-43399 . Sending this issue back to integration.
            Hide
            marina Marina Glancy added a comment -

            Thanks Jerome, integrated in 2.4, 2.5, 2.6 and master

            Show
            marina Marina Glancy added a comment - Thanks Jerome, integrated in 2.4, 2.5, 2.6 and master
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.4, 2.5, 2.6 and master.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.4, 2.5, 2.6 and master.
            Hide
            damyon Damyon Wiese added a comment -

            Twas the week before Christmas,
            And all though HQ
            Devs were scrambling to finish peer review.
            They sent all their issues,
            and rushed out the door -
            "To the beach!" someone heard them roar!

            This issue has been released upstream. Thanks!

            Show
            damyon Damyon Wiese added a comment - Twas the week before Christmas, And all though HQ Devs were scrambling to finish peer review. They sent all their issues, and rushed out the door - "To the beach!" someone heard them roar! This issue has been released upstream. Thanks!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14

                  Agile