Skip to content
This repository has been archived by the owner on Apr 16, 2024. It is now read-only.

Commit

Permalink
Merge branch 'feature/#964-code-kata-hide-submit' of https://github.c…
Browse files Browse the repository at this point in the history
…om/geli-lms/geli into feature/#964-code-kata-hide-submit
  • Loading branch information
David Boschmann committed Dec 17, 2018
2 parents 21abf73 + f6ceb64 commit 1028194
Show file tree
Hide file tree
Showing 35 changed files with 710 additions and 424 deletions.
21 changes: 19 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,40 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- More predefined custom containers. [#996](https://github.com/geli-lms/geli/issues/996)
- Styled code snippets. [#1017](https://github.com/geli-lms/geli/issues/1017)
- `LectureController` success (`200`), access denial (`403`) and not found (`404`) unit tests for all routes. [#1041](https://github.com/geli-lms/geli/issues/1041)
- Various `NotificationController` unit tests (`200`s, `400`s, `403`s, `404`s). [#1065](https://github.com/geli-lms/geli/issues/1065)
- `TestHelper` request methods for `PUT` & `DELETE`. [#1041](https://github.com/geli-lms/geli/issues/1041)
- CodeKata validation service. [#844](https://github.com/geli-lms/geli/issues/844)
- Course: Added switch for file <-> video unit. [#912](https://github.com/geli-lms/geli/issues/912)

### Fixed
- `bundle.scss` not available in api container. [#1052](https://github.com/geli-lms/geli/issues/1052)

### Changed
- Update `mongoose` to `5.3.x`. [#1003](https://github.com/geli-lms/geli/issues/1003) [#1004](https://github.com/geli-lms/geli/pull/1004) [#1044](https://github.com/geli-lms/geli/pull/1044)
- Refactor `LectureController` `GET`/`POST`/`PUT` routes to use `async`/`await`. [#1041](https://github.com/geli-lms/geli/issues/1041)
- Refactor `NotificationController` unit tests in general. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Refactor `NotificationController` to utilize `.orFail` and the `errorCodes` file. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Refactor `ExportController` & `LectureController` to utilize `.orFail`. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Sanitize `{post} /api/lecture/` route parameters by reducing the arbitrary `ILecture` input to `name` & `description`. [#1041](https://github.com/geli-lms/geli/issues/1041)
- Submit button for code kata will be disabled when deadline is over. [#964](https://github.com/geli-lms/geli/issues/964)
- Sanitize `NotificationController` `POST` route parameters by taking a `targetType` and `targetId` instead of the separate `changedCourse`/`changedLecture`/`changedUnit` which needed a _(missing)_ consistency check. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Empty success response object in the two `NotificationController` `POST` routes. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Disable unit submit button when deadline is over. [#964](https://github.com/geli-lms/geli/issues/964)

### Removed
- Unused `Notification` class in the front-end. [#1065](https://github.com/geli-lms/geli/issues/1065)

### Fixed
- Some incorrect `FixtureUtils` return types. [#1041](https://github.com/geli-lms/geli/issues/1041)
- Some incorrect `FixtureUtils` return types. [#1041](https://github.com/geli-lms/geli/issues/1041) [#1065](https://github.com/geli-lms/geli/issues/1065)
- `LectureController` `404` error handling. [#1041](https://github.com/geli-lms/geli/issues/1041)
- `NotificationController` `404` error handling. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Course list broken when course image in invalid state. [#1053](https://github.com/geli-lms/geli/issues/1053)

### Security
- Fix multiple security issues of the `LectureController`. [#1041](https://github.com/geli-lms/geli/issues/1041)
- Fix missing `teacher` authorization check for the two `NotificationController` `POST` routes. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Fix missing `NotificationController` `POST` `teacher` authorization check. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Fix `{get} /api/notification/` response leaks by introducing `INotificationView`, a reduced and safe variant of the `INotification` interface. [#1065](https://github.com/geli-lms/geli/issues/1065)
- Secure `{get} /api/notification/` by using the `@CurrentUser` instead of allowing arbitrary id requests. [#1065](https://github.com/geli-lms/geli/issues/1065)

## [[0.8.3](https://github.com/geli-lms/geli/releases/tag/v0.8.3)] - 2018-11-29 - WS 18/19 🚀-Release
### Added
Expand Down
2 changes: 1 addition & 1 deletion api/fixtures/FixtureUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class FixtureUtils {
return this.getRandom<IUnitModel>(array, hash);
}

public static async getRandomUnitFromLecture(lecture: ILecture, hash?: string): Promise<IUnit> {
public static async getRandomUnitFromLecture(lecture: ILecture, hash?: string): Promise<IUnitModel> {
const unitId = await this.getRandom<IUnit>(lecture.units, hash);
return Unit.findById(unitId);
}
Expand Down
12 changes: 6 additions & 6 deletions api/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"markdown-it-emoji": "^1.4.0",
"markdown-it-mark": "^2.0.0",
"migrate-mongoose": "^3.2.2",
"moment": "^2.22.1",
"moment": "^2.23.0",
"mongoose": "~5.3.16",
"morgan": "^1.9.1",
"multer": "^1.4.1",
Expand Down Expand Up @@ -88,7 +88,7 @@
"@types/passport-jwt": "^3.0.1",
"@types/passport-local": "^1.0.33",
"@types/raven": "^2.5.1",
"@types/validator": "^9.4.3",
"@types/validator": "^9.4.4",
"chai": "4.2.0",
"chai-http": "^4.2.0",
"coveralls": "^3.0.2",
Expand Down
22 changes: 22 additions & 0 deletions api/src/config/errorCodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,5 +147,27 @@ export const errorCodes = {
code: 'emptyQuery',
text: 'Query was empty.'
}
},
notification: {
missingCourseOfLecture: {
code: 'missingCourseOfLecture',
text: 'Course of given existing lecture is missing'
},
missingCourseOfUnit: {
code: 'missingCourseOfUnit',
text: 'Course of given existing unit is missing'
},
invalidTargetType: {
code: 'invalidTargetType',
text: 'targetType is invalid'
},
textOnlyWithoutText: {
code: 'textOnlyWithoutText',
text: 'Requested text-only notification creation without supplying any text'
},
targetUserNotFound: {
code: 'targetUserNotFound',
text: 'Target user not found'
}
}
};
85 changes: 66 additions & 19 deletions api/src/controllers/CourseController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {ICourse} from '../../../shared/models/ICourse';
import {ICourseDashboard} from '../../../shared/models/ICourseDashboard';
import {ICourseView} from '../../../shared/models/ICourseView';
import {IUser} from '../../../shared/models/IUser';
import {Course} from '../models/Course';
import {Course, ICourseModel} from '../models/Course';
import {WhitelistUser} from '../models/WhitelistUser';
import emailService from '../services/EmailService';

Expand All @@ -21,8 +21,7 @@ import * as fs from 'fs';
import * as path from 'path';
import ResponsiveImageService from '../services/ResponsiveImageService';
import {IResponsiveImageData} from '../../../shared/models/IResponsiveImageData';

import { Picture } from '../models/mediaManager/File';
import {Picture} from '../models/mediaManager/File';

const coursePictureUploadOptions = {
storage: multer.diskStorage({
Expand Down Expand Up @@ -681,18 +680,29 @@ export class CourseController {
return {};
}


/**
* @api {delete} /api/courses/picture/:id Delete course picture
* @apiName DeleteCoursePicture
* @apiGroup Course
*
* @apiParam {String} id Course ID.
* @apiParam {IUser} currentUser Currently logged in user.
*
* @apiSuccess {Object} Empty object.
*
* @apiSuccessExample {json} Success-Response:
* {}
*
* @apiError NotFoundError
* @apiError ForbiddenError
*/
@Authorized(['teacher', 'admin'])
@Delete('/picture/:id')
async deleteCoursePicture(
@Param('id') id: string,
@CurrentUser() currentUser: IUser) {

const course = await Course.findById(id);

if (!course) {
throw new NotFoundError();
}
const course = await Course.findById(id).orFail(new NotFoundError());

if (!course.checkPrivileges(currentUser).userCanEditCourse) {
throw new ForbiddenError();
Expand All @@ -702,13 +712,53 @@ export class CourseController {
throw new NotFoundError();
}

const picture = await Picture.findOne(course.image);
await picture.remove();
const picture = await Picture.findById(course.image);
if (picture) {
picture.remove();
}

await Course.updateOne({_id: id}, {$unset: {image: 1}});
return {};
}

/**
* @api {post} /api/courses/picture/:id Add course picture
* @apiName AddCoursePicture
* @apiGroup Course
*
* @apiParam {String} id Course ID.
* @apiParam responsiveImageDataRaw Image as data object.
* @apiParam {IUser} currentUser Currently logged in user.
*
* @apiSuccess {Object} Empty object.
*
* @apiSuccessExample {json} Success-Response:
* {
* "breakpoints":[
* {
* "screenSize":0,
* "imageSize":{
* "width":284,
* "height":190
* },
* "pathToImage":"uploads/courses/5c0fa2770315e73d6c7babfe_1544542544919_0.jpg",
* "pathToRetinaImage":"uploads/courses/[email protected]"
* }
* ],
* "_id":"5c0fd95871707a3a888ae70a",
* "__t":"Picture",
* "name":"5c0fa2770315e73d6c7babfe_1544542544919.jpg",
* "link":"-",
* "size":0,
* "mimeType":"image/jpeg",
* "createdAt":"2018-12-11T15:35:52.423Z",
* "updatedAt":"2018-12-11T15:35:52.423Z",
* "__v":0
* }
*
* @apiError NotFoundError
* @apiError ForbiddenError
*/
@Authorized(['teacher', 'admin'])
@Post('/picture/:id')
async addCoursePicture(
Expand All @@ -718,27 +768,25 @@ export class CourseController {
@CurrentUser() currentUser: IUser) {

// Remove the old picture if the course already has one.
const course = await Course.findById(id);
const course = await Course.findById(id).orFail(new NotFoundError());

if (!course.checkPrivileges(currentUser).userCanEditCourse) {
throw new ForbiddenError();
}

const responsiveImageData = <IResponsiveImageData>JSON.parse(responsiveImageDataRaw.imageData);

const mimeFamily = file.mimetype.split('/', 1)[0];
if (mimeFamily !== 'image') {
// Remove the file if the type was not correct.
await fs.unlinkSync(file.path);

throw new BadRequestError('Forbidden format of uploaded picture: ' + mimeFamily);
}

if (course.image) {
const picture = await Picture.findById(course.image);
await picture.remove();
const picture = await Picture.findById(course.image);
if (picture) {
picture.remove();
}

const responsiveImageData = <IResponsiveImageData>JSON.parse(responsiveImageDataRaw.imageData);
await ResponsiveImageService.generateResponsiveImages(file, responsiveImageData);

const image: any = new Picture({
Expand All @@ -749,7 +797,6 @@ export class CourseController {
mimeType: file.mimetype,
breakpoints: responsiveImageData.breakpoints
});

await image.save();

course.image = image;
Expand Down
15 changes: 3 additions & 12 deletions api/src/controllers/ExportController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ export class ExportController {
*/
@Get('/course/:id')
async exportCourse(@Param('id') id: string, @CurrentUser() currentUser: IUser) {
const course = await Course.findById(id);
if (!course) {
throw new NotFoundError();
}
const course = await Course.findById(id).orFail(new NotFoundError());
ExportController.assertUserExportAuthorization(currentUser, course);
return course.exportJSON();
}
Expand Down Expand Up @@ -85,10 +82,7 @@ export class ExportController {
*/
@Get('/lecture/:id')
async exportLecture(@Param('id') id: string, @CurrentUser() currentUser: IUser) {
const lecture = await Lecture.findById(id);
if (!lecture) {
throw new NotFoundError();
}
const lecture = await Lecture.findById(id).orFail(new NotFoundError());
const course = await Course.findOne({lectures: id});
ExportController.assertUserExportAuthorization(currentUser, course);
return lecture.exportJSON();
Expand Down Expand Up @@ -120,10 +114,7 @@ export class ExportController {
*/
@Get('/unit/:id')
async exportUnit(@Param('id') id: string, @CurrentUser() currentUser: IUser) {
const unit = await Unit.findById(id);
if (!unit) {
throw new NotFoundError();
}
const unit = await Unit.findById(id).orFail(new NotFoundError());
const course = await Course.findById(unit._course);
ExportController.assertUserExportAuthorization(currentUser, course);
return unit.exportJSON();
Expand Down
20 changes: 4 additions & 16 deletions api/src/controllers/LectureController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ export class LectureController {
*/
@Get('/:id')
async getLecture(@Param('id') id: string, @CurrentUser() currentUser: IUser) {
const lecture = await Lecture.findById(id);
if (!lecture) {
throw new NotFoundError();
}
const lecture = await Lecture.findById(id).orFail(new NotFoundError());
const course = await Course.findOne({lectures: id});
if (!course.checkPrivileges(currentUser).userCanViewCourse) {
throw new ForbiddenError();
Expand Down Expand Up @@ -78,10 +75,7 @@ export class LectureController {
@BodyParam('description', {required: true}) description: string,
@BodyParam('courseId', {required: true}) courseId: string,
@CurrentUser() currentUser: IUser) {
const course = await Course.findById(courseId);
if (!course) {
throw new NotFoundError();
}
const course = await Course.findById(courseId).orFail(new NotFoundError());
if (!course.checkPrivileges(currentUser).userCanEditCourse) {
throw new ForbiddenError();
}
Expand Down Expand Up @@ -121,10 +115,7 @@ export class LectureController {
@Authorized(['teacher', 'admin'])
@Put('/:id')
async updateLecture(@Param('id') id: string, @Body() lectureUpdate: ILecture, @CurrentUser() currentUser: IUser) {
const course = await Course.findOne({lectures: id});
if (!course) {
throw new NotFoundError();
}
const course = await Course.findOne({lectures: id}).orFail(new NotFoundError());
if (!course.checkPrivileges(currentUser).userCanEditCourse) {
throw new ForbiddenError();
}
Expand Down Expand Up @@ -152,10 +143,7 @@ export class LectureController {
@Authorized(['teacher', 'admin'])
@Delete('/:id')
async deleteLecture(@Param('id') id: string, @CurrentUser() currentUser: IUser) {
const course = await Course.findOne({lectures: id});
if (!course) {
throw new NotFoundError();
}
const course = await Course.findOne({lectures: id}).orFail(new NotFoundError());
if (!course.checkPrivileges(currentUser).userCanEditCourse) {
throw new ForbiddenError();
}
Expand Down
Loading

0 comments on commit 1028194

Please sign in to comment.