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

Nulls are displayed as "0" in the report builder numeric columns

XMLWordPrintable

    • MOODLE_400_STABLE
    • MOODLE_400_STABLE
    • MDL-75358-master
    • Hide

      Testing requires some code modifications because there are not many datasources in Moodle LMS yet

      1. Create two cohorts, add a member to one of them (does not matter who)
      2. In Site administration>Users>User profile fields add user profile fields with the types "checkbox" and "date"
      3. Go to Site administration>Reports>Custom reports and create a report from the "Cohorts" datasource
      4. Add columns:
      • Cohort: Name
      • Cohort member: Time added
      • User: Full name
      • User: Confirmed
      • two user profile fields you added in the step 2 above

      5. View the report. In the row for the empty cohort all cells have to be completely empty (note that user profile field 'date' displays "Not set")

      6. Modify the code in the file cohort/classes/local/entities/cohort_member.php around lines 86-95:

      Modification 1:
      Change the type to INTEGER and change callback from 'userdate' to 'percent'. View report - the "Time added" cell in the empty cohort row should be empty

      diff --git a/cohort/classes/reportbuilder/local/entities/cohort_member.php b/cohort/classes/reportbuilder/local/entities/cohort_member.php
      index b05a661714b..bcf353d0a10 100644
      --- a/cohort/classes/reportbuilder/local/entities/cohort_member.php
      +++ b/cohort/classes/reportbuilder/local/entities/cohort_member.php
      @@ -89,10 +89,10 @@ class cohort_member extends base {
                   $this->get_entity_name()
               ))
                   ->add_joins($this->get_joins())
      -            ->set_type(column::TYPE_TIMESTAMP)
      +            ->set_type(column::TYPE_INTEGER)
                   ->add_fields("{$tablealias}.timeadded")
                   ->set_is_sortable(true)
      -            ->set_callback([format::class, 'userdate']);
      +            ->set_callback([format::class, 'percent']);
       
               return $columns;
           }
      

      Modification 2:
      For the same column leave the type as INTEGER but remove callback completely. View report - the "Time added" cell in the empty cohort row should be empty

      diff --git a/cohort/classes/reportbuilder/local/entities/cohort_member.php b/cohort/classes/reportbuilder/local/entities/cohort_member.php
      index b05a661714b..519d1dbc50e 100644
      --- a/cohort/classes/reportbuilder/local/entities/cohort_member.php
      +++ b/cohort/classes/reportbuilder/local/entities/cohort_member.php
      @@ -89,10 +89,10 @@ class cohort_member extends base {
                   $this->get_entity_name()
               ))
                   ->add_joins($this->get_joins())
      -            ->set_type(column::TYPE_TIMESTAMP)
      +            ->set_type(column::TYPE_INTEGER)
                   ->add_fields("{$tablealias}.timeadded")
      -             ->set_is_sortable(true)
      -            ->set_callback([format::class, 'userdate']);
      +            ->set_is_sortable(true);
       
               return $columns;
           }
      

      Modification 3. Change the type to column::TYPE_FLOAT - assert the same result in the report

      Aggregation

      1. Rollback modifications from the previous test
      2. Use the same report, but only columns "Cohort: Name", "Cohort member: Time added" and "User: Confirmed"
      3. Aggregate the columns "Confirmed" and "Time added" using different aggregation methods (except for "count" and "count distinct")
      4. Assert that the aggregated values for the empty cohort show empty cells and the aggregated values for the non-empty cohort show values
      Show
      Testing requires some code modifications because there are not many datasources in Moodle LMS yet Create two cohorts, add a member to one of them (does not matter who) In Site administration>Users>User profile fields add user profile fields with the types "checkbox" and "date" Go to Site administration>Reports>Custom reports and create a report from the "Cohorts" datasource Add columns: Cohort: Name Cohort member: Time added User: Full name User: Confirmed two user profile fields you added in the step 2 above 5. View the report. In the row for the empty cohort all cells have to be completely empty (note that user profile field 'date' displays "Not set") 6. Modify the code in the file cohort/classes/local/entities/cohort_member.php around lines 86-95: Modification 1: Change the type to INTEGER and change callback from 'userdate' to 'percent'. View report - the "Time added" cell in the empty cohort row should be empty diff --git a/cohort/classes/reportbuilder/local/entities/cohort_member.php b/cohort/classes/reportbuilder/local/entities/cohort_member.php index b05a661714b..bcf353d0a10 100644 --- a/cohort/classes/reportbuilder/local/entities/cohort_member.php +++ b/cohort/classes/reportbuilder/local/entities/cohort_member.php @@ -89,10 +89,10 @@ class cohort_member extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TIMESTAMP) + ->set_type(column::TYPE_INTEGER) ->add_fields("{$tablealias}.timeadded") ->set_is_sortable(true) - ->set_callback([format::class, 'userdate']); + ->set_callback([format::class, 'percent']); return $columns; } Modification 2: For the same column leave the type as INTEGER but remove callback completely. View report - the "Time added" cell in the empty cohort row should be empty diff --git a/cohort/classes/reportbuilder/local/entities/cohort_member.php b/cohort/classes/reportbuilder/local/entities/cohort_member.php index b05a661714b..519d1dbc50e 100644 --- a/cohort/classes/reportbuilder/local/entities/cohort_member.php +++ b/cohort/classes/reportbuilder/local/entities/cohort_member.php @@ -89,10 +89,10 @@ class cohort_member extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TIMESTAMP) + ->set_type(column::TYPE_INTEGER) ->add_fields("{$tablealias}.timeadded") - ->set_is_sortable(true) - ->set_callback([format::class, 'userdate']); + ->set_is_sortable(true); return $columns; } Modification 3. Change the type to column::TYPE_FLOAT - assert the same result in the report Aggregation Rollback modifications from the previous test Use the same report, but only columns "Cohort: Name", "Cohort member: Time added" and "User: Confirmed" Aggregate the columns "Confirmed" and "Time added" using different aggregation methods (except for "count" and "count distinct") Assert that the aggregated values for the empty cohort show empty cells and the aggregated values for the non-empty cohort show values

      When a column type is defined as TYPE_INTEGER/TYPE_FLOAT/TYPE_BOOLEAN we display "0" / "No" in the column instead of the empty string (null)

      Aggregation works correctly (i.e. the null values do not affect AVG or MIN). Edit - aggregation itself works correctly when there are both null and non-null values. However AVG(null) is still displayed as "0" and it's a different problem than what was initially reported, also addressed in this patch.

      Example:
      with MDL-75245 create a tag collection without tags, create a report from the "Tags" datasource and add all columns there

        1. before_fix.png
          before_fix.png
          34 kB
        2. after_fix.png
          after_fix.png
          34 kB
        3. image-2022-10-31-09-46-46-309.png
          image-2022-10-31-09-46-46-309.png
          41 kB
        4. image-2022-10-31-09-47-31-719.png
          image-2022-10-31-09-47-31-719.png
          38 kB
        5. image-2022-10-31-09-48-06-889.png
          image-2022-10-31-09-48-06-889.png
          36 kB
        6. image-2022-10-31-09-50-18-969.png
          image-2022-10-31-09-50-18-969.png
          27 kB

            marina Marina Glancy
            marina Marina Glancy
            Paul Holden Paul Holden
            Jun Pataleta Jun Pataleta
            Amaia Anabitarte Amaia Anabitarte
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 day, 2 hours, 2 minutes
                1d 2h 2m

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