Fix grade_quiz_auto() silently treating false-grade question IDs as zero-grade#8030
Open
thisismyurl wants to merge 5 commits into
Open
Fix grade_quiz_auto() silently treating false-grade question IDs as zero-grade#8030thisismyurl wants to merge 5 commits into
thisismyurl wants to merge 5 commits into
Conversation
get_question_grade() returns false (not 0) for post IDs that are not of the 'question' post type — e.g. deleted posts or multiple_question container IDs. PHP's loose `0 == false` comparison caused these to fall into the zero-grade branch, storing `false` via set_user_grades() and potentially deflating the computed percentage. Add a strict `false === $achievable_grade` guard before the loose check so invalid IDs are skipped entirely (no entry written to the grade map). Add a unit test confirming that a non-'question' post ID included in the $submitted array does not appear in the stored per-question grades. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rtions Add three precondition assertions (post type check, get_question_grade() returns false), assert the returned percentage is 100 (correct answer on the one valid question), and assert the valid question ID IS stored — so the test fails for multiple distinct reasons without the fix rather than a single key-absence check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four alignment fixes and one missing short description (pre-existing) in the grading test file, detected by phpcs.xml.dist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…question_grade() The malformed `@return int $question_grade | bool` was parsed by Psalm as `@return int`, making `false === $achievable_grade` in grade_quiz_auto() a type contradiction. The function legitimately returns false for non-question post types; the correct docblock is `@return int|false`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The corrected @return int|false docblock on get_question_grade() revealed three call sites where the false return was used directly in arithmetic or string concatenation without guarding. In PHP, false coerces to 0 in arithmetic and empty string in concatenation — add explicit (int) casts to match the pre-existing runtime behavior and satisfy Psalm. Affected files: - class-sensei-grading-user-quiz.php:141 — $quiz_grade_total += (int) $question_grade_total - class-sensei-utils.php:749 — $quiz_total += (int) $question_grade - class-sensei-question.php:1012 — '/' . (int) $question_grade in format_question_points() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #7818
Proposed Changes
get_question_grade()returnsfalse— not0— for any post ID that is not of the'question'post type (such asmultiple_questioncontainer IDs or deleted-post remnants). Ingrade_quiz_auto(), the existing checkif ( 0 == $achievable_grade )uses PHP's loose comparison operator, which evaluates0 == falseastrue. This caused those IDs to silently fall into the zero-grade branch and be written to the per-question grade map asfalseviaset_user_grades(), corrupting the per-question grade record and potentially deflating the computed percentage.This PR adds a strict
false === $achievable_gradeguard before the existing loose check. Invalid IDs are now skipped entirely (continue) — no grade entry is created for them.Testing Instructions
true). As a student, submit the quiz with the correct answer plus an answer keyed to a non-'question'post ID (simulating amultiple_questioncontainer or deleted-post remnant in the submitted array). Check the stored per-question grades — the invalid ID must not appear, and the percentage must reflect only the valid questions.New/Updated Hooks
None
Deprecated Code
None
Pre-Merge Checklist
Changelog entry
A
changelog/fix-grade-quiz-auto-false-question-idfile is included (patch/fixed).(full disclosure: AI helped me identify the issue and verify my work)