Gradebook: Export list view scores as numeric percentage, score and max columns in Excel

[ybtbeez]

Gradebook: Export list view scores as numeric percentage, score and max columns in Excel

Fixes #1642

Problem

The Excel export of the Gradebook list view (Assessments → List view → "Export Excel",
/api/gradebook/export/flat.xls) writes each evaluation result, and the total, as one text cell,
e.g. 6.00 / 10.00 (60.00%). Excel cannot sum, average or chart these values. #1642 asked to split them
into separate numbers, e.g. 0.90;18;20. Current master still exports one text cell.

In 2017 the request was postponed to 2.0 because the 1.11 sheet had one column per student. The 3.x
list-view export has one row per learner, so the split is now simple.

Cause

GradebookExportService builds a single table of strings (buildFlatTable()), shared by the CSV, DOCX,
PDF and XLS exports. createXlsResponse() then writes every cell with DataType::TYPE_STRING.

Fix

Only the XLS export changes, in src/CoreBundle/Service/Gradebook/GradebookExportService.php. CSV,
DOCX and PDF keep their current layout.

• The name, username, official code and extra-field columns are unchanged. They are reused from
  buildFlatTable().
• Every score column, plus the Total, becomes three numeric columns, in the order the issue asks for:
  • <title> (Percentage): a fraction with the 0.00% number format, so Excel shows 90.00% and
    computes with 0.9;
  • <title> (Score);
  • <title> (Max score).
• Learners with no result get empty cells, not 0.
• The optional "Ranking" (custom score) column stays at the end, as text.
• Text cells, including the column titles entered by teachers, still go through
  sanitizeSpreadsheetValue(), the formula-injection guard.
• The generic createXlsResponse() was only used by this export, so it is removed.
• No new translation strings: Percentage, Score, Max score, Total and Ranking already exist.

How it was tested

The branch is rebased on master 26b01e9. All runs below are from Oct 20.

• PHPUnit, new test tests/CoreBundle/Service/Gradebook/GradebookExportServiceTest.php
  (php bin/phpunit tests/CoreBundle/Service/Gradebook/GradebookExportServiceTest.php). It generates the
  XLS, reads it back with PhpSpreadsheet and checks:

  • the headers;
  • that the score cells are TYPE_NUMERIC, with the right values and the percent format;
  • that a learner without a result gets empty cells.

  With the fix: OK (1 test, 22 assertions). With the service reverted to master: 1 failure
  (Failed asserting that two arrays are identical, because the headers are still Quiz 1, Total).

• PHPUnit, related existing tests:

  • tests/CoreBundle/Repository/GradebookCertificateExpiryTest.php: OK (7 tests).
  • tests/CoreBundle/Api/GradebookLinkSecurityTest.php: 10 errors, all
    Access denied for user 'root'@'localhost'. That comes from the local test-DB setup, not from this
    change.

• Playwright, new feature tests/playwright/features/gradebookExcelExport.feature, plus 3 new
  generic steps in common.steps.ts: open a path with a resolved course id, download from an icon link,
  and search the downloaded file.

  • The feature creates its own course GBXLS, subscribes and grades norizales, clicks "Export Excel"
    in the list view, checks that the downloaded file has the split headers, then deletes the course.
  • Command:
    npx playwright test --config=tests/playwright/playwright.config.ts <spec> --retries=0 --workers=1
  • Without the fix: 1 failed ("xlsexam (Percentage)" in the downloaded file), 2 passed.
  • With the fix: 3/3 passed.

• Playwright, existing Gradebook suite toolAssessments.feature, run with the fix: 10/10 passed.
  It covers the PDF exports, among other things. With the new feature: 13/13 passed.

• Static checks:

  • php -l: OK.
  • vendor/bin/ecs check on both PHP files: no findings.
  • vendor/bin/phpstan analyse: no findings.
  • vendor/bin/psalm --no-cache on the service: clean.
  • Prettier: no findings in the added steps block. The file as a whole already has Prettier warnings
    on master.

Security review

• No new route, and no change to authorization. The existing export operation calls the service.
• Text cells, including the headers built from teacher-entered titles, go through the existing
  sanitizeSpreadsheetValue().
• Score cells are written only when the value passes is_numeric(), and are cast to float.

Notes for reviewers

• The issue is labelled Enhancement + wontfix (stale bot, 2020). It is still open, and in project
  milestone 3.1.
• This changes the column layout of the Excel export. A macro or import that reads the old
  single-column layout would need to be adapted.
• CSV is left as it is on purpose. Say so if you want the same split there.
• The Playwright scenario checks the headers of the downloaded file. The numeric cell types and values
  are checked by the PHPUnit test.
• This PR and the one for Exercises: keep filtering between pages #1643 both append a step block at the end of common.steps.ts. Whichever is
  merged second needs a trivial rebase.

[codecov]

Codecov Report

❌ Patch coverage is 94.91525% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.20%. Comparing base (26b01e9) to head (3b4c19a).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...undle/Service/Gradebook/GradebookExportService.php	94.91%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #9279      +/-   ##
============================================
+ Coverage     17.13%   17.20%   +0.06%     
- Complexity    62160    62176      +16     
============================================
  Files          2522     2522              
  Lines        233279   233319      +40     
============================================
+ Hits          39981    40133     +152     
+ Misses       193298   193186     -112     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.