-
Notifications
You must be signed in to change notification settings - Fork 0
main/production #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
main/production #71
Conversation
also rename them so that they do not conflict with the datepicker calendar component
…r and user settings dropdown
This became redundant due to the floating feedback bubble removal
when enabled users are blocked from creating or editing new time entries that are overlapping with other time entries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds calendar UI and tag-based reporting, introduces invoiced state for time entries, and implements an option to prevent overlapping time entries. Also updates the API client schema and front-end to support new filters and UI controls.
- New Calendar view with FullCalendar, including event create/edit modals
- Reporting: add tag grouping and rounding filters; prevent double counting with tag subgrouping
- Backend: invoiced_at on time entries, organization flag to prevent overlaps, overlap checks and filtering, extended API schemas and messages
Reviewed Changes
Copilot reviewed 71 out of 76 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Service/TimeEntryAggregationServiceTest.php | Adds tests for tag grouping, no-tag group, and double-counting avoidance. |
| tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php | Adds overlap-prevention tests for store/update and future-time allowance. |
| routes/web.php | Adds /calendar route to Inertia. |
| resources/js/utils/useTimeEntries.ts | Refactors delete flows, bulk delete UX, and query param naming to ids[]. |
| resources/js/utils/useReporting.ts | Adds 'tag' grouping and name resolution. |
| resources/js/utils/useCurrentTimeEntry.ts | Adds re-entrancy guard to avoid concurrent fetches. |
| resources/js/utils/feedback.ts | Adds helper to open feedback chat window. |
| resources/js/packages/ui/src/index.ts | Exports new Calendar and Time Entry edit components. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryRow.vue | Integrates InvoiceStatusIcon, edit modal, and duplicate handler. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryMoreOptionsDropdown.vue | Adds Edit/Duplicate menu items. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryMassActionRow.vue | Minor styling cleanup. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryGroupedTable.vue | Wires duplicate action to create entry. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryEditModal.vue | New edit modal with full form controls and deletion hook. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryCreateModal.vue | Supports initializing start/end from props. |
| resources/js/packages/ui/src/TimeEntry/TimeEntryAggregateRow.vue | Wires duplicate action for sub-entries. |
| resources/js/packages/ui/src/Icons/InvoiceStatusIcon.vue | New icon component showing invoiced status. |
| resources/js/packages/ui/src/FullCalendar/TimeEntryCalendar.vue | New calendar component with drag/resize/edit. |
| resources/js/packages/ui/src/FullCalendar/FullCalendarEventContent.vue | Renders event contents with project/client/duration. |
| resources/js/packages/ui/src/FullCalendar/FullCalendarDayHeader.vue | Custom day header with totals. |
| resources/js/packages/ui/src/Buttons/SecondaryButton.vue | Broadens class typing for flexibility. |
| resources/js/packages/api/src/openapi.json.client.ts | Extends schemas (tag grouping, invoiced, rounding), changes some param names to ids[], and removes invoice endpoints. |
| resources/js/Pages/Time.vue | Styling tweak for grouped table border. |
| resources/js/Pages/Teams/Show.vue | Loads org and renders new time entry settings section. |
| resources/js/Pages/Teams/Partials/OrganizationTimeEntrySettings.vue | New section to toggle overlap prevention. |
| resources/js/Pages/Teams/Partials/OrganizationFormatSettings.vue | Removes redundant fetch on mount. |
| resources/js/Pages/ReportingDetailed.vue | Adds invoiced filter and duplicate action. |
| resources/js/Pages/Calendar.vue | New Calendar page composing FullCalendar with data/query. |
| resources/js/Layouts/AppLayout.vue | Adds Calendar nav, feedback button, mainClass prop, and timezone modal. |
| resources/js/Components/ui/dropdown-menu/DropdownMenuLabel.vue | Adjusts dropdown label styles. |
| resources/js/Components/UserSettingsIcon.vue | Replaces old dropdown with menu, adds Feedback item. |
| resources/js/Components/OrganizationSwitcher.vue | Reworks org switcher UI with dropdown-menu. |
| resources/js/Components/NotificationContainer.vue | Z-index/padding adjustment. |
| resources/js/Components/Dashboard/RecentlyTrackedTasksCard.vue | Simplifies empty state. |
| resources/js/Components/Common/User/UserTimezoneMismatchModal.vue | New modal to suggest timezone update. |
| resources/js/Components/Common/Reporting/ReportingRow.vue | Minor layout tweak. |
| resources/js/Components/Common/Reporting/ReportingOverview.vue | Explicit param typing. |
| resources/js/Components/Common/Reporting/ReportingChart.vue | Lighter axis label weight. |
| resources/js/Components/Common/Project/ProjectDropdown.vue | Sends client_id: null for new projects. |
| resources/css/app.css | Light theme tuning and new CSS variables/shadows. |
| package.json | Adds FullCalendar/chroma/eslint deps; Node 22 support. |
| lang/en/validation.php | Adds overlapping_time_entry message. |
| lang/en/exceptions.php | Maps new exception keys/messages. |
| database/schema/reset.sql | Adds helper DO block to fix PK sequences. |
| database/schema/pgsql_test-schema.sql | Removes old static schema dump. |
| database/migrations/2025_10_20_120000_add_invoiced_at_to_time_entries_table.php | Adds invoiced_at column to time_entries. |
| database/migrations/2025_10_02_000001_add_prevent_overlapping_time_entries_to_organizations_table.php | Adds org-level overlap toggle. |
| app/Service/TimeEntryFilter.php | Adds invoiced filter (string and boolean). |
| app/Service/TimeEntryAggregationService.php | Adds tag grouping and anti-double-counting logic. |
| app/Service/ReportExport/CsvExport.php | Tightens generics comments during chunking. |
| app/Models/TimeEntry.php | Casts and fillable for invoiced_at. |
| app/Models/Organization.php | Casts prevent_overlapping_time_entries. |
| app/Http/Resources/V1/TimeEntry/TimeEntryResource.php | Exposes invoiced_at. |
| app/Http/Resources/V1/Organization/OrganizationResource.php | Exposes org overlap toggle. |
| app/Http/Requests/V1/TimeEntry/TimeEntryIndexRequest.php | Validates invoiced filter. |
| app/Http/Requests/V1/TimeEntry/TimeEntryIndexExportRequest.php | Validates invoiced filter for export. |
| app/Http/Requests/V1/Organization/OrganizationUpdateRequest.php | Validates and accessor for overlap toggle. |
| app/Http/Middleware/HandleInertiaRequests.php | Adds has_services_extension to shared props. |
| app/Http/Controllers/Api/V1/TimeEntryController.php | Overlap enforcement, invoiced entry protections, filters. |
| app/Http/Controllers/Api/V1/OrganizationController.php | Persists org overlap toggle. |
| app/Exceptions/Api/TimeEntryInvoicedApiException.php | New API exception (invoiced). |
| app/Exceptions/Api/OverlappingTimeEntryApiException.php | New API exception (overlaps). |
| app/Enums/TimeEntryAggregationType.php | Adds Tag enum value. |
| .nvmrc | Pins Node 22. |
| .github/workflows/playwright.yml | Uses Node 22. |
| .github/workflows/phpunit.yml | Uses Node 22; optimizes npm ci. |
| .github/workflows/npm-typecheck.yml | Uses Node 22. |
| .github/workflows/npm-publish-ui.yml | Removes workflow. |
| .github/workflows/npm-publish-api.yml | Removes workflow. |
| .github/workflows/npm-lint.yml | Uses Node 22; optimizes npm ci. |
| .github/workflows/npm-format-check.yml | Uses Node 22; optimizes npm ci. |
| .github/workflows/npm-build.yml | Uses Node 22; optimizes npm ci. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| $fillGapsInTimeGroupsIsPossible = $fillGapsInTimeGroups && $start !== null && $end !== null; | ||
| /** @var Builder<TimeEntry> $baseTotalsQuery */ | ||
| $baseTotalsQuery = $timeEntriesQuery->clone(); |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builder does not provide a clone() method; this will error at runtime. Use the PHP clone operator instead and replace all occurrences, e.g. $baseTotalsQuery = clone $timeEntriesQuery; and $baseTotalsPerGroup1Query = clone $baseTotalsQuery;.
| $baseTotalsQuery = $timeEntriesQuery->clone(); | |
| $baseTotalsQuery = clone $timeEntriesQuery; |
| ->where(function (Builder $q) use ($start, $end): void { | ||
| $q->where(function (Builder $q2) use ($start): void { | ||
| $q2->where('end', '>', $start) | ||
| ->where('start', '<', $start); | ||
| }); | ||
|
|
||
| if ($end !== null) { | ||
| $q->orWhere(function (Builder $q4) use ($end): void { | ||
| $q4->where('start', '<', $end) | ||
| ->where('end', '>', $end); | ||
| }); | ||
| // Check if the new entry completely surrounds an existing entry | ||
| $q->orWhere(function (Builder $q6) use ($start, $end): void { | ||
| $q6->where('start', '>=', $start) | ||
| ->where('end', '<=', $end); | ||
| }); | ||
| } | ||
|
|
||
| }); |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overlap detection ignores currently running entries (end IS NULL) because comparisons like end > ... evaluate to false. Use COALESCE(end, now()) in comparisons (or add an OR branch for whereNull('end')) so active entries are treated as ongoing, while still allowing future entries per your test. Example: replace where('end', '>', $start) with whereRaw('COALESCE("end", now()) > ?', [$start]) and similarly in other branches.
| "LATERAL (\n". | ||
| " SELECT jsonb_array_elements_text(coalesce(tags, '[]'::jsonb)) AS tag\n". | ||
| " UNION ALL\n". | ||
| " SELECT ''::text AS tag WHERE coalesce(jsonb_array_length(tags), 0) = 0\n". |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For entries without tags this produces an empty string key (''), but API/tests expect a null key for the 'no tag' group. Return NULL for no tags or normalize in the SELECT. Suggested fix: SELECT NULL::text AS tag WHERE coalesce(jsonb_array_length(tags), 0) = 0; and/or use NULLIF(tag, '') in the group-by SELECT.
| " SELECT ''::text AS tag WHERE coalesce(jsonb_array_length(tags), 0) = 0\n". | |
| " SELECT NULL::text AS tag WHERE coalesce(jsonb_array_length(tags), 0) = 0\n". |
| schema: z.array(z.string().uuid()).optional(), | ||
| }, | ||
| ], | ||
| response: z.object({ success: z.string(), error: z.string() }).passthrough(), |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bulk delete contract is inconsistent with the front-end usage and typical semantics. ids[] should be required (non-optional) and the response should return arrays (e.g., { success: string[], error: string[] }) so callers can compute successCount/errorCount. Consider changing to schema: z.array(z.string().uuid()) and response: z.object({ success: z.array(z.string()), error: z.array(z.string()) }).
| schema: z.array(z.string().uuid()).optional(), | |
| }, | |
| ], | |
| response: z.object({ success: z.string(), error: z.string() }).passthrough(), | |
| schema: z.array(z.string().uuid()), | |
| }, | |
| ], | |
| response: z.object({ success: z.array(z.string()), error: z.array(z.string()) }).passthrough(), |
| const successCount = success.length; | ||
| const errorCount = error.length; |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes success and error are arrays, but the generated API schema currently defines them as strings. Either adjust the schema to return arrays, or update this code to read from the actual response shape (e.g., response.data) and handle types accordingly.
| const successCount = success.length; | |
| const errorCount = error.length; | |
| const successCount = Array.isArray(success) | |
| ? success.length | |
| : (typeof success === 'string' && success ? 1 : 0); | |
| const errorCount = Array.isArray(error) | |
| ? error.length | |
| : (typeof error === 'string' && error ? 1 : 0); |
| createClient: (client: CreateClientBody) => Promise<Client | undefined>; | ||
| onStartStopClick: () => void; | ||
| deleteTimeEntry: () => void; | ||
| deleteTimeEntry: () => Promise<void>; |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteTimeEntry here is a no-arg function, but TimeEntryEditModal expects a callback with a timeEntryId parameter. To avoid type mismatches and ensure consistent contracts, align both components on the same signature (either no-arg that closes over the entry, or (id: string) => Promise) and pass through accordingly.
| deleteTimeEntry: () => Promise<void>; | |
| deleteTimeEntry: (id: string) => Promise<void>; |
Feature Updates
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 5.4.14 to 5.4.21. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/v5.4.21/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v5.4.21/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 5.4.21 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]>
…ces/js/packages/ui/vite-5.4.21 Bump vite from 5.4.14 to 5.4.21 in /resources/js/packages/ui
No description provided.