Course evaluation#110
Merged
Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…lament-LMS into course_evaluation
…lament-LMS into course_evaluation
swilla
requested changes
Jul 6, 2026
swilla
left a comment
swilla
left a comment
Member
There was a problem hiding this comment.
We need a way for this to work with a scorm course since the menu is hidden for those.
- Widen embeddedCourseExitUrl()'s parameter to Authenticatable&FilamentLmsUserInterface so $user->id resolves under PHPStan - Scope video/test step completions to the active evaluation primary course, matching form steps - Guard evaluationPrimaryCourseIdFromRequest() against untrusted query params when the route's course can't be verified - Remove an evaluationStepAvailableForPrimary() check that let mislinked evaluation courses skip step ordering
Step page mount() resolves an implicit primaryCourse via activePrimaryCourseForEvaluation() when it's absent from the query string, but progress lookups, completion percentage, and navigation links all read evaluationPrimaryCourseIdFromRequest() directly, which only sees an explicit query param. That mismatch left a step's own progress reading unscoped rows right after completing it under the resolved primary course. Write the resolved id back into the request's query bag once mount() settles on it, so every other consumer within the same request sees the same scope.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1082a51. Configure here.
| return $this->hasOne(StepUser::class)->ofMany([ | ||
| // TODO is this started_at => max needed? | ||
| 'created_at' => 'max', | ||
| ], function ($query) use ($currentUserId) { |
Contributor
There was a problem hiding this comment.
Video progress ignores evaluation scope
Medium Severity
videoProgress still reads and writes lms_step_user rows without evaluation_primary_course_id, while evaluation progress and completion use primary-scoped rows. On a shared evaluation course with a video step, progress and completion can attach to the wrong pivot row and never satisfy scoped completion checks.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1082a51. Configure here.
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
2 participants
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.
Details
Schema
evaluation_course_idonlms_courses— links a training course to its evaluation courseCompletion flow
completed_atand certificate wait until evaluation is doneConfig (
filament-lms.evaluations.enabled) — opt-in feature flagComplete instructions added on README file.
Note
High Risk
Changes core completion, certificate eligibility, and step progress scoping across Course/Step/Livewire pages; misconfiguration or shared-evaluation edge cases could block certificates or show wrong progress.
Overview
Adds an opt-in course evaluation flow (
filament-lms.evaluations.enabled) where a training course links to a separate private evaluation course viaevaluation_course_id. Learners must finish that linked form course before the primary course getscompleted_at, a certificate, or the normal completion UI.Schema & admin: New migrations add
evaluation_course_idon courses, plusevaluation_primary_course_idandfilament_form_user_idonlms_step_userso one shared evaluation course can track per-training progress and form submissions. Admin Course resource gets an Evaluation course select with validation (private targets only, no reverse/nested links). README documents setup;tapp/filament-form-builderis bumped to^4.3.5.Completion & UX: New
CourseEvaluationServicecentralizes gating, auto-enrollment, URLs withprimaryCoursequery param, and finalizing primary completion after evaluation.maybeSetCompletedAtForUserdefers primary completion until evaluation passes (and still respects test score rules). Course completed page shows Complete Evaluation when pending and View Evaluation after submit; evaluation-only courses are hidden from the dashboard listing. Step / FormStep / embedded SCORM exit paths redirect into evaluation; evaluation form steps hide Next, allow resubmissions, and tie submissions to the scopedStepUserrow.Reviewed by Cursor Bugbot for commit 1082a51. Bugbot is set up for automated code reviews on this repo. Configure here.