prisma/migrations/20241002055837_dynamic_form_builder/migration.sql (4)

21-26: LGTM! Consider adding metadata fields.

The FormStructure table is correctly defined with id as the primary key. This minimal design allows for flexibility in form structures. However, consider adding metadata fields such as name, description, or createdAt to provide more context and improve manageability.

Example of additional fields:

CREATE TABLE "FormStructure" (
    "id" STRING NOT NULL,
    "name" STRING NOT NULL,
    "description" STRING,
    "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,

    CONSTRAINT "FormStructure_pkey" PRIMARY KEY ("id")
);

---

28-35: LGTM! Consider making the statement field non-nullable.

The Answer table structure is well-designed with a composite primary key ensuring one answer per question per submitter. However, the statement field is currently nullable. Consider making it non-nullable to ensure all answers have a value, even if it's an empty string.

Suggested change:

CREATE TABLE "Answer" (
-    "statement" STRING,
+    "statement" STRING NOT NULL,
    "addressedQuestionId" STRING NOT NULL,
    "submitterId" STRING NOT NULL,

    CONSTRAINT "Answer_pkey" PRIMARY KEY ("addressedQuestionId","submitterId")
);

---

45-50: LGTM! Consider using a separate id as the primary key.

The FormQuestionCategory table structure is simple, using the category name as the primary key. While this works, consider using a separate id field as the primary key. This would allow for easier renaming of categories in the future without affecting foreign key relationships.

Suggested structure:

CREATE TABLE "FormQuestionCategory" (
    "id" STRING NOT NULL,
    "name" STRING NOT NULL,

    CONSTRAINT "FormQuestionCategory_pkey" PRIMARY KEY ("id")
);

CREATE UNIQUE INDEX "FormQuestionCategory_name_key" ON "FormQuestionCategory"("name");

---

61-68: LGTM! Consider RESTRICT on delete for questionId.

The foreign key constraints for FormStructureQuestion are generally well-defined. However, consider changing the ON DELETE action for questionId from CASCADE to RESTRICT. This would prevent accidental deletion of questions that are part of existing form structures.

Current constraints:

1. formStructureId references FormStructure.id with CASCADE on delete and update.
2. questionId references Question.id with CASCADE on delete and update.
3. categoryId references FormQuestionCategory.name with CASCADE on delete and update.

Suggested change for the questionId foreign key:

ALTER TABLE "FormStructureQuestion" ADD CONSTRAINT "FormStructureQuestion_questionId_fkey" FOREIGN KEY ("questionId") REFERENCES "Question"("id") 
- ON DELETE CASCADE ON UPDATE CASCADE;
+ ON DELETE RESTRICT ON UPDATE CASCADE;

prisma/schema.prisma (4)

Line range hint 112-165: New DH10Application model as a transitional step

The new DH10Application model is introduced with a comment indicating its future deprecation once application data is fully migrated to FormSubmissions. This suggests a phased approach to updating the application system.

Recommendations:

1. Document the migration plan from DH10Application to FormSubmissions, including timelines and steps.
2. Ensure that any new application submissions use the new FormSubmission system to avoid adding more data to this transitional model.
3. Create a task or issue to track the migration progress and eventual removal of this model.

Would you like assistance in creating a GitHub issue to track the migration from DH10Application to FormSubmissions?

---

182-201: New FormStructure and FormStructureQuestion models for dynamic forms

The introduction of FormStructure and FormStructureQuestion models provides a flexible framework for creating dynamic forms, aligning with the PR objective of enabling dynamic form creation with minimal development effort.

Key features:

1. FormStructure acts as a container for form submissions and question structures.
2. FormStructureQuestion links questions to forms and categories, with a display priority for ordering.
3. The composite unique constraint on displayPriority and formStructureId ensures proper ordering of questions within a form.

Recommendations:

1. Document the process of creating and managing dynamic forms using these new models.
2. Consider creating helper functions or utilities to simplify the creation and modification of form structures.
3. Ensure that the UI components are updated to render forms based on this new structure.

Would you like assistance in creating documentation for the new dynamic form structure system?

---

223-227: Addition of FormQuestionCategory model

The new FormQuestionCategory model enables the categorization of questions within form structures. This addition can significantly enhance the organization of forms and potentially improve the user experience.

Considerations:

1. This model allows for flexible grouping of questions, which can be particularly useful for long or complex forms.
2. Categories can be used to implement features like section-based form rendering or category-based form analytics.

Recommendations:

1. Define a set of standard categories that can be used across different types of forms to maintain consistency.
2. Update the UI components to utilize these categories for improved form layout and navigation.
3. Consider implementing category-based validation or conditional rendering of form sections based on these categories.

Would you like assistance in defining a set of standard categories or implementing category-based form features?

---

Incomplete Deprecation of the Review Model

The Review model is still referenced in multiple parts of the codebase, indicating that its deprecation is not yet complete.

Recommended Actions:

1. Update the Deprecation Comment:

   -/// @deprecated Use `User.status` instead
   +/// @deprecated Use `FormSubmission.status` instead

2. Plan for Complete Removal:
   • Refactor all usages of the Review model to utilize the FormSubmission model.
   • Ensure that all dependencies and references are updated accordingly to prevent residual usage.

🔗 Analysis chain

Line range hint 66-75: Deprecation of the Review model

The entire Review model has been deprecated with the comment suggesting to use User.status instead. However, this comment seems outdated as User.status is also deprecated in favor of FormSubmission.status.

Consider the following actions:

1. Update the deprecation comment to refer directly to FormSubmission.status.
2. Plan for the complete removal of this model in a future update.
3. Ensure all parts of the application that use the Review model are updated to use the new FormSubmission model.

Update the deprecation comment as follows:

-/// @deprecated Use `User.status` instead
+/// @deprecated Use `FormSubmission.status` instead

To verify the usage of the Review model in the codebase, run:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find references to the Review model in the codebase

rg -t ts -t tsx 'Review\s*\{|Review\s*\[' src/

Length of output: 598

src/server/router/reviewers.ts (3)

9-18: Consider preserving null values for the email field

The updated ApplicationForReview schema improves the structure of the application data. However, transforming null emails to empty strings may lead to ambiguity between an empty email and a missing one. It's generally better to keep null to represent the absence of data.

Consider modifying the email field to retain null values:

email: z
  .string()
  .nullable()
-  .transform((v) => (v === null ? "" : v)),

The replacement of dH10ApplicationId with formStructureId is a good change, suggesting a more flexible approach to identifying applications.

---

32-66: Excellent refactoring of getApplications query

The refactoring of the getApplications query significantly improves the robustness and flexibility of the application retrieval process. The use of DirectPrismaQuerier for fetching the current application form name is a good approach for efficient database querying.

The error handling for parsing applications is a valuable addition. Consider enhancing it further by including more specific error information:

 } catch (error) {
   throw new TRPCError({
     code: "INTERNAL_SERVER_ERROR",
-    message: "Failed to parse applications.",
+    message: `Failed to parse applications: ${error instanceof Error ? error.message : 'Unknown error'}`,
   });
 }

This change will provide more detailed error information, which can be helpful for debugging.

---

143-170: Consider simplifying the status icon logic

The update logic for formSubmission and the enhanced logging for status changes are good improvements. However, the nested ternary operators for icon assignment, while functional, can be hard to read and maintain.

Consider simplifying this logic using an object mapping:

const statusIcons = {
  [Status.ACCEPTED]: "✅",
  [Status.REJECTED]: "❌",
  [Status.WAITLISTED]: "🕰️",
  [Status.RSVP]: "🎟️",
};

icon: statusIcons[input.application.status] ?? "🤔",

This approach is more readable and easier to maintain, especially if more statuses are added in the future.

src/pages/checkin.tsx (2)

16-16: Remove unused import TRPCError

The TRPCError import appears to be unused in this file. It's best to remove unused imports to keep the code clean and prevent potential confusion.

Apply this change to remove the unused import:

-import { TRPCError } from "@trpc/server";

---

Line range hint 1-245: Overall improvements with a minor optimization suggestion

The changes in this file significantly enhance its quality:

1. Improved type safety with InferGetServerSidePropsType.
2. Better error handling in getServerSideProps.
3. Optimized data fetching with DirectPrismaQuerier.
4. Simplified component logic by leveraging server-side props.

These modifications align well with Next.js best practices and address previous issues.

As a minor optimization, consider memoizing the stateMap object to prevent unnecessary re-creation on each render:

const stateMap = useMemo(() => ({
  [Status.IN_REVIEW]: <PreCheckedIn />,
  [Status.ACCEPTED]: <PreCheckedIn />,
  [Status.WAITLISTED]: <PreCheckedIn />,
  [Status.REJECTED]: <PreCheckedIn />,
  [Status.RSVP]: <PreCheckedIn />,
  [Status.CHECKED_IN]: <PostCheckedIn />,
}), []);

This change would require adding useMemo to the imports from 'react'.

src/server/router/application.ts (7)

1-1: LGTM: New imports and interface enhance functionality.

The addition of DirectPrismaQuerier and trpcAssert imports, along with the AnswerForRouter interface, improves database querying, assertion handling, and answer structuring. These changes align well with the new schema focus on formSubmissions.

Consider adding a brief comment above the AnswerForRouter interface to explain its purpose and usage within the router.

Also applies to: 7-8, 85-89

---

109-122: LGTM: Improved getStatusCount procedure with better authorization and querying.

The updates to getStatusCount enhance security with trpcAssert for authorization checks and improve querying by using DirectPrismaQuerier. The change to group by formStructureId aligns with the new schema structure.

Consider extracting the authorization check into a separate helper function for reusability across other admin/reviewer-only procedures.

---

158-176: LGTM with a suggestion: rsvp procedure updated to use formSubmission.

The changes to the rsvp procedure align well with the new schema focusing on formSubmissions. However, there's a potential issue with the assertion:

trpcAssert(form.status === Status.ACCEPTED, "UNAUTHORIZED");

This assertion will throw an "UNAUTHORIZED" error if the status is not ACCEPTED, which is correct. However, it might be clearer to use a more specific error message:

trpcAssert(form.status === Status.ACCEPTED, "NOT_ACCEPTED");

Consider using a more specific error message for the status check to improve clarity.

---

203-367: LGTM with suggestions: submit procedure updated for new schema.

The submit procedure has been significantly updated to work with the new schema, using applicationSchema for input validation and storing data in a more granular format. These changes improve data integrity and flexibility.

However, there are a few points to consider:

1. The procedure is quite long and complex. Consider refactoring it into smaller, more manageable functions for better readability and maintainability.

2. The date handling for gradDate is correct, but you might want to consider using a utility function for date parsing to keep the main logic cleaner.

3. Consider using a more descriptive name for the answers array, such as formattedAnswers, to better reflect its purpose.

Consider breaking down the submit procedure into smaller functions, each handling a specific part of the submission process (e.g., formatAnswers, upsertFormSubmission, upsertAnswers). This will improve readability and maintainability.

---

485-492: LGTM: Simplified getPrevAutofill procedure.

The getPrevAutofill procedure has been simplified to directly return the user's dh10application. This change reduces external API dependencies and aligns with the new schema structure.

Consider renaming this procedure to getDh10Application or similar to better reflect its current functionality, as it no longer specifically relates to autofill.

---

494-526: LGTM: Improved deleteApplication procedure with better error handling.

The deleteApplication procedure has been updated to work with the new schema, deleting the formSubmission instead of dh10Application. The addition of error handling and logging enhances the robustness of the procedure.

Consider adding a check to ensure the user has the necessary permissions to delete an application, especially if this procedure can be called for applications other than the current user's.

---

Line range hint 1-526: Overall LGTM: Significant improvements to application router.

This update to the application router brings substantial enhancements:

1. Alignment with a new schema structure, focusing on formSubmission.
2. Improved error handling and input validation.
3. New procedures for better application management (getApplicationShallow).
4. Simplified and more efficient data retrieval (getPrevAutofill).
5. Enhanced security with trpcAssert for authorization checks.

These changes collectively improve the clarity, efficiency, and robustness of the application submission and retrieval processes. While there are some areas for potential further refinement (as noted in individual comments), the overall direction is very positive.

Consider creating a separate service layer for complex business logic, especially for procedures like submit. This would improve separation of concerns and make the router more focused on request handling and routing.

Configuration used: CodeRabbit UI
Review profile: CHILL

src/server/db/directQueries.ts (2)

18-35: Consider adding an explicit return type.

The getDeltaHacksApplicationFormName method is well-implemented with proper error handling using assertions. To further improve type safety and code readability, consider adding an explicit return type:

async getDeltaHacksApplicationFormName(): Promise<string> {
  // ... existing implementation ...
}

This change would make the method's contract clearer to callers.

---

37-53: Consider adding an explicit return type.

The hasKilledApplications method is well-implemented with proper error handling. To enhance type safety and code clarity, consider adding an explicit return type:

async hasKilledApplications(): Promise<boolean> {
  // ... existing implementation ...
}

This change would make the method's contract clearer to callers.

package.json (1)

Line range hint 5-11: Consider reviewing and updating scripts

Given the significant updates to dependencies, especially Prisma, it might be worth reviewing the scripts section to ensure they're still optimal and up-to-date. For example, you might want to consider:

1. Adding a script for Prisma migrations if not already handled elsewhere.
2. Updating the dev script to use prisma db push instead of prisma generate if you're using Prisma in prototype mode.
3. Adding a postinstall script to run prisma generate automatically after package installation.

Example:

"scripts": {
  "postinstall": "prisma generate",
  "dev": "pnpm i && prisma db push && next dev",
  // ... other scripts
}

These are just suggestions; please adapt them to your project's specific needs.

prisma/seed/data.ts (3)

1-127: LGTM! Consider adding question properties for enhanced flexibility.

The QUESTIONS constant is well-structured and uses the as const assertion for type safety. This approach ensures that all question IDs are valid at compile-time, which is excellent.

Consider enhancing the question objects with additional properties such as:

• required: boolean to indicate if a question is mandatory
• type: string to specify the expected answer format (e.g., 'text', 'date', 'select')

This would provide more flexibility and clarity in form rendering and validation.

Example:

{
  id: "first_name",
  statement: "First Name",
  required: true,
  type: "text"
},

---

151-219: Well-structured form definition. Consider future scalability.

The FORM_STRUCTURES constant effectively defines the DeltaHacks X Application Form using the previously defined interfaces. The organization of questions into categories is logical and comprehensive, covering all aspects of the application process.

While the current implementation is solid, consider the following suggestion for future scalability:

Instead of hardcoding the form structure, consider creating a function that generates the form structure based on configuration objects. This approach would make it easier to create and maintain multiple form structures in the future.

Example:

function createFormStructure(id: string, categoryConfigs: Array<{categoryId: FormQuestionCategoryId, questionIds: QuestionId[]}>): FormStructure {
  return {
    id,
    categories: categoryConfigs.map(config => ({
      categoryId: config.categoryId,
      questions: config.questionIds
    }))
  };
}

export const FORM_STRUCTURES: FormStructure[] = [
  createFormStructure("DeltaHacks X Application Form", [
    { categoryId: "Personal Information", questionIds: ["first_name", "last_name", /* ... */] },
    // ... other categories
  ])
];

This approach would make it easier to maintain and update form structures in the future.

---

221-225: Consistent form configuration. Minor suggestion for improvement.

The DELTAHACKS_APPLICATION_FORM_CONFIG constant provides a centralized configuration for the DeltaHacks application form, which is good for maintainability and consistency across the application.

For improved consistency, consider aligning the id and name values with the id used in the FORM_STRUCTURES constant:

export const DELTAHACKS_APPLICATION_FORM_CONFIG = {
  id: "DeltaHacks X Application Form",
  name: "DeltaHacks X Application Form",
  value: "DeltaHacks X Application Form",
};

This change would ensure that the form is consistently identified across different parts of the application.

src/server/router/reviewers.ts (5)

5-26: LGTM! Consider simplifying email transformation

The new imports, updated ApplicationForReview schema, and FormItem interface are well-structured and align with the changes in the file. The nested submitter object in ApplicationForReview is a good improvement for data organization.

Consider simplifying the email transformation:

email: z
  .string()
  .nullable()
-  .transform((v) => (v === null ? "" : v)),
+  .transform((v) => v ?? ""),

This change uses the nullish coalescing operator, which is more concise and achieves the same result.

---

32-66: LGTM! Consider enhancing error logging

The updates to the getApplications procedure are well-implemented:

• The authorization check using trpcAssert is more concise.
• Using DirectPrismaQuerier to get the application form name is a good approach.
• The query is more specific by using formStructureId.
• Error handling for parsing applications is a valuable addition.

Consider enhancing the error logging to include more details:

 } catch (error) {
   throw new TRPCError({
     code: "INTERNAL_SERVER_ERROR",
-    message: "Failed to parse applications.",
+    message: "Failed to parse applications.",
+    cause: error,
   });
 }

This change will provide more context for debugging if an error occurs.

---

71-123: LGTM! Consider adding type assertion for better type safety

The updates to the getApplication procedure are well-implemented:

• Using submitterId instead of dH10ApplicationId aligns with the new schema.
• The complex query structure efficiently retrieves all necessary data in a single query.
• Building a Map of FormItems is a good approach for organizing the data.

Consider adding a type assertion for better type safety when accessing questionStructure.question.answers[0]:

 answer: questionStructure.question.answers[0]?.statement ?? null,
+answer: (questionStructure.question.answers[0] as { statement: string | null } | undefined)?.statement ?? null,

This change ensures that TypeScript understands the structure of the answers array elements.

---

125-171: LGTM! Consider simplifying status icon logic

The updates to the updateApplicationShallow procedure (formerly updateStatus) are well-implemented:

• The renaming better reflects the function's purpose.
• Using submitterId and formStructureId for updating aligns with the new schema.
• The use of DirectPrismaQuerier is consistent with other procedures.
• The LogSnag tracking update is comprehensive.

Consider simplifying the status icon logic using an object lookup:

-icon:
-  input.application.status === Status.ACCEPTED
-    ? "✅"
-    : input.application.status === Status.REJECTED
-    ? "❌"
-    : input.application.status === Status.WAITLISTED
-    ? "🕰️"
-    : input.application.status === Status.RSVP
-    ? "🎟️"
-    : "🤔",
+icon: {
+  [Status.ACCEPTED]: "✅",
+  [Status.REJECTED]: "❌",
+  [Status.WAITLISTED]: "🕰️",
+  [Status.RSVP]: "🎟️"
+}[input.application.status] ?? "🤔",

This change makes the code more readable and easier to maintain.

---

Line range hint 172-214: Consider updating authorization check for consistency

The submit procedure remains largely unchanged. For consistency with other procedures in this file, consider updating the authorization check to use trpcAssert.

Update the authorization check:

-if (
-  !(
-    ctx.session.user.role.includes(Role.ADMIN) ||
-    ctx.session.user.role.includes(Role.REVIEWER)
-  )
-) {
-  throw new TRPCError({ code: "UNAUTHORIZED" });
-}
+trpcAssert(
+  ctx.session.user.role.includes(Role.ADMIN) ||
+    ctx.session.user.role.includes(Role.REVIEWER),
+  "UNAUTHORIZED"
+);

This change will make the authorization check consistent with other procedures in the file.

prisma/seed/seed.ts (3)

90-95: Avoid logging sensitive error details in production

Logging detailed error messages and stack traces can expose sensitive information in production environments.

Consider adjusting the logging to be less verbose in production:

---

88-102: Handle errors gracefully without abrupt termination

Using process.exit(1) can abruptly terminate the process, potentially interrupting asynchronous operations. It's better to allow the process to exit naturally after completing all pending tasks.

Consider removing process.exit(1) and allowing the script to exit gracefully:

---

31-36: Type annotation for formStructureQuestion may be unnecessary

The explicit type annotation for formStructureQuestion can be omitted as TypeScript can infer the type based on the assigned object.

You can simplify the code by removing the redundant type annotation:

Configuration used: CodeRabbit UI
Review profile: CHILL

prisma/seed/data.ts (4)

1-7: LGTM! Consider enhancing type safety for questions.

The FormStructure interface is well-defined and provides a clear structure for forms. It allows for flexible form creation with multiple categories and questions.

To enhance type safety, consider defining an enum or union type for common question types, which could be used instead of string[] for the questions array. This would provide better type checking and autocompletion for specific question types. For example:

type QuestionType = 'text' | 'number' | 'date' | 'multipleChoice' | 'checkbox';

interface Question {
  text: string;
  type: QuestionType;
}

export interface FormStructure {
  id: string;
  categories: {
    name: string;
    questions: Question[];
  }[];
}

---

9-77: LGTM! Consider data privacy for sensitive questions.

The FORM_STRUCTURES constant is well-structured and follows the FormStructure interface correctly. It provides a comprehensive set of questions for the DeltaHacks application.

Please ensure that appropriate data privacy measures are in place for handling sensitive information such as gender, race, and emergency contact details. Consider the following:

1. Implement strict access controls for this data in the database.
2. Encrypt sensitive data at rest and in transit.
3. Provide clear privacy policies to users about how their data will be used and stored.
4. Ensure compliance with relevant data protection regulations (e.g., GDPR, CCPA) based on your user base.

---

79-83: LGTM! Consider simplifying the naming convention.

The DELTAHACKS_APPLICATION_FORM_CONFIG constant provides a clear configuration for the DeltaHacks application form. The value field correctly matches the id in the FORM_STRUCTURES array, ensuring consistency.

Consider simplifying the id and name fields to avoid repetition:

export const DELTAHACKS_APPLICATION_FORM_CONFIG = {
  id: "Application",
  name: "Application",
  value: "DeltaHacks X Application Form",
};

This change would make the configuration more concise while still maintaining its clarity and purpose.

---

1-83: Great foundation for dynamic form creation and management!

This file provides a well-structured and extensible foundation for creating and managing application forms. The FormStructure interface, FORM_STRUCTURES constant, and DELTAHACKS_APPLICATION_FORM_CONFIG work together to create a flexible and type-safe system for defining forms.

For future development:

1. Consider adding more form structures to the FORM_STRUCTURES array as needed for different types of applications or surveys.
2. Implement a validation layer that uses these structures to ensure data integrity when processing form submissions.
3. Create utility functions that can generate frontend form components based on these structures, promoting consistency between the backend definitions and frontend rendering.

prisma/migrations/20241003064705_dynamic_form_builder/migration.sql (1)

56-72: LGTM: Appropriate foreign key constraints

The foreign key constraints are well-defined and maintain referential integrity between the tables. The use of ON DELETE RESTRICT and ON UPDATE CASCADE is generally a good practice.

Consider: Evaluate delete behaviors

While ON DELETE RESTRICT is a safe default, consider if different delete behaviors might be more appropriate for some relationships:

1. For FormSubmission_formStructureId_fkey and QuestionCategory_formStructureId_fkey, you might want ON DELETE CASCADE if deleting a FormStructure should remove all associated submissions and categories.
2. For Answer_addressedQuestionId_fkey, ON DELETE CASCADE might be appropriate if deleting a Question should remove all associated answers.

Evaluate these based on your application's requirements and data lifecycle management needs.

prisma/seed/seed.ts (1)

38-41: Consider relaxing the assertion condition.

The current assertion might be too strict if there are existing categories in the database. Consider modifying it to ensure that at least the new categories have been added:

 assert(
-  categories.length >= formStructure.categories.length,
+  categories.length >= formStructure.categories.length,
   "All categories should've been added before"
 );

This change allows for the possibility of existing categories while still ensuring that all new categories have been added successfully.

prisma/schema.prisma (7)

49-53: Approve changes to User model with a note on data management shift

The changes to the User model, including the deprecation of several fields and the addition of new relationships, indicate a positive shift towards a more flexible form submission system. This approach should allow for easier management of diverse application types in the future.

However, there's a minor inconsistency in the deprecation comments:

The comment for the status field suggests using FormSubmission.status, while other comments suggest using FormSubmission without specifying a field. Consider updating the comment for consistency:

-  status               Status     @default(IN_REVIEW) /// @deprecated Use `FormSubmission.status` instead
+  status               Status     @default(IN_REVIEW) /// @deprecated Use `FormSubmission` instead

Also applies to: 61-63

---

Line range hint 66-76: Update deprecation comment for Review model

The deprecation of the Review model aligns with the shift towards the new FormSubmission system. However, the current deprecation comment may lead to confusion.

Update the deprecation comment to point directly to the new FormSubmission model:

-/// @deprecated Use `User.status` instead
+/// @deprecated Use `FormSubmission` instead

This change will provide clearer guidance and prevent potential confusion caused by referencing another deprecated field.

---

Line range hint 112-157: Acknowledge transition approach and suggest detailed migration plan

The addition of the DH10Application model with a note about future deprecation shows good foresight in managing the transition to the new FormSubmission system.

To improve the migration process:

1. Consider adding a TODO comment with a target date or milestone for the migration.
2. Create a migration guide or plan that outlines how each field in DH10Application will map to the new FormSubmission structure.
3. Implement a migration script to automate the data transfer process.

This will help ensure a smooth transition and prevent the temporary solution from becoming permanent.

---

169-180: Approve FormSubmission model structure with a suggestion

The new FormSubmission model is well-structured and aligns with the changes made to the User model. The use of a composite primary key allows for multiple submissions per user across different form structures, which provides good flexibility.

To enhance data integrity:

Consider adding a unique constraint on the submissionTime for each user to prevent duplicate submissions:

model FormSubmission {
  submissionTime DateTime @default(now())
  status         Status   @default(IN_REVIEW)

  formStructure   FormStructure @relation(fields: [formStructureId], references: [id])
  formStructureId String

  submitter   User   @relation(fields: [submitterId], references: [id])
  submitterId String

  @@id([formStructureId, submitterId])
+ @@unique([submitterId, submissionTime])
}

This ensures that each submission for a user has a unique timestamp, preventing potential issues with duplicate submissions.

---

182-213: Approve form structure models with a suggestion for consistency

The new FormStructure, QuestionCategory, and Question models create a well-organized, hierarchical structure for managing forms and questions. The use of position fields with unique constraints ensures proper ordering within each level.

For consistency and clarity:

Consider renaming the formPosition field in the QuestionCategory model to categoryPosition to match the naming convention used in the Question model:

model QuestionCategory {
  id           String @id @default(cuid())
  name         String
- formPosition Int
+ categoryPosition Int

  formStructure   FormStructure @relation(fields: [formStructureId], references: [id])
  formStructureId String

  questions Question[]

  @@unique([name, formStructureId])
- @@unique([formPosition, formStructureId])
+ @@unique([categoryPosition, formStructureId])
}

This change would make the naming more consistent across the models and clarify that the position is relative to other categories within the same form structure.

---

215-225: Approve Answer model with suggestion for submission association

The new Answer model effectively captures user responses to specific questions. The composite primary key ensures data integrity by allowing only one answer per question per user.

To improve data management and querying capabilities:

Consider adding a link to the FormSubmission model. This would allow you to associate answers with specific submissions, which can be useful for tracking changes over time if a user submits multiple applications. Here's a suggested modification:

model Answer {
  statement String?

  addressedQuestion   Question @relation(fields: [addressedQuestionId], references: [id])
  addressedQuestionId String

  submitter   User   @relation(fields: [submitterId], references: [id])
  submitterId String

+ formSubmission   FormSubmission @relation(fields: [formStructureId, submitterId], references: [formStructureId, submitterId])
+ formStructureId  String

- @@id([addressedQuestionId, submitterId])
+ @@id([addressedQuestionId, submitterId, formStructureId])
}

model FormSubmission {
  // ... existing fields ...
+ answers Answer[]
}

This change would allow you to easily retrieve all answers for a specific submission and track changes in user responses across multiple submissions.

---

Line range hint 1-225: Summary of schema changes and their impact

The changes to the Prisma schema represent a significant improvement in the application's data model:

1. The introduction of FormSubmission, FormStructure, QuestionCategory, Question, and Answer models creates a flexible system for managing diverse types of forms and submissions.
2. Deprecation of old fields and models (like Review) with clear notices helps manage the transition to the new system.
3. The new structure allows for dynamic form creation and submission, which should make the application more adaptable to changing requirements.

These changes will likely have a substantial impact on the application:

• Data migration: Ensure a comprehensive plan is in place to migrate existing data to the new structure.
• API updates: The application's API will need to be updated to work with the new data model.
• Frontend changes: Forms and data display components will need to be refactored to work with the new, more flexible structure.
• Performance considerations: With the more complex relationships, pay attention to query optimization, especially when retrieving full form submissions with all related data.

Overall, these changes provide a solid foundation for a more flexible and maintainable application. Great work on improving the data model!

src/server/router/reviewers.ts (2)

35-72: LGTM: Improved getApplications procedure with robust error handling

The updates to the getApplications procedure are well-implemented:

1. The authorization check is more concise.
2. Using DirectPrismaQuerier for retrieving the form name is a good practice.
3. The query now correctly uses formStructureId for filtering applications.
4. The added error handling for parsing applications is a valuable safeguard.

One minor suggestion:

Consider adding a log or metric for parsing failures to help track the frequency of such errors:

catch (error) {
  console.error("Failed to parse applications:", error);
  // Consider adding a metric here if you have a metrics system
  throw new TRPCError({
    code: "INTERNAL_SERVER_ERROR",
    message: "Failed to parse applications.",
  });
}

---

Line range hint 210-255: LGTM: Submit procedure unchanged

The submit procedure remains largely unchanged, which is fine if it's still functioning as intended. It retains the necessary authorization checks and logic for submitting reviews.

However, for consistency with the rest of the file:

Consider updating the authorization check to use trpcAssert like in other procedures:

trpcAssert(
  ctx.session.user.role.includes(Role.ADMIN) ||
    ctx.session.user.role.includes(Role.REVIEWER),
  "UNAUTHORIZED"
);

This would make the error handling consistent across all procedures in this file.

src/components/Applicant.tsx (1)

136-141: Approve conditional rendering, suggest consistent data access

The conditional rendering based on the email domain is implemented correctly. However, consider applying the refactored data access method suggested earlier to maintain consistency and improve readability.

Example using the suggested helper function:

{submitter.email.endsWith("mcmaster.ca") && (
  <FormCheckbox
    label={getQuestionAnswer(5, 0).question}
    checked={getQuestionAnswer(5, 0).answer === "true"}
    readOnly
  />
)}

src/server/router/application.ts (4)

109-126: Consider improving error handling when formName is not found.

The changes to use DirectPrismaQuerier and group by formStructureId are good improvements. However, when formName is not found, the function returns an empty array without logging or throwing an error. Consider adding more robust error handling in this case.

 if (!formName) {
+  console.error("DeltaHacks application form name not found");
   return [];
 }

This will help in debugging if the form name is unexpectedly missing.

---

162-191: Improve error message specificity in rsvp procedure.

The changes to use formSubmission instead of user for RSVP status are good. However, the error message for the first assertion could be more specific:

 trpcAssert(
   formName,
   "NOT_FOUND",
-  "No application found for the User to RSVP"
+  "No active application form found for RSVP"
 );

This provides more clarity about what exactly was not found.

---

Line range hint 457-489: Improve user-friendliness of error messages in checkIn procedure.

The changes to the checkIn procedure improve the robustness of the check-in process. Consider making the error messages more user-friendly:

 if (user?.qrcode !== null) {
   throw new TRPCError({
     code: "FORBIDDEN",
-    message: "You are already checked in.",
+    message: "You have already checked in. If you believe this is an error, please contact support.",
   });
 }

 if (qrCount != 0) {
   throw new TRPCError({
     code: "FORBIDDEN",
-    message: "This QR code is already in use",
+    message: "This QR code is already in use. Please request a new QR code from the registration desk.",
   });
 }

These messages provide more context and guidance to the user.

---

Line range hint 490-545: Improve error handling in getUser procedure.

The changes to the getUser procedure align with the new requirements. However, consider improving the error handling when typeform_response_id is null or undefined:

 if (
   user?.typeform_response_id === null ||
   user?.typeform_response_id === undefined
 ) {
-  throw new TRPCError({ code: "NOT_FOUND" });
+  throw new TRPCError({ 
+    code: "NOT_FOUND",
+    message: "User's Typeform response not found. The user may not have completed the application form."
+  });
 }

This provides more context about why the data couldn't be found, which can be helpful for debugging and user communication.

src/pages/apply.tsx (1)

Line range hint 767-786: Improved application status handling

The addition of the killed prop and its associated conditional rendering is a good way to control the visibility of the application form. However, consider using a more descriptive variable name like isApplicationClosed for better clarity.

Apply this change:

-const Apply: NextPage<
-  InferGetServerSidePropsType<typeof getServerSideProps>
-> = ({ email, killed }) => {
+const Apply: NextPage<
+  InferGetServerSidePropsType<typeof getServerSideProps>
+> = ({ email, isApplicationClosed }) => {
   // ...

-  {killed && (
+  {isApplicationClosed && (
     // ...
   )}

Remember to update the variable name in other parts of the code as well.

src/server/router/fileBuilder.ts (1)

1-100: Enhance input validation

To ensure data integrity, consider adding more detailed validations to your Zod schemas, such as minimum length for strings, positive numbers for positions, and UUID formats for IDs.

Example enhancements:

 z.object({
   id: z.string(),
+  // If IDs are UUIDs
+  id: z.string().uuid(),
 })

 z.object({
   name: z.string(),
+  name: z.string().min(1),
   formPosition: z.number(),
+  formPosition: z.number().nonnegative(),
   formStructureId: z.string(),
+  formStructureId: z.string().uuid(),
 })

Configuration used: CodeRabbit UI
Review profile: CHILL

src/server/db/configQueries.ts (2)

4-21: LGTM with a minor suggestion for improved error handling.

The function getDeltaHacksApplicationFormName is well-structured and follows good practices. It correctly uses async/await, has appropriate error handling through assertions, and is focused on a single responsibility.

Consider adding a try-catch block to handle potential database errors gracefully:

 export const getDeltaHacksApplicationFormName = async (
   prisma: PrismaClient
 ) => {
+  try {
     const deltaHacksApplicationFormConfig = await prisma.config.findUnique({
       where: {
         id: "DeltaHacksApplication",
       },
     });
     if (!deltaHacksApplicationFormConfig) {
       return null;
     }

     assert(
       deltaHacksApplicationFormConfig.value.length > 0,
       "Invalid DeltaHacksApplicationConfig value"
     );
     return deltaHacksApplicationFormConfig.value;
+  } catch (error) {
+    console.error("Error fetching DeltaHacksApplication config:", error);
+    return null;
+  }
 };

This change would ensure that any unexpected database errors are logged and handled gracefully.

---

1-39: Overall good implementation with room for improvement in error handling and consistency.

This file provides essential functionality for managing application configurations. The functions are well-structured and use Prisma effectively. To further enhance the code:

1. Implement consistent error handling across both functions.
2. Consider creating a reusable utility function for config retrieval to reduce code duplication.
3. Add JSDoc comments to improve function documentation.

Example of a reusable utility function:

async function getConfig(prisma: PrismaClient, configName: string): Promise<string | null> {
  try {
    const config = await prisma.config.findFirst({
      where: { name: configName },
      select: { value: true },
    });
    return config?.value ?? null;
  } catch (error) {
    console.error(`Error fetching ${configName} config:`, error);
    return null;
  }
}

This approach would simplify both functions and ensure consistent error handling.

src/pages/welcome.tsx (2)

88-91: LGTM: Robust form name retrieval and error handling.

The addition of getDeltaHacksApplicationFormName and the subsequent null check improves the robustness of the application. Returning a 404 response when the form doesn't exist is appropriate.

Consider adding a log statement when the form name is not found to aid in debugging:

if (!formName) {
  console.error("DeltaHacks application form name not found");
  return { notFound: true };
}

---

93-97: LGTM: Updated application retrieval logic.

The new query using formSubmission.findFirst is more specific and aligns well with the new schema. It correctly uses the retrieved form name and the user's ID.

Consider adding a select clause to only fetch the necessary fields, which can improve query performance:

const application = await prisma.formSubmission.findFirst({
  where: {
    formStructureId: formName,
    submitterId: session?.user.id,
  },
  select: {
    id: true,
    // Add other necessary fields
  }
});

prisma/schema.prisma (6)

62-63: New relationships added to User model

The addition of applicationSubmissions and answers fields establishes the necessary relationships for the new form submission structure. This change aligns well with the PR objectives.

Consider adding inline comments or documentation to clarify the nature of these relationships, especially since they are not typical one-to-many or one-to-one relationships as mentioned in the past review comments.

---

112-113: Plan for DH10Application model deprecation

The comment indicates that the DH10Application model will be deprecated after data migration to FormSubmissions. To ensure a smooth transition:

1. Create a detailed migration plan with timelines.
2. Implement tracking mechanisms to monitor the progress of data migration.
3. Set up alerts or reminders to revisit this model's deprecation once migration is complete.

Would you like assistance in creating a migration plan or setting up tracking mechanisms?

---

169-180: Approve FormSubmission model with index suggestion

The FormSubmission model effectively captures the core of the new form submission structure, aligning well with the PR objectives. The composite primary key ensures uniqueness of submissions per user and form structure.

Consider adding an index on the status field if you anticipate frequent queries filtering or sorting by status:

@@index([status])

This can improve query performance, especially as the number of submissions grows.

---

189-201: Approve QuestionCategory model with documentation suggestion

The QuestionCategory model is well-structured with appropriate unique constraints ensuring proper organization within each form structure. This aligns well with the dynamic form creation objective.

Consider adding a comment to explain the purpose and behavior of the formPosition field:

model QuestionCategory {
  // ... other fields ...
  formPosition Int    // Determines the order of categories within the form
  // ... rest of the model ...
}

This will help other developers understand the significance of this field in organizing the form structure.

---

203-226: Approve Question and Answer models with suggestion

The Question and Answer models effectively complete the form submission structure, allowing for flexible question types and organized responses. The constraints and relationships are well-defined.

Consider adding a reference to the FormSubmission in the Answer model to directly link answers to specific submissions. This could be useful for querying all answers for a particular submission:

model Answer {
  statement String?

  addressedQuestion   Question @relation(fields: [addressedQuestionId], references: [id])
  addressedQuestionId String

  submitter   User   @relation(fields: [submitterId], references: [id])
  submitterId String

+ formSubmission   FormSubmission @relation(fields: [formStructureId, submitterId], references: [formStructureId, submitterId])
+ formStructureId  String

  @@id([addressedQuestionId, submitterId])
+ @@index([formStructureId, submitterId])
}

This change would allow for more efficient querying of all answers related to a specific form submission.

---

Line range hint 1-226: Overall assessment of schema changes

The changes introduced in this schema represent a significant shift towards a more flexible and dynamic form management system, aligning well with the PR objectives. Key points:

1. The new models (FormSubmission, FormStructure, QuestionCategory, Question, and Answer) provide a robust structure for dynamic form creation and submission.
2. Deprecation of old fields in the User model and the Review model indicates a clear move towards the new system.
3. The DH10Application model serves as a transitional step, with plans for future deprecation.

These changes will greatly enhance the application's ability to handle diverse form types with minimal development effort. However, ensure that:

1. A comprehensive migration strategy is in place for transitioning data from old to new structures.
2. All parts of the application using deprecated fields are updated to use the new models.
3. Adequate documentation is provided, especially for complex relationships and field purposes.
4. Performance considerations (like suggested indexes) are addressed as the system scales.

Great work on redesigning the schema to support more flexible form management!

src/server/router/reviewers.ts (3)

32-71: LGTM: Improved getApplications procedure with minor suggestion

The updates to the getApplications procedure are well-implemented:

1. The use of trpcAssert for authorization checks enhances error handling.
2. The query now correctly uses formStructureId, aligning with the new schema.
3. Error handling for parsing failures adds robustness.

However, consider simplifying the parsing logic:

return ApplicationForReview.array().parse(application);

If ApplicationForReview.array().parse() is type-safe, the try-catch block might be unnecessary. This could make the code more concise without sacrificing safety.

---

73-153: LGTM: Comprehensive improvements to getApplication procedure

The updates to the getApplication procedure are well-implemented:

1. Using submitterId aligns with the new schema.
2. The complex query structure efficiently fetches all necessary data in a single database call.
3. Sorting logic ensures consistent ordering of categories and questions.

Consider extracting the sorting logic into a separate function for better readability:

function sortQuestionCategories(categories: any[]): void {
  categories.sort((a, b) => a.formPosition - b.formPosition);
  categories.forEach(category => {
    category.questions.sort((a, b) => a.categoryPosition - b.categoryPosition);
  });
}

// Usage
sortQuestionCategories(applicationSubmission.formStructure.questionCategories);

This refactoring would make the main function more concise and the sorting logic more reusable.

---

155-207: LGTM: Well-implemented updateApplicationShallow procedure

The updates to the updateApplicationShallow procedure (formerly updateStatus) are well-implemented:

1. The new name better reflects the function's purpose.
2. Using submitterId and formStructureId for unique identification aligns with the new schema.
3. LogSnag tracking provides better visibility into status changes.

Consider refactoring the status icon logic for better readability:

const statusIcons = {
  [Status.ACCEPTED]: "✅",
  [Status.REJECTED]: "❌",
  [Status.WAITLISTED]: "🕰️",
  [Status.RSVP]: "🎟️",
};

const getStatusIcon = (status: Status) => statusIcons[status] || "🤔";

// Usage
icon: getStatusIcon(input.application.status),

This approach would make the code more maintainable and easier to extend in the future.

src/server/router/application.ts (6)

109-122: LGTM with a minor suggestion: Improved authorization and form name handling.

The changes enhance error handling with trpcAssert and add flexibility by dynamically fetching the form name. The early return for missing form name is a good practice.

Consider adding a more specific error message when the form name is not found:

 if (!formName) {
+  throw new TRPCError({
+    code: "NOT_FOUND",
+    message: "DeltaHacks application form name not configured",
+  });
   return [];
 }

This would provide more context for debugging if the issue occurs.

---

163-191: LGTM with a suggestion: RSVP procedure updated to use formSubmission.

The changes align well with the new schema focusing on formSubmissions. The use of trpcAssert improves error handling and robustness. However, there's a potential issue with the assertion:

trpcAssert(form, "NOT_FOUND");
trpcAssert(form.status === Status.ACCEPTED, "UNAUTHORIZED");

This assertion will throw an "UNAUTHORIZED" error if the status is not ACCEPTED, but it will throw a "NOT_FOUND" error if the form is null. Consider separating these checks for clarity:

trpcAssert(form, "NOT_FOUND", "No application found for the User to RSVP");
trpcAssert(form.status === Status.ACCEPTED, "UNAUTHORIZED", "User's application is not in ACCEPTED status");

This change would provide more specific error messages based on the actual condition that failed.

---

195-220: LGTM: New getApplicationShallow procedure added.

The new getApplicationShallow procedure is a good addition that aligns with the new schema changes. It includes proper null checks and allows fetching applications for different users, which can be useful for admin functions.

Consider adding a permission check to ensure only authorized users (e.g., admins) can fetch applications for other users:

if (input.submitterId !== ctx.session.user.id) {
  trpcAssert(
    ctx.session.user.role.includes(Role.ADMIN),
    "UNAUTHORIZED",
    "Only admins can fetch applications for other users"
  );
}

This would prevent potential unauthorized access to other users' applications.

---

222-462: LGTM with suggestions: submit procedure updated for new schema.

The submit procedure has been significantly updated to work with the new schema, using applicationSchema for input validation and storing data in a more granular format. These changes improve data integrity and flexibility.

However, there are a few points to consider:

1. The procedure is quite long and complex. Consider refactoring it into smaller, more manageable functions for better readability and maintainability.

2. There's a potential issue with the date handling for gradDate:

if (!Number.isNaN(possible.getTime())) {
  gradDate = possible;
}

Consider adding a null check before calling getTime():

if (possible && !Number.isNaN(possible.getTime())) {
  gradDate = possible;
}

3. Consider using a more descriptive name for the answers array, such as formattedAnswers, to better reflect its purpose.

4. The use of Promise.all for inserting answers is good for performance, but consider adding error handling:

try {
  await Promise.all(
    answers.map(async (answerPartial) => {
      // ... existing code ...
    })
  );
} catch (error) {
  console.error("Error inserting answers:", error);
  throw new TRPCError({
    code: "INTERNAL_SERVER_ERROR",
    message: "Failed to save application answers",
  });
}

This would provide better error handling and logging for potential issues during answer insertion.

---

589-626: LGTM with a suggestion: deleteApplication procedure updated for new schema.

The deleteApplication procedure has been updated to work with the new schema, deleting a formSubmission instead of a dH10Application. The addition of error handling and logging improves the robustness of the procedure.

Consider adding a check to ensure the user has the necessary permissions to delete the application:

trpcAssert(
  ctx.session.user.id === input.userId || ctx.session.user.role.includes(Role.ADMIN),
  "UNAUTHORIZED",
  "You don't have permission to delete this application"
);

This would prevent unauthorized deletion of applications.

---

Line range hint 1-626: Overall LGTM: Significant improvements to application router.

The changes in this file align well with the new database schema and introduce several improvements:

1. Enhanced error handling with trpcAssert throughout the file.
2. Improved input validation using applicationSchema.
3. New procedures like getApplicationShallow that add functionality.
4. Updated procedures that work with the new formSubmission-based schema.

While the changes are generally positive, consider the following for future improvements:

1. Refactor long procedures (like submit) into smaller, more manageable functions.
2. Add more specific error messages to improve debugging and user experience.
3. Implement consistent permission checks across procedures that handle sensitive operations.

These changes significantly enhance the robustness and flexibility of the application router.

src/pages/apply.tsx (2)

797-808: LGTM: Improved configuration handling and error checking

The changes in the getServerSideProps function are well-implemented:

1. Handling the case where the form name might be uninitialized during initial migration is a good edge case consideration.
2. Using Config functions improves modularity and separates concerns.
3. Passing the 'killed' status to the component is consistent with the PR objectives.

Consider adding more detailed error logging when the form name is not initialized:

 const formName = await Config.getDeltaHacksApplicationFormName(prisma);
 if (!formName) {
+  console.error('DeltaHacks application form name is not initialized');
   return { redirect: { destination: "welcome", permanent: false } };
 }

This will help with debugging in case of initialization issues.

---

Line range hint 1-811: LGTM: Overall improvements in configuration handling and modularity

The changes in this file are well-implemented and consistent with the PR objectives. The improvements in configuration handling and the generalization of the submission process enhance the modularity and maintainability of the code.

To further improve code organization, consider extracting the ApplyForm component into a separate file. This would reduce the size of the current file and improve readability. You can create a new file src/components/ApplyForm.tsx and import it in this file:

+import ApplyForm from '../components/ApplyForm';
 
 const Apply: NextPage<
   InferGetServerSidePropsType<typeof getServerSideProps>
 > = ({ email, killed }) => {
   // ... existing code ...
 };

This change would make the file structure more modular and easier to maintain.

Configuration used: CodeRabbit UI
Review profile: CHILL