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

Specifying invalid default column sorting in a report should trigger exception

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Fixed
    • 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

    Description

      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

      Attachments

        Issue Links

          Activity

            People

              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

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

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

                  Clockify

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