Skip to content

Conversation

@hanxuan-lin
Copy link
Contributor

@hanxuan-lin hanxuan-lin commented Oct 8, 2025

NTP-based time synchronization to prevent users from bypassing timeline validation by manipulating their device time.

Summary by CodeRabbit

  • New Features
    • App time now syncs with a trusted server for more accurate current time across the app.
  • Bug Fixes
    • More reliable determination of past vs. upcoming events, ensuring correct visibility of event lists, details, and actions.
  • Refactor
    • Standardized event “ended” checks and centralized time retrieval for consistent behavior.
  • Chores
    • Added a time synchronization dependency and initialized it during app startup.

@hanxuan-lin hanxuan-lin self-assigned this Oct 8, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Introduces ServerTimeService for NTP-based time sync, initializes it during app startup, and updates time usage: TimezoneService now derives current time from ServerTimeService. Event screens switch past-event checks from direct DateTime.now() comparisons to hasEnded. Adds ntp dependency. No public API signatures changed.

Changes

Cohort / File(s) Summary of changes
Event screens: past-event logic
lib/features/event/presentation/screens/booked_events_screen.dart, lib/features/event/presentation/screens/event_detail_screen.dart, lib/features/event/presentation/screens/event_screen.dart
Replace end-time checks using DateTime.now() with hasEnded for filtering and UI conditionals; sorting and other flows unchanged.
Time services: introduction and integration
lib/shared/infrastructure/services/server_time_service.dart, lib/shared/infrastructure/services/timezone_service.dart
Add ServerTimeService singleton with NTP sync, cached offset, and getAccurateNow(). Update TimezoneService.stockholmNow() to use ServerTimeService for accurate UTC time before converting to Stockholm.
App startup initialization
lib/main.dart
Import and await ServerTimeService.instance.initialize() after timezone init, before service locator setup.
Dependencies
pubspec.yaml
Add ntp: ^2.0.0 dependency.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as main.dart
  participant TZ as TimezoneService
  participant ST as ServerTimeService
  participant NTP as NTP Server

  rect rgba(221,238,255,0.5)
  note over App: App startup
  App->>TZ: initialize timezones
  App->>ST: initialize()
  activate ST
  ST->>NTP: fetch current time (timeout 5s)
  alt success
    NTP-->>ST: ntpTime
    ST-->>ST: compute offset (ntp - device)
  else failure
    ST-->>ST: set zero offset
  end
  deactivate ST
  App->>App: continue startup
  end
Loading
sequenceDiagram
  autonumber
  participant UI as Event/Timezone callers
  participant TZ as TimezoneService
  participant ST as ServerTimeService

  rect rgba(232,252,244,0.6)
  note over UI: Need current Stockholm time
  UI->>TZ: stockholmNow()
  TZ->>ST: getAccurateNow()
  ST-->>TZ: accurateUtcNow (with cached offset)
  TZ-->>UI: tz.TZDateTime(Stockholm)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • Hugo-Persson

Poem

A rabbit checked the ticking sky, ⏱️
Synced with clouds where NTPs fly.
No more guessing “now” by ear—
Stockholm’s time is crystal-clear.
Events that ended hop aside,
The accurate clock’s our guide.
Thump-thump—bugs can’t hide! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the primary change—adding NTP-based time synchronization—without extraneous detail, making it clear to reviewers what the main feature of the PR is.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-timeline-validation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Oct 8, 2025

PR Preview Action v1.6.2

🚀 View preview at
https://careerfairsystems.github.io/arkad-app-flutter/pr-preview/pr-33/

Built to branch gh-pages at 2025-10-08 22:18 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d48262 and 61cbf8a.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • lib/features/event/presentation/screens/booked_events_screen.dart (1 hunks)
  • lib/features/event/presentation/screens/event_detail_screen.dart (1 hunks)
  • lib/features/event/presentation/screens/event_screen.dart (1 hunks)
  • lib/main.dart (2 hunks)
  • lib/shared/infrastructure/services/server_time_service.dart (1 hunks)
  • lib/shared/infrastructure/services/timezone_service.dart (2 hunks)
  • pubspec.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/presentation/screens/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

lib/**/presentation/screens/**/*.dart: Reset command state in initState using addPostFrameCallback to avoid stale UI state
Use GoRouter context.go/push/pop for navigation; do not use Navigator directly
Only navigate after a command isCompleted and has no error
Invoke ViewModel methods for actions; never execute commands directly from UI

Files:

  • lib/features/event/presentation/screens/event_detail_screen.dart
  • lib/features/event/presentation/screens/booked_events_screen.dart
  • lib/features/event/presentation/screens/event_screen.dart
**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.dart: Use snake_case for file names, PascalCase for classes, camelCase for variables, and _prefix for private members
Wrap debug logging with kDebugMode and avoid production prints of sensitive data

Files:

  • lib/features/event/presentation/screens/event_detail_screen.dart
  • lib/features/event/presentation/screens/booked_events_screen.dart
  • lib/features/event/presentation/screens/event_screen.dart
  • lib/shared/infrastructure/services/timezone_service.dart
  • lib/main.dart
  • lib/shared/infrastructure/services/server_time_service.dart
pubspec.yaml

📄 CodeRabbit inference engine (CLAUDE.md)

Never use 'any' for dependencies; specify version ranges (e.g., dio: ^5.x)

Files:

  • pubspec.yaml
⏰ 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). (1)
  • GitHub Check: Build Flutter Web

Comment on lines +49 to +55
// Log to Sentry for monitoring (graceful fallback, not an error)
Sentry.logger.info(
'NTP sync failed, using device time',
attributes: {
'component': SentryLogAttribute.string('ServerTimeService'),
'error': SentryLogAttribute.string(e.toString()),
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Replace unsupported Sentry logging API

Sentry.logger and SentryLogAttribute are not part of sentry_flutter 9.6.0, so this block will not compile. Please switch to a supported API (e.g., Sentry.captureMessage with SentryLevel.info, or a breadcrumb) or drop the structured attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants