Skip to content

[Bugfix:RainbowGrades] Guard null bucket access in customizations#12545

Closed
SergioChan wants to merge 1 commit into
Submitty:mainfrom
SergioChan:fix/rainbow-customization-null-guard
Closed

[Bugfix:RainbowGrades] Guard null bucket access in customizations#12545
SergioChan wants to merge 1 commit into
Submitty:mainfrom
SergioChan:fix/rainbow-customization-null-guard

Conversation

@SergioChan

@SergioChan SergioChan commented Mar 7, 2026

Copy link
Copy Markdown

Why is this Change Important & Necessary?

Fixes #12544.

Old courses can contain null bucket entries in stored rainbow customization data. The current code passes that null directly to array_filter, which throws a fatal TypeError and crashes the customization page.

What is the New Behavior?

When reading an existing customization bucket, the code now normalizes missing or non-array bucket data to an empty array before filtering removed/swapped gradeables. This prevents the crash while preserving existing filtering logic for valid array inputs.

What steps should a reviewer take to reproduce or test the bug or new feature?

  1. Use a course where rainbow customization JSON includes a bucket key with null value (or temporarily inject one).
  2. Open the rainbow grades customization page.
  3. Before this fix: page crashes with array_filter(): Argument #1 ($array) must be of type array, null given.
  4. After this fix: page loads successfully and bucket cleanup proceeds.

Automated Testing & Documentation

  • Could not run PHP lint/test commands in this runner because php is not installed (php: command not found).
  • Change is scoped to a defensive type guard around existing logic; no behavior change for valid array inputs.
  • No documentation changes required.

Other information

  • Not a breaking change.
  • No migrations.
  • No additional security concerns.

@codecov

codecov Bot commented Mar 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 21.66%. Comparing base (e039359) to head (3f9c7d7).
⚠️ Report is 37 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12545   +/-   ##
=========================================
  Coverage     21.66%   21.66%           
- Complexity     9638     9639    +1     
=========================================
  Files           268      268           
  Lines         36233    36236    +3     
  Branches        486      486           
=========================================
+ Hits           7849     7851    +2     
- Misses        27902    27903    +1     
  Partials        482      482           
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.68% <75.00%> (+<0.01%) ⬆️
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 91.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@SergioChan SergioChan changed the title fix: prevent null bucket crash in rainbow customization [Bugfix:Rainbow] Prevent null bucket crash in rainbow customization (#12544) Mar 7, 2026
@SergioChan SergioChan changed the title [Bugfix:Rainbow] Prevent null bucket crash in rainbow customization (#12544) [Bugfix:RainbowGrades] Prevent null bucket crash in rainbow customization Mar 7, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Prevent null bucket crash in rainbow customization [Bugfix:RainbowGrades] Guard against null buckets in rainbow customization Mar 7, 2026
@github-actions github-actions Bot added the Abandoned PR - Needs New Owner No activity on PR for more than 2 weeks -- seeking new owner to complete label Mar 22, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Guard against null buckets in rainbow customization [Bugfix:RainbowGrades] Guard null buckets in data Mar 22, 2026
@SergioChan

Copy link
Copy Markdown
Author

Updated the PR title to match the documented format constraints:

[Bugfix:RainbowGrades] Guard null buckets in data

That should satisfy the type/module + subject length checks from submitty/action-pr-title.

The two existing Title Check failures are from the original run and remain as stale checks on this PR. I don’t have permission to rerun those historical workflow runs directly, so a fresh workflow run (new push or maintainer rerun) is needed to clear the stale failures.

@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Guard null buckets in data [Bugfix:RainbowGrades] Handle null grade buckets Mar 22, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Handle null grade buckets [Bugfix:RainbowGrades] Handle null grade buckets safely Mar 22, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Handle null grade buckets safely [Bugfix:RainbowGrades] Handle null buckets safely Mar 22, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Handle null buckets safely [Bugfix: RainbowGrades] Handle null buckets safely Mar 23, 2026
@SergioChan SergioChan changed the title [Bugfix: RainbowGrades] Handle null buckets safely [Bugfix:RainbowGrades] Handle null buckets safely Mar 23, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Handle null buckets safely [Bugfix:RainbowGrades] Guard null buckets in data Mar 23, 2026
@SergioChan

Copy link
Copy Markdown
Author

Updated this PR’s title to:

[Bugfix:RainbowGrades] Guard null buckets in data

to align with the repository’s required [<TYPE>:<MODULE>] <SUBJECT> format.

If the two existing Title Check failures still show up, they look like stale historical runs from the original PR open event and may need a maintainer rerun to refresh status.

@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Guard null buckets in data [Bugfix:RainbowGrades] Guard null buckets in customizations Mar 23, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Guard null buckets in customizations [Bugfix:RainbowGrades] Guard null bucket access in customizations Mar 23, 2026
@SergioChan SergioChan changed the title [Bugfix:RainbowGrades] Guard null bucket access in customizations [Bugfix: RainbowGrades] Guard null bucket access in customizations Mar 23, 2026
@SergioChan SergioChan changed the title [Bugfix: RainbowGrades] Guard null bucket access in customizations [Bugfix:RainbowGrades] Guard null bucket access in customizations Mar 23, 2026
@hyeenaa

hyeenaa commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

I've tested it thoroughly and have reached the conclusion that the instructions presented to re-create the error are not sufficient to to show malfunction. If you choose to continue working on this issue, include screenshots in demonstrating the error and re-creation steps. Once that is fulfilled feel free to re-open the PR and tag me for re-review.

@hyeenaa hyeenaa closed this Mar 31, 2026
@github-project-automation github-project-automation Bot moved this from Seeking Reviewer to Done in Submitty Development Mar 31, 2026
bmcutler pushed a commit that referenced this pull request Jun 8, 2026
### Why is this Change Important & Necessary?

Fixes #12544.

The Rainbow Grades customization page crashes for some older courses
when legacy `gui_customization.json` data contains:

- bucket types not present in the current syllabus bucket list
- `"ids": null` instead of an array

The backend assumes these values are arrays and directly calls
`array_filter()` or iterates using `foreach`, resulting in a PHP 8
`TypeError`.

---

### What is the New Behavior?

- Bucket gradeable data is normalized to an empty array if missing or
invalid
- `json_bucket->ids` is normalized before iteration
- Legacy bucket metadata is initialized when needed

Malformed or legacy data is now treated as an empty bucket instead of
causing a crash. Existing valid behavior is unchanged.

#### Before (bug)

Legacy customization data with `"ids": null` causes a crash on the
customization page:

<img width="1470" height="956" alt="Screenshot 2026-04-01 at 10 52 16"
src="https://github.com/user-attachments/assets/1a9af415-d8c7-4a53-bee6-8bd8fb0ad64e"
/>

#### After (fixed)

The same input no longer crashes and the page loads successfully:

<img width="1470" height="956" alt="Screenshot 2026-04-01 at 10 54 29"
src="https://github.com/user-attachments/assets/b2267620-7510-4009-9f1b-daeb52acccda"
/>

---

### What steps should a reviewer take to reproduce or test the bug or
new feature?

1. Create or edit `rainbow_grades/gui_customization.json` in a course
directory
2. Add a legacy bucket entry such as:

```json
{
  "gradeables": [
    {
      "type": "legacy-bucket",
      "count": 1,
      "remove_lowest": 0,
      "percent": 0.25,
      "ids": null
    }
  ]
}
```

3. Visit:
   `/courses/<term>/<course>/reports/rainbow_grades_customization`

4. Before this patch:
   - Page crashes with:
`array_filter(): Argument #1 ($array) must be of type array, null given`

5. After this patch:
   - Page loads successfully and treats the malformed bucket as empty

---

### Automated Testing & Documentation

- Added regression test:
  `tests/app/models/RainbowCustomizationTester.php`

- Ran:
- `php vendor/bin/phpunit
tests/app/models/RainbowCustomizationTester.php`
- `php vendor/bin/phpunit
tests/app/controllers/admin/ReportControllerTester.php`

- No documentation changes required

---

### Other information

- Not a breaking change
- No migrations required
- No security concerns
- Supersedes abandoned PR #12545

---------

Co-authored-by: D'Andre Collins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Abandoned PR - Needs New Owner No activity on PR for more than 2 weeks -- seeking new owner to complete

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

rainbow grades customization null/array error

2 participants