Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add shared attribute, add markshared mutation, add templateCour… #914

Merged
merged 31 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
fb13997
feat: add shared attribute, add markshared mutation, add templateCour…
paul-jaworski Dec 2, 2023
1837993
fix: fix authorization
paul-jaworski Dec 2, 2023
4bbd331
feat: add logging to courseMarkShared mutation
paul-jaworski Dec 2, 2023
7ed6d73
add shared to public fiields
paul-jaworski Dec 2, 2023
17f52f1
integrate courseSearch method call in templateCourses mutation
paul-jaworski Dec 2, 2023
ace89df
Merge branch 'master' into feat-974
paul-jaworski Jan 13, 2024
b72f00d
feat: implement pr comments
paul-jaworski Jan 17, 2024
9ba7e7d
Merge branch 'feat-974' of https://github.com/corona-school/backend i…
paul-jaworski Jan 17, 2024
f0d7984
fix: pr comments
paul-jaworski Jan 18, 2024
0a234d6
feat: add integration tests for templateCourses
paul-jaworski Jan 18, 2024
0182728
fix: pr comment
paul-jaworski Jan 19, 2024
45415b2
fix: pr comments
paul-jaworski Jan 22, 2024
a1ea25c
Merge branch 'master' into feat-974
paul-jaworski Jan 22, 2024
8674a32
Merge branch 'master' into feat-974
paul-jaworski Jan 27, 2024
a656cbd
Merge branch 'master' into feat-974
paul-jaworski Feb 1, 2024
81db359
fix integration test
paul-jaworski Feb 1, 2024
33f7da8
add ) back
paul-jaworski Feb 4, 2024
381fbbf
pass studentid to createsubcourse
paul-jaworski Feb 4, 2024
a3db03c
add ,
paul-jaworski Feb 4, 2024
92eb0a8
Merge branch 'master' into feat-974
paul-jaworski Feb 6, 2024
ebebc1d
fix integration tests
paul-jaworski Feb 11, 2024
6c663d3
Merge branch 'master' into feat-974
paul-jaworski Feb 11, 2024
927e7fc
fix linting
paul-jaworski Feb 11, 2024
e51f428
fix pr comments
paul-jaworski Feb 12, 2024
9904d1a
remove whitespace
paul-jaworski Feb 12, 2024
db66075
fix pr comments
paul-jaworski Feb 12, 2024
449fe94
test query templateCourses eith non empty search string
paul-jaworski Feb 12, 2024
5bd4a7e
remove secondtest
paul-jaworski Feb 18, 2024
9e9d56d
Merge branch 'master' into feat-974
paul-jaworski Feb 18, 2024
db9455e
Merge branch 'master' into feat-974
paul-jaworski Mar 8, 2024
ddd0684
rewrite prisma graphql query
Mar 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions graphql/authorizations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ export const authorizationModelEnhanceMap: ModelsEnhanceMap = {
| 'description'
| 'createdAt'
| 'updatedAt'
| 'shared'
>({
screeningComment: adminOrOwner,
correspondentId: adminOrOwner,
Expand Down
36 changes: 36 additions & 0 deletions graphql/course/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,40 @@ export class ExtendedFieldsCourseResolver {
const student = await getSessionStudent(context, studentId);
return (await prisma.course_instructors_student.count({ where: { courseId: course.id, studentId: student.id } })) > 0;
}

@FieldResolver((returns) => [Course])
paul-jaworski marked this conversation as resolved.
Show resolved Hide resolved
@Authorized(Role.ADMIN, Role.OWNER, Role.INSTRUCTOR)
async templateCourses(
@Arg('studentId', { nullable: true }) studentId: number,
@Arg('search') search: string,
@Arg('take', () => GraphQLInt) take,
@Arg('skip', () => GraphQLInt, { nullable: true }) skip: number = 0
) {
const courseSearchFilters = await courseSearch(search);
const courses = await prisma.course.findMany({
where: {
OR: [
{
AND: [
{
course_instructors_student: {
some: {
studentId: studentId,
},
},
},
courseSearchFilters,
paul-jaworski marked this conversation as resolved.
Show resolved Hide resolved
],
},
{
shared: true,
},
],
},
take,
skip,
});

return courses;
}
}
16 changes: 15 additions & 1 deletion graphql/course/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as GraphQLModel from '../generated/models';
import { getCourse, getStudent, getSubcoursesForCourse } from '../util';
import { putFile, DEFAULT_BUCKET } from '../../common/file-bucket';

import { course_schooltype_enum as CourseSchooltype, course_subject_enum as CourseSubject, course_coursestate_enum as CourseState } from '../generated';
import { course_schooltype_enum as CourseSchooltype, course_subject_enum as CourseSubject, course_coursestate_enum as CourseState, Course } from '../generated';
import { ForbiddenError } from '../error';
import { addCourseInstructor, allowCourse, denyCourse, subcourseOver } from '../../common/courses/states';
import { getCourseImageKey } from '../../common/courses/util';
Expand Down Expand Up @@ -82,6 +82,20 @@ export class MutateCourseResolver {
return result;
}

@Mutation((returns) => GraphQLModel.Course)
@AuthorizedDeferred(Role.ADMIN, Role.OWNER)
async courseMarkShared(@Ctx() context: GraphQLContext, @Arg('courseId') courseId: number, @Arg('shared') shared: boolean): Promise<GraphQLModel.Course> {
const course = await getCourse(courseId);
await hasAccess(context, 'Course', course);

const updatedCourse = await prisma.course.update({
where: { id: course.id },
data: { shared: shared },
});
paul-jaworski marked this conversation as resolved.
Show resolved Hide resolved
logger.info(`Course(${course.id} was ${shared ? 'shared' : 'unshared'} by User(${context.user.userID})`);
return updatedCourse;
}

@Mutation((returns) => GraphQLModel.Course)
@AuthorizedDeferred(Role.ADMIN, Role.OWNER)
async courseEdit(
Expand Down
14 changes: 12 additions & 2 deletions graphql/subcourse/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class PublicLectureInput {
@Resolver((of) => GraphQLModel.Subcourse)
export class MutateSubcourseResolver {
@Mutation((returns) => GraphQLModel.Subcourse)
@AuthorizedDeferred(Role.ADMIN, Role.OWNER)
@Authorized(Role.INSTRUCTOR)
async subcourseCreate(
@Ctx() context: GraphQLContext,
@Arg('courseId') courseId: number,
Expand All @@ -80,6 +80,17 @@ export class MutateSubcourseResolver {
): Promise<GraphQLModel.Subcourse> {
const course = await getCourse(courseId);
await hasAccess(context, 'Course', course);
paul-jaworski marked this conversation as resolved.
Show resolved Hide resolved
const courseInstructorAssociation = await prisma.course_instructors_student.findFirst({
where: {
courseId: courseId,
studentId: studentId,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this for sure is wrong, we cannot trust our users to pass in their id. Please do

const student = await getSessionStudent(context, studentId);

... studentId: student.id

},
});
const isCourseSharedOrOwned = !!courseInstructorAssociation || course.shared;
if (!isCourseSharedOrOwned) {
logger.error(`Course(${courseId}) is not shared or Student(${studentId}) is not an instructor of this course`);
throw new PrerequisiteError('This course is not shared or you are not the instructor of this course!');
}

const { joinAfterStart, minGrade, maxGrade, maxParticipants, allowChatContactParticipants, allowChatContactProspects, groupChatType } = subcourse;
const result = await prisma.subcourse.create({
Expand All @@ -95,7 +106,6 @@ export class MutateSubcourseResolver {
groupChatType,
},
});

const student = await getSessionStudent(context, studentId);
await prisma.subcourse_instructors_student.create({ data: { subcourseId: result.id, studentId: student.id } });

Expand Down
42 changes: 40 additions & 2 deletions integration-tests/07_course.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { test } from './base';
import { adminClient } from './base/clients';
import { pupilOne } from './01_user';
import { pupilOne, studentOne } from './01_user';
import * as assert from 'assert';
import { screenedInstructorOne, screenedInstructorTwo } from './02_screening';
import { ChatType } from '../common/chat/types';
import { expectFetch } from './base/mock';
import { course_coursestate_enum as CourseState } from '@prisma/client';
import { prisma } from '../common/prisma';

const appointmentTitle = 'Group Appointment 1';

Expand All @@ -21,6 +22,7 @@ const courseOne = test('Create Course One', async () => {
description: "Why should I test if my users can do that for me in production?"
category: club
allowContact: true
shared: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need a separate mutation markShared afterwards

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please rephrase this comment? Do you mean that we need a separate test for shared and nonshared courses? For the current integration test I manually set the shared value to false. See line 414

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No the courseCreate Mutation does not take a shared attribute, thus the test is failing?

subject: Informatik
schooltype: gymnasium
}) {
Expand Down Expand Up @@ -113,6 +115,7 @@ export const subcourseOne = test('Create Subcourse', async () => {

const { client, instructor } = await screenedInstructorOne;
const { courseId } = await courseOne;
const { student } = await studentOne;

const {
subcourseCreate: { id: subcourseId },
Expand All @@ -126,7 +129,7 @@ export const subcourseOne = test('Create Subcourse', async () => {
allowChatContactProspects: true
allowChatContactParticipants: true
groupChatType: ${ChatType.NORMAL}
}) { id }
} studentId: ${student.userID}) { id }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and this can be reverted as we now use the session student

}
`);

Expand Down Expand Up @@ -390,3 +393,38 @@ void test('Add / Remove another instructor', async () => {
subcourseDeleteInstructor(subcourseId: ${subcourseId} studentId: ${instructor2.student.id})
}`);
});

void test('Find shared course', async () => {
const { instructor: instructor1 } = await screenedInstructorTwo;
const { instructor, courseId } = await subcourseOne;

const { courseSearch: courseSearch } = await adminClient.request(`
query FindSharedCourses {
templateCourses(studentId: ${instructor1.student.id}) { id }
}
`);

assert.ok(courseSearch.some((it) => it.id === courseId));
});

void test('Test template course search', async () => {
const { instructor: instructor1 } = await screenedInstructorTwo;
paul-jaworski marked this conversation as resolved.
Show resolved Hide resolved
const { instructor, courseId } = await subcourseOne;

await prisma.course.update({ where: { id: courseId }, data: { shared: false } });
paul-jaworski marked this conversation as resolved.
Show resolved Hide resolved

const { courseSearch: courseSearch } = await adminClient.request(`
query FindSharedCourses {
templateCourses(studentId: ${instructor1.student.id}) { id }
}
`);

assert.ok(courseSearch.every((it) => it.id != courseId));

const { courseSearch: courseSearch2 } = await adminClient.request(`
query FindSharedCourses {
templateCourses(studentId: ${instructor.student.id}) { id }
}
`);
assert.ok(courseSearch2.some((it) => it.id === courseId));
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- AlterTable
ALTER TABLE "course" ADD COLUMN "shared" BOOLEAN NOT NULL DEFAULT false;
2 changes: 2 additions & 0 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ model course {
course_instructors_student course_instructors_student[]
course_tags_course_tag course_tags_course_tag[]
subcourse subcourse[]
shared Boolean @default(false)

}

// DEPRECATED: This was filled from BBB Meetings, replaced with the zoomMeetingReports field of lectures
Expand Down
Loading