Claudio/comp 106 add implementation widget#467
Merged
Conversation
- Added FloatingOnboardingChecklist component to display onboarding progress. - Introduced getOnboardingStatus action to fetch onboarding status from the database. - Created generateChecklistItems function to generate checklist items based on onboarding data. - Updated Layout and Page components to integrate onboarding status handling and error management. - Refactored Checklist component to use ChecklistItemProps for better type safety.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the routing paths by replacing "/employees" with "/people" and integrates an onboarding checklist widget via new actions and UI components. Key changes include:
- Updating revalidation paths across multiple action files and components.
- Introducing a new onboarding checklist with corresponding types, pages, and actions.
- Removing obsolete "employees/all" pages and components.
Reviewed Changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/people/[employeeId]/actions/update-employee.ts | Updated revalidation paths from "/employees" to "/people". |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/people/[employeeId]/actions/update-employee-status.ts | Updated revalidation paths to reflect new routing. |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/people/[employeeId]/actions/update-employee-details.ts | Revalidation paths updated to "/people". |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/people/[employeeId]/actions/update-department.ts | Revalidation paths updated to "/people". |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/layout.tsx | Added onboarding checklist logic with error logging for fetching onboarding status. |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/implementation/types.ts | Updated the ChecklistItemProps by making description required. |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/implementation/page.tsx | Updated onboarding status retrieval and enhanced error messaging in the UI. |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/implementation/components/Checklist.tsx | Modified type import and key usage for Checklist items. |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/implementation/checklist-items.tsx | New file generating checklist items with updated routing. |
| apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/implementation/actions.ts | New file that consolidates onboarding status logic. |
| apps/app/src/actions/organization/remove-employee.ts and invite-employee.ts | Updated revalidation paths to remove "/employees/all" references. |
| Other employees/all pages/components | Removed obsolete files related to the "employees/all" route. |
Comments suppressed due to low confidence (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/implementation/components/Checklist.tsx:10
- [nitpick] Using 'dbColumn' as a React key might lead to duplicate keys if its values are not guaranteed to be unique; consider using a more unique identifier if available.
<ChecklistItem key={item.dbColumn} {...item} />
| if ("error" in onboardingStatus) { | ||
| // Decide how to handle the error - maybe log it or show a generic error message | ||
| // For now, we'll proceed without the checklist | ||
| console.error("Error fetching onboarding status:", onboardingStatus.error); |
There was a problem hiding this comment.
[nitpick] While console.error is useful for debugging during development, integrating a structured error tracking or user feedback mechanism may improve error handling in production.
| const completedItems = checklistItems.filter((item) => item.completed).length; | ||
| const totalItems = checklistItems.length; | ||
|
|
||
| return { checklistItems, completedItems, totalItems }; |
There was a problem hiding this comment.
[nitpick] The getOnboardingStatus function returns an error object on failure and a status object on success; standardizing error handling (e.g., by throwing exceptions) could avoid ambiguity in downstream consumers.
This file contains hidden or 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
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.
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist