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

Jump to bottom

Feat: Question Bank, Exam Questions Randomization #21

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Feat: Question Bank, Exam Questions Randomization #21

wants to merge 7 commits into from

Conversation

Ardent10
Copy link

@Ardent10 Ardent10 commented Sep 1, 2024 •
edited
Loading

Demo: https://www.loom.com/share/07fb4db156744b0b8b29e49a0b21fb84?sid=31a9409b-3ddc-4ad9-99e2-9d39985ff899

Features:

  1. Questions Bank:

    • Create New Questions (One or Multiple)
    • Update Existing Questions (One or Multiple)
    • Added Image support to a question.

  2. Exams

    • Create new exams
    • update existing exams
    • Soft delete existing exams (isDeleted flag in DB)

  3. Exam Questions Randomization based on the no of questions being set for that exam.

  4. Global Store with Exam, User and Question slices.

  5. Logo for the application

  6. Reusable Components and code structure.

@hkirat
Copy link
Contributor

hkirat commented Sep 1, 2024

@devsargam

@devsargam
Copy link

Pushing onto the stack

@Ardent10
Copy link
Author

Ardent10 commented Sep 6, 2024

@devsargam Is it reviewed ? I checked there was a bug while creating a new exam, I fixed that as well.
You can take a fresh pull.

@devsargam
Copy link

@Ardent10 yeah. I will pull the latest and hopefully review it by today. :cheers

@Ardent10
Copy link
Author

Ardent10 commented Sep 13, 2024 •
edited
Loading

@devsargam @hkirat is it reviewed? Do let me know if this is fine or needs changes.

@hkirat
Copy link
Contributor

hkirat commented Sep 15, 2024

@devsargam

@Ardent10
Copy link
Author

@devsargam reviewed?

devsargam
devsargam approved these changes Sep 20, 2024
View reviewed changes
Copy link

@devsargam devsargam left a comment

Choose a reason for hiding this comment

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

Code is very clearn. But the actions are concerning me. I might be wrong here but are we checking for global authentication for all of the server actions?

prisma/schema.prisma Outdated
Copy link

Choose a reason for hiding this comment

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

Share prisma migrations as well

Copy link
Author

Choose a reason for hiding this comment

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

Added

src/actions/exams.ts
}

export const createExam = cache(async (examData: CreateExamInput) => {
try {
Copy link

Choose a reason for hiding this comment

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

Can u add authentication to all the actions

Copy link
Author

Choose a reason for hiding this comment

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

Done

src/actions/exams.ts
})

export const updateExam = cache(async (examData: UpdateExamInput) => {
try {
Copy link

Choose a reason for hiding this comment

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

same as above

Copy link
Author

Choose a reason for hiding this comment

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

Added auth.

src/actions/exams.ts

export const getExams = async () => {
try {
const response = await db.exam.findMany({
Copy link

Choose a reason for hiding this comment

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

Check logic if user can see exam without login we can ignore this else add auth

Copy link
Author

@Ardent10 Ardent10 Sep 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It is a protected route currently which makes more sense, reason being when a user without login redirected to the available exams page (which is a dashboard), it contains all user details, exams taken and everything else on its sidebar.

If a user is NOT LOGGED IN CURRENTLY and they are to take an exam they should pick the exam and then click on the "Pay & Take" test afterwards he is supposed to be redirected to -> login page -> dashboard -> available exams -> select exams that they wish to attempt -> Pay -> Take test. Which is most likely a not an ideal UX for an exam portal.

src/actions/exams.ts

// Get Random Questions for an Exam
export const getRandomQuestionsForExam = async (examId: string) => {
try {
Copy link

Choose a reason for hiding this comment

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

same

Copy link
Author

Choose a reason for hiding this comment

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

Added auth.

src/actions/exams.ts
// Get Exam Data - Fetch exam details and random questions
export async function getExamData(examId: string) {
try {
const exam = await db.exam.findUnique({
Copy link

Choose a reason for hiding this comment

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

same as above

Copy link
Author

Choose a reason for hiding this comment

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

Added auth.

src/actions/questions.ts
} from '@/schemas'

export const createQuestions = async (values: createQuestionsValues) => {
try {
Copy link

Choose a reason for hiding this comment

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

Auth here

Copy link
Author

Choose a reason for hiding this comment

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

Added

src/components/exams/avaiable.tsx Outdated
@@ -1,11 +1,21 @@
// components/AvailableExams.tsx
Copy link

Choose a reason for hiding this comment

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

rm this comment

Copy link
Author

Choose a reason for hiding this comment

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

Done

devsargam
devsargam suggested changes Sep 20, 2024
View reviewed changes
Copy link

@devsargam devsargam left a comment

Choose a reason for hiding this comment

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

Check for auth in actions

@devsargam
Copy link

@Ardent10 update

@Ardent10
Copy link
Author

@devsargam Updated, Review required.

@Ardent10
Copy link
Author

Ardent10 commented Oct 2, 2024

@devsargam reviewed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devsargam devsargam devsargam requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ardent10 @hkirat @devsargam