Skip to content

Exercise: Keep the status filter of the results list when coming back to it - #9280

Merged
ywarnier merged 1 commit into
chamilo:masterfrom
ybtbeez:fix/issue-1643-exercise-report-keep-status-filter
Oct 20, 2026
Merged

ywarnier merged 1 commit into
chamilo:masterfrom
ybtbeez:fix/issue-1643-exercise-report-keep-status-filter

Conversation

@ybtbeez

@ybtbeez ybtbeez commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Exercise: Keep the status filter of the results list when coming back to it

Fixes #1643

Problem

A teacher can open a test's results page (main/exercise/exercise_report.php) and set the Status
column filter to "Not validated" to see only the attempts still to correct. After opening an attempt to
correct it and coming back to the list, the filter is back to "All". The teacher has to set it again
for every attempt (#1643).

In 3.x, the main entry point (Tests list → Results) is the Vue report
(/resources/exercise/<node>/<id>/report). It already keeps its filters in the URL query, so they
survive going back.

The legacy page is still reachable and still lost the status filter. It is linked from:

  • the legacy question editor (admin.php);
  • overview.php;
  • exercise_show.php, whose correction form posts back to exercise_report.php;
  • pending.php, stats.php, live_stats.php, question_stats.php, exercise_history.php;
  • my_space/myStudents.php, reports.lib.php;
  • the exercise runtime provider and skills.

Cause

exercise_report.php builds the jqGrid filter toolbar on every page load. It tries to remember the
group filter in a cookie (default_group_<exerciseId>), but nothing remembers the status filter
(#gs_results_status). Each load starts again from the default, "All".

Fix

One block in the page's inline script, in public/main/exercise/exercise_report.php, right after the
filter toolbar is built and before the first triggerToolbar():

  • It restores the selected status from a per-test cookie, exercise_report_status_<exerciseId>.
  • It saves the status to that cookie whenever the teacher changes it.

This follows the same pattern as the existing group-filter code. The select is addressed by its real
id, gs_results_status: free-jqGrid's filter toolbar uses idMode: "new", which names the fields
gs_<grid id>_<column>. The first grid request already asks the server for the filtered list, so
there is no extra request.

How it was tested

The branch is rebased on master 26b01e9. All runs below are from Oct 20, against a local portal
(APP_ENV=prod), with
npx playwright test --config=tests/playwright/playwright.config.ts <spec> --retries=0 --workers=1.

  • Playwright, new feature tests/playwright/features/exerciseReportFilter.feature, with its
    generated spec and one new step in common.steps.ts: "I go to the legacy results page of the current
    test". The Vue pages no longer link to the legacy page, so the step reads the test id and cid/sid
    from the current Vue URL.
    • The feature creates its own course EXRPTF and a test, then opens the legacy results page and
      chooses "Not validated".
    • It leaves for the course home page, comes back to the saved results URL, and checks that the Status
      filter still says "Not validated" (gs_results_status = 0).
    • Then it deletes the course.
    • Without the fix: 1 failed (expect(locator).toHaveValue("0"), received ""), 2 passed.
    • With the fix: 3/3 passed.
  • Playwright, existing exercise suites with the fix: toolExerciseAdmin.feature 22/22 passed and
    toolExerciseTeacher.feature 26/26 passed. With the new feature: 51/51.
  • There is no PHPUnit test: the change is only client-side page script.
  • Static checks:
    • php -l: OK.
    • vendor/bin/ecs check public/main/exercise/exercise_report.php: one auto-fixable finding, the same
      as on master, not in the changed block.
    • vendor/bin/phpstan analyse: 18 findings, identical to master, none in the changed block.
    • Prettier: no findings in the added steps block. The file as a whole already has Prettier warnings
      on master.

Security review

  • The exercise id written into the cookie name is the int-cast $exercise_id.
  • The cookie value is applied only if it matches one of the select's own option values, so a forged
    cookie cannot inject anything.
  • The value only drives the existing toolbar filter, which is already validated by model.ajax.php.
  • No new endpoint, and access control is unchanged.

Notes for reviewers

  • The issue is labelled Enhancement + wontfix (stale bot, 2020). It is still open, and in project
    milestone 3.1. ywarnier wrote in the issue: "have a first look then maybe ask questions to finetune
    this quick report". This PR is that first look.
  • The group filter has the same problem. The existing group-cookie code on this page still uses
    #gs_group_name. With free-jqGrid's idMode: "new" that id does not exist (the real one is
    gs_results_group_name), so on 3.x the group filter is not remembered either.
    • It is left out on purpose. Fixing it would turn on the beforeRequest / group_id_in_toolbar
      logic, which has not been running and needs its own testing.
    • Worth a follow-up.
  • The Vue report, the main 3.x flow, already behaves as the issue asks. This PR only fixes the legacy
    page. If exercise_report.php is going to be retired soon, this PR can be dropped.
  • The cookie is per test and per browser. Like the existing group cookie, it lives for the browser
    session (the js-cookie default). The teacher clears the filter by choosing "All" again.
  • This PR and the one for Gradebook: split Excel export into more columns #1642 both append a step block at the end of common.steps.ts. Whichever is
    merged second needs a trivial rebase.

… to it - Fixes chamilo#1643

On the legacy test results page (exercise_report.php), the Status filter of
the grid toolbar (e.g. "Not validated") was reset to "All" every time the
page was loaded, so after opening an attempt to correct it the teacher had
to set it again. Remember the selected status per test in a cookie and
restore it before the first grid request.
@ybtbeez
ybtbeez force-pushed the fix/issue-1643-exercise-report-keep-status-filter branch from 0d5f49a to f3ed15f Compare October 20, 2026 10:32
@ywarnier
ywarnier merged commit 2b9532f into chamilo:master Oct 20, 2026
4 of 7 checks passed
@codecov

codecov Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 17.20%. Comparing base (f84ab3f) to head (f3ed15f).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #9280   +/-   ##
=========================================
  Coverage     17.20%   17.20%           
  Complexity    62176    62176           
=========================================
  Files          2522     2522           
  Lines        233319   233319           
=========================================
  Hits          40150    40150           
  Misses       193169   193169           

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exercises: keep filtering between pages

2 participants