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

Specifying invalid default column sorting in a report should trigger exception

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Fixed
    • Icon: Minor Minor
    • 4.3
    • 4.3
    • Report builder
    • MOODLE_403_STABLE
    • MOODLE_403_STABLE
    • Hide
      1. Make the following manual change:

        $ git diff
        diff --git a/badges/classes/reportbuilder/datasource/badges.php b/badges/classes/reportbuilder/datasource/badges.php
        index 9944204ee65..a529161ff3e 100644
        --- a/badges/classes/reportbuilder/datasource/badges.php
        +++ b/badges/classes/reportbuilder/datasource/badges.php
        @@ -147,7 +147,7 @@ class badges extends datasource {
             public function get_default_column_sorting(): array {
                 return [
                     'badge:name' => SORT_ASC,
        -            'user:fullname' => SORT_ASC,
        +            'user:fullnames' => SORT_ASC,
                     'badge_issued:issued' => SORT_ASC,
                 ];
             }
        

      2. Run the following test:

        $ vendor/bin/phpunit badges/tests/reportbuilder/datasource/badges_test.php --filter test_datasource_default
        

      3. Confirm you see the following failure:

        There was 1 error:
         
        1) core_badges\reportbuilder\datasource\badges_test::test_datasource_default
        coding_exception: Coding error detected, it must be fixed by a programmer: Invalid column name (user:fullnames)
        

      4. CI will test everything else
      Show
      Make the following manual change: $ git diff diff --git a/badges/classes/reportbuilder/datasource/badges.php b/badges/classes/reportbuilder/datasource/badges.php index 9944204ee65..a529161ff3e 100644 --- a/badges/classes/reportbuilder/datasource/badges.php +++ b/badges/classes/reportbuilder/datasource/badges.php @@ -147,7 +147,7 @@ class badges extends datasource { public function get_default_column_sorting(): array { return [ 'badge:name' => SORT_ASC, - 'user:fullname' => SORT_ASC, + 'user:fullnames' => SORT_ASC, 'badge_issued:issued' => SORT_ASC, ]; } Run the following test: $ vendor/bin/phpunit badges/tests/reportbuilder/datasource/badges_test.php --filter test_datasource_default Confirm you see the following failure: There was 1 error:   1) core_badges\reportbuilder\datasource\badges_test::test_datasource_default coding_exception: Coding error detected, it must be fixed by a programmer: Invalid column name (user:fullnames) CI will test everything else

      This was discussed during implementation of the initial "default column sorting" of a report source in MDL-75525 here, but currently the invalid column is silently discarded

      This is a problem because it can introduce subtle bugs if the developer thinks their report is being sorted by default (and writes tests with that assumption), but because of a typo in the column name they end up with random sorting test failures

      This is exactly what happened when I was looking at MDL-78755. Let's be consistent and a bit stricter - throw a \coding_exception and fail early so the developer knows to fix their report source

            pholden Paul Holden
            pholden Paul Holden
            Pedro Jordao Pedro Jordao
            David Carrillo David Carrillo
            Kim Jared Lucas Kim Jared Lucas
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 hour, 47 minutes
                1h 47m

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.