Interview template validation #2836
Conversation
| } | ||
| } | ||
|
|
||
| export class InterviewTemplateValidator { |
There was a problem hiding this comment.
IMO we don't really need classes here, it can be just a function that performs checks and throws error if something is wrong
| descriptionHtml?: string; | ||
| } | ||
|
|
||
| export class QuestionCategoryValidator { |
There was a problem hiding this comment.
Same with this class, it can be just a part of the function I mentioned above
There was a problem hiding this comment.
Pull Request Overview
Adds validation for interview templates to ensure category and question IDs are unique, and fails the build with a clear error when duplicates are detected.
- Introduces TemplateValidationError, InterviewTemplateValidator, and QuestionCategoryValidator
- Validates templates during module initialization; adds unit tests for validators
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| client/src/data/interviews/templateValidator.ts | Adds validation classes and error type to enforce unique category/question IDs |
| client/src/data/interviews/index.ts | Instantiates validators for each template during export to fail builds on duplicates |
| client/src/data/interviews/testes/templateValidator.test.ts | Adds unit tests for validation behavior and error messaging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| const uniqueQuestionIds = new Set<number>(questions?.map(q => q.id) ?? []); | ||
|
|
||
| if (uniqueQuestionIds.size !== questions?.length) { |
There was a problem hiding this comment.
If questions is undefined, questions?.length yields undefined, so 0 !== undefined evaluates to true and incorrectly throws. Guard the check when questions is provided, or coalesce length to 0.
| if (uniqueQuestionIds.size !== questions?.length) { | |
| if (uniqueQuestionIds.size !== (questions?.length ?? 0)) { |
Copilot uses AI. Check for mistakes.
| corejs1: new InterviewTemplateValidator(corejs1Template), | ||
| corejs2: new InterviewTemplateValidator(corejs2Template), | ||
| react: new InterviewTemplateValidator(reactTemplate), | ||
| angular: new InterviewTemplateValidator(angularTemplate), | ||
| shortTrackScreening: new InterviewTemplateValidator(shortTrackScreeningTemplate), | ||
| shortTrackJavaScript: new InterviewTemplateValidator(shortTrackJavaScriptTemplate), | ||
| shortTrackTypeScript: new InterviewTemplateValidator(shortTrackTypeScriptTemplate), | ||
| shortTrackPerformance: new InterviewTemplateValidator(shortTrackPerformanceTemplate), |
There was a problem hiding this comment.
Replacing template objects with InterviewTemplateValidator instances changes the exported API shape and inferred type of templates, potentially breaking consumers that expect InterviewTemplate. Validate as a side-effect but export the original template objects (and annotate templates as Record<string, InterviewTemplate>) to preserve the public API.
Copilot uses AI. Check for mistakes.
| questions: [mockQuestion1, mockQuestionDuplicateId], // IDs are 101, 101 | ||
| }; | ||
|
|
||
| describe('TemplateValidationError', () => { |
There was a problem hiding this comment.
The folder name 'testes' appears to be a typo; rename to 'tests' so standard test runners detect and execute these tests.
Copilot uses AI. Check for mistakes.
| categories.forEach(category => { | ||
| return new QuestionCategoryValidator(category); | ||
| }); |
There was a problem hiding this comment.
[nitpick] The return inside forEach is unnecessary and misleading; the callback's return value is ignored. Simplify to categories.forEach(category => new QuestionCategoryValidator(category)).
| categories.forEach(category => { | |
| return new QuestionCategoryValidator(category); | |
| }); | |
| categories.forEach(category => new QuestionCategoryValidator(category)); |
Copilot uses AI. Check for mistakes.
|
Please follow https://github.com/rolling-scopes/rsschool-app/blob/master/CONTRIBUTING.md#pull-requests
|
3 participants
Issue:
Description:
InterviewTemplateValidatorandQuestionCategoryValidatorclasses to validate unique category and question IDs.next build) will now fail with aTemplateValidationErrorif duplicates are found in an interview template.TemplateValidationErrorfor duplicate category IDs includes the template name in the error message.TemplateValidationErrorfor duplicate question IDs includes the category ID.Usage
Self-Check