Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
      None
    • Testing Instructions:
      Hide

      This needs to be tested in multiple databases.
      As teacher

      1. set course layout setting to "show one section per page"
      2. access a course (topic/week format) and view a few section (eg: http://moodle/course/view.php?id=xx&section=3)
      3. check the log report for the course and make sure there's "course view section" entry pointing to the correct section.

      As admin

      1. backup the course and tick on "include course logs"
      2. restore the course as new course
      3. check the log for the new course and make sure there's "course view section" entry which takes you to the right section.
      Show
      This needs to be tested in multiple databases. As teacher set course layout setting to "show one section per page" access a course (topic/week format) and view a few section (eg: http://moodle/course/view.php?id=xx&section=3 ) check the log report for the course and make sure there's "course view section" entry pointing to the correct section. As admin backup the course and tick on "include course logs" restore the course as new course check the log for the new course and make sure there's "course view section" entry which takes you to the right section.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-33465-master
    • Rank:
      41364

      Description

      Log entries for "course view section" should use sectionid parameter isntead of section parameter while generating action urls.

      The reason being that section number is a variable thing, and changes when section are moved around, which leads to incorrect urls being stored in the log entries.

        Issue Links

          Activity

          Hide
          Ankit Agarwal added a comment -

          @integrator
          This must land after MDL-33369 is integrated.
          Thanks

          Show
          Ankit Agarwal added a comment - @integrator This must land after MDL-33369 is integrated. Thanks
          Hide
          Rossiani Wijaya added a comment -

          This looks good Ankit.

          Show
          Rossiani Wijaya added a comment - This looks good Ankit.
          Hide
          Ankit Agarwal added a comment -

          Thanks for the review Rosie,
          Sending this for integration

          @integrator
          Master only

          Show
          Ankit Agarwal added a comment - Thanks for the review Rosie, Sending this for integration @integrator Master only
          Hide
          Dan Poltawski added a comment -

          Note to integrator: this is blocked by MDL-33369

          Show
          Dan Poltawski added a comment - Note to integrator: this is blocked by MDL-33369
          Hide
          Aparup Banerjee added a comment -

          Thanks Ankit, this has been integrated into master (as well as its dependant issue).

          They are ready for testing.

          Show
          Aparup Banerjee added a comment - Thanks Ankit, this has been integrated into master (as well as its dependant issue). They are ready for testing.
          Hide
          Dan Poltawski added a comment -

          Debug info: ERROR: COALESCE types character varying and bigint cannot be matched
          LINE 1: SELECT COALESCE(name, section) FROM mdl_course_sections WHER...
          ^
          SELECT COALESCE(name, section) FROM mdl_course_sections WHERE id = $1
          [array (
          0 => '3',
          )]
          Error code: dmlreadexception
          Stack trace:
          line 413 of /lib/dml/moodle_database.php: dml_read_exception thrown
          line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
          line 703 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
          line 1341 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql()
          line 1414 of /lib/dml/moodle_database.php: call to moodle_database->get_record_sql()
          line 1395 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql()
          line 1374 of /lib/dml/moodle_database.php: call to moodle_database->get_field_select()
          line 387 of /course/lib.php: call to moodle_database->get_field()
          line 157 of /report/log/index.php: call to print_log()

          Show
          Dan Poltawski added a comment - Debug info: ERROR: COALESCE types character varying and bigint cannot be matched LINE 1: SELECT COALESCE(name, section) FROM mdl_course_sections WHER... ^ SELECT COALESCE(name, section) FROM mdl_course_sections WHERE id = $1 [array ( 0 => '3', )] Error code: dmlreadexception Stack trace: line 413 of /lib/dml/moodle_database.php: dml_read_exception thrown line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end() line 703 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end() line 1341 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql() line 1414 of /lib/dml/moodle_database.php: call to moodle_database->get_record_sql() line 1395 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql() line 1374 of /lib/dml/moodle_database.php: call to moodle_database->get_field_select() line 387 of /course/lib.php: call to moodle_database->get_field() line 157 of /report/log/index.php: call to print_log()
          Hide
          Ankit Agarwal added a comment - - edited

          The above error was introduced in MDL-32766, any way I have added a second commit to fix up the issue. Also created MDL-33606 to improve the code to use section number if section name is null.

          Thanks

          Show
          Ankit Agarwal added a comment - - edited The above error was introduced in MDL-32766 , any way I have added a second commit to fix up the issue. Also created MDL-33606 to improve the code to use section number if section name is null. Thanks
          Hide
          Dan Poltawski added a comment -

          Thanks Ankit for that fix to at least get the logs working on postgres (albeit without section name).

          Show
          Dan Poltawski added a comment - Thanks Ankit for that fix to at least get the logs working on postgres (albeit without section name).
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note that this was integrated incorrectly, with one wrong conflict resolution, so this line:

          1109) $this->set_mapping('course_sectionnumber', $oldsection, ....

          was kept at it's leading to broken restore operation each time one restore is tried onto blank course.

          Such merge needs to be reviewed and fixed. It's causing any testing involving restore to fail badly (like MDL-33080).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Note that this was integrated incorrectly, with one wrong conflict resolution, so this line: 1109) $this->set_mapping('course_sectionnumber', $oldsection, .... was kept at it's leading to broken restore operation each time one restore is tried onto blank course. Such merge needs to be reviewed and fixed. It's causing any testing involving restore to fail badly (like MDL-33080 ). Ciao
          Hide
          Dan Poltawski added a comment -

          My testing is failing due to this problem.

          Show
          Dan Poltawski added a comment - My testing is failing due to this problem.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm reviewing the offending merge right now... stay tuned...

          Show
          Eloy Lafuente (stronk7) added a comment - I'm reviewing the offending merge right now... stay tuned...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Done, added commit, deleting the 2 lines incorrectly kept by the merge resolution.

          Show
          Eloy Lafuente (stronk7) added a comment - Done, added commit, deleting the 2 lines incorrectly kept by the merge resolution.
          Hide
          Dan Poltawski added a comment -

          Tested and thanks.

          Show
          Dan Poltawski added a comment - Tested and thanks.
          Hide
          Aparup Banerjee added a comment -

          ah noted! thanks for fixing that up!

          Show
          Aparup Banerjee added a comment - ah noted! thanks for fixing that up!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

          Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

          Many thanks for your collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: