feat: add team templates (#912) #82
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Someone is attempting to deploy a commit to a Personal Account owned by @SergeWilfried on Vercel. @SergeWilfried first needs to authorize it. |
WalkthroughWalkthroughThe updates span across the dashboard and team sections of a web application, focusing on refactoring components, enhancing template functionality, and improving import syntax. New features include handling template ownership and team-specific templates, alongside modifications to server logic and database schema to support these enhancements. This comprehensive overhaul aims at improving code maintainability, user experience, and system capabilities for template management. 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 your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![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: 9
Configuration used: CodeRabbit UI
Files selected for processing (41)
- apps/web/src/app/(dashboard)/documents/[id]/document-page-view.tsx (2 hunks)
- apps/web/src/app/(dashboard)/documents/[id]/page.tsx (1 hunks)
- apps/web/src/app/(dashboard)/documents/documents-page-view.tsx (2 hunks)
- apps/web/src/app/(dashboard)/documents/page.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/edit-template.tsx (3 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/page.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/[id]/template-page-view.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/data-table-action-dropdown.tsx (3 hunks)
- apps/web/src/app/(dashboard)/templates/data-table-templates.tsx (3 hunks)
- apps/web/src/app/(dashboard)/templates/delete-template-dialog.tsx (2 hunks)
- apps/web/src/app/(dashboard)/templates/duplicate-template-dialog.tsx (3 hunks)
- apps/web/src/app/(dashboard)/templates/new-template-dialog.tsx (3 hunks)
- apps/web/src/app/(dashboard)/templates/page.tsx (1 hunks)
- apps/web/src/app/(dashboard)/templates/templates-page-view.tsx (1 hunks)
- apps/web/src/app/(teams)/t/[teamUrl]/documents/[id]/page.tsx (2 hunks)
- apps/web/src/app/(teams)/t/[teamUrl]/documents/page.tsx (1 hunks)
- apps/web/src/app/(teams)/t/[teamUrl]/templates/[id]/page.tsx (1 hunks)
- apps/web/src/app/(teams)/t/[teamUrl]/templates/page.tsx (1 hunks)
- apps/web/src/components/(dashboard)/layout/desktop-nav.tsx (1 hunks)
- apps/web/src/components/(dashboard)/layout/menu-switcher.tsx (4 hunks)
- apps/web/src/components/(dashboard)/layout/mobile-navigation.tsx (1 hunks)
- packages/app-tests/e2e/templates/manage-templates.spec.ts (1 hunks)
- packages/lib/constants/teams.ts (1 hunks)
- packages/lib/server-only/field/get-fields-for-template.ts (1 hunks)
- packages/lib/server-only/field/set-fields-for-template.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 (2 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 (2 hunks)
- packages/lib/server-only/template/find-templates.ts (1 hunks)
- packages/lib/server-only/template/get-template-by-id.ts (1 hunks)
- packages/lib/utils/teams.ts (1 hunks)
- packages/prisma/migrations/20240206051948_add_teams_templates/migration.sql (1 hunks)
- packages/prisma/schema.prisma (2 hunks)
- packages/prisma/seed/templates.ts (1 hunks)
- packages/trpc/server/field-router/router.ts (1 hunks)
- packages/trpc/server/recipient-router/router.ts (2 hunks)
- packages/trpc/server/template-router/router.ts (3 hunks)
- packages/trpc/server/template-router/schema.ts (2 hunks)
Additional comments: 65
packages/prisma/migrations/20240206051948_add_teams_templates/migration.sql (1)
- 5-5: The foreign key constraint is correctly set up to cascade on delete and update. This ensures that when a team is deleted, all associated templates are also deleted, maintaining referential integrity.
apps/web/src/app/(dashboard)/documents/[id]/page.tsx (1)
- 1-1: The import statement for
DocumentPageViewcorrectly uses destructuring, aligning with the project's move towards named imports for consistency and clarity.apps/web/src/app/(dashboard)/templates/[id]/page.tsx (2)
- 3-4: The import statements for
TemplatePageViewPropsandTemplatePageVieware correctly structured, using destructuring for consistency with the project's coding standards.- 8-9: The refactoring to delegate rendering to
TemplatePageViewsimplifies theTemplatePagecomponent, improving modularity and maintainability.apps/web/src/app/(dashboard)/documents/page.tsx (1)
- 4-4: The change to use destructuring for importing
DocumentsPageViewaligns with the project's coding standards for consistency and clarity in imports.apps/web/src/app/(dashboard)/templates/page.tsx (2)
- 5-5: The import of
TemplatesPageViewusing destructuring is consistent with the project's coding standards, enhancing clarity and maintainability.- 16-17: Refactoring
TemplatesPageto delegate rendering toTemplatesPageViewsimplifies the component structure, improving code readability and maintainability.packages/lib/server-only/template/delete-template.ts (1)
- 11-29: The deletion logic now correctly considers both direct user ownership and team membership, enhancing the security and correctness of the template deletion process.
apps/web/src/app/(teams)/t/[teamUrl]/documents/[id]/page.tsx (2)
- 4-4: The import statement for
DocumentPageViewcorrectly uses destructuring, aligning with the project's move towards named imports for consistency and clarity.- 19-19: Passing both
paramsandteamtoDocumentPageViewcorrectly extends the component's functionality to support team-specific document handling.packages/lib/server-only/field/get-fields-for-template.ts (1)
- 13-26: The addition of team membership condition in the query logic for
getFieldsForTemplatecorrectly expands the search criteria, ensuring that team members can access fields associated with their team's templates.packages/lib/server-only/template/get-template-by-id.ts (1)
- 10-26: The updated
wherefilter logic correctly incorporates both direct user ownership and team membership, enhancing the security and correctness of template retrieval.packages/lib/server-only/recipient/get-recipients-for-template.ts (1)
- 16-29: The expanded query criteria for retrieving recipients now correctly include both direct user association and team membership, broadening the scope of potential recipients based on user or team relationships.
packages/lib/server-only/template/create-template.ts (2)
- 15-26: The check to ensure the user is a member of the specified team before creating a template with a
teamIdenhances security by enforcing team membership validation.- 33-33: Including
teamIdin the template creation process correctly extends the functionality to support team-specific templates, aligning with the feature's objectives.apps/web/src/app/(teams)/t/[teamUrl]/documents/page.tsx (1)
- 5-5: The change to use destructuring for importing
DocumentsPageViewaligns with the project's coding standards for consistency and clarity in imports.apps/web/src/app/(teams)/t/[teamUrl]/templates/[id]/page.tsx (2)
- 6-7: The import statements for
TemplatePageViewPropsandTemplatePageVieware correctly structured, using destructuring for consistency with the project's coding standards.- 15-21: Passing both
paramsandteamtoTemplatePageViewcorrectly extends the component's functionality to support team-specific template handling.packages/trpc/server/template-router/schema.ts (2)
- 4-5: Adding
teamIdas an optional field inZCreateTemplateMutationSchemacorrectly extends the schema to support team-specific templates.- 15-15: Including
teamIdas an optional field inZDuplicateTemplateMutationSchemacorrectly extends the schema to support duplicating templates within a team context.packages/prisma/seed/templates.ts (2)
- 14-14: Including
teamIdas an optional parameter inSeedTemplateOptionscorrectly extends the seeding functionality to support team-specific templates.- 33-33: Correctly including
teamIdin the template creation process during seeding aligns with the feature's objectives to support team-specific templates.apps/web/src/app/(teams)/t/[teamUrl]/templates/page.tsx (2)
- 6-7: The import statements for
TemplatesPageViewPropsandTemplatesPageVieware correctly structured, using destructuring for consistency with the project's coding standards.- 16-25: Passing both
searchParamsandteamtoTemplatesPageViewcorrectly extends the component's functionality to support team-specific template handling.packages/lib/server-only/template/find-templates.ts (1)
- 22-32: The conditional logic to adjust
whereFilterbased onteamIdcorrectly supports filtering templates by team membership, enhancing the functionality to support team-specific templates.packages/lib/utils/teams.ts (1)
- 15-17: Adding
formatTemplatesPathfunction correctly extends utility functions to generate paths for team templates, improving code reuse and consistency.packages/lib/constants/teams.ts (1)
- 4-4: Adding
TEAM_URL_REGEXconstant with a regular expression pattern for team URLs complements the existingTEAM_URL_ROOT_REGEX, enhancing the utility and flexibility of URL validation.apps/web/src/app/(dashboard)/templates/delete-template-dialog.tsx (2)
- 38-45: The integration of error handling logic directly into the
onErrorcallback simplifies the component, removing unnecessary abstraction layers.- 61-72: Directly calling
deleteTemplatefrom theDeletebutton and usingisLoadingto disable the button when necessary simplifies the component's logic and improves user feedback.apps/web/src/app/(dashboard)/templates/duplicate-template-dialog.tsx (2)
- 17-17: Including
teamIdas a parameter inDuplicateTemplateDialogPropscorrectly extends the component to support team-specific template duplication.- 73-84: Updating the
Duplicatebutton'sonClickhandler to callduplicateTemplatewithtemplateIdandteamIdparameters correctly aligns with the feature's objectives to support team-specific template duplication.packages/lib/server-only/template/create-document-from-template.ts (2)
- 14-29: The additional conditions in the query for the template, checking for
userIdin different contexts, correctly enhance the security and correctness of document creation from a template.- 53-53: Explicitly setting
teamIdin the created document data correctly extends the functionality to support team-specific documents, aligning with the feature's objectives.packages/lib/server-only/recipient/set-recipients-for-template.ts (1)
- 23-36: The additional condition that checks for the presence of
userIdin either the direct user field or within a team's members expands the search criteria for the template based on user or team membership.apps/web/src/app/(dashboard)/templates/data-table-action-dropdown.tsx (3)
- 24-25: Adding
templateRootPathandteamIdprops inDataTableActionDropdownPropscorrectly extends the component to support team-specific template actions.- 43-43: The logic to determine
isTeamTemplatebased on theteamIdprop correctly supports conditional rendering of actions based on template ownership and team membership.- 54-72: The links and actions within the component now consider both the ownership of the template and whether it belongs to a team before enabling certain functionalities, enhancing the component's usability and security.
packages/trpc/server/recipient-router/router.ts (1)
- 36-36: The updated error message from "We were unable to sign this field." to "We were unable to set this field." in the
recipientRouterfunction provides clearer feedback to the user in case of an error.packages/lib/server-only/template/duplicate-template.ts (2)
- 15-33: The handling of a
teamIdparameter and adjustment of thetemplateWhereFilterbased on the presence ofteamIdcorrectly supports duplicating templates within a team context, enhancing the functionality to support team-specific templates.- 59-59: Including
teamIdin the duplicated template creation process correctly aligns with the feature's objectives to support team-specific templates.apps/web/src/components/(dashboard)/layout/mobile-navigation.tsx (1)
- 42-48: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [45-70]
The removal of specific filtering logic for the 'Templates' link results in all links being displayed without this specific filter, potentially affecting the navigation experience. Ensure this change aligns with the intended navigation structure and user experience.
apps/web/src/app/(dashboard)/templates/templates-page-view.tsx (1)
- 56-70: Rendering logic correctly handles both scenarios where templates are available and when no templates exist, providing a good user experience by displaying relevant content or an empty state message.
apps/web/src/components/(dashboard)/layout/desktop-nav.tsx (1)
- 55-70: The modification in the rendering logic of navigation links within the
DesktopNavcomponent to display all links without filtering based on specific conditions may affect the navigation experience. Ensure this change aligns with the intended navigation structure and user experience.packages/lib/server-only/field/set-fields-for-template.ts (1)
- 30-43: The OR condition in the query correctly allows for finding templates by either
userIdor team membership. Ensure that theuserIdin the team members subquery is correctly scoped to avoid potential ambiguity.packages/trpc/server/template-router/router.ts (3)
- 22-27: Adding
teamIdto thecreateTemplatemutation is correct. Ensure that the rest of the application correctly handles this new parameter, including permission checks for team access.- 68-73: Adding
teamIdto theduplicateTemplatemutation is correct. Similar tocreateTemplate, verify thatteamIdis properly utilized throughout the application.- 93-93: The update to the
deleteTemplatecall to passuserIdbeforeidaligns with standard practices for ensuring that the operation is performed within the context of the requesting user.packages/trpc/server/field-router/router.ts (1)
- 42-42: Updating the error message in the
addFieldsmutation to "We were unable to set this field. Please try again later." improves clarity and is appropriate for the operation being performed.apps/web/src/app/(dashboard)/documents/[id]/document-page-view.tsx (1)
- 28-28: Converting
DocumentPageViewto an arrow function and assigning it to a constant is correct and aligns with React component best practices. Ensure async operations are handled appropriately within the component lifecycle.apps/web/src/app/(dashboard)/templates/[id]/edit-template.tsx (2)
- 31-31: Adding
templateRootPathtoEditTemplateFormPropsis correct and enhances navigation flexibility by allowing dynamic path construction based on the team context.- 103-103: Using
templateRootPathinrouter.pushto navigate after form submission is correct and ensures that navigation respects the current team context.apps/web/src/app/(dashboard)/templates/data-table-templates.tsx (3)
- 31-33: Adding
documentRootPath,templateRootPath, and an optionalteamIdtoTemplatesDataTablePropsis correct and supports enhanced navigation and action handling within the component.- 79-79: Using
documentRootPathinrouter.pushfor navigation after creating a document from a template is correct and ensures that navigation respects the current team context.- 141-145: Passing
teamIdandtemplateRootPathtoDataTableActionDropdownis correct and ensures that actions within the dropdown can utilize these props for accurate operation and navigation.apps/web/src/app/(dashboard)/documents/documents-page-view.tsx (1)
- 36-36: Converting
DocumentsPageViewto an arrow function and assigning it to a constant is correct and aligns with React component best practices. Ensure async operations are handled appropriately within the component lifecycle.apps/web/src/app/(dashboard)/templates/new-template-dialog.tsx (3)
- 46-48: Adding
teamIdandtemplateRootPathtoNewTemplateDialogPropsis correct and supports creating templates within a team context and navigating appropriately after creation.- 108-108: Passing
teamIdtocreateTemplateis correct and ensures that the new template is associated with the correct team when provided.- 122-122: Using
templateRootPathinrouter.pushto navigate after creating a new template is correct and ensures that navigation respects the current team context.apps/web/src/components/(dashboard)/layout/menu-switcher.tsx (4)
- 9-9: Including
TEAM_URL_REGEXin the import statement is correct and supports the new functionality for handling URL formatting based on team context.- 74-88: Adding
formatRedirectUrlOnSwitchfunction is correct and enhances navigation flexibility by allowing seamless switching between documents and templates pages for both teams and personal accounts. Ensure the logic correctly handles different pathname scenarios.- 119-119: Using
formatRedirectUrlOnSwitchwithout a team URL for personal account navigation is correct and ensures that navigation respects the absence of a team context.- 171-171: Using
formatRedirectUrlOnSwitchwith a team URL for team context navigation is correct and ensures that navigation respects the current team context.packages/prisma/schema.prisma (3)
- 337-338: The addition of the
templatesfield to theTeammodel is correctly implemented, establishing a one-to-many relationship betweenTeamandTemplate. This change aligns with the objectives of enhancing template management at the team level.- 419-419: The
teamIdfield in theTemplatemodel is optional (Int?), which correctly allows for templates that are not associated with a team. This is a sensible design choice for supporting both team-specific and general templates.- 424-424: The relation
teamin theTemplatemodel is correctly defined, using theteamIdfield to establish a many-to-one relationship with theTeammodel. This setup enables the association of templates with specific teams, supporting the feature's objectives.
| team?: Team; | ||
| }; | ||
|
|
||
| export const TemplatePageView = async ({ params, team }: TemplatePageViewProps) => { |
There was a problem hiding this comment.
The use of async for a React component function is incorrect. React components cannot be async. Consider moving async operations to useEffect or a data fetching library hook.
Comment on lines
+32
to
+34
| if (!templateId || Number.isNaN(templateId)) { | ||
| redirect(templateRootPath); | ||
| } |
There was a problem hiding this comment.
Redirection using redirect from next/navigation inside a component is not effective. Use useEffect to handle redirection based on conditions.
Comment on lines
+36
to
+41
| const { user } = await getRequiredServerComponentSession(); | ||
|
|
||
| const template = await getTemplateById({ | ||
| id: templateId, | ||
| userId: user.id, | ||
| }).catch(() => null); |
There was a problem hiding this comment.
Fetching data directly inside the component without handling loading or error states. Consider using a data fetching library like react-query or trpc to handle these states.
Comment on lines
+43
to
+45
| if (!template || !template.templateDocumentData) { | ||
| redirect(templateRootPath); | ||
| } |
There was a problem hiding this comment.
Redirection logic repeated without considering error handling for failed data fetches. Consolidate redirection logic and include error handling.
Comment on lines
+49
to
+58
| const [templateRecipients, templateFields] = await Promise.all([ | ||
| getRecipientsForTemplate({ | ||
| templateId, | ||
| userId: user.id, | ||
| }), | ||
| getFieldsForTemplate({ | ||
| templateId, | ||
| userId: user.id, | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
Using Promise.all without error handling. Consider wrapping this in a try-catch block to handle potential errors from either getRecipientsForTemplate or getFieldsForTemplate.
Comment on lines
+60
to
+85
| return ( | ||
| <div className="mx-auto max-w-screen-xl px-4 md:px-8"> | ||
| <Link href="/templates" className="flex items-center text-[#7AC455] hover:opacity-80"> | ||
| <ChevronLeft className="mr-2 inline-block h-5 w-5" /> | ||
| Templates | ||
| </Link> | ||
|
|
||
| <h1 className="mt-4 truncate text-2xl font-semibold md:text-3xl" title={template.title}> | ||
| {template.title} | ||
| </h1> | ||
|
|
||
| <div className="mt-2.5 flex items-center gap-x-6"> | ||
| <TemplateType inheritColor type={template.type} className="text-muted-foreground" /> | ||
| </div> | ||
|
|
||
| <EditTemplateForm | ||
| className="mt-8" | ||
| template={template} | ||
| user={user} | ||
| recipients={templateRecipients} | ||
| fields={templateFields} | ||
| documentData={templateDocumentData} | ||
| templateRootPath={templateRootPath} | ||
| /> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Rendering logic does not handle loading or error states, potentially leading to a blank or broken page state for the user. Implement conditional rendering to handle these states.
| @@ -0,0 +1,5 @@ | |||
| -- AlterTable | |||
| ALTER TABLE "Template" ADD COLUMN "teamId" INTEGER; | |||
There was a problem hiding this comment.
Missing NOT NULL constraint on the teamId column. Consider if this column should allow NULL values. If not, add NOT NULL constraint to ensure data integrity.
- ALTER TABLE "Template" ADD COLUMN "teamId" INTEGER;
+ ALTER TABLE "Template" ADD COLUMN "teamId" INTEGER NOT NULL;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| ALTER TABLE "Template" ADD COLUMN "teamId" INTEGER; | |
| ALTER TABLE "Template" ADD COLUMN "teamId" INTEGER NOT NULL; |
Comment on lines
+17
to
+19
| let whereFilter: Prisma.TemplateWhereInput = { | ||
| userId, | ||
| teamId: null, |
There was a problem hiding this comment.
Initializing whereFilter with userId and teamId: null might not be necessary if teamId is later conditionally set. Consider initializing whereFilter without teamId: null for clarity.
- userId,
- teamId: null,
+ userIdCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| let whereFilter: Prisma.TemplateWhereInput = { | |
| userId, | |
| teamId: null, | |
| let whereFilter: Prisma.TemplateWhereInput = { | |
| userId |
| team?: Team; | ||
| }; | ||
|
|
||
| export const TemplatesPageView = async ({ searchParams = {}, team }: TemplatesPageViewProps) => { |
There was a problem hiding this comment.
The TemplatesPageView function should not be async. Server-side logic (like fetching data) should be handled outside of the component or in hooks like getServerSideProps or useEffect for client-side fetching.
- export const TemplatesPageView = async ({ searchParams = {}, team }: TemplatesPageViewProps) => {
+ export const TemplatesPageView = ({ searchParams = {}, team }: TemplatesPageViewProps) => {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| export const TemplatesPageView = async ({ searchParams = {}, team }: TemplatesPageViewProps) => { | |
| export const TemplatesPageView = ({ searchParams = {}, team }: TemplatesPageViewProps) => { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Viewcomponents in the dashboard and teams sections.Bug Fixes
DesktopNavcomponent.MobileNavigationcomponent.Chores
field-routerandrecipient-router.