Skip to content

Refactor installation tracker reset and Fix initialize location service - #430

Closed
bamboo0403 wants to merge 6 commits into
mainfrom
fix/installation-tracker-reset
Closed

Refactor installation tracker reset and Fix initialize location service#430
bamboo0403 wants to merge 6 commits into
mainfrom
fix/installation-tracker-reset

Conversation

@bamboo0403

@bamboo0403 bamboo0403 commented Nov 5, 2025

Copy link
Copy Markdown
Member

這是什麼類型的 PR?

選擇所有符合的項目

  • 重構
  • 新功能
  • 錯誤修復
  • 最佳化
  • 技術文件更新

描述

  • 重構 設定重設
  • 修復 手機重開機後啟動自動定位

相關 issue

QA 指南、截圖、錄像

請將這行替換成:如何測試您的 PR 的步驟,已測試的裝置註釋,以及任何相關的 UI 更改圖片。

UI 無障礙清單

如果您的 PR 包含 UI 更改,請使用此清單:

  • 變數名稱實現語意化命名?
  • 測試通過 AA 顏色對比?

Replaces install time tracking with a UUID-based installId and adds build number tracking for more robust installation and upgrade detection. Updates Preference keys and accessors accordingly, and simplifies installation state logic.
@claude

claude Bot commented Nov 5, 2025

Copy link
Copy Markdown

Claude finished @bamboo0403's task —— View job


Code Review Complete ✓

I've reviewed this PR and have the following feedback:

Todo List:

  • Read changed files to understand the modifications
  • Analyze the installation tracker refactoring
  • Review the location service initialization fix
  • Check for bugs, performance, and security concerns
  • Provide comprehensive feedback

Summary

This PR refactors the installation tracking mechanism and fixes a location service initialization issue. Overall, the changes are well-implemented and follow Flutter best practices. Here are my findings:

🟢 Positive Changes

  1. Better Installation Tracking (lib/core/installation_tracker.dart)

    • Switched from installTime (which can be unreliable) to installId (UUID) for tracking installations
    • Added buildNumber tracking for more granular version change detection
    • Simplified logic with early returns, improving readability
    • Properly awaits Preference.instance.clear() (was missing await before)
  2. Improved Preference Key Management (lib/core/preference.dart)

    • Centralized preference keys in PreferenceKeys class (good practice)
    • Added proper getters/setters for buildNumber and installId
    • Consistent naming convention with constants
  3. Critical Bug Fix (lib/core/service.dart)

    • Fixed initialization order: Now checks locationAuto preference BEFORE creating notification
    • Prevents service from running when automatic location is disabled
    • Avoids unnecessary resource usage on device reboot

🟡 Suggestions for Improvement

1. Missing Space After if (Code Style)

lib/core/service.dart:223

if(Preference.locationAuto != true) {

Should be:

if (Preference.locationAuto != true) {

This follows Dart style guidelines for consistent spacing.

2. Consider More Explicit Null Handling

lib/core/service.dart:223

if(Preference.locationAuto != true) {

This condition catches both null and false values. While this works, it might be clearer to write:

if (Preference.locationAuto != true) {

or

if (Preference.locationAuto == false || Preference.locationAuto == null) {

The current approach is fine, but explicit null handling can improve code readability.

3. UUID Dependency

lib/core/installation_tracker.dart:4

  • The PR adds a uuid package dependency. Ensure this is added to pubspec.yaml with an appropriate version constraint.

🔒 Security Review

✓ No security concerns identified:

  • UUID generation is appropriate for installation tracking
  • No sensitive data exposure
  • Proper preference isolation

⚡ Performance Review

✓ Good performance characteristics:

  • Early returns prevent unnecessary work
  • Async operations properly awaited
  • Location service now exits early if disabled, saving resources

🧪 Test Coverage Recommendations

Consider adding tests for:

  1. Installation tracking scenarios:

    • First-time installation
    • Version upgrade (same build)
    • Build number upgrade
    • Reinstallation detection
  2. Location service initialization:

    • Service stops when locationAuto is false or null
    • Service continues when locationAuto is true

📝 Minor Issues

  1. Unused installTime field: The installTime preference is still defined but no longer used in the logic. Consider removing it in a future cleanup if it's truly unnecessary, or document why it's being kept for backward compatibility.

Overall Assessment

Verdict: ✅ APPROVE with minor style suggestions

The changes are sound and solve real problems:

  • ✅ Fixes the auto-location initialization bug on reboot
  • ✅ Improves installation tracking reliability
  • ✅ Better code organization and readability
  • ⚠️ Minor style issue (missing space after if)

The code quality is good, and the refactoring improves maintainability. The critical bug fix for location service initialization is particularly important for user experience.


@lowrt lowrt added this to the v3.1.2 milestone Nov 5, 2025
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Nov 5, 2025
@ExpTechTW ExpTechTW deleted a comment from claude Bot Nov 5, 2025
lowrt and others added 2 commits November 5, 2025 09:24
Moved installId storage from SharedPreferences to flutter_secure_storage for enhanced security. Updated installation tracking logic and registered the secure storage plugin for macOS. Added flutter_secure_storage dependency.
@ExpTechTW ExpTechTW deleted a comment from claude Bot Nov 5, 2025
@bamboo0403

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Nov 5, 2025

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@ExpTechTW ExpTechTW deleted a comment from claude Bot Nov 5, 2025
@bamboo0403 bamboo0403 closed this Nov 5, 2025
@lowrt lowrt removed this from the v3.1.2 milestone Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

偵測重置設定不穩定 [Bug]: 更新、重開機 會被啟用自動定位功能(設定是被關閉的)

2 participants