Moodle
  1. Moodle
  2. MDL-26263 Standardise use of the cm_info object and fix type hinting where stdClass is still being used.
  3. MDL-26265

moodle_page::set_cm should require a cm_info object and convert a stdClass $cm into a cm_info if required

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Libraries
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16505

      Description

      moodle_page::set_cm currently gets a cm_info object if it is being called from a require_login function, however code manually setting a cm is going to be passing in a stdClass object from the db.
      This should be tidied up and moodle_page::set_cm should really convert the stdClass object to a cm_info instance if required.

      Cheers
      Sam

        Activity

        Hide
        Sam Hemelryk added a comment -

        Hi Andrew,
        I've pushed a fix for this, could you please peer-review it for me: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-26265

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Andrew, I've pushed a fix for this, could you please peer-review it for me: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-26265 Cheers Sam
        Hide
        Andrew Davis added a comment -

        Should that change to version.php be in there?

        Show
        Andrew Davis added a comment - Should that change to version.php be in there?
        Hide
        Andrew Davis added a comment -

        version info issue fixed.

        Is it worth outputting a debug message if the supplied $cm isnt from get_fast_modinfo() so that the calling code is aware that they are doing something suboptimal?

        Otherwise, +1.

        Show
        Andrew Davis added a comment - version info issue fixed. Is it worth outputting a debug message if the supplied $cm isnt from get_fast_modinfo() so that the calling code is aware that they are doing something suboptimal? Otherwise, +1.
        Hide
        Sam Hemelryk added a comment -

        Created PULL-239

        Show
        Sam Hemelryk added a comment - Created PULL-239
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Copied from the PULL request, to have it present:

        Integrating this in blind mode 100x100.

        • I don't like things to be introduced and marked as deprecated at the same time.
        • I don't like to have different $cm here and there having completely different information.
        • I don't like having PAGE->context, cm_info->context, $context and $anythingelse->context.
        • I don't like $cm being returned as stdClass sometimes or cm_info in other places.
        • I don't like get_instance_from_XXXX() returning on $cm that later is used to build cm_info

        I don't like, I don't like. I hope it's the last time we add one change like this in stable. Tons of things need clarification and information should be only in one place and in one format.

        Said that, I rely one Petr's opinion and have pushed this.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Copied from the PULL request, to have it present: Integrating this in blind mode 100x100. I don't like things to be introduced and marked as deprecated at the same time. I don't like to have different $cm here and there having completely different information. I don't like having PAGE->context, cm_info->context, $context and $anythingelse->context. I don't like $cm being returned as stdClass sometimes or cm_info in other places. I don't like get_instance_from_XXXX() returning on $cm that later is used to build cm_info I don't like, I don't like. I hope it's the last time we add one change like this in stable. Tons of things need clarification and information should be only in one place and in one format. Said that, I rely one Petr's opinion and have pushed this. Ciao
        Hide
        Petr Škoda added a comment -

        I have reworked the patch and fixed some more issues there:
        1/ added cm_info context to increase BC (deprecated, hopefully we may get rid of it in 2.1)
        2/ fixed setting of page context in set_cm() so that PAGE->context matches PAGE->cm->context (this might have been creating some nasty bugs)
        3/ fixed inline docs

        We need to test it a lot more than last week...

        Show
        Petr Škoda added a comment - I have reworked the patch and fixed some more issues there: 1/ added cm_info context to increase BC (deprecated, hopefully we may get rid of it in 2.1) 2/ fixed setting of page context in set_cm() so that PAGE->context matches PAGE->cm->context (this might have been creating some nasty bugs) 3/ fixed inline docs We need to test it a lot more than last week...
        Hide
        Carson Tam added a comment -

        I think this fix has broken some unit tests:

        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_set_cm
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 588 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_setting_cm_sets_context
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 626 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_activity_record_loaded_if_not_set
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 638 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_set_activity_record
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 646 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_cannot_set_inconsistent_activity_record_course
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 656 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_cannot_set_inconsistent_activity_record_instance
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 667 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_settin_cm_sets_course
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 679 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_set_cm_with_course_and_activity_no_db
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 690 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_get_activity_name
        Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146]
        
            * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm()
            * line 711 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm()
            * line ... of ...
            * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run()
            * line ... of ...
            * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        
        
        Show
        Carson Tam added a comment - I think this fix has broken some unit tests: Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_set_cm Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 588 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_setting_cm_sets_context Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 626 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_activity_record_loaded_if_not_set Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 638 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_set_activity_record Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 646 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_cannot_set_inconsistent_activity_record_course Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 656 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_cannot_set_inconsistent_activity_record_instance Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 667 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_settin_cm_sets_course Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 679 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_set_cm_with_course_and_activity_no_db Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 690 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run() Exception: lib/simpletest/testpagelib_moodlepage.php / ► moodle_page_cm_test / ► test_get_activity_name Unexpected exception of type [moodle_exception] with message [Invalid course module ID] in [C:\xampp\htdocs\moodle\lib\modinfolib.php line 146] * line 805 of \lib\pagelib.php: call to course_modinfo->get_cm() * line 711 of \lib\simpletest\testpagelib_moodlepage.php: call to moodle_page->set_cm() * line ... of ... * line 37 of \admin\report\unittest\ex_simple_test.php: call to TestSuite->run() * line ... of ... * line 100 of \admin\report\unittest\index.php: call to autogroup_test_coverage->run()
        Hide
        Carson Tam added a comment -

        Created MDL-27393 for the above error.

        Show
        Carson Tam added a comment - Created MDL-27393 for the above error.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: