Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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

        Gliffy Diagrams

          Activity

          Hide
          samhemelryk 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
          samhemelryk 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
          andyjdavis Andrew Davis added a comment -

          Should that change to version.php be in there?

          Show
          andyjdavis Andrew Davis added a comment - Should that change to version.php be in there?
          Hide
          andyjdavis 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
          andyjdavis 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
          samhemelryk Sam Hemelryk added a comment -

          Created PULL-239

          Show
          samhemelryk Sam Hemelryk added a comment - Created PULL-239
          Hide
          stronk7 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
          stronk7 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
          skodak Petr Skoda 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
          skodak Petr Skoda 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
          carsontam 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
          carsontam 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
          carsontam Carson Tam added a comment -

          Created MDL-27393 for the above error.

          Show
          carsontam 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:
                Fix Release Date:
                21/Feb/11