-
-
Notifications
You must be signed in to change notification settings - Fork 21
Fix/cannot create event for not splitable tasks #3240
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
Conversation
WalkthroughThis set of changes primarily focuses on UI improvements, code refactoring, and scheduler logic updates. It includes reordering CSS utility classes for consistency, replacing input fields with popover-based date pickers, updating scheduling logic to use a new function, refining type definitions, and minor debugging adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddEventModal
participant Popover
participant Calendar
User->>AddEventModal: Clicks "Start" or "End" date field
AddEventModal->>Popover: Opens popover
Popover->>Calendar: Renders calendar UI
User->>Calendar: Selects date
Calendar->>Popover: Returns selected date
Popover->>AddEventModal: Closes popover, updates field
sequenceDiagram
participant API
participant Scheduler
participant DB
API->>Scheduler: Collects tasks, flexible events, locked events
Scheduler->>Scheduler: Validates and promotes events to tasks
Scheduler->>DB: Upserts new scheduled events (locked: false)
Scheduler->>API: Returns scheduling result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3240 +/- ##
==========================================
- Coverage 1.40% 1.39% -0.01%
==========================================
Files 1854 1854
Lines 242429 242463 +34
Branches 2465 2461 -4
==========================================
- Hits 3399 3391 -8
- Misses 237519 237561 +42
Partials 1511 1511 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Summary of Changes
Hello @phatgg221, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily resolves an issue where events could not be properly created for tasks designated as non-splittable, by refining the underlying scheduling algorithm to correctly process such tasks. It also introduces a significant user interface improvement for date selection in the event creation flow, enhancing usability. Furthermore, the changes include general code cleanup and formatting, improving overall code quality.
Highlights
- Core Scheduling Logic Fix: The
promoteEventToTask
function in the scheduling algorithm has been enhanced to correctly handle events promoted to tasks. It now ensures these tasks are treated as non-splittable by setting theirminDuration
andmaxDuration
to their actual duration, and explicitly markingallowSplit
asfalse
. Validation for invalid events and a minimum duration of 15 minutes for promoted tasks have also been added. - Improved Auto-Scheduling Integration: The auto-scheduling API now utilizes a more robust
scheduleTasks
function, which incorporates the refined logic for promoted events. Additionally, the system now correctly filters out locked events from being upserted to the database, preventing unintended modifications to protected time blocks. - Enhanced Date Selection User Interface: The 'Add Event' dialog has received a significant user experience upgrade. Date input fields now leverage a
Calendar
component within aPopover
, replacing the previous nativedatetime-local
inputs, offering a more intuitive and visually appealing date selection process. - Code Quality and Formatting: Numerous files across the codebase have undergone a cleanup pass, including the reordering of Tailwind CSS classes for consistency and the removal of debugging
console.log
statements and redundant comments, contributing to a cleaner and more maintainable codebase.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
🚨 Biome Check ReportIssues Found: 5647📊 Analysis Summary
🔍 Issue Breakdown
🛠️ Next StepsRun the following command to fix issues: bun format-and-lint:fix Then commit and push the changes.
🤖 Auto-generated by Biome Check workflow • Last updated: 7/11/2025, 6:25:34 AM
|
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.
Code Review
This pull request aims to fix an issue with creating events for non-splittable tasks. The changes in the scheduling algorithm and the auto-schedule API route seem to correctly address this by properly handling locked events and promoting other events to tasks for rescheduling.
However, the review identified a few areas for improvement:
- Debugging Code: Several
console.log
statements and commented-out code blocks were found across multiple files. These should be removed before merging. - Functional Regression: A significant change in the
add-event-dialog.tsx
component has removed the ability for users to select a time for start and end dates, which is a regression from the previous implementation. This should be addressed.
Overall, the core logic fix appears sound, but the collateral changes need attention to ensure code quality and prevent loss of functionality.
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
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.
Actionable comments posted: 4
♻️ Duplicate comments (3)
apps/web/src/app/api/[wsId]/task/create/route.ts (1)
121-127
: Remove debugconsole.log
before merging
console.log('Scheduled events:', newScheduledEvents);
will leak internal data in production logs and is inconsistent with earlier review guidance that all debug logs be removed.- console.log('Scheduled events:', newScheduledEvents); + // console.log('Scheduled events:', newScheduledEvents); // ← uncomment only when actively debuggingapps/web/src/app/api/[wsId]/calendar/auto-schedule/route.ts (1)
76-76
: Remove debugging console.log statements.These console.log statements appear to be for debugging purposes and should be removed before merging to production.
Also applies to: 232-235, 289-289
packages/ai/src/scheduling/algorithm.ts (1)
378-389
: Remove commented-out code.This block of commented-out code should be removed to keep the codebase clean and avoid confusion.
🧹 Nitpick comments (2)
packages/types/src/supabase.ts (2)
8171-8182
: Provide a single overloaded signature instead of two variant records
generate_cross_app_token
now exposes two overloads that differ only by optional keys. Postgres does not support true function overloading on record types, and the generated d.ts unions add noise for callers. Prefer one argument object with all keys optional when nullable, keeping server and client typings trivial.
8195-8196
: Return-type field shuffling is pure noiseReordering columns inside composite return types (
day / total_income / total_expense
, etc.) does not change the runtime shape but bloats git diffs and increases merge-conflict risk. Consider pinningsupabase gen types
to a fixed CLI version and committing generated files only when actual schema changes occur.Also applies to: 8203-8205, 8231-8233
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx
(17 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
(6 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsx
(0 hunks)apps/web/src/app/api/[wsId]/calendar/auto-schedule/route.ts
(4 hunks)apps/web/src/app/api/[wsId]/task/create/route.ts
(1 hunks)packages/ai/src/scheduling/algorithm.ts
(1 hunks)packages/types/src/supabase.ts
(23 hunks)packages/ui/src/components/ui/date-time-picker.tsx
(1 hunks)packages/ui/src/components/ui/legacy/calendar/event-form-components.tsx
(14 hunks)packages/ui/src/components/ui/legacy/calendar/event-modal.tsx
(19 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsx
🧰 Additional context used
🧠 Learnings (1)
packages/ui/src/components/ui/legacy/calendar/event-modal.tsx (1)
Learnt from: DennieDan
PR: tutur3u/platform#2891
File: packages/ui/src/hooks/use-calendar.tsx:1511-1513
Timestamp: 2025-05-21T09:22:15.348Z
Learning: In the `GoogleCalendarSettings` component's `handleSyncNow` function, the boolean return value from `syncGoogleCalendarNow` is used to determine if changes were made during a successful sync, while errors are handled through a separate try/catch block with detailed error messages.
🧬 Code Graph Analysis (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/test-event-generator-button.tsx (1)
TestEventGeneratorButton
(20-94)
🪛 GitHub Check: codecov/patch
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx
[warning] 38-38: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx#L38
Added line #L38 was not covered by tests
apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx
[warning] 171-171: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L171
Added line #L171 was not covered by tests
[warning] 224-224: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L224
Added line #L224 was not covered by tests
[warning] 248-248: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L248
Added line #L248 was not covered by tests
[warning] 267-267: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L267
Added line #L267 was not covered by tests
[warning] 307-307: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L307
Added line #L307 was not covered by tests
[warning] 310-310: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L310
Added line #L310 was not covered by tests
[warning] 341-341: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L341
Added line #L341 was not covered by tests
[warning] 359-359: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L359
Added line #L359 was not covered by tests
[warning] 370-370: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L370
Added line #L370 was not covered by tests
[warning] 388-388: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L388
Added line #L388 was not covered by tests
[warning] 417-417: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L417
Added line #L417 was not covered by tests
[warning] 461-461: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L461
Added line #L461 was not covered by tests
[warning] 467-467: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L467
Added line #L467 was not covered by tests
[warning] 473-473: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L473
Added line #L473 was not covered by tests
[warning] 479-479: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L479
Added line #L479 was not covered by tests
[warning] 544-544: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L544
Added line #L544 was not covered by tests
[warning] 558-558: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L558
Added line #L558 was not covered by tests
[warning] 570-570: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L570
Added line #L570 was not covered by tests
[warning] 593-593: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L593
Added line #L593 was not covered by tests
[warning] 605-605: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L605
Added line #L605 was not covered by tests
[warning] 619-619: apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx#L619
Added line #L619 was not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
[warning] 3-3: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L3
Added line #L3 was not covered by tests
[warning] 15-15: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L15
Added line #L15 was not covered by tests
[warning] 25-25: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L25
Added line #L25 was not covered by tests
[warning] 68-69: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L68-L69
Added lines #L68 - L69 were not covered by tests
[warning] 625-625: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L625
Added line #L625 was not covered by tests
[warning] 627-633: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L627-L633
Added lines #L627 - L633 were not covered by tests
[warning] 635-672: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L635-L672
Added lines #L635 - L672 were not covered by tests
[warning] 675-675: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L675
Added line #L675 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
🔇 Additional comments (12)
packages/ui/src/components/ui/date-time-picker.tsx (1)
345-346
: Consistent utility-class ordering – LGTMOnly the order of the
text-destructive
/text-xs
utilities changed. Styling & behaviour remain identical.apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx (1)
37-39
: UI hierarchy improvedPlacing
TestEventGeneratorButton
after the primary “Add Event” action reads better for end-users. No functional issues spotted.apps/calendar/src/app/[locale]/(root)/scheduler/page.tsx (1)
171-178
: Class-order cosmetic changes confirmedAll touched lines only reorder Tailwind utility classes (e.g.
font-bold text-lg
→text-lg font-bold
). No rendering or functional impact.Also applies to: 224-230, 248-252, 267-270, 307-312, 341-348, 358-361, 417-423, 461-468, 473-480
packages/ui/src/components/ui/legacy/calendar/event-form-components.tsx (1)
71-72
: Minor Tailwind re-ordering approvedLike elsewhere in the PR, only the sequence of utility classes changed for readability/consistency – behaviour unaffected.
Also applies to: 89-90, 278-279, 289-291, 329-330, 338-339, 376-377, 394-395, 402-403, 435-436, 466-467, 475-476, 518-519, 531-533, 568-569, 572-573
apps/web/src/app/api/[wsId]/calendar/auto-schedule/route.ts (2)
3-3
: Import change aligns with algorithm refactoring.The import correctly reflects the removal of
scheduleWithFlexibleEvents
and use ofscheduleTasks
directly.
189-199
: Good approach to preserve locked events.Filtering out locked events before upsert and explicitly setting
locked: false
ensures the scheduler doesn't modify protected events.packages/ui/src/components/ui/legacy/calendar/event-modal.tsx (2)
70-73
: Good consolidation of calendar utilities.Moving
createAllDayEvent
andisAllDayEvent
to a shared utilities module improves code organization and reusability.
881-881
: CSS class reordering for consistency.The CSS class reordering improves consistency across the codebase without affecting functionality.
Also applies to: 932-932, 1218-1218
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx (1)
68-69
: Add test coverage for new date picker functionality.The new popover date picker implementation lacks test coverage. Consider adding tests for:
- Popover open/close behavior
- Date selection and formatting
- State management for
startDateOpen
andendDateOpen
Are there plans to add test coverage for these UI changes in a follow-up PR?
Also applies to: 625-721
packages/ai/src/scheduling/algorithm.ts (2)
123-154
: Excellent validation and error handling improvements.The added validation checks prevent invalid events from being promoted to tasks, and the nullish coalescing operators are more semantically correct than logical OR for these use cases.
156-156
: Good architectural simplification.Removing
scheduleWithFlexibleEvents
and integrating its logic directly into the route handler reduces unnecessary abstraction layers and makes the code flow clearer.packages/types/src/supabase.ts (1)
8682-8688
: Verify function calls updated after argument-order change
transactions_have_same_abs_amount
andtransactions_have_same_amount
swapped argument order (transaction_id_1
,transaction_id_2
). Ensure every SQL or TypeScript invocation passes the IDs in the new order or uses named notation.
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Show resolved
Hide resolved
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.
LGTM! Thanks @phatgg221.
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes
Chores
Documentation
Tests
Revert