Merge pull request #380 from fujaba/fix/assignments

Assignments Bugfixes
fujaba · Nov 7, 2023 · 18b18ae · 18b18ae
2 parents 1c79a1c + 1ea1388
commit 18b18ae
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 67 deletions.
diff --git a/services/apps/assignments/src/assignment/assignment.service.ts b/services/apps/assignments/src/assignment/assignment.service.ts
@@ -3,7 +3,7 @@ import {UserToken} from '@app/keycloak-auth';
 import {Injectable} from '@nestjs/common';
 import {InjectModel} from '@nestjs/mongoose';
 import {FilterQuery, Model, UpdateQuery} from 'mongoose';
-import {generateToken, idFilter} from '../utils';
+import {generateToken} from '../utils';
 import {CreateAssignmentDto, ReadAssignmentDto, ReadTaskDto, UpdateAssignmentDto} from './assignment.dto';
 import {Assignment, AssignmentDocument, Task} from './assignment.schema';
 import {MemberService} from "@app/member";
@@ -42,11 +42,14 @@ export class AssignmentService {
   }
 
   async findAll(where: FilterQuery<Assignment> = {}): Promise<AssignmentDocument[]> {
-    return this.model.find(where).sort({title: 1}).exec();
+    return this.model.find(where).sort({title: 1}).collation({
+      locale: 'en',
+      numericOrdering: true,
+    }).exec();
   }
 
   async findOne(id: string): Promise<AssignmentDocument | null> {
-    return this.model.findOne(idFilter(id)).exec();
+    return this.model.findById(id).exec();
   }
 
   mask(assignment: Assignment): ReadAssignmentDto {
@@ -77,13 +80,13 @@ export class AssignmentService {
         update[`classroom.${key}`] = value;
       }
     }
-    const updated = await this.model.findOneAndUpdate(idFilter(id), update, {new: true}).exec();
+    const updated = await this.model.findByIdAndUpdate(id, update, {new: true}).exec();
     updated && this.emit('updated', updated);
     return updated;
   }
 
   async remove(id: string): Promise<AssignmentDocument | null> {
-    const deleted = await this.model.findOneAndDelete(idFilter(id)).exec();
+    const deleted = await this.model.findByIdAndDelete(id).exec();
     deleted && this.emit('deleted', deleted);
     return deleted;
   }

diff --git a/services/apps/assignments/src/comment/comment.service.ts b/services/apps/assignments/src/comment/comment.service.ts
@@ -3,7 +3,6 @@ import {UserToken} from '@app/keycloak-auth';
 import {Injectable} from '@nestjs/common';
 import {InjectModel} from '@nestjs/mongoose';
 import {FilterQuery, Model} from 'mongoose';
-import {idFilter} from '../utils';
 import {CreateCommentDto, UpdateCommentDto} from './comment.dto';
 import {Comment, CommentDocument} from './comment.schema';
 
@@ -34,17 +33,17 @@ export class CommentService {
   }
 
   async findOne(id: string): Promise<CommentDocument | null> {
-    return this.model.findOne(idFilter(id)).exec();
+    return this.model.findById(id).exec();
   }
 
   async update(id: string, dto: UpdateCommentDto): Promise<Comment | null> {
-    const updated = await this.model.findOneAndUpdate(idFilter(id), dto, {new: true}).exec();
+    const updated = await this.model.findByIdAndUpdate(id, dto, {new: true}).exec();
     updated && this.emit('updated', updated);
     return updated;
   }
 
   async remove(id: string): Promise<CommentDocument | null> {
-    const deleted = await this.model.findOneAndDelete(idFilter(id)).exec();
+    const deleted = await this.model.findByIdAndDelete(id).exec();
     deleted && this.emit('deleted', deleted);
     return deleted;
   }

diff --git a/services/apps/assignments/src/course-member/course-auth.guard.ts b/services/apps/assignments/src/course-member/course-auth.guard.ts
@@ -5,6 +5,7 @@ import {Observable} from 'rxjs';
 import {notFound} from '@mean-stream/nestx';
 import {CourseService} from "../course/course.service";
 import {MemberService} from "@app/member";
+import {Types} from "mongoose";
 
 @Injectable()
 export class CourseAuthGuard implements CanActivate {
@@ -22,7 +23,7 @@ export class CourseAuthGuard implements CanActivate {
   }
 
   async checkAuth(id: string, user: UserToken): Promise<boolean> {
-    const course = await this.courseService.findOne(id) ?? notFound(id);
+    const course = await this.courseService.find(new Types.ObjectId(id)) ?? notFound(id);
     return course.createdBy === user.sub || !!await this.memberService.findOne({
       parent: course._id,
       user: user.sub,

diff --git a/services/apps/assignments/src/course/course.controller.ts b/services/apps/assignments/src/course/course.controller.ts
@@ -1,12 +1,12 @@
 import {Auth, AuthUser, UserToken} from '@app/keycloak-auth';
-import {NotFound} from '@mean-stream/nestx';
+import {NotFound, ObjectIdPipe} from '@mean-stream/nestx';
 import {Body, Controller, Delete, Get, Param, ParseArrayPipe, Patch, Post, Query} from '@nestjs/common';
 import {ApiCreatedResponse, ApiOkResponse, ApiOperation, ApiTags} from '@nestjs/swagger';
 import {CourseStudent, CreateCourseDto, UpdateCourseDto} from './course.dto';
 import {Course} from './course.schema';
 import {CourseService} from './course.service';
 import {CourseAuth} from "../course-member/course-auth.decorator";
-import {FilterQuery} from "mongoose";
+import {FilterQuery, Types} from "mongoose";
 import {MemberService} from "@app/member";
 
 const forbiddenResponse = 'Not owner.';
@@ -27,7 +27,7 @@ export class CourseController {
     @Body() dto: CreateCourseDto,
     @AuthUser() user: UserToken,
   ): Promise<Course> {
-    return this.courseService.create(dto, user.sub);
+    return this.courseService.create({...dto, createdBy: user.sub});
   }
 
   @Get()
@@ -50,8 +50,10 @@ export class CourseController {
   @Get(':id')
   @NotFound()
   @ApiOkResponse({type: Course})
-  async findOne(@Param('id') id: string): Promise<Course | null> {
-    return this.courseService.findOne(id);
+  async findOne(
+    @Param('id', ObjectIdPipe) id: Types.ObjectId,
+  ): Promise<Course | null> {
+    return this.courseService.find(id);
   }
 
   @Get(':id/students')
@@ -60,17 +62,18 @@ export class CourseController {
   @NotFound()
   @ApiOkResponse({type: [CourseStudent]})
   async getStudents(
-    @Param('id') id: string,
+    @Param('id', ObjectIdPipe) id: Types.ObjectId,
+    @AuthUser() user: UserToken,
   ): Promise<CourseStudent[]> {
-    return this.courseService.getStudents(id);
+    return this.courseService.getStudents(id, user.sub);
   }
 
   @Patch(':id')
   @CourseAuth({forbiddenResponse})
   @NotFound()
   @ApiOkResponse({type: Course})
   async update(
-    @Param('id') id: string,
+    @Param('id', ObjectIdPipe) id: Types.ObjectId,
     @Body() dto: UpdateCourseDto,
   ): Promise<Course | null> {
     return this.courseService.update(id, dto);
@@ -81,8 +84,8 @@ export class CourseController {
   @NotFound()
   @ApiOkResponse({type: Course})
   async remove(
-    @Param('id') id: string,
+    @Param('id', ObjectIdPipe) id: Types.ObjectId,
   ): Promise<Course | null> {
-    return this.courseService.remove(id);
+    return this.courseService.delete(id);
   }
 }
diff --git a/services/apps/assignments/src/course/course.service.ts b/services/apps/assignments/src/course/course.service.ts
@@ -1,47 +1,43 @@
-import {EventService} from '@mean-stream/nestx';
+import {EventRepository, EventService, MongooseRepository} from '@mean-stream/nestx';
 import {Injectable} from '@nestjs/common';
 import {InjectModel} from '@nestjs/mongoose';
-import {FilterQuery, Model} from 'mongoose';
+import {Model, Types} from 'mongoose';
 import {AuthorInfo} from '../solution/solution.schema';
 import {SolutionService} from '../solution/solution.service';
-import {idFilter} from '../utils';
-import {CourseStudent, CreateCourseDto, UpdateCourseDto} from './course.dto';
+import {CourseStudent} from './course.dto';
 import {Course, CourseDocument} from './course.schema';
+import {MemberService} from "@app/member";
 
 @Injectable()
-export class CourseService {
+@EventRepository()
+export class CourseService extends MongooseRepository<Course> {
   constructor(
-    @InjectModel(Course.name) private model: Model<Course>,
+    @InjectModel(Course.name) model: Model<Course>,
     private solutionService: SolutionService,
     private eventService: EventService,
+    private memberService: MemberService,
   ) {
+    super(model);
   }
 
-  async create(dto: CreateCourseDto, userId?: string): Promise<CourseDocument> {
-    const created = await this.model.create({
-      ...dto,
-      createdBy: userId,
-    });
-    this.emit('created', created);
-    return created;
-  }
-
-  async findAll(filter: FilterQuery<Course> = {}): Promise<CourseDocument[]> {
-    return this.model.find(filter).exec();
-  }
-
-  async findOne(id: string): Promise<CourseDocument | null> {
-    return this.model.findOne(idFilter(id)).exec();
-  }
-
-  async getStudents(id: string): Promise<CourseStudent[]> {
-    const course = await this.findOne(id);
+  async getStudents(id: Types.ObjectId, user: string): Promise<CourseStudent[]> {
+    const course = await this.find(id);
     if (!course) {
       return [];
     }
+
+    const userMembers = await this.memberService.findAll({
+      parent: {$in: course.assignments.map(a => new Types.ObjectId(a))},
+      user,
+    });
+    const courseAssignmentsWhereUserIsMember = userMembers.map(m => m.parent.toString());
+    if (!courseAssignmentsWhereUserIsMember.length) {
+      return [];
+    }
+
     const students = new Map<string, CourseStudent>();
     const solutions = await this.solutionService.model.aggregate([
-      {$match: {assignment: {$in: course.assignments}}},
+      {$match: {assignment: {$in: courseAssignmentsWhereUserIsMember}}},
       {$addFields: {id: {$toString: '$_id'}}},
       {
         $lookup: {
@@ -107,19 +103,7 @@ export class CourseService {
     return Array.from(new Set(students.values()));
   }
 
-  async update(id: string, dto: UpdateCourseDto): Promise<Course | null> {
-    const updated = await this.model.findOneAndUpdate(idFilter(id), dto, {new: true}).exec();
-    updated && this.emit('updated', updated);
-    return updated;
-  }
-
-  async remove(id: string): Promise<CourseDocument | null> {
-    const deleted = await this.model.findOneAndDelete(idFilter(id)).exec();
-    deleted && this.emit('deleted', deleted);
-    return deleted;
-  }
-
-  private emit(event: string, course: CourseDocument) {
+  emit(event: string, course: CourseDocument) {
     this.eventService.emit(`courses.${course.id}.${event}`, course);
   }
 }
diff --git a/services/apps/assignments/src/solution/solution.service.ts b/services/apps/assignments/src/solution/solution.service.ts
@@ -3,7 +3,7 @@ import {UserToken} from '@app/keycloak-auth';
 import {Injectable} from '@nestjs/common';
 import {InjectModel} from '@nestjs/mongoose';
 import {FilterQuery, Model, UpdateQuery} from 'mongoose';
-import {generateToken, idFilter} from '../utils';
+import {generateToken} from '../utils';
 import {BatchUpdateSolutionDto, CreateSolutionDto, ReadSolutionDto, UpdateSolutionDto} from './solution.dto';
 import {Solution, SolutionDocument} from './solution.schema';
 
@@ -37,11 +37,11 @@ export class SolutionService {
   }
 
   async findOne(id: string): Promise<SolutionDocument | null> {
-    return this.model.findOne(idFilter(id)).exec();
+    return this.model.findById(id).exec();
   }
 
   async update(id: string, dto: UpdateSolutionDto): Promise<SolutionDocument | null> {
-    const updated = await this.model.findOneAndUpdate(idFilter(id), dto, {new: true}).exec();
+    const updated = await this.model.findByIdAndUpdate(id, dto, {new: true}).exec();
     updated && this.emit('updated', updated);
     return updated;
   }
@@ -83,7 +83,7 @@ export class SolutionService {
   }
 
   async remove(id: string): Promise<SolutionDocument | null> {
-    const deleted = await this.model.findOneAndDelete(idFilter(id)).exec();
+    const deleted = await this.model.findByIdAndDelete(id).exec();
     deleted && this.emit('deleted', deleted);
     return deleted;
   }

diff --git a/services/apps/assignments/src/utils.ts b/services/apps/assignments/src/utils.ts
@@ -11,10 +11,6 @@ export function generateToken(): string {
   return new Array(4).fill(0).map((_, index) => hex.substr(index * 4, 4)).join('-');
 }
 
-export function idFilter(id: string): FilterQuery<any> {
-  return isUUID(id) ? {id} : {_id: id};
-}
-
 export function eventStream<T>(source: Observable<WsResponse<T>>, name: string): Observable<MessageEvent> {
   return merge(
     source.pipe(