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.

Sign up for GitHub

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

Remove removed, adds removedAt and removedBy fields to Exercise model #2660

Merged
merged 9 commits into from
Jan 1, 2023
Merged

Remove removed, adds removedAt and removedBy fields to Exercise model #2660

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
68a5a3d
feat: Add Remove exercise fields to Exercise schema
flacial Jan 1, 2023
6ebe9e0
feat: Update Exercise type definition with the new fields
flacial Jan 1, 2023
bb3af6d
Revert "feat: Add Remove exercise fields to Exercise schema"
flacial Jan 1, 2023
cc4210a
Revert "feat: Update Exercise type definition with the new fields"
flacial Jan 1, 2023
76aae44
feat: Add Remove exercise fields to Exercise schema
flacial Jan 1, 2023
25982cd
feat: Update Exercise type definition with the new fields
flacial Jan 1, 2023
938c626
Update getExercise query
flacial Jan 1, 2023
b52f45b
Update getExercisesData mock
flacial Jan 1, 2023
c81384b
Merge branch 'master' into 2630-remove-flagged-exercises-in-accordanc…
flacial Jan 1, 2023
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
6 changes: 3 additions & 3 deletions __dummy__/getExercisesData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const getExercisesData: GetExercisesQuery = {
email: '[email protected]',
discordId: '8321'
},
removed: false,
removedAt: null,
description: '```\nlet a = 5\na = a + 10\n// what is a?\n```',
answer: '15',
explanation: 'You can reassign variables that were created with "let".',
Expand All @@ -91,7 +91,7 @@ const getExercisesData: GetExercisesQuery = {
email: '[email protected]',
discordId: '8321'
},
removed: false,
removedAt: null,
description: '```\nlet a = 1\na += 2\n// what is a?\n```',
answer: '3',
explanation: '`a += 2` is a shorter way to write `a = a + 2`',
Expand All @@ -112,7 +112,7 @@ const getExercisesData: GetExercisesQuery = {
email: '[email protected]',
discordId: '8321'
},
removed: false,
removedAt: null,
description: '```\nlet a = 1\na -= 2\n// what is a?\n```',
answer: '-1',
explanation: null,
Expand Down
20 changes: 14 additions & 6 deletions graphql/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ export type Exercise = {
flaggedById?: Maybe<Scalars['Int']>
id: Scalars['Int']
module: Module
removed?: Maybe<Scalars['Boolean']>
removedAt?: Maybe<Scalars['String']>
removedBy?: Maybe<User>
removedById?: Maybe<Scalars['Int']>
testStr?: Maybe<Scalars['String']>
}

Expand Down Expand Up @@ -1007,7 +1009,7 @@ export type GetExercisesQuery = {
exercises: Array<{
__typename?: 'Exercise'
id: number
removed?: boolean | null
removedAt?: string | null
description: string
answer: string
explanation?: string | null
Expand Down Expand Up @@ -1746,7 +1748,9 @@ export type ExerciseResolvers<
flaggedById?: Resolver<Maybe<ResolversTypes['Int']>, ParentType, ContextType>
id?: Resolver<ResolversTypes['Int'], ParentType, ContextType>
module?: Resolver<ResolversTypes['Module'], ParentType, ContextType>
removed?: Resolver<Maybe<ResolversTypes['Boolean']>, ParentType, ContextType>
removedAt?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>
removedBy?: Resolver<Maybe<ResolversTypes['User']>, ParentType, ContextType>
removedById?: Resolver<Maybe<ResolversTypes['Int']>, ParentType, ContextType>
testStr?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>
}>
Expand Down Expand Up @@ -4354,7 +4358,7 @@ export const GetExercisesDocument = gql`
slug
}
}
removed
removedAt
description
answer
explanation
Expand Down Expand Up @@ -6542,7 +6546,9 @@ export type ExerciseKeySpecifier = (
| 'flaggedById'
| 'id'
| 'module'
| 'removed'
| 'removedAt'
| 'removedBy'
| 'removedById'
| 'testStr'
| ExerciseKeySpecifier
)[]
Expand All @@ -6557,7 +6563,9 @@ export type ExerciseFieldPolicy = {
flaggedById?: FieldPolicy<any> | FieldReadFunction<any>
id?: FieldPolicy<any> | FieldReadFunction<any>
module?: FieldPolicy<any> | FieldReadFunction<any>
removed?: FieldPolicy<any> | FieldReadFunction<any>
removedAt?: FieldPolicy<any> | FieldReadFunction<any>
removedBy?: FieldPolicy<any> | FieldReadFunction<any>
removedById?: FieldPolicy<any> | FieldReadFunction<any>
testStr?: FieldPolicy<any> | FieldReadFunction<any>
}
export type ExerciseCommentKeySpecifier = (
Expand Down
2 changes: 1 addition & 1 deletion graphql/queries/getExercises.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const GET_EXERCISES = gql`
slug
}
}
removed
removedAt
description
answer
explanation
Expand Down
4 changes: 3 additions & 1 deletion graphql/typeDefs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,9 @@ export default gql`
answer: String!
testStr: String
explanation: String
removed: Boolean
removedAt: String
removedBy: User
removedById: Int
flaggedAt: String
flagReason: String
flaggedBy: User
Expand Down
13 changes: 13 additions & 0 deletions prisma/migrations/20230101050656_add_removed_column_date_and_user_to_exercise/migration.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
Warnings:

- You are about to drop the column `removed` on the `exercises` table. All the data in the column will be lost.

*/
-- AlterTable
ALTER TABLE "exercises" DROP COLUMN "removed",
ADD COLUMN "removedAt" TIMESTAMP(3),
ADD COLUMN "removedById" INTEGER;

-- AddForeignKey
ALTER TABLE "exercises" ADD CONSTRAINT "exercises_removedById_fkey" FOREIGN KEY ("removedById") REFERENCES "users"("id") ON DELETE SET NULL ON UPDATE CASCADE;
7 changes: 5 additions & 2 deletions prisma/schema.prisma
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ model User {
comments Comment[]
exercises Exercise[]
exerciseSubmissions ExerciseSubmission[]
Exercise Exercise[] @relation("flaggedExercises")
FlaggedExercise Exercise[] @relation("flaggedExercises")
RemovedExercise Exercise[] @relation("removedExercises")
modules Module[]
starsMentor Star[] @relation("starMentor")
starsGiven Star[] @relation("starStudent")
Expand Down Expand Up @@ -187,7 +188,9 @@ model Exercise {
answer String
testStr String?
explanation String?
removed Boolean? @default(false)
removedAt DateTime?
Copy link
Member Author

Choose a reason for hiding this comment

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

It's only possible to set removedAt default value to a DateTime, and in the client, we will use it like the following:

if (exercise.removedAt) // do something

If it has a default, value, the above condition will be true, and we'd get an opposite behavior for what we want.

Copy link
Member

Choose a reason for hiding this comment

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

If you don't set a default value postgresql will set it to null so we don't need to explicitly set the default to null in prisma, postgresql will do it and we'll get the desired behaviour. Being limited to datetime as a default value is due to prisma, prisma doesn't let you set null as the default for an optional value but that is the default behaviour.
Just adding removedAt DateTime? like you have done is enough.

removedById Int?
removedBy User? @relation("removedExercises", fields: [removedById], references: [id])
Copy link
Member Author

Choose a reason for hiding this comment

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

Something I discovered during testing the changes with Apollo playground: removedBy and flaggedBy returns null because they refer to a different model. In order to include them in the result of querying the db for exercises, we'd have to explicitly tell Prisma to include them:

export const exercises = () => {
  return prisma.exercise.findMany({
    include: {
      author: true,
+     flaggedBy: true,
+     removedBy: true,
      module: {
        include: { lesson: true }
      }
    }
  })
}

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, getting the related fields requires a sql JOIN so prisma will not do the extra work unless needed and requested by us.

flagReason String?
flaggedAt DateTime?
flaggedById Int?
Expand Down