feat: templates #537
feat: templates #537
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
apps/marketing/src/components/(marketing)/single-player-mode/single-player-mode-success.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/(dashboard)/template/create-template.action.ts
Outdated
Show resolved
Hide resolved
apps/web/src/app/(dashboard)/template/create-template.action.ts
Outdated
Show resolved
Hide resolved
WalkthroughThe codebase has been updated with functionalities to manage templates, including creating, editing, duplicating, and deleting templates. New properties like Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 47
Configuration used: CodeRabbit UI
Files selected for processing (57)
- apps/marketing/src/app/(marketing)/singleplayer/client.tsx (1 hunks)
- apps/marketing/src/components/(marketing)/single-player-mode/single-player-mode-success.tsx (1 hunks)
- apps/web/src/app/(dashboard)/documents/data-table-action-dropdown.tsx (2 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/edit-template.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/page.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/data-table-action-dropdown.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/data-table-templates.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/data-table-title.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/delete-template-dialog.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/duplicate-template-dialog.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/empty-state.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/new-template-dialog.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/page.tsx (1 hunks)
- apps/web/src/components/(dashboard)/layout/desktop-nav.tsx (3 hunks)
- apps/web/src/components/(dashboard)/layout/header.tsx (1 hunks)
- apps/web/src/components/(dashboard)/layout/profile-dropdown.tsx (2 hunks)
- apps/web/src/components/formatter/template-type.tsx (1 hunks)
- apps/web/src/components/forms/edit-template/add-template-fields.action.ts (1 hunks)
- apps/web/src/components/forms/edit-template/add-template-placeholders.action.ts (1 hunks)
- packages/lib/server-only/admin/get-recipients-stats.ts (1 hunks)
- packages/lib/server-only/document/find-documents.ts (2 hunks)
- packages/lib/server-only/field/get-fields-for-template.ts (1 hunks)
- packages/lib/server-only/field/remove-signed-field-with-token.ts (1 hunks)
- packages/lib/server-only/field/set-fields-for-template.ts (1 hunks)
- packages/lib/server-only/field/sign-field-with-token.ts (1 hunks)
- packages/lib/server-only/recipient/get-recipients-for-template.ts (1 hunks)
- packages/lib/server-only/recipient/set-recipients-for-template.ts (1 hunks)
- packages/lib/server-only/template/create-document-from-template.ts (1 hunks)
- packages/lib/server-only/template/create-template.ts (1 hunks)
- packages/lib/server-only/template/delete-template.ts (1 hunks)
- packages/lib/server-only/template/duplicate-template.ts (1 hunks)
- packages/lib/server-only/template/get-template-by-id.ts (1 hunks)
- packages/lib/server-only/template/get-templates.ts (1 hunks)
- packages/prisma/migrations/20231007013737_templates/migration.sql (1 hunks)
- packages/prisma/migrations/20231007014431_templates_type/migration.sql (1 hunks)
- packages/prisma/migrations/20231007021427_reuse_document_data/migration.sql (1 hunks)
- packages/prisma/migrations/20231007033447_remove_inserted_on_template_field/migration.sql (1 hunks)
- packages/prisma/migrations/20231007080315_document_name_to_template/migration.sql (1 hunks)
- packages/prisma/migrations/20231007211915_template_created_date/migration.sql (1 hunks)
- packages/prisma/migrations/20231017041643_placeholder_recipients/migration.sql (1 hunks)
- packages/prisma/migrations/20231017042227_fix_typo/migration.sql (1 hunks)
- packages/prisma/migrations/20231019043226_template_recipient_email/migration.sql (1 hunks)
- packages/prisma/migrations/20231020032507_template_recipient_token/migration.sql (1 hunks)
- packages/prisma/migrations/20231021193915_template_token_for_recipient/migration.sql (1 hunks)
- packages/prisma/migrations/20231030061522_recipient_name_not_placeholder/migration.sql (1 hunks)
- packages/prisma/migrations/20231208090322_remove_template_specific_models/migration.sql (1 hunks)
- packages/prisma/schema.prisma (6 hunks)
- packages/trpc/server/router.ts (2 hunks)
- packages/trpc/server/template-router/router.ts (1 hunks)
- packages/trpc/server/template-router/schema.ts (1 hunks)
- packages/ui/primitives/dialog.tsx (1 hunks)
- packages/ui/primitives/document-dropzone.tsx (2 hunks)
- packages/ui/primitives/document-flow/types.ts (1 hunks)
- packages/ui/primitives/template-flow/add-template-fields.tsx (1 hunks)
- packages/ui/primitives/template-flow/add-template-fields.types.ts (1 hunks)
- packages/ui/primitives/template-flow/add-template-placeholder-recipients.tsx (1 hunks)
- packages/ui/primitives/template-flow/add-template-placeholder-recipients.types.ts (1 hunks)
Files skipped from review due to trivial changes (6)
- apps/marketing/src/components/(marketing)/single-player-mode/single-player-mode-success.tsx
- packages/prisma/migrations/20231007014431_templates_type/migration.sql
- packages/prisma/migrations/20231007080315_document_name_to_template/migration.sql
- packages/prisma/migrations/20231019043226_template_recipient_email/migration.sql
- packages/prisma/migrations/20231020032507_template_recipient_token/migration.sql
- packages/prisma/migrations/20231021193915_template_token_for_recipient/migration.sql
Additional comments: 77
apps/marketing/src/app/(marketing)/singleplayer/client.tsx (2)
151-157: The addition of the
templateTokenproperty to theplaceholderRecipientobject is consistent with the PR's objective of enhancing template functionality. Ensure that this new property is properly integrated and utilized whereverplaceholderRecipientis used.151-157: Verify that all instances of
Recipientobject creation or manipulation across the codebase have been updated to include the newtemplateTokenproperty.apps/web/src/app/(dashboard)/documents/data-table-action-dropdown.tsx (2)
101-102:
The addition ofnonSignedRecipientsis a good use of filtering to identify recipients who have not signed the document, which can be used for further actions like resending the document.131-134:
Please confirm if the "Duplicate" menu item is intended to be permanently disabled. If it should be conditionally enabled, the disabling logic needs to be implemented.apps/web/src/app/(dashboard)/templates/data-table-action-dropdown.tsx (2)
26-79: Ensure that the
DuplicateTemplateDialogandDeleteTemplateDialogcomponents have proper error handling and state management, especially if they perform asynchronous operations.32-34: Returning
nullfrom a component is a valid way to prevent rendering, but ensure that this is the desired UX when there is no session.apps/web/src/app/(dashboard)/templates/data-table-title.tsx (1)
- 14-15: The previous suggestion to provide a fallback or message when there's no session has not been implemented. Consider updating the code to improve the user experience.
if (!session) { - return null; + return <p>Please log in to view templates.</p>; }apps/web/src/app/(dashboard)/templates/empty-state.tsx (1)
- 3-17: The
EmptyTemplateStatecomponent is well-structured and follows React functional component best practices. The use of theBirdicon fromlucide-reactand the instructional text provides a clear and friendly user interface for cases where no templates exist.apps/web/src/app/(dashboard)/templates/new-template-dialog.tsx (1)
- 1-228: The
NewTemplateDialogcomponent is well-structured and follows best practices for form handling, validation, and state management in React. It integrates with the existing design system and uses tRPC for server interactions. Ensure that the new UI components and server-side functions are properly tested, especially the file upload and template creation functionalities, to maintain the application's reliability.apps/web/src/app/(dashboard)/templates/page.tsx (1)
- 10-14: Clarify how the
searchParamsprop is managed and updated, and ensure there's a mechanism in place to trigger re-renders when it changes.apps/web/src/components/(dashboard)/layout/desktop-nav.tsx (5)
6-7: The addition of
LinkandusePathnameimports is consistent with the new navigation links feature.16-25: The
navigationLinksarray is added to store the navigation links, which is a good practice for maintainability and scalability.50-65: The rendering logic for the navigation links has been refactored to use the
navigationLinksarray, which is correctly implemented.58-58: The use of
pathname?.startsWith(href)is a good way to set the active class on the current navigation link.44-47: The use of
cnutility function to combine class names is a good practice for conditional class rendering.apps/web/src/components/(dashboard)/layout/header.tsx (1)
- 52-52: The addition of the
md:ml-8class to thedivelement is a styling change that will apply a left margin on medium-sized screens and above. This change seems appropriate for responsive design adjustments.apps/web/src/components/(dashboard)/layout/profile-dropdown.tsx (3)
7-7: The addition of the
FileSpreadsheetimport aligns with the new feature implementation.110-116: The addition of the Templates link in the
ProfileDropdowncomponent is correctly implemented and supports the new feature.107-119: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [4-119]
No further issues or improvements are identified in the
ProfileDropdowncomponent related to the new feature addition.apps/web/src/components/formatter/template-type.tsx (1)
- 1-50: The
TemplateTypecomponent and its associated types and constants are well-defined and correctly implemented. The use of types from the Prisma client ensures consistency with the backend schema, and the component's props provide flexibility for styling and behavior. The code is clean, maintainable, and follows best practices for a React component.apps/web/src/components/forms/edit-template/add-template-placeholders.action.ts (1)
- 5-5: There appears to be a typo in the type name
TAddTemplatePlacholderRecipientsFormSchema. Please verify if the type name should beTAddTemplatePlaceholderRecipientsFormSchema.packages/lib/server-only/document/find-documents.ts (2)
- 8-14: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [11-22]
The
orderByproperty withinFindDocumentsOptionshas been modified to usecolumninstead offield. Verify that all references to this property have been updated to reflect this change.Verification successful
The search results show two occurrences of
FindDocumentsOptions. The first one is infind-documents.ts, which is the file where the type was changed, and it shows the updatedorderByproperty usingcolumninstead offield. The second occurrence is inget-all-documents.ts, which defines a differentFindDocumentsOptionsinterface without anorderByproperty.Since the
ast-grepcommand did not find any instances of the oldfieldproperty and thergcommand only found the updated usage infind-documents.tsand a different interface inget-all-documents.ts, it seems that the change fromfieldtocolumnhas been correctly updated in the codebase.Based on the information gathered, it appears that all references to the
orderByproperty withinFindDocumentsOptionshave been updated to reflect the change fromfieldtocolumn.* 24-25: The `findDocuments` function appears to be correctly implemented with the updated `FindDocumentsOptions` type.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that might be using the old `field` property. ast-grep --lang typescript --pattern $'FindDocumentsOptions { $$$ orderBy?: { field: $_, $$$ }; $$$ }'Length of output: 620
Script:
#!/bin/bash # Corrected search for files that might be using the old `field` property within `FindDocumentsOptions`. ast-grep --lang typescript --pattern $'type FindDocumentsOptions = { $$$ orderBy?: { field: $_, $$$ }; $$$ }'Length of output: 126
Script:
#!/bin/bash # Search for any TypeScript files that might be using the `FindDocumentsOptions` type to ensure the `orderBy` property is correctly updated. rg --type ts 'FindDocumentsOptions' -A 10Length of output: 3052
packages/lib/server-only/field/remove-signed-field-with-token.ts (3)
30-36: The added error checks for document existence and completion status are consistent with the PR's objectives and enhance the robustness of the
removeSignedFieldWithTokenfunction by ensuring that operations are not performed on non-existent or completed documents.30-36: Consider verifying that the error handling strategy used here, particularly the error messages, is consistent with the rest of the application. Consistency in error handling can improve maintainability and the debugging process.
Verification successful
The error handling strategy in
remove-signed-field-with-token.ts, particularly the error messages, appears to be consistent with the rest of the application. The error messages throughout the codebase follow a similar pattern of specificity and clarity, often including the entity type and its ID for context, which aligns with the approach taken in the file under review.* 34-36: Ensure that the `Promise.all` does not contain operations that have dependencies on each other, as this could lead to race conditions or unintended side effects if they are executed concurrently.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar error patterns to ensure consistency rg "throw new Error" --type=tsLength of output: 13219
packages/lib/server-only/field/set-fields-for-template.ts (2)
87-88: The
createblock in theupsertoperation setscustomTextto an empty string andinsertedto false. Verify if these are appropriate default values or if they should be configurable or derived from some other state.94-98: The
Recipientconnect block in thecreateoperation uses bothidandsignerIdorsignerEmailmight be optional and that the correct fields are used for establishing the connection.packages/lib/server-only/field/sign-field-with-token.ts (1)
- 36-42: The addition of error handling for a missing
documentobject and a check for theCOMPLETEDstatus of the document before proceeding with the signing process is a good practice to ensure data integrity and prevent illegal state transitions.packages/lib/server-only/recipient/set-recipients-for-template.ts (1)
- 79-80: The generation of
templateTokenfor each recipient might be correct, but it's important to verify that this token is intended to be unique per recipient and not per template. If it should be unique per template, this could lead to unintended behavior.packages/lib/server-only/template/create-template.ts (1)
- 1-20: The implementation of the
createTemplatefunction aligns with the PR's objectives and correctly uses the Prisma client to interact with the database.packages/lib/server-only/template/get-template-by-id.ts (1)
- 1-18: The function
getTemplateByIdis well-structured and uses Prisma'sfindFirstOrThrowmethod appropriately to retrieve a template by its ID and the user ID. The use of TypeScript for typing the function parameters with theGetTemplateByIdOptionsinterface is a good practice for type safety. The function is marked as async and correctly uses await for the asynchronous operation. However, ensure that the calling code is prepared to handle the potential error thrown byfindFirstOrThrowwhen no record is found.packages/prisma/migrations/20231007013737_templates/migration.sql (6)
- 7-7: The
userIdfield references theUsertable. Ensure that theUsertable'sidfield is also of typeINTEGERto maintain referential integrity.Verification successful
The output from the script confirms that the
idfield of theUsertable is defined asSERIALand notINTEGER. In PostgreSQL, theSERIALdata type is used for auto-incrementing integer columns and it implies that the column is of typeINTEGERinternally, but also includes auto-increment functionality.Given this information, the
userIdfield in theTemplatetable, which is of typeINTEGER, should be compatible with theidfield of theUsertable for the purpose of foreign key referencing, as both are integer types. Therefore, the referential integrity between theTemplatetable'suserIdfield and theUsertable'sidfield is maintained.Reply:
The
Usertable'sidfield is defined asSERIAL, which is compatible with theINTEGERtype used for theuserIdfield in theTemplatetable. This ensures referential integrity between these fields. No further action is required.* 10-10: The default value for `status` is set to `'PRIVATE'`. Confirm that this default aligns with the application's business logic and privacy requirements.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the User table's id field is of type INTEGER rg 'CREATE TABLE "User"' -A 10 | rg '"id" INTEGER'Length of output: 52
Script:
#!/bin/bash # Search for the CREATE TABLE "User" statement and check the type of the "id" field rg 'CREATE TABLE "User" {.*"id" INTEGER.*};' -ULength of output: 192
Script:
#!/bin/bash # Search for the CREATE TABLE "User" statement in SQL migration files and check the type of the "id" field rg --type sql 'CREATE TABLE "User".*"id" INTEGER' -ULength of output: 54
Script:
#!/bin/bash # Find the SQL migration file(s) that contain the CREATE TABLE "User" statement fd --type f --extension sql '.*' --exec rg 'CREATE TABLE "User"'Length of output: 88
Script:
#!/bin/bash # Find the SQL migration file(s) that contain the CREATE TABLE "User" statement and display the lines after the match to include the id field definition fd --type f --extension sql '.*' --exec rg 'CREATE TABLE "User"' -A 5Length of output: 218
32-35: The default values for
positionX,positionY,width, andheightin theTemplateFieldtable are set to0and-1. Ensure that these defaults are intentional and documented, as-1is an unusual default for dimensions.36-36: The
customTextfield in theTemplateFieldtable is marked asNOT NULLwith no default value. This requires that every insert operation must provide a value forcustomText. Confirm that this is the desired behavior.43-43: The unique index on
templateDataIdin theTemplatetable is good for ensuring that each template has a unique data reference. However, ensure that the application logic supports this constraint and that there are no use cases where a template might share data with another template.46-52: Foreign key constraints are set to
ON DELETE CASCADEandON UPDATE CASCADE. This is a critical behavior that should be confirmed with the application's data management strategy, as it will automatically delete or update related records, which might not always be desirable.packages/prisma/migrations/20231007211915_template_created_date/migration.sql (1)
- 8-9: The issue with the
updatedAtcolumn not having a default value has not been resolved. Existing rows in theTemplatetable will lack a value for this column, which could lead to errors. Consider implementing the previous suggestion to add a default value or use a trigger to update this field.packages/prisma/migrations/20231017042227_fix_typo/migration.sql (2)
5-20: Before applying the unique constraint on the
templateDocumentDataIdcolumn, ensure that there are no existing duplicate values in the table, as this will cause the migration to fail.22-23: Verify that the
ON DELETE CASCADE ON UPDATE CASCADEactions on the foreign key are intended and align with the application's data integrity requirements.packages/prisma/migrations/20231208090322_remove_template_specific_models/migration.sql (5)
1-6: Before executing this migration, verify that there are backups of the
TemplateFieldandTemplateRecipienttables, and that the data is either no longer needed or has been migrated to a new structure.6-6: Ensure a pre-migration script is run to identify and resolve any duplicate
[templateId,email]entries in theRecipienttable before applying the unique constraint.22-30: Confirm that the application logic has been updated to handle the new
templateIdcolumn and the nullable state of the previously NOT NULL columns in theFieldandRecipienttables.38-54: Check that the newly created indices and foreign key constraints are correctly set up and reference the intended columns and tables, especially after the schema changes.
47-54: Ensure that the cascading delete and update behavior introduced by the new foreign key constraints is intentional and won't lead to unintended data loss or inconsistency.
packages/prisma/schema.prisma (6)
43-43: The addition of the
Templaterelation to theUsermodel aligns with the PR's objectives to enhance template functionality.158-158: The addition of the optional
Templaterelation to theDocumentDatamodel aligns with the PR's objectives to allow documents to be associated with templates.185-207: The changes to the
Recipientmodel, including the addition oftemplateId,templateToken, and the adjustments to the nullability of certain fields, are consistent with the PR's objectives to link recipients to templates and documents.228-240: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [220-237]
The changes to the
Fieldmodel, including the addition oftemplateIdand the new relation toTemplate, are consistent with the PR's objectives to allow fields to be associated with both documents and templates.
268-287: The introduction of the
Templatemodel with its fields and relations is consistent with the PR's objectives to support template functionality within the application.191-191: The addition of the
templateTokenfield to theRecipientmodel has been previously discussed and explained by the developers, indicating its purpose in linking recipients to the original template recipients.packages/trpc/server/router.ts (3)
9-9: The addition of
templateRouterto the imports is consistent with the PR's objective to handle template-related operations.23-23: The integration of
templateRouterinto theappRouterobject is correctly implemented, aligning with the PR's objectives to introduce new routing logic for templates.6-12: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [6-26]
The changes to
router.tsare well-contained and align with the PR's objectives. Ensure that the newtemplateTokenproperty in theSinglePlayerClientfunction, as mentioned in the PR overview, is also correctly implemented in the relevant part of the codebase.packages/trpc/server/template-router/router.ts (1)
- 1-94: The
templateRoutersetup and the associated CRUD operations for templates are implemented correctly, following standard practices for a TRPC router in a TypeScript project.packages/trpc/server/template-router/schema.ts (2)
3-18:
The Zod schemas for template operations are well-defined. Ensure that the validation rules align with the corresponding database schema and the rest of the application's validation logic.20-26:
The inferred types from the Zod schemas are correctly exported. Verify that these types are utilized in the relevant parts of the codebase to maintain type safety.packages/ui/primitives/dialog.tsx (1)
- 23-26: The change in z-index to
z-[9999]ensures the dialog overlays other elements, but consider verifying that this does not introduce any visual stacking issues with other fixed or absolute elements in the application.packages/ui/primitives/document-dropzone.tsx (4)
91-91: The addition of the
typeproperty toDocumentDropzonePropsis consistent with the PR's objectives to handle different types of documents.78-85: The
DocumentDescriptionobject is a clean way to manage different headlines for theDocumentDropzonecomponent based on thetypeproperty.99-99: Setting the default value of
typeto'document'ensures backward compatibility for existing uses ofDocumentDropzonewhere thetypemay not be provided.171-171: The use of
DocumentDescription[type].headlinecorrectly displays the headline based on thetypeproperty. Ensure that thetypeproperty is validated elsewhere in the code to only allow'document'or'template'.packages/ui/primitives/template-flow/add-template-fields.tsx (13)
3-35: The imports are well-organized and seem to be relevant to the functionality of the component. It's good to see that unused imports, such as the commented-out Tooltip imports, are being removed to keep the code clean.
41-46: The
fontCaveatis defined outside of the component, which is good for performance as it won't be redefined on each render. However, ensure that theCaveatfunction and thefontCaveatvariable are used elsewhere in the application to justify their inclusion.54-59: The
AddTemplateFieldsFormPropstype definition is clear and well-defined. It's good to see TypeScript being used to enforce type safety for component props.62-68: The
AddTemplateFieldsFormPartialcomponent is well-structured and the props are destructured, which improves readability.73-95: The use of
useFormfromreact-hook-formis appropriate for managing form state and handling submissions. The default values are set based on thefieldsprop, which is a good practice. Ensure that theTAddTemplateFieldsFormSchematype accurately reflects the expected structure of the form data.109-117: The use of
useStatefor managing local component state is standard in React. The naming of the state variables is clear and indicates their purpose.124-140: The
onMouseMovecallback is usinguseCallbackto memoize the function, which can help with performance if the function is passed as a prop to child components or if it's used in a dependency array of effects or other callbacks.199-227: The
onFieldResizecallback is quite complex. Consider breaking it down into smaller, more manageable functions. This will improve readability and maintainability.
- 229-250: The
onFieldMovecallback is quite complex. Consider breaking it down into smaller, more manageable functions. This will improve readability and maintainability.
252-262: The
useEffecthook is used to add and remove event listeners based on theselectedFieldstate. This is a good use of the hook to ensure that event listeners are cleaned up when they are no longer needed.264-288: The
useEffecthook is used to observe changes in the document body and update thefieldBoundsref accordingly. This is a good use ofMutationObserverto react to DOM changes. However, ensure that the observed changes are relevant to the component's functionality and that this does not lead to unnecessary updates or performance issues.290-292: The
useEffecthook is used to set the default selected signer. This is a good practice to initialize component state based on props.294-538: The component's return statement is well-structured and uses conditional rendering effectively. The use of utility classes and conditional class names with
cnis a good practice for dynamic styling. Ensure that all the utility classes used are defined and behave as expected.packages/ui/primitives/template-flow/add-template-fields.types.ts (1)
- 1-23: The Zod schema and TypeScript type definition for adding template fields appear to be correctly defined and make use of the
zodlibrary for runtime validation and TypeScript for compile-time type safety. The use ofFieldTypefrom@documenso/prisma/clientensures consistency with the database schema.
packages/prisma/migrations/20231030061522_recipient_name_not_placeholder/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20231007013737_templates/migration.sql
Outdated
Show resolved
Hide resolved
packages/prisma/migrations/20231007013737_templates/migration.sql
Outdated
Show resolved
Hide resolved
|
Just the last few comments and request to change up the server actions to tRPC endpoints since we've since tossed them 🙏 We're so close now, sorry for taking so long boss 🫡 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- apps/marketing/src/app/(marketing)/singleplayer/client.tsx (2 hunks)
- packages/lib/server-only/recipient/set-recipients-for-template.ts (1 hunks)
- packages/lib/server-only/template/create-document-from-template.ts (1 hunks)
- packages/lib/server-only/template/duplicate-template.ts (1 hunks)
- packages/prisma/migrations/20231214081915_remove_template_token_column/migration.sql (1 hunks)
- packages/prisma/schema.prisma (6 hunks)
- packages/trpc/server/singleplayer-router/router.ts (1 hunks)
- packages/ui/primitives/template-flow/add-template-fields.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/lib/server-only/template/create-document-from-template.ts
- packages/lib/server-only/template/duplicate-template.ts
Files skipped from review as they are similar to previous changes (2)
- apps/marketing/src/app/(marketing)/singleplayer/client.tsx
- packages/lib/server-only/recipient/set-recipients-for-template.ts
Additional comments: 19
packages/prisma/schema.prisma (5)
43-43: The addition of the
Templaterelation to theUsermodel aligns with the PR's objectives to enhance template functionality.158-158: The addition of the optional
Templaterelation to theDocumentDatamodel is consistent with the PR's objectives.263-287: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [185-287]
The changes to the
Recipientmodel, including making certain fields nullable and adding new fields and relations, are in line with the PR's objectives. Ensure that the new unique constraints and indexes are properly reviewed and tested to maintain data integrity and performance.
- 227-239: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [219-236]
The updates to the
Fieldmodel, including the addition oftemplateIdand the update to relations, support the new template features as described in the PR.
- 267-287: The new
Templatemodel with its fields and relations is a key addition to support the template functionality as outlined in the PR.packages/trpc/server/singleplayer-router/router.ts (1)
- 66-66: The addition of
templateId: nullis noted. Please ensure that setting this tonullis intentional and that it aligns with the current implementation and future plans for template functionality. If this is a placeholder for future work, consider adding a comment or TODO to clarify its purpose.packages/ui/primitives/template-flow/add-template-fields.tsx (13)
1-35: The imports and component declarations seem to be in order, with no apparent issues regarding unused imports or missing dependencies.
41-46: The
fontCaveatis defined but not used within the provided code. If it's not used elsewhere, consider removing it to keep the code clean.Verification successful
The
fontCaveatvariable is used in multiple places within the codebase, including within the same file where it is defined. Therefore, it should not be removed. The code does not require any changes regarding this variable.* 54-60: The `AddTemplateFieldsFormProps` type definition is clear and well-defined.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of `fontCaveat` in the codebase. rg 'fontCaveat'Length of output: 1945
62-97: The component
AddTemplateFieldsFormPartialis well-structured, and the use of hooks for form management and field arrays is appropriate.109-117: The state management for
selectedField,selectedSigner, andshowRecipientsSelectoris straightforward and follows React best practices.124-140: The
onMouseMovecallback is well-implemented usinguseCallbackfor performance optimization.143-197: The
onMouseClickcallback is complex and handles several operations. Ensure that all edge cases are covered, especially with the conditional checks and DOM interactions.199-227: The
onFieldResizecallback is complex. Consider breaking it down into smaller, more manageable functions. This will improve readability and maintainability.
- This comment has been made in a previous review and is still valid.
- 229-250: The
onFieldMovecallback is complex. Consider breaking it down into smaller, more manageable functions. This will improve readability and maintainability.
- This comment has been made in a previous review and is still valid.
252-262: The
useEffectfor adding and removing event listeners is correctly implemented with cleanup to prevent memory leaks.264-288: The
useEffectfor observing DOM mutations and setting field bounds is a good use ofMutationObserver. Ensure that it does not cause performance issues with frequent DOM changes.290-292: The
useEffecthook that sets the default selected signer is clear and concise.294-538: The render method is well-organized, and the use of conditional rendering is appropriate. Ensure that all UI components have been tested for proper rendering and functionality.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/prisma/schema.prisma (6 hunks)
Additional comments: 4
packages/prisma/schema.prisma (4)
44-44: Adding a new relation
Templateto theUsermodel is consistent with the PR's objective to introduce template functionality. Ensure that the necessary migrations are created and applied to handle this new relationship in the database.184-203: The
Recipientmodel has been updated with new fields and relations to support templates. Ensure that the unique and index constraints ontemplateIdand226-238: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [218-235]
The
Fieldmodel now includes optional relations to bothDocumentandTemplate. Verify that the business logic correctly handles cases where aFieldmight be associated with either aDocumentor aTemplate, but not both.
- 266-286: The new
Templatemodel has been added with appropriate fields and relations. Ensure that theTemplateTypeenum is used correctly throughout the application and that thetemplateDocumentDataIdfield is handled properly, especially in terms of deletion behavior.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (7)
- apps/marketing/src/components/constants.ts (1 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/edit-template.tsx (1 hunks)
- packages/trpc/server/field-router/router.ts (2 hunks)
- packages/trpc/server/field-router/schema.ts (1 hunks)
- packages/trpc/server/recipient-router/router.ts (2 hunks)
- packages/trpc/server/recipient-router/schema.ts (1 hunks)
- packages/ui/primitives/template-flow/add-template-placeholder-recipients.types.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/marketing/src/components/constants.ts
Additional comments: 2
packages/trpc/server/recipient-router/schema.ts (1)
- 26-47: The new
ZAddTemplateSignersMutationSchemaand its corresponding typeTAddTemplateSignersMutationSchemaare consistent with the existing patterns and correctly enforce unique emails.packages/trpc/server/field-router/schema.ts (1)
- 24-41: The
ZAddTemplateFieldsMutationSchemaand its corresponding typeTAddTemplateFieldsMutationSchemaare correctly defined and follow the existing patterns.
packages/ui/primitives/template-flow/add-template-placeholder-recipients.types.ts
Show resolved
Hide resolved
packages/ui/primitives/template-flow/add-template-placeholder-recipients.types.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (11)
- apps/web/src/app/(dashboard)/documents/duplicate-document-dialog.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/data-table-templates.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/delete-template-dialog.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/duplicate-template-dialog.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/new-template-dialog.tsx (1 hunks)
- packages/lib/server-only/admin/get-recipients-stats.ts (1 hunks)
- packages/lib/server-only/field/set-fields-for-document.ts (1 hunks)
- packages/lib/server-only/field/set-fields-for-template.ts (1 hunks)
- packages/lib/server-only/template/get-templates.ts (1 hunks)
- packages/prisma/migrations/20231221070056_make_recipient_status_columns_non_null_again/migration.sql (1 hunks)
- packages/prisma/schema.prisma (6 hunks)
Additional comments: 7
packages/lib/server-only/admin/get-recipients-stats.ts (1)
- 20-27: Ensure that the aggregation logic in
getRecipientsStatscorrectly handles cases where a recipient may have multiple statuses. If a recipient can only have one status at a time, this logic is correct. Otherwise, the counts may be inflated.packages/lib/server-only/field/set-fields-for-document.ts (1)
- 2-3: The change to use the
typekeyword for importingFieldTypeis a good practice in TypeScript to ensure that only the type is imported and not the runtime object.packages/prisma/schema.prisma (5)
44-44: The addition of the
Templaterelation to theUsermodel is consistent with the PR's objectives. Ensure that the cascade behavior is correctly defined for this relation to maintain referential integrity.157-157: The addition of an optional
Templaterelation to theDocumentDatamodel aligns with the PR's objective. However, consider if there should be any constraints or cascading behaviors defined for this relation.184-203: The
Recipientmodel now includes atemplateIdfield and a relation to theTemplatemodel. Verify that the unique and index constraints are correctly applied and consider the implications of theonDeletebehavior for theTemplaterelation.226-238: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [218-235]
The
Fieldmodel now includes atemplateIdfield and a relation to theTemplatemodel. Ensure that the indexes are correctly defined and review theonDeletebehavior for theTemplaterelation.
- 272-286: The new
Templatemodel is introduced with fields and relations that align with the PR's objectives. Ensure that the unique constraint ontemplateDocumentDataIdis appropriate and that theonDeletebehavior for theUserrelation is correctly defined.
.../prisma/migrations/20231221070056_make_recipient_status_columns_non_null_again/migration.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/prisma/migrations/20231221101005_add_templates/migration.sql (1 hunks)
Additional comments: 9
packages/prisma/migrations/20231221101005_add_templates/migration.sql (9)
8-8: Creation of the
TemplateTypeenum is a good practice for ensuring data integrity for thetypefield in theTemplatetable.11-11: Verify the impact of dropping the foreign key constraint on the
Fieldtable. Ensure that this change is coordinated with any necessary data migrations or application logic updates.14-15: Adding a
templateIdcolumn and makingdocumentIdnullable in theFieldtable allows for associating fields with either a document or a template, but not both. Ensure that the application logic enforces this constraint correctly.19-22: The CHECK constraint added to the
Fieldtable is a good practice to ensure data integrity by allowing only one oftemplateIdordocumentIdto be set.25-26: Similar to the
Fieldtable, adding atemplateIdcolumn and makingdocumentIdnullable in theRecipienttable allows for associating recipients with either a document or a template. Verify that the application logic correctly enforces this constraint.30-33: The CHECK constraint added to the
Recipienttable is a good practice to ensure data integrity by allowing only one oftemplateIdordocumentIdto be set.36-45: The creation of the
Templatetable with appropriate fields and constraints is well-defined and supports the new template functionality.49-58: The creation of unique and non-unique indexes on the
Template,Field, andRecipienttables is a good practice for optimizing query performance.61-73: The addition of foreign key constraints to the
Recipient,Field, andTemplatetables is a good practice for maintaining referential integrity across the database.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (2)
- apps/web/src/app/(dashboard)/documents/data-table-action-dropdown.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/page.tsx (1 hunks)
Additional comments: 8
apps/web/src/app/(dashboard)/templates/[id]/page.tsx (6)
1-12: Imports are well-organized and relevant to the functionality of the page.
17-21: The
TemplatePagePropstype is correctly defined and used to type the component's props.26-30: Good validation check for
templateIdto ensure it's a valid number before proceeding with data fetching.39-41: Proper validation to ensure that the
templateand itstemplateDocumentDataare present before rendering the component.45-54: Efficient use of
Promise.allto fetchtemplateRecipientsandtemplateFieldsconcurrently, reducing wait time.71-78: The
EditTemplateFormcomponent is correctly rendered with props derived from the fetched data, ensuring it has all the necessary information to function.apps/web/src/app/(dashboard)/documents/data-table-action-dropdown.tsx (2)
101-102: The addition of
nonSignedRecipientsis a logical change that filters out recipients who have signed the document. This ensures that actions such as resending the document are only available to recipients who have not yet signed.99-105: The logic within
DataTableActionDropdownis well-structured, with clear conditions for enabling and disabling menu items based on document status and user role.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Database and Migrations
Documentation