Feature Relaunch #40
Feature Relaunch #40
Conversation
There was a problem hiding this comment.
Pull Request Overview
This is a comprehensive feature relaunch that transforms the project from an open-source version into an internal/private version. The changes include disabling deletion functionality, adding search capabilities, improving UI/UX, and upgrading the runtime infrastructure.
- Deletion functionality for projects and clients has been disabled while maintaining archiving capabilities
- Search functionality added to projects and clients pages with client-name matching
- Database configuration updated for read/write host separation and PHP 8.4 runtime upgrade
Reviewed Changes
Copilot reviewed 65 out of 67 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vite-module-loader.js | Enhanced error handling with graceful fallbacks for missing module files |
| resources/js/utils/useReporting.ts | Added type annotations for Project and Client parameters |
| resources/js/utils/useProjects.ts | Removed deleteProject function, replaced with comment |
| resources/js/utils/useClients.ts | Removed deleteClient function, replaced with comment |
| resources/js/Pages/Projects.vue | Added search functionality with client-name matching and MagnifyingGlassIcon |
| resources/js/Pages/ProjectShow.vue | Added Project type import and annotation |
| resources/js/Pages/Clients.vue | Added search functionality with client filtering |
| resources/js/Layouts/AppLayout.vue | Added sidebar counts for projects and clients |
| resources/js/Components/NavigationSidebarLink.vue | Added count prop and badge display |
| resources/js/Components/NavigationSidebarItem.vue | Added count prop forwarding |
| Multiple table components | Removed delete functionality and status columns across all tables |
| app/Http/Controllers/Api/V1/* | Modified delete endpoints to return early without deletion |
| app/Http/Middleware/ShareInertiaData.php | Added sidebar counts calculation with visibility awareness |
| config/database.php | Added read/write host separation configuration |
| composer.json | Upgraded to PHP 8.4 with platform constraints |
| e2e/*.spec.ts | Disabled deletion tests, updated to verify archive functionality |
| docker/*, LICENSE.md, etc. | Removed open-source infrastructure and documentation files |
| let stat; | ||
| try { | ||
| stat = await fs.stat(viteConfigPath); | ||
| } catch { |
There was a problem hiding this comment.
Empty catch block should at least include a comment explaining why the error is being ignored. Consider adding a comment like // Skip modules with missing vite.config.js files
| } catch { | |
| } catch { | |
| // Skip modules with missing vite.config.js files |
| } | ||
| } catch (error) { | ||
| console.error(`Error reading module statuses or module configurations: ${error}`); | ||
| } catch { |
There was a problem hiding this comment.
Empty catch block should include a comment explaining the intention. Consider adding // Silently handle missing modules directory
| return `grid-template-columns: minmax(300px, 1fr) minmax(150px, auto) minmax(140px, auto) minmax(130px, auto) ${props.showBillableRate ? 'minmax(130px, auto)' : ''} minmax(120px, auto) 80px;`; | ||
| // Name → Client → Total Time → Progress → [spacer] → Actions (right-aligned) | ||
| return `grid-template-columns: max-content max-content max-content max-content 1fr 80px`; | ||
| }); |
There was a problem hiding this comment.
[nitpick] Hard-coded grid template with magic numbers makes it difficult to understand column purposes. Consider using descriptive CSS custom properties or a more maintainable approach with named grid areas.
| <div | ||
| data-testid="client_table" | ||
| class="grid min-w-full" | ||
| style="grid-template-columns: max-content max-content 1fr 80px"> |
There was a problem hiding this comment.
[nitpick] Inline style with hard-coded grid template values reduces maintainability. Consider moving this to a computed property or CSS class for better organization.
|
|
||
| return response() | ||
| ->json(null, 204); | ||
| // Deletion disabled: return early to keep data intact |
There was a problem hiding this comment.
Commenting out functionality rather than properly removing it can lead to confusion and dead code. Consider removing the commented deletion logic entirely or implementing a feature flag system.
| // Deletion disabled: return early to keep data intact |
|
|
||
| return response()->json(null, 204); | ||
| // Deletion disabled: return early to keep data intact | ||
| return response()->json(['message' => 'Client deletion disabled'], 200); |
There was a problem hiding this comment.
Similar to the project controller, this approach of disabling functionality by early return may confuse API consumers expecting standard HTTP behavior. Consider returning HTTP 405 (Method Not Allowed) or 403 (Forbidden) instead of 200.
| return response()->json(['message' => 'Client deletion disabled'], 200); | |
| return response()->json(['message' => 'Client deletion is disabled'], 405); |
No description provided.