Skip to content

Commit 04946de

Browse files
authored
Merge pull request #728 from Penielka/fix/penielka-issues-615-course-dto-validation
fix(backend): enforce course DTO validation and canonical taxonomy (BA-047)
2 parents 03f77cb + 7b6a559 commit 04946de

5 files changed

Lines changed: 366 additions & 10 deletions

File tree

BackendAcademy/src/courses/course.service.spec.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,84 @@ describe('CourseService', () => {
381381
});
382382

383383
// ---------------------------------------------------------------------------
384+
// ---------------------------------------------------------------------------
385+
// Taxonomy normalization (BA-047)
386+
// ---------------------------------------------------------------------------
387+
388+
it('normalizes free-text fields on create', async () => {
389+
const course = await service.create({
390+
title: ' Rust Basics ',
391+
description: ' An intro to Rust. ',
392+
level: 'INTERMEDIATE' as CourseLevel,
393+
order: 1,
394+
learningPathId: 'path-1',
395+
duration: 30,
396+
});
397+
398+
expect(course.title).toBe('Rust Basics');
399+
expect(course.description).toBe('An intro to Rust.');
400+
expect(course.level).toBe(CourseLevel.INTERMEDIATE);
401+
expect(course.slug).toBe('rust-basics');
402+
});
403+
404+
it('canonicalizes taxonomy arrays (trim, lowercase, dedupe, drop blanks) on create', async () => {
405+
const course = await service.create({
406+
title: 'Rust Basics',
407+
description: 'An intro',
408+
level: CourseLevel.BEGINNER,
409+
order: 1,
410+
learningPathId: 'path-1',
411+
duration: 30,
412+
category: 'WASM',
413+
categories: ['WASM', ' Rust ', 'Rust', ' '],
414+
tags: [' Ownership ', 'ownership'],
415+
prerequisites: [' borrowed ', 'lifetimes'],
416+
skills: ['Memory Safety', 'memory safety'],
417+
});
418+
419+
expect(course.category).toBe('wasm');
420+
expect(course.categories).toEqual(['wasm', 'rust']);
421+
expect(course.tags).toEqual(['ownership']);
422+
expect(course.prerequisites).toEqual(['borrowed', 'lifetimes']);
423+
expect(course.skills).toEqual(['memory safety']);
424+
});
425+
426+
it('normalizes taxonomy fields on update only when provided', async () => {
427+
const course = await service.create({
428+
title: 'Rust Basics',
429+
description: 'An intro',
430+
level: CourseLevel.BEGINNER,
431+
order: 1,
432+
learningPathId: 'path-1',
433+
duration: 30,
434+
tags: [' original '],
435+
prerequisites: ['stay'],
436+
});
437+
438+
const updated = await service.update(course.id, { skills: [' WASM ', 'wasm'] });
439+
440+
// Fields that were not part of the update payload must be untouched.
441+
expect(updated!.tags).toEqual(['original']);
442+
expect(updated!.prerequisites).toEqual(['stay']);
443+
// Provided taxonomy is canonicalized.
444+
expect(updated!.skills).toEqual(['wasm']);
445+
});
446+
447+
it('keeps the canonical title bound reflected in the persisted slug on update', async () => {
448+
const course = await service.create({
449+
title: 'Rust Basics',
450+
description: 'An intro',
451+
level: CourseLevel.BEGINNER,
452+
order: 1,
453+
learningPathId: 'path-1',
454+
duration: 30,
455+
});
456+
457+
const updated = await service.update(course.id, { title: ' Rust Advanced ' });
458+
expect(updated!.title).toBe('Rust Advanced');
459+
expect(updated!.slug).toBe('rust-advanced');
460+
});
461+
384462
// Revision lookup
385463
// ---------------------------------------------------------------------------
386464

BackendAcademy/src/courses/course.service.ts

Lines changed: 98 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Injectable, Logger, NotFoundException, Optional, ConflictException } fr
22
import { InjectRepository } from '@nestjs/typeorm';
33
import { Repository } from 'typeorm';
44
import { CourseEntity } from './course.entity';
5+
import { CourseLevel } from './interfaces/course-level.enum';
56
import {
67
CourseRevisionEntity,
78
CourseRevisionReason,
@@ -71,12 +72,14 @@ export class CourseService {
7172
* observe a course without an audit trail entry.
7273
*/
7374
async create(dto: CreateCourseDto): Promise<CourseEntity> {
74-
const slug = await this.createUniqueSlug(dto.title);
75+
// BA-047: bound + canonicalize taxonomy before anything is persisted.
76+
const normalized = this.normalizeCourseInput(dto);
77+
const slug = await this.createUniqueSlug(normalized.title ?? dto.title);
7578
const course = this.courseRepo.create({
7679
id: crypto.randomUUID(),
7780
version: CourseService.INITIAL_VERSION,
7881
slug,
79-
...dto,
82+
...normalized,
8083
});
8184

8285
const txResult = await this.transactionManager.runAtomic(async (tx) => {
@@ -128,14 +131,17 @@ export class CourseService {
128131
const course = await this.courseRepo.findOne({ where: { id } });
129132
if (!course) return null;
130133

134+
// BA-047: bound + canonicalize taxonomy so only canonical values persist.
135+
const normalized = this.normalizeCourseInput(dto);
136+
131137
const previousVersion = course.version;
132138
course.version = previousVersion + 1;
133139
course.updatedAt = new Date();
134-
Object.assign(course, dto);
135-
if (dto.title !== undefined) {
136-
course.slug = await this.createUniqueSlug(dto.title, course.id);
140+
Object.assign(course, normalized);
141+
if (normalized.title !== undefined) {
142+
course.slug = await this.createUniqueSlug(normalized.title, course.id);
137143
}
138-
this.syncCourseTaxonomy(course, dto);
144+
this.syncCourseTaxonomy(course, normalized);
139145
const saved = await this.courseRepo.save(course);
140146

141147
// #449: Rollback the course update if the revision append fails
@@ -524,6 +530,92 @@ export class CourseService {
524530
}
525531
}
526532

533+
/**
534+
* BA-047: Canonicalize course taxonomy input so that only bounded,
535+
* normalized values reach persistence.
536+
*
537+
* - Free-text fields (title, description, category) are trimmed and
538+
* inner whitespace is collapsed.
539+
* - The enum level is lower-cased to its canonical `CourseLevel` value.
540+
* - Taxonomy arrays (categories, tags, prerequisites, skills) are
541+
* trimmed, lower-cased, de-duplicated, and have blank items removed.
542+
*/
543+
private normalizeCourseInput<T extends Partial<CreateCourseDto>>(
544+
input: T,
545+
): Partial<CourseEntity> {
546+
const normalized: Partial<CourseEntity> = { ...input } as Partial<
547+
CourseEntity
548+
>;
549+
550+
if (typeof normalized.title === 'string') {
551+
normalized.title = this.collapseWhitespace(normalized.title);
552+
}
553+
if (typeof normalized.description === 'string') {
554+
normalized.description = normalized.description.trim();
555+
}
556+
if (typeof normalized.category === 'string') {
557+
normalized.category = this.normalizeTaxonomyItem(normalized.category);
558+
}
559+
if (typeof normalized.level === 'string') {
560+
normalized.level = this.canonicalizeLevel(normalized.level);
561+
}
562+
563+
// Only normalize taxonomy arrays that were actually provided, so an
564+
// absent field on update never overwrites already-persisted values with
565+
// an empty/undefined array.
566+
if (Array.isArray(input.categories)) {
567+
normalized.categories = this.normalizeTaxonomy(input.categories);
568+
}
569+
if (Array.isArray(input.tags)) {
570+
normalized.tags = this.normalizeTaxonomy(input.tags);
571+
}
572+
if (Array.isArray(input.prerequisites)) {
573+
normalized.prerequisites = this.normalizeTaxonomy(input.prerequisites);
574+
}
575+
if (Array.isArray(input.skills)) {
576+
normalized.skills = this.normalizeTaxonomy(input.skills);
577+
}
578+
579+
return normalized;
580+
}
581+
582+
private collapseWhitespace(value: string): string {
583+
return value.trim().replace(/\s+/g, ' ');
584+
}
585+
586+
private normalizeTaxonomyItem(value: string): string {
587+
return value
588+
.trim()
589+
.toLowerCase()
590+
.replace(/\s+/g, ' ');
591+
}
592+
593+
private normalizeTaxonomy(values: string[]): string[] {
594+
const seen = new Set<string>();
595+
const result: string[] = [];
596+
for (const raw of values) {
597+
const item = this.normalizeTaxonomyItem(raw ?? '');
598+
if (item && !seen.has(item)) {
599+
seen.add(item);
600+
result.push(item);
601+
}
602+
}
603+
return result;
604+
}
605+
606+
private canonicalizeLevel(level: string): CourseLevel {
607+
const canonical = level.trim().toLowerCase();
608+
if (
609+
canonical === CourseLevel.BEGINNER ||
610+
canonical === CourseLevel.INTERMEDIATE ||
611+
canonical === CourseLevel.ADVANCED ||
612+
canonical === CourseLevel.WEB3
613+
) {
614+
return canonical as CourseLevel;
615+
}
616+
return level as CourseLevel;
617+
}
618+
527619
private async createUniqueSlug(title: string, excludeId?: string): Promise<string> {
528620
const baseSlug = this.normalizeSlug(title);
529621
let slug = baseSlug;
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { validate } from 'class-validator';
2+
import { CreateCourseDto } from './create-course.dto';
3+
import { UpdateCourseDto } from './update-course.dto';
4+
import { CourseLevel } from '../interfaces/course-level.enum';
5+
6+
describe('CreateCourseDto validation (BA-047)', () => {
7+
const valid = () => {
8+
const dto = new CreateCourseDto();
9+
Object.assign(dto, {
10+
title: 'Rust Basics',
11+
description: 'An intro to Rust.',
12+
level: CourseLevel.BEGINNER,
13+
order: 1,
14+
learningPathId: 'path-1',
15+
duration: 60,
16+
category: 'blockchain',
17+
categories: ['blockchain'],
18+
tags: ['ownership'],
19+
prerequisites: ['borrowing'],
20+
skills: ['memory-safety'],
21+
});
22+
return dto;
23+
};
24+
25+
it('accepts a valid payload', async () => {
26+
expect(await validate(valid())).toEqual([]);
27+
});
28+
29+
it('rejects a title that is too short', async () => {
30+
const dto = valid();
31+
dto.title = 'Ru';
32+
const errors = await validate(dto);
33+
expect(errors.some((e) => e.property === 'title')).toBe(true);
34+
});
35+
36+
it('rejects an over-long title', async () => {
37+
const dto = valid();
38+
dto.title = 'x'.repeat(121);
39+
const errors = await validate(dto);
40+
expect(errors.some((e) => e.property === 'title')).toBe(true);
41+
});
42+
43+
it('rejects a blank required field', async () => {
44+
const dto = valid();
45+
dto.description = '';
46+
const errors = await validate(dto);
47+
expect(errors.some((e) => e.property === 'description')).toBe(true);
48+
});
49+
50+
it('rejects an out-of-enum level', async () => {
51+
const dto = valid();
52+
dto.level = 'extreme' as CourseLevel;
53+
const errors = await validate(dto);
54+
expect(errors.some((e) => e.property === 'level')).toBe(true);
55+
});
56+
57+
it('rejects blank taxonomy items in skills', async () => {
58+
const dto = valid();
59+
dto.skills = [''];
60+
const errors = await validate(dto);
61+
expect(errors.some((e) => e.property === 'skills')).toBe(true);
62+
});
63+
64+
it('rejects an over-long taxonomy item in tags', async () => {
65+
const dto = valid();
66+
dto.tags = ['x'.repeat(61)];
67+
const errors = await validate(dto);
68+
expect(errors.some((e) => e.property === 'tags')).toBe(true);
69+
});
70+
71+
it('rejects taxonomy arrays that exceed the max item count', async () => {
72+
const dto = valid();
73+
dto.skills = Array.from({ length: 21 }, (_, i) => `skill-${i}`);
74+
const errors = await validate(dto);
75+
expect(errors.some((e) => e.property === 'skills')).toBe(true);
76+
});
77+
});
78+
79+
describe('UpdateCourseDto validation (BA-047)', () => {
80+
it('accepts a valid partial payload', async () => {
81+
const dto = new UpdateCourseDto();
82+
Object.assign(dto, { title: 'Rust Basics', skills: ['mem'] });
83+
expect(await validate(dto)).toEqual([]);
84+
});
85+
86+
it('rejects a title that is too short on update', async () => {
87+
const dto = new UpdateCourseDto();
88+
Object.assign(dto, { title: 'Ru' });
89+
const errors = await validate(dto);
90+
expect(errors.some((e) => e.property === 'title')).toBe(true);
91+
});
92+
93+
it('rejects an over-long taxonomy item in categories on update', async () => {
94+
const dto = new UpdateCourseDto();
95+
Object.assign(dto, { categories: ['c'.repeat(61)] });
96+
const errors = await validate(dto);
97+
expect(errors.some((e) => e.property === 'categories')).toBe(true);
98+
});
99+
});

0 commit comments

Comments
 (0)