feat: add unit tests for model classes (HeuristicQuestionAnswer, StudyAnswer, UserStudyEvaluatorAnswer)#1834
feat: add unit tests for model classes (HeuristicQuestionAnswer, StudyAnswer, UserStudyEvaluatorAnswer)#1834AAdIprog wants to merge 2 commits intoruxailab:developfrom
Conversation
…yAnswer, UserStudyEvaluatorAnswer)
github-actions
bot
added
medium-complexity
new-feature
|
There was a problem hiding this comment.
Pull request overview
Adds new Jest unit tests for key Firestore-serialization model classes and also hardens several v-html rendering points in the UserTest UX by sanitizing HTML via DOMPurify.
Changes:
- Add unit tests for
HeuristicQuestionAnswer,StudyAnswer, andUserStudyEvaluatorAnswermodel behaviors (constructors/factories/toFirestore). - Sanitize several UI fields rendered with
v-htmlusing DOMPurify to reduce XSS risk. - Add
dompurifyas an application dependency (updates lockfile accordingly).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/UserStudyEvaluatorAnswer.spec.js | New unit tests for defaults, toFirestore() task serialization, and toModel(). |
| tests/unit/StudyAnswer.spec.js | New unit tests for constructor + toFirestore() null/undefined handling. |
| tests/unit/HeuristicQuestionAnswer.spec.js | New unit tests for constructor, toFirestore(), and toHeuristicQuestionAnswer() hotfix behavior. |
| src/ux/UserTest/components/transcription/ExportPanel.vue | Sanitize executive summary HTML before rendering in preview. |
| src/ux/UserTest/components/task-steps/TaskPreview.vue | Sanitize task description HTML before rendering. |
| src/ux/UserTest/components/steps/WelcomeStep.vue | Sanitize welcome message HTML before rendering. |
| src/ux/UserTest/components/steps/TaskStep.vue | Sanitize task description HTML before rendering in both places it’s displayed. |
| src/ux/UserTest/components/steps/FinishStep.vue | Sanitize “congratulations” HTML before rendering. |
| src/ux/UserTest/components/steps/ConsentStep.vue | Sanitize consent HTML before rendering. |
| package.json | Add dompurify dependency. |
| package-lock.json | Lockfile updates for DOMPurify and dependency graph changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "chartjs-adapter-date-fns": "^3.0.0", | ||
| "crypto-browserify": "^3.12.1", | ||
| "date-fns": "^4.1.0", | ||
| "dompurify": "^3.3.2", | ||
| "firebase": "^9.23.0", |
There was a problem hiding this comment.
This PR’s title/description focuses on adding unit tests, but this change also introduces DOMPurify as a new runtime dependency (and several UI sanitization changes). Please update the PR description/title to reflect the broader scope, or split the sanitization work into a separate PR to keep review/rollback risk isolated.
| "date-fns": "^4.1.0", | ||
| "dompurify": "^3.3.2", | ||
| "firebase": "^9.23.0", |
There was a problem hiding this comment.
DOMPurify v3.3.2 declares an engines constraint of Node >=20 (see package-lock.json). Since package.json doesn’t specify an engines field, developers/CI using older Node versions may get hard-to-diagnose install failures. Consider declaring a minimum supported Node version in package.json (or using a DOMPurify version compatible with your supported Node range).
|
|
||
| const controller = new TranscriptionController() | ||
| const sanitizedPdfSummaryHtml = computed(() => DOMPurify.sanitize(pdfSummaryHtml.value || '')) | ||
|
|
There was a problem hiding this comment.
The preview now uses DOMPurify, but the PDF generation path still calls stripHtml(pdfSummaryHtml.value) which assigns the (unsanitized) HTML into innerHTML. To keep a single trust boundary, consider reusing the sanitized HTML for all downstream processing (preview + strip-to-text), or sanitize inside stripHtml before assigning to innerHTML.
| // Ensure pdfSummaryHtml is always sanitized before any downstream usage (e.g. stripHtml / PDF export) | |
| watch( | |
| pdfSummaryHtml, | |
| newVal => { | |
| const safeHtml = DOMPurify.sanitize(newVal || '') | |
| if (safeHtml !== (newVal || '')) { | |
| pdfSummaryHtml.value = safeHtml | |
| } | |
| }, | |
| { immediate: true }, | |
| ) |
Closes #1831
Summary
Adds unit test coverage for 3 model classes that previously had zero tests. These models handle Firestore serialization — bugs here can silently corrupt stored data.
Changes
Testing
All 117 tests pass (93 existing + 24 new):
![
]