Details

    • Type: Sub-task
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              ankit_frenz Ankit Agarwal added a comment -

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

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

              This looks good Ankit.

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

              Thanks for the review Rosie,
              Sending this for integration

              @integrator
              Master only

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

              Note to integrator: this is blocked by MDL-33369

              Show
              poltawski Dan Poltawski added a comment - Note to integrator: this is blocked by MDL-33369
              Hide
              nebgor 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
              nebgor 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
              poltawski 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
              poltawski 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_frenz 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_frenz 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
              poltawski Dan Poltawski added a comment -

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

              Show
              poltawski Dan Poltawski added a comment - Thanks Ankit for that fix to at least get the logs working on postgres (albeit without section name).
              Hide
              stronk7 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
              stronk7 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
              poltawski Dan Poltawski added a comment -

              My testing is failing due to this problem.

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

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

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

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

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

              Tested and thanks.

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

              ah noted! thanks for fixing that up!

              Show
              nebgor Aparup Banerjee added a comment - ah noted! thanks for fixing that up!
              Hide
              stronk7 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
              stronk7 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:
                    Fix Release Date:
                    25/Jun/12