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

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

    XMLWordPrintable

Details

    • MOODLE_400_STABLE
    • MOODLE_400_STABLE
    • MDL-75358-400
    • 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

    Description

      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

      Attachments

        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

        Issue Links

          Activity

            People

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

              Dates

                Created:
                Updated:
                Resolved:
                14/Nov/22

                Time Tracking

                  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