Refactor/dart define config#267
Conversation
WalkthroughMoves runtime dotenv to compile-time ChangesCalendar Configuration & Event Creation Enhancement
Sequence DiagramssequenceDiagram
participant Build as Build System
participant AppConfig
participant SupabaseService
participant AIService
Build->>AppConfig: String.fromEnvironment()
Note over AppConfig: SUPABASE_URL<br/>SUPABASE_ANON_KEY<br/>GEMINI_API_KEY<br/>OAUTH_REDIRECT_URL
SupabaseService->>AppConfig: read supabaseUrl/supabaseAnonKey
SupabaseService->>SupabaseService: validate and initialize Supabase
AIService->>AppConfig: read geminiApiKey
AIService->>AIService: validate API key
sequenceDiagram
participant User
participant CalendarScreen
participant Dialog
participant CreateScreen
participant ChatScreen
User->>CalendarScreen: tap create / AI option
CalendarScreen->>Dialog: _showCreateDialog()
Dialog->>User: show options
User->>Dialog: select Meeting/Task/Ticket/AI
alt Standard creation
Dialog->>CalendarScreen: _handleCreate(type)
CalendarScreen->>CreateScreen: navigate with initialDateTime
CreateScreen-->>CalendarScreen: pop(result=true)
CalendarScreen->>CalendarScreen: _loadEventsWithCache (if mounted)
else AI creation
Dialog->>CalendarScreen: _handleCreateWithAI()
CalendarScreen->>ChatScreen: navigate(arguments: {'initial_message': message})
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
lib/main.dart (1)
18-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not swallow initialization failures or log raw exception payloads.
If service init fails, the app still boots in a partially initialized state, and
"$e"may leak sensitive details into logs. Prefer a controlled startup-failure path (error screen / guarded mode) and sanitize logged errors.Suggested fix
void main() async { WidgetsFlutterBinding.ensureInitialized(); + var startupOk = true; try { await SupabaseService().initialize(); await AIService().initialize(); - } catch (e) { - debugPrint('Error initializing services: $e'); + } catch (_) { + startupOk = false; + debugPrint('Error initializing core services'); } final themeController = await ThemeController.create(); runApp( WidgetsBindingObserverWidget( child: ChangeNotifierProvider<ThemeController>.value( value: themeController, - child: const MyApp(), + child: startupOk ? const MyApp() : const _StartupErrorApp(), ), ), ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/main.dart` around lines 18 - 23, The current catch around SupabaseService().initialize() and AIService().initialize() swallows failures and logs raw exception payloads via debugPrint; replace this with a controlled startup-failure flow: catch specific exceptions from SupabaseService.initialize and AIService.initialize, sanitize/log only a non-sensitive message (e.g., "Service initialization failed"), and then route the app into a guarded error state instead of continuing normal boot (for example by rethrowing to the app bootstrap, calling a StartupController.showErrorScreen(errorCode) or setting an AppState.isStartupFailed flag so the UI shows an error screen). Ensure you update the catch to avoid interpolating "$e" directly and reference SupabaseService.initialize and AIService.initialize and the debug/log call to implement the sanitized logging + controlled failure transition.lib/services/supabase_service.dart (2)
66-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale
.enverror text.Line 68 tells users to check
.env file, but this PR migrated to--dart-define. This will mislead debugging and onboarding.Proposed fix
- throw Exception( - 'Missing required Supabase configuration. Please check your .env file.'); + throw Exception( + 'Missing required Supabase configuration. Provide SUPABASE_URL and SUPABASE_ANON_KEY via --dart-define.');Based on learnings: "Do not use
.envloading; provide values via--dart-define=<NAME>=...."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/supabase_service.dart` around lines 66 - 69, The exception thrown when supabaseUrl or supabaseAnonKey are empty currently tells users to "check your .env file"; update the message thrown in the check that references supabaseUrl and supabaseAnonKey (the throw Exception(...) in lib/services/supabase_service.dart) to instruct users to provide values via --dart-define (e.g. --dart-define=SUPABASE_URL=... and --dart-define=SUPABASE_ANON_KEY=...) or appropriate environment configuration, so the error text reflects the migrated configuration method.
524-526:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove
dotenvusage from Supabase OAuth redirect URL
lib/services/supabase_service.dartusesdotenv.env['OAUTH_REDIRECT_URL']insignInWithGoogle, but the file has nodotenvimport/symbol defined; this will fail to compile—switch to the build-timeAppConfig.oauthRedirectUrl(fromlib/services/app_config.dart) and keep the fallback URL.- Update the config error message at line ~68 that still references checking the “.env file” to match the new
--dart-define/String.fromEnvironmentflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/supabase_service.dart` around lines 524 - 526, The signInWithGoogle code uses dotenv.env['OAUTH_REDIRECT_URL'] but dotenv is not imported and should be replaced with the build-time config; change the redirectUrl assignment in signInWithGoogle to use AppConfig.oauthRedirectUrl with the existing fallback 'io.supabase.ellena://login-callback' (i.e. prefer AppConfig.oauthRedirectUrl ?? fallback). Also update the configuration error message (the one that currently mentions checking the “.env file”) to instruct users to use --dart-define / String.fromEnvironment or to check AppConfig.oauthRedirectUrl instead so the message matches the new build-time configuration flow.lib/services/ai_service.dart (1)
46-55:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove runtime dotenv loading from AI service initialization.
lib/services/ai_service.dartstill callsawait dotenv.load()and the “Missing Gemini API key” error mentions.env, which conflicts with the build-time config strategy.Proposed fix
- // Load API key from .env file - await dotenv.load().catchError((e) { - debugPrint('Error loading .env file: $e'); - }); - _apiKey = AppConfig.geminiApiKey; if (_apiKey == null || _apiKey!.isEmpty) { - throw Exception('Missing Gemini API key. Please check your .env file.'); + throw Exception('Missing GEMINI_API_KEY. Provide it via --dart-define.'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/ai_service.dart` around lines 46 - 55, Remove the runtime dotenv loading and any reference to `.env` in the error message: delete the await dotenv.load() call and its catchError handler in the AIService initialization, rely solely on AppConfig.geminiApiKey to populate _apiKey, and change the thrown Exception text from 'Missing Gemini API key. Please check your .env file.' to a neutral message like 'Missing Gemini API key in configuration' (reference symbols: dotenv.load, AppConfig.geminiApiKey, _apiKey).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/screens/calendar/calendar_screen.dart`:
- Around line 213-216: The code is updating _lastFetchTimeKey unconditionally
after concurrent category fetches (via Future.wait / prefs.setString), which
marks partial failures as a fresh cache; change the logic so you only call
prefs.setString(_lastFetchTimeKey, ...) when all category loaders completed
successfully and returned non-stale data — e.g., have each loader (the functions
passed to Future.wait) return a success flag or throw on failure, inspect the
combined results or await forAny failure, and only update _lastFetchTimeKey when
every result is successful; apply this change to the blocks that write
_lastFetchTimeKey around lines where prefs.setString is called so stale/partial
loads do not mark the cache fresh.
- Around line 166-205: If decoding any cached blob fails and you fall back to
network, clear the partial in-memory state first: reset the _events collection
(or use a local temporary list/map) before calling _loadEventsFromNetwork() so
previously processed data from _processTasksData, _processTicketsData, or
_processMeetingsData isn't duplicated; alternatively decode each JSON into a
temporary variable and only merge into _events after all three decodings
succeed, then call _loadEventsFromNetwork() on error.
- Around line 329-352: The parsed timestamps in _processTicketsData and
_processMeetingsData (variables createdAt and meetingDate built with
DateTime.parse(...)) are left in UTC when the ISO string contains 'Z' or
offsets, causing dateOnly and TimeOfDay to be computed incorrectly for local
users; convert the parsed DateTime to local by calling .toLocal() (e.g.,
createdAt = DateTime.parse(...).toLocal()) before extracting year/month/day or
building TimeOfDay, and update both occurrences (createdAt in
_processTicketsData and meetingDate in _processMeetingsData) so calendar slots
reflect the local timezone.
In `@lib/screens/meetings/create_meeting_screen.dart`:
- Around line 7-16: The new CreateMeetingScreen parameter initialDateTime is
never applied to the form state; in _CreateMeetingScreenState.initState() set
state from widget.initialDateTime by initializing _selectedDate and
_selectedTime (e.g., _selectedDate = widget.initialDateTime?.toLocal() and
_selectedTime = widget.initialDateTime != null ?
TimeOfDay.fromDateTime(widget.initialDateTime!) : null) so the calendar/time
picker is prefilled; ensure you reference widget.initialDateTime and update any
related form controllers or UI after assigning these values.
In `@lib/screens/tasks/create_task_screen.dart`:
- Around line 5-14: The screen never uses the incoming initialDateTime to
pre-populate the internal due date; in _CreateTaskScreenState set the state
field _selectedDueDate from widget.initialDateTime (e.g., in initState) so the
passed DateTime is applied when the screen is shown, and ensure any null
handling keeps existing behavior when initialDateTime is null; update places
that initialize or reset _selectedDueDate to prefer widget.initialDateTime.
In `@lib/screens/tickets/create_ticket_screen.dart`:
- Around line 5-14: CreateTicketScreen exposes initialDateTime but the state
never reads it; update _CreateTicketScreenState to consume
widget.initialDateTime (e.g., in initState) and initialize the internal selected
date/time or calendar/controller with that value so the incoming selected
calendar slot is preserved; reference CreateTicketScreen.initialDateTime and
_CreateTicketScreenState.initState (and whichever internal variable/controller
you use, e.g., selectedDateTime or calendarController) and ensure you only call
setState if necessary when assigning the initial value.
In `@lib/services/ai_service.dart`:
- Around line 10-25: Remove the duplicate declarations and unresolved dotenv
usage: keep a single AIService class (the one that initializes _apiKey and
checks for emptiness), delete the extra AIService and the local AppConfig class,
and reference the project's AppConfig (AppConfig.geminiApiKey) instead of
redefining it; then either add the missing import for dotenv (import
'package:flutter_dotenv/flutter_dotenv.dart') if you intend to call
dotenv.load(), or remove/replace the dotenv.load() call so no unresolved
identifier remains. Ensure AIService's constructor still uses
AppConfig.geminiApiKey and throws on empty key.
---
Outside diff comments:
In `@lib/main.dart`:
- Around line 18-23: The current catch around SupabaseService().initialize() and
AIService().initialize() swallows failures and logs raw exception payloads via
debugPrint; replace this with a controlled startup-failure flow: catch specific
exceptions from SupabaseService.initialize and AIService.initialize,
sanitize/log only a non-sensitive message (e.g., "Service initialization
failed"), and then route the app into a guarded error state instead of
continuing normal boot (for example by rethrowing to the app bootstrap, calling
a StartupController.showErrorScreen(errorCode) or setting an
AppState.isStartupFailed flag so the UI shows an error screen). Ensure you
update the catch to avoid interpolating "$e" directly and reference
SupabaseService.initialize and AIService.initialize and the debug/log call to
implement the sanitized logging + controlled failure transition.
In `@lib/services/ai_service.dart`:
- Around line 46-55: Remove the runtime dotenv loading and any reference to
`.env` in the error message: delete the await dotenv.load() call and its
catchError handler in the AIService initialization, rely solely on
AppConfig.geminiApiKey to populate _apiKey, and change the thrown Exception text
from 'Missing Gemini API key. Please check your .env file.' to a neutral message
like 'Missing Gemini API key in configuration' (reference symbols: dotenv.load,
AppConfig.geminiApiKey, _apiKey).
In `@lib/services/supabase_service.dart`:
- Around line 66-69: The exception thrown when supabaseUrl or supabaseAnonKey
are empty currently tells users to "check your .env file"; update the message
thrown in the check that references supabaseUrl and supabaseAnonKey (the throw
Exception(...) in lib/services/supabase_service.dart) to instruct users to
provide values via --dart-define (e.g. --dart-define=SUPABASE_URL=... and
--dart-define=SUPABASE_ANON_KEY=...) or appropriate environment configuration,
so the error text reflects the migrated configuration method.
- Around line 524-526: The signInWithGoogle code uses
dotenv.env['OAUTH_REDIRECT_URL'] but dotenv is not imported and should be
replaced with the build-time config; change the redirectUrl assignment in
signInWithGoogle to use AppConfig.oauthRedirectUrl with the existing fallback
'io.supabase.ellena://login-callback' (i.e. prefer AppConfig.oauthRedirectUrl ??
fallback). Also update the configuration error message (the one that currently
mentions checking the “.env file”) to instruct users to use --dart-define /
String.fromEnvironment or to check AppConfig.oauthRedirectUrl instead so the
message matches the new build-time configuration flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27036309-2a2f-4c2f-9e8a-f62fab6fbc97
⛔ Files ignored due to path filters (6)
linux/flutter/generated_plugin_registrant.ccis excluded by!**/linux/**linux/flutter/generated_plugins.cmakeis excluded by!**/linux/**macos/Flutter/GeneratedPluginRegistrant.swiftis excluded by!**/macos/**pubspec.lockis excluded by!**/*.lockwindows/flutter/generated_plugin_registrant.ccis excluded by!**/windows/**windows/flutter/generated_plugins.cmakeis excluded by!**/windows/**
📒 Files selected for processing (9)
lib/main.dartlib/screens/calendar/calendar_screen.dartlib/screens/meetings/create_meeting_screen.dartlib/screens/tasks/create_task_screen.dartlib/screens/tickets/create_ticket_screen.dartlib/services/ai_service.dartlib/services/app_config.dartlib/services/supabase_service.dartpubspec.yaml
| if (tasksJson != null) { | ||
| final tasks = List<Map<String, dynamic>>.from( | ||
| jsonDecode(tasksJson).map((x) => Map<String, dynamic>.from(x))); | ||
| final tasks = | ||
| List<Map<String, dynamic>>.from( | ||
| (jsonDecode(tasksJson) as List) | ||
| .map((x) => Map<String, dynamic>.from(x)), | ||
| ); | ||
|
|
||
| _processTasksData(tasks); | ||
| } | ||
|
|
||
| // Load tickets | ||
| final ticketsJson = prefs.getString(_ticketsKey); | ||
| final ticketsJson = | ||
| prefs.getString(_ticketsKey); | ||
|
|
||
| if (ticketsJson != null) { | ||
| final tickets = List<Map<String, dynamic>>.from( | ||
| jsonDecode(ticketsJson).map((x) => Map<String, dynamic>.from(x))); | ||
| final tickets = | ||
| List<Map<String, dynamic>>.from( | ||
| (jsonDecode(ticketsJson) as List) | ||
| .map((x) => Map<String, dynamic>.from(x)), | ||
| ); | ||
|
|
||
| _processTicketsData(tickets); | ||
| } | ||
|
|
||
| // Load meetings | ||
| final meetingsJson = prefs.getString(_meetingsKey); | ||
| final meetingsJson = | ||
| prefs.getString(_meetingsKey); | ||
|
|
||
| if (meetingsJson != null) { | ||
| final meetings = List<Map<String, dynamic>>.from( | ||
| jsonDecode(meetingsJson).map((x) => Map<String, dynamic>.from(x))); | ||
| final meetings = | ||
| List<Map<String, dynamic>>.from( | ||
| (jsonDecode(meetingsJson) as List) | ||
| .map((x) => Map<String, dynamic>.from(x)), | ||
| ); | ||
|
|
||
| _processMeetingsData(meetings); | ||
| } | ||
|
|
||
| debugPrint('Events loaded from cache'); | ||
| debugPrint('Loaded events from cache'); | ||
| } catch (e) { | ||
| debugPrint('Error loading events from cache: $e'); | ||
| // If cache loading fails, fall back to network | ||
| debugPrint('Cache load error: $e'); | ||
| await _loadEventsFromNetwork(); |
There was a problem hiding this comment.
Clear partial _events state before falling back to network.
If one cached blob fails after earlier blobs were already processed, _loadEventsFromNetwork() appends onto those partial results and duplicates them in memory. Reset _events before the fallback, or decode into a temporary map first.
Suggested fix
} catch (e) {
debugPrint('Cache load error: $e');
+ _events.clear();
await _loadEventsFromNetwork();
}📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (tasksJson != null) { | |
| final tasks = List<Map<String, dynamic>>.from( | |
| jsonDecode(tasksJson).map((x) => Map<String, dynamic>.from(x))); | |
| final tasks = | |
| List<Map<String, dynamic>>.from( | |
| (jsonDecode(tasksJson) as List) | |
| .map((x) => Map<String, dynamic>.from(x)), | |
| ); | |
| _processTasksData(tasks); | |
| } | |
| // Load tickets | |
| final ticketsJson = prefs.getString(_ticketsKey); | |
| final ticketsJson = | |
| prefs.getString(_ticketsKey); | |
| if (ticketsJson != null) { | |
| final tickets = List<Map<String, dynamic>>.from( | |
| jsonDecode(ticketsJson).map((x) => Map<String, dynamic>.from(x))); | |
| final tickets = | |
| List<Map<String, dynamic>>.from( | |
| (jsonDecode(ticketsJson) as List) | |
| .map((x) => Map<String, dynamic>.from(x)), | |
| ); | |
| _processTicketsData(tickets); | |
| } | |
| // Load meetings | |
| final meetingsJson = prefs.getString(_meetingsKey); | |
| final meetingsJson = | |
| prefs.getString(_meetingsKey); | |
| if (meetingsJson != null) { | |
| final meetings = List<Map<String, dynamic>>.from( | |
| jsonDecode(meetingsJson).map((x) => Map<String, dynamic>.from(x))); | |
| final meetings = | |
| List<Map<String, dynamic>>.from( | |
| (jsonDecode(meetingsJson) as List) | |
| .map((x) => Map<String, dynamic>.from(x)), | |
| ); | |
| _processMeetingsData(meetings); | |
| } | |
| debugPrint('Events loaded from cache'); | |
| debugPrint('Loaded events from cache'); | |
| } catch (e) { | |
| debugPrint('Error loading events from cache: $e'); | |
| // If cache loading fails, fall back to network | |
| debugPrint('Cache load error: $e'); | |
| await _loadEventsFromNetwork(); | |
| if (tasksJson != null) { | |
| final tasks = | |
| List<Map<String, dynamic>>.from( | |
| (jsonDecode(tasksJson) as List) | |
| .map((x) => Map<String, dynamic>.from(x)), | |
| ); | |
| _processTasksData(tasks); | |
| } | |
| final ticketsJson = | |
| prefs.getString(_ticketsKey); | |
| if (ticketsJson != null) { | |
| final tickets = | |
| List<Map<String, dynamic>>.from( | |
| (jsonDecode(ticketsJson) as List) | |
| .map((x) => Map<String, dynamic>.from(x)), | |
| ); | |
| _processTicketsData(tickets); | |
| } | |
| final meetingsJson = | |
| prefs.getString(_meetingsKey); | |
| if (meetingsJson != null) { | |
| final meetings = | |
| List<Map<String, dynamic>>.from( | |
| (jsonDecode(meetingsJson) as List) | |
| .map((x) => Map<String, dynamic>.from(x)), | |
| ); | |
| _processMeetingsData(meetings); | |
| } | |
| debugPrint('Loaded events from cache'); | |
| } catch (e) { | |
| debugPrint('Cache load error: $e'); | |
| _events.clear(); | |
| await _loadEventsFromNetwork(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/calendar/calendar_screen.dart` around lines 166 - 205, If
decoding any cached blob fails and you fall back to network, clear the partial
in-memory state first: reset the _events collection (or use a local temporary
list/map) before calling _loadEventsFromNetwork() so previously processed data
from _processTasksData, _processTicketsData, or _processMeetingsData isn't
duplicated; alternatively decode each JSON into a temporary variable and only
merge into _events after all three decodings succeed, then call
_loadEventsFromNetwork() on error.
| await prefs.setString( | ||
| _lastFetchTimeKey, DateTime.now().toIso8601String()); | ||
|
|
||
| // Tasks, tickets, and meetings are saved in their respective methods | ||
| _lastFetchTimeKey, | ||
| DateTime.now().toIso8601String(), | ||
| ); |
There was a problem hiding this comment.
Don't mark partial network loads as a fresh cache.
Each category loader swallows its own failure, so Future.wait still resolves and _lastFetchTimeKey is updated even when one fetch failed or stayed stale. That leaves the calendar serving incomplete data as “fresh” for the next 5 minutes.
Also applies to: 224-228, 255-260, 316-321, 371-376
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/calendar/calendar_screen.dart` around lines 213 - 216, The code
is updating _lastFetchTimeKey unconditionally after concurrent category fetches
(via Future.wait / prefs.setString), which marks partial failures as a fresh
cache; change the logic so you only call prefs.setString(_lastFetchTimeKey, ...)
when all category loaders completed successfully and returned non-stale data —
e.g., have each loader (the functions passed to Future.wait) return a success
flag or throw on failure, inspect the combined results or await forAny failure,
and only update _lastFetchTimeKey when every result is successful; apply this
change to the blocks that write _lastFetchTimeKey around lines where
prefs.setString is called so stale/partial loads do not mark the cache fresh.
| final createdAt = | ||
| DateTime.parse(ticket['created_at']); | ||
|
|
||
| final dateOnly = DateTime( | ||
| createdAt.year, | ||
| createdAt.month, | ||
| createdAt.day, | ||
| ); | ||
|
|
||
| _events.putIfAbsent(dateOnly, () => []); | ||
|
|
||
| _events[dateOnly]!.add( | ||
| CalendarEvent( | ||
| title: | ||
| ticket['title'] ?? 'Untitled Ticket', | ||
| startTime: TimeOfDay( | ||
| hour: createdAt.hour, | ||
| minute: createdAt.minute, | ||
| ), | ||
| endTime: TimeOfDay.fromDateTime( | ||
| createdAt.add( | ||
| const Duration(hours: 1), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
How does Dart DateTime.parse handle ISO-8601 strings with a Zor timezone offset, and should Flutter apps call.toLocal() before deriving local calendar days and TimeOfDay values from Supabase/PostgREST timestamp fields?
💡 Result:
Dart's DateTime.parse method handles ISO-8601 strings, including those with a 'Z' (UTC) or a numeric timezone offset (e.g., +01:30) [1][2]. When a timezone or offset is provided, the resulting DateTime object is marked as UTC (isUtc = true) and the time is normalized to UTC [1][3]. If no timezone information is present, the string is parsed as a local time [1]. Regarding your second question: Yes, Flutter apps typically should call.toLocal on DateTime objects derived from Supabase/PostgREST timestamptz fields if you need to display or derive local calendar days and times [3][4]. Because Supabase stores and returns timestamptz values in UTC [5][4], DateTime.parse will return a UTC object [1][3]. If you use this object directly for local calculations—such as extracting the hour for a TimeOfDay or determining the day of the month—you will be performing those operations on the UTC time rather than the user's local time [6][4]. Calling.toLocal converts the time to the local timezone of the device where the app is running [7], ensuring that calendar dates and clock times align with the user's expected context [6]. In summary: 1. DateTime.parse interprets 'Z' or offsets by normalizing to UTC and setting isUtc to true [1][3]. 2. Always call.toLocal on incoming UTC timestamps from your database before using them for UI logic that depends on local calendar days or wall-clock times [6][4]. Top results: [1], [3], [7], [4]
Citations:
- 1: https://api.dart.dev/stable/2.18.6/dart-core/DateTime/parse.html
- 2: https://api.flutter.dev/flutter/dart-core/DateTime/parse.html
- 3: https://stackoverflow.com/questions/52318867/datetime-timezone-deserialization
- 4: https://community.flutterflow.io/database-and-apis/post/formatting-a-supabase-timestamptz-in-hm-shows-the-utc-time-not-the-AwcVw9X4fBEon6V
- 5: DateTime send to supabase database lacks timezone details FlutterFlow/flutterflow-issues#2759
- 6: https://dev.to/bharath_kumar_39293/how-i-replaced-react-calendar-with-a-timezone-safe-unifieddatepicker-in-a-production-oss-codebase-4n5e
- 7: https://api.flutter.dev/flutter/dart-core/DateTime/toLocal.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '300,430p' lib/screens/calendar/calendar_screen.dart | nl -ba | sed -n '1,160p'Repository: AOSSIE-Org/Ell-ena
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '300,430p' lib/screens/calendar/calendar_screen.dart | nl -baRepository: AOSSIE-Org/Ell-ena
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '300,430p' lib/screens/calendar/calendar_screen.dart | nl -baRepository: AOSSIE-Org/Ell-ena
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '300,430p' lib/screens/calendar/calendar_screen.dart | nl -baRepository: AOSSIE-Org/Ell-ena
Length of output: 105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '320,380p' lib/screens/calendar/calendar_screen.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 1492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '380,430p' lib/screens/calendar/calendar_screen.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 1371
Convert parsed timestamp strings to local time before building calendar slots
In lib/screens/calendar/calendar_screen.dart, _processTicketsData and _processMeetingsData use DateTime.parse(...) to derive dateOnly and TimeOfDay. When created_at/meeting_date ISO strings include a Z or timezone offset (common for Supabase timestamptz), DateTime.parse normalizes to UTC, so extracting year/month/day and hour/minute can shift events for non-UTC users.
Suggested fix
- final createdAt =
- DateTime.parse(ticket['created_at']);
+ final createdAt =
+ DateTime.parse(ticket['created_at']).toLocal();
...
- final meetingDate =
- DateTime.parse(meeting['meeting_date']);
+ final meetingDate =
+ DateTime.parse(meeting['meeting_date']).toLocal();Applies to lines 329-352 and 384-408.
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final createdAt = | |
| DateTime.parse(ticket['created_at']); | |
| final dateOnly = DateTime( | |
| createdAt.year, | |
| createdAt.month, | |
| createdAt.day, | |
| ); | |
| _events.putIfAbsent(dateOnly, () => []); | |
| _events[dateOnly]!.add( | |
| CalendarEvent( | |
| title: | |
| ticket['title'] ?? 'Untitled Ticket', | |
| startTime: TimeOfDay( | |
| hour: createdAt.hour, | |
| minute: createdAt.minute, | |
| ), | |
| endTime: TimeOfDay.fromDateTime( | |
| createdAt.add( | |
| const Duration(hours: 1), | |
| ), | |
| ), | |
| final createdAt = | |
| DateTime.parse(ticket['created_at']).toLocal(); | |
| final dateOnly = DateTime( | |
| createdAt.year, | |
| createdAt.month, | |
| createdAt.day, | |
| ); | |
| _events.putIfAbsent(dateOnly, () => []); | |
| _events[dateOnly]!.add( | |
| CalendarEvent( | |
| title: | |
| ticket['title'] ?? 'Untitled Ticket', | |
| startTime: TimeOfDay( | |
| hour: createdAt.hour, | |
| minute: createdAt.minute, | |
| ), | |
| endTime: TimeOfDay.fromDateTime( | |
| createdAt.add( | |
| const Duration(hours: 1), | |
| ), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/calendar/calendar_screen.dart` around lines 329 - 352, The parsed
timestamps in _processTicketsData and _processMeetingsData (variables createdAt
and meetingDate built with DateTime.parse(...)) are left in UTC when the ISO
string contains 'Z' or offsets, causing dateOnly and TimeOfDay to be computed
incorrectly for local users; convert the parsed DateTime to local by calling
.toLocal() (e.g., createdAt = DateTime.parse(...).toLocal()) before extracting
year/month/day or building TimeOfDay, and update both occurrences (createdAt in
_processTicketsData and meetingDate in _processMeetingsData) so calendar slots
reflect the local timezone.
| final DateTime? initialDateTime; | ||
|
|
||
| const CreateMeetingScreen({ | ||
| super.key, | ||
| this.initialDateTime, | ||
| }); | ||
|
|
||
| @override | ||
| State<CreateMeetingScreen> createState() => _CreateMeetingScreenState(); | ||
| State<CreateMeetingScreen> createState() => | ||
| _CreateMeetingScreenState(); |
There was a problem hiding this comment.
initialDateTime is added but never applied to form state.
Line 7/Line 11 introduces initialDateTime, but _selectedDate and _selectedTime are never initialized from it, so calendar prefill does not work.
Suggested fix
class _CreateMeetingScreenState extends State<CreateMeetingScreen> {
@@
bool _isGoogleMeetUrl = true;
+
+ `@override`
+ void initState() {
+ super.initState();
+ final initial = widget.initialDateTime;
+ if (initial != null) {
+ _selectedDate = DateTime(initial.year, initial.month, initial.day);
+ _selectedTime = TimeOfDay.fromDateTime(initial);
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/meetings/create_meeting_screen.dart` around lines 7 - 16, The new
CreateMeetingScreen parameter initialDateTime is never applied to the form
state; in _CreateMeetingScreenState.initState() set state from
widget.initialDateTime by initializing _selectedDate and _selectedTime (e.g.,
_selectedDate = widget.initialDateTime?.toLocal() and _selectedTime =
widget.initialDateTime != null ? TimeOfDay.fromDateTime(widget.initialDateTime!)
: null) so the calendar/time picker is prefilled; ensure you reference
widget.initialDateTime and update any related form controllers or UI after
assigning these values.
| final DateTime? initialDateTime; | ||
|
|
||
| const CreateTaskScreen({ | ||
| super.key, | ||
| this.initialDateTime, | ||
| }); | ||
|
|
||
| @override | ||
| State<CreateTaskScreen> createState() => _CreateTaskScreenState(); | ||
| State<CreateTaskScreen> createState() => | ||
| _CreateTaskScreenState(); |
There was a problem hiding this comment.
initialDateTime is not wired into _selectedDueDate.
Line 5/Line 9 adds the input, but the screen never consumes widget.initialDateTime, so due date pre-population is missing.
Suggested fix
`@override`
void initState() {
-
super.initState();
+ final initial = widget.initialDateTime;
+ if (initial != null) {
+ _selectedDueDate = DateTime(initial.year, initial.month, initial.day);
+ }
_loadTeamMembers();
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/tasks/create_task_screen.dart` around lines 5 - 14, The screen
never uses the incoming initialDateTime to pre-populate the internal due date;
in _CreateTaskScreenState set the state field _selectedDueDate from
widget.initialDateTime (e.g., in initState) so the passed DateTime is applied
when the screen is shown, and ensure any null handling keeps existing behavior
when initialDateTime is null; update places that initialize or reset
_selectedDueDate to prefer widget.initialDateTime.
| final DateTime? initialDateTime; | ||
|
|
||
| const CreateTicketScreen({ | ||
| super.key, | ||
| this.initialDateTime, | ||
| }); | ||
|
|
||
| @override | ||
| State<CreateTicketScreen> createState() => _CreateTicketScreenState(); | ||
| State<CreateTicketScreen> createState() => | ||
| _CreateTicketScreenState(); |
There was a problem hiding this comment.
initialDateTime is currently a no-op in CreateTicketScreen.
Line 5/Line 9 exposes the parameter, but nothing reads widget.initialDateTime, so selected calendar slot context is dropped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/tickets/create_ticket_screen.dart` around lines 5 - 14,
CreateTicketScreen exposes initialDateTime but the state never reads it; update
_CreateTicketScreenState to consume widget.initialDateTime (e.g., in initState)
and initialize the internal selected date/time or calendar/controller with that
value so the incoming selected calendar slot is preserved; reference
CreateTicketScreen.initialDateTime and _CreateTicketScreenState.initState (and
whichever internal variable/controller you use, e.g., selectedDateTime or
calendarController) and ensure you only call setState if necessary when
assigning the initial value.
| class AIService { | ||
| late final String _apiKey; | ||
|
|
||
| AIService() { | ||
| _apiKey = AppConfig.geminiApiKey; | ||
|
|
||
| if (_apiKey.isEmpty) { | ||
| throw Exception("Missing GEMINI_API_KEY"); | ||
| } | ||
| } | ||
| } | ||
| class AppConfig { | ||
| static const geminiApiKey = | ||
| String.fromEnvironment('GEMINI_API_KEY'); | ||
| } | ||
| class AIService { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify duplicate class declarations and local AppConfig shadowing
rg -n --type=dart -C2 "^class AIService|^class AppConfig" lib/services/ai_service.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 502
🏁 Script executed:
#!/bin/bash
# Inspect ai_service.dart around dotenv.load and other initialization code
sed -n '35,80p' lib/services/ai_service.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 1355
🏁 Script executed:
#!/bin/bash
# Show top of lib/services/ai_service.dart to inspect duplicate class bodies and dotenv import/type declarations
sed -n '1,80p' lib/services/ai_service.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 2294
🏁 Script executed:
#!/bin/bash
# Check whether dotenv is imported/available and find dotenv.load usage context
rg -n --type=dart "dotenv\.load|import ['\"].*dotenv" lib/services/ai_service.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Find all dotenv occurrences in the file to see if it's imported or otherwise defined
rg -n --type=dart "\bdotenv\b" lib/services/ai_service.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 108
Fix duplicate AIService/AppConfig declarations and missing dotenv import in lib/services/ai_service.dart.
lib/services/ai_service.dartdefines twoclass AIServicedeclarations and also declares a localclass AppConfig, conflicting with the importedpackage:ell_ena/config/app_config.dartand blocking compilation.initialize()callsdotenv.load()butdotenvis not imported in this file, causing an unresolved identifier compile error.
Proposed fix
-class AIService {
- late final String _apiKey;
-
- AIService() {
- _apiKey = AppConfig.geminiApiKey;
-
- if (_apiKey.isEmpty) {
- throw Exception("Missing GEMINI_API_KEY");
- }
- }
-}
-class AppConfig {
- static const geminiApiKey =
- String.fromEnvironment('GEMINI_API_KEY');
-}
class AIService {Add dotenv import (or remove/replace dotenv.load()), and keep only one AIService class plus the correct AppConfig source.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/services/ai_service.dart` around lines 10 - 25, Remove the duplicate
declarations and unresolved dotenv usage: keep a single AIService class (the one
that initializes _apiKey and checks for emptiness), delete the extra AIService
and the local AppConfig class, and reference the project's AppConfig
(AppConfig.geminiApiKey) instead of redefining it; then either add the missing
import for dotenv (import 'package:flutter_dotenv/flutter_dotenv.dart') if you
intend to call dotenv.load(), or remove/replace the dotenv.load() call so no
unresolved identifier remains. Ensure AIService's constructor still uses
AppConfig.geminiApiKey and throws on empty key.
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/screens/calendar/calendar_screen.dart (3)
48-48: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMake
markerOpacityastatic constand rename the shadowing local.
markerOpacityis a public, mutable instance field that is never reassigned, and it is shadowed insidemarkerBuilder(Lines 500-501) by a theme-dependent local of the same name with a different value (0.55/0.3 vs 0.5). The collision makes the two opacity semantics hard to reason about.♻️ Proposed change
- static const Duration _cacheDuration = Duration(minutes: 5); - - double markerOpacity = 0.5; + static const Duration _cacheDuration = Duration(minutes: 5); + + static const double markerOpacity = 0.5;And rename the local in
markerBuilder:- final markerOpacity = - isLight ? 0.55 : 0.3; + final badgeOpacity = + isLight ? 0.55 : 0.3; ... - color: Colors.green - .withOpacity( - markerOpacity, - ), + color: Colors.green + .withValues(alpha: badgeOpacity),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/screens/calendar/calendar_screen.dart` at line 48, Make the instance field markerOpacity a static const (e.g., static const double markerOpacity = 0.5) since it is never reassigned, and rename the theme-dependent local inside markerBuilder (currently shadowing markerOpacity) to a different name (e.g., themeMarkerOpacity) and update its uses in markerBuilder so there is no shadowing between the class-level markerOpacity and the local opacity variable.
511-513:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMigrate deprecated
Color.withOpacity()towithValues(alpha:)
Color.withOpacity()is deprecated in Flutter’s wide-gamut Color framework; migrate toColor.withValues(alpha: ...)to avoid precision-loss issues. This screen still useswithOpacityat lines 512, 651, and 797 (Issue#255).♻️ Proposed change (apply to all three sites)
- color: Colors.green - .withOpacity( - markerOpacity, - ), + color: Colors.green + .withValues(alpha: markerOpacity),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/screens/calendar/calendar_screen.dart` around lines 511 - 513, Replace deprecated Color.withOpacity usages with the new wide-gamut API: locate the three occurrences in calendar_screen.dart where Colors.green.withOpacity(markerOpacity) (and similar calls at the other two sites) are used and change them to use Color.withValues(alpha: markerOpacity) instead; ensure the alpha argument remains a double in the 0.0–1.0 range and update any imports or type expectations if required so the calls compile (search for "withOpacity(" to find all instances and update to "withValues(alpha: ...)").
819-872:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix calendar-slot prefill:
selectedDateTimeis computed but discarded in_handleCreate.In
lib/screens/calendar/calendar_screen.dart,_handleCreatebuildsselectedDateTime(827-833) but the subsequentNavigator.pushcalls pushCreateMeetingScreen(),CreateTaskScreen(), andCreateTicketScreen()with no arguments (842-863), leaving a dead local and preventing the#255slot prefill.
CreateMeetingScreenhas noinitialDateTimeconstructor param, so meeting prefill requires adding/using one.CreateTaskScreen/CreateTicketScreendeclarefinal DateTime? initialDateTime, but it isn’t referenced anywhere else in those files, so task/ticket prefill won’t work even if it’s passed until they wirewidget.initialDateTimeinto their initial selected date/due-date state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/screens/calendar/calendar_screen.dart` around lines 819 - 872, _handleCreate builds selectedDateTime but never passes it into the pushed screens; update _handleCreate to pass selectedDateTime into the constructors of CreateMeetingScreen, CreateTaskScreen, and CreateTicketScreen (e.g. add an initialDateTime argument when calling Navigator.push), add an initialDateTime DateTime? parameter and corresponding field to CreateMeetingScreen if missing, and wire widget.initialDateTime inside CreateTaskScreen and CreateTicketScreen to initialize their internal selected/due-date state so the screens are prefilled; also update any callers/constructors to match the new param signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/screens/meetings/create_meeting_screen.dart`:
- Line 7: Restore the optional initialDateTime parameter on the
CreateMeetingScreen constructor so the create-screen can be prefilled from the
calendar; update the constructor signature to include an optional DateTime?
initialDateTime parameter and ensure any corresponding field/property (e.g.,
this.initialDateTime) is set and used by the widget (look for
CreateMeetingScreen class and its constructor) so existing datetime-prefill
logic continues to work.
---
Outside diff comments:
In `@lib/screens/calendar/calendar_screen.dart`:
- Line 48: Make the instance field markerOpacity a static const (e.g., static
const double markerOpacity = 0.5) since it is never reassigned, and rename the
theme-dependent local inside markerBuilder (currently shadowing markerOpacity)
to a different name (e.g., themeMarkerOpacity) and update its uses in
markerBuilder so there is no shadowing between the class-level markerOpacity and
the local opacity variable.
- Around line 511-513: Replace deprecated Color.withOpacity usages with the new
wide-gamut API: locate the three occurrences in calendar_screen.dart where
Colors.green.withOpacity(markerOpacity) (and similar calls at the other two
sites) are used and change them to use Color.withValues(alpha: markerOpacity)
instead; ensure the alpha argument remains a double in the 0.0–1.0 range and
update any imports or type expectations if required so the calls compile (search
for "withOpacity(" to find all instances and update to "withValues(alpha:
...)").
- Around line 819-872: _handleCreate builds selectedDateTime but never passes it
into the pushed screens; update _handleCreate to pass selectedDateTime into the
constructors of CreateMeetingScreen, CreateTaskScreen, and CreateTicketScreen
(e.g. add an initialDateTime argument when calling Navigator.push), add an
initialDateTime DateTime? parameter and corresponding field to
CreateMeetingScreen if missing, and wire widget.initialDateTime inside
CreateTaskScreen and CreateTicketScreen to initialize their internal
selected/due-date state so the screens are prefilled; also update any
callers/constructors to match the new param signature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 82f4c737-7613-43ad-8f26-12ae4bbd98a3
📒 Files selected for processing (3)
lib/main.dartlib/screens/calendar/calendar_screen.dartlib/screens/meetings/create_meeting_screen.dart
| @@ -7,7 +7,8 @@ class CreateMeetingScreen extends StatefulWidget { | |||
| CreateMeetingScreen({super.key,}); | |||
There was a problem hiding this comment.
Restore optional initialDateTime in constructor to preserve prefill flow.
At Line 7, removing initialDateTime prevents calendar-driven prefill for meeting creation, which breaks this PR’s create-screen datetime-support contract.
Suggested fix
class CreateMeetingScreen extends StatefulWidget {
- CreateMeetingScreen({super.key,});
+ const CreateMeetingScreen({super.key, this.initialDateTime});
+ final DateTime? initialDateTime;📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CreateMeetingScreen({super.key,}); | |
| class CreateMeetingScreen extends StatefulWidget { | |
| const CreateMeetingScreen({super.key, this.initialDateTime}); | |
| final DateTime? initialDateTime; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/screens/meetings/create_meeting_screen.dart` at line 7, Restore the
optional initialDateTime parameter on the CreateMeetingScreen constructor so the
create-screen can be prefilled from the calendar; update the constructor
signature to include an optional DateTime? initialDateTime parameter and ensure
any corresponding field/property (e.g., this.initialDateTime) is set and used by
the widget (look for CreateMeetingScreen class and its constructor) so existing
datetime-prefill logic continues to work.
Closes #255
📝 Description
This PR refactors the application’s configuration system by removing
flutter_dotenvand migrating to Flutter’s--dart-defineapproach.Environment variables are now injected at build/run time instead of being loaded from a runtime
.envfile. This improves security, reliability, and production readiness.🔧 Changes Made
flutter_dotenvdependency frompubspec.yaml.envasset configurationdotenv.load()callsdotenv.env[...]usage withAppConfigAppConfig)String.fromEnvironmentString.fromEnvironment.envfile is no longer used--dart-definevalues✅ Checklist
Summary by CodeRabbit
New Features
Improvements