Repository navigation
feat(weather): add configurable iOS weather widget locations - #579
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
05eb988 to
8b1da0e
Compare
8b1da0e to
09bf6fb
Compare
09bf6fb to
d5cf12a
Compare
The merge-base changed after approval.
d5cf12a to
2e6173c
Compare
Platform: ios New(zh-Hant): 天氣小工具支援每個實例選擇所在地或已儲存地區，並使用獨立地區快取 New(en-US): Allow each iOS weather widget to select Current Location or a saved region with independent per-location snapshots
2e6173c to
0ed4cfd
Compare
|
🔍 OpenCodeReview found 8 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| guard let data = try? Data(contentsOf: url) else { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
WidgetLocationCatalogStore 的 load() 與 decode(_:) 方法使用了 try? 來處理檔案讀取與 JSON 解碼，這會導致錯誤被靜默忽略。雖然在 Intent Extension 中，當載入失敗時退回預設選項（僅顯示「目前位置」）是一種穩健的 UX 決策，但這會增加除錯難度，使開發者難以區分是檔案不存在、權限問題還是 JSON 格式不正確。建議改用 do-catch 並在錯誤發生時進行記錄（例如使用 os_log 或 Logger）。
| displayString: | ||
| "\(location.displayName) — " | ||
| + location.administrativeAreaName |
There was a problem hiding this comment.
地點顯示名稱的組合方式使用硬編碼的字串拼接 ("\(location.displayName) — \(location.administrativeAreaName)")，這未考慮不同語言對地址排列順序或分隔符號的需求。建議使用本地化的字串格式化工具（例如 String(format:) 或在 .strings 檔案中定義帶有佔位符的格式字串）來處理。
| displayString: | ||
| "\(location.displayName) — " | ||
| + location.administrativeAreaName |
There was a problem hiding this comment.
使用者可見的字串使用了字串拼接（String interpolation and concatenation），這會導致在地化（Localization）困難。在不同的語言中，顯示名稱與行政區之間的順序或分隔符號（如 ' — '）可能需要不同。建議使用在地化的格式化字串（Localized format string）來確保不同語言都能正確顯示。
Suggestion:
| displayString: | |
| "\(location.displayName) — " | |
| + location.administrativeAreaName | |
| displayString: String(format: NSLocalizedString("widget.location.display_format", comment: "Format for displaying location in widget options"), location.displayName, location.administrativeAreaName) |
| func matches( | ||
| snapshot: CurrentWeatherWidgetSnapshot | ||
| ) -> Bool { | ||
| guard | ||
| let sourceIdentifier = snapshot.sourceIdentifier, | ||
| let address = CurrentWeatherSnapshotAddress( | ||
| sourceIdentifier: sourceIdentifier | ||
| ) | ||
| else { | ||
| return false | ||
| } |
There was a problem hiding this comment.
遷移邏輯會失效。在 WidgetLocationTarget.matches(snapshot:) 中，使用了 guard let sourceIdentifier = snapshot.sourceIdentifier。由於舊版 (schema < 5) 的快照其 sourceIdentifier 為 nil，這會導致 matches 直接回傳 false。這會連帶導致 WidgetSnapshotStore.loadCurrentWeatherSnapshot(for:) 中的遷移回退機制 (fallback) 無法運作，因為即使找到了舊版的全域快照，target.matches(snapshot: legacySnapshot) 也會因為上述原因永遠回傳 false。
Suggestion:
| func matches( | |
| snapshot: CurrentWeatherWidgetSnapshot | |
| ) -> Bool { | |
| guard | |
| let sourceIdentifier = snapshot.sourceIdentifier, | |
| let address = CurrentWeatherSnapshotAddress( | |
| sourceIdentifier: sourceIdentifier | |
| ) | |
| else { | |
| return false | |
| } | |
| func matches( | |
| snapshot: CurrentWeatherWidgetSnapshot | |
| ) -> Bool { | |
| guard let snapshotSourceIdentifier = snapshot.sourceIdentifier else { | |
| // 舊版快照 (schema < 5) 沒有 sourceIdentifier，應僅匹配預設的 .currentLocation | |
| return self == .currentLocation | |
| } | |
| guard let address = CurrentWeatherSnapshotAddress( | |
| sourceIdentifier: snapshotSourceIdentifier | |
| ) else { | |
| return false | |
| } |
| if let url = currentWeatherSnapshotURL( | ||
| for: sourceIdentifier | ||
| ), | ||
| let data = try? Data(contentsOf: url), | ||
| let snapshot = try? JSONDecoder().decode( | ||
| CurrentWeatherWidgetSnapshot.self, | ||
| from: data | ||
| ), | ||
| target.matches(snapshot: snapshot) | ||
| { | ||
| return snapshot | ||
| } |
There was a problem hiding this comment.
在 WidgetSnapshotStore.loadCurrentWeatherSnapshot(for:) 中，讀取檔案 (Data(contentsOf:)) 與解碼 JSON (JSONDecoder().decode(...)) 都使用了 try?。這會導致當檔案因損毀、權限問題或 schema 不相容而無法讀取時，錯誤會被靜默忽略並直接跳轉到 fallback 邏輯。雖然在 Widget 背景程序中靜默失敗是常見做法，但這會讓開發者難以察覺底層儲存異常。建議考慮使用 do-catch 並在適當的情況下記錄錯誤，以利除錯。
| 'kind': kind.name, | ||
| 'json': json, | ||
| 'sourceIdentifier': ?sourceIdentifier, |
There was a problem hiding this comment.
?sourceIdentifier 是無效的 Dart 語法，會導致編譯錯誤。此外，為了符合測試中對 arguments 的預期（即當 sourceIdentifier 為 null 時，Map 中不應包含該 key），建議使用 if (sourceIdentifier != null) 語法。
Suggestion:
| 'kind': kind.name, | |
| 'json': json, | |
| 'sourceIdentifier': ?sourceIdentifier, | |
| 'kind': kind.name, | |
| 'json': json, | |
| if (sourceIdentifier != null) 'sourceIdentifier': sourceIdentifier, |
| 'kind': kind.name, | ||
| 'json': json, | ||
| 'sourceIdentifier': ?sourceIdentifier, |
There was a problem hiding this comment.
'sourceIdentifier': ?sourceIdentifier 是一個語法錯誤。在 Dart 中，這是不合法的語法。如果你想將 sourceIdentifier 的值傳遞給 Map，應該直接寫 'sourceIdentifier': sourceIdentifier。如果你希望僅在 sourceIdentifier 不為 null 時才將該鍵值對加入 Map，可以使用集合中的 if 語句：if (sourceIdentifier != null) 'sourceIdentifier': sourceIdentifier。
Suggestion:
| 'kind': kind.name, | |
| 'json': json, | |
| 'sourceIdentifier': ?sourceIdentifier, | |
| 'kind': kind.name, | |
| 'json': json, | |
| if (sourceIdentifier != null) 'sourceIdentifier': sourceIdentifier, |
| Future<Result<void>> write({ | ||
| required WidgetSnapshotKind kind, | ||
| required String json, | ||
| String? sourceIdentifier, | ||
| }) async { | ||
| writes.add((kind: kind, json: json)); |
There was a problem hiding this comment.
Mock 類別 _RecordingWidgetSnapshotWriter 與 _BlockingFirstWriteWidgetSnapshotWriter 雖然在 write 方法中接收了 sourceIdentifier 參數，但卻沒有將其存入 writes 列表中。這會導致在測試中使用這些 Mock 物件時，無法驗證 sourceIdentifier 是否被正確傳遞。
這個 PR 做了什麼
讓每個 iOS Weather Widget instance 可以獨立選擇：
並將目前天氣 snapshot 從單一全域檔案擴充為 per-location cache，使多個 Widget 可以同時顯示不同地點。
此 PR 是 Widget 開發堆疊的第三階段，建立可設定 Widget 與 per-location snapshot contract；Widget Extension 自行抓取天氣則由後續 PR 處理。
主要變更
Widget configuration
WeatherWidgetConfigurationIntentIntentConfigurationLocation target
穩定 identifier：
current-locationregion:<regionCode>無效或未知 identifier 會 fail closed，不會自動 fallback 到所在地。
Per-location snapshot
目前天氣 snapshot schema 升級至 v5，新增：
sourceIdentifier檔案範例：
WidgetSnapshots/current-weather/current-location.jsonWidgetSnapshots/current-weather/region-242.json每個 Widget instance 只讀取與自身 target 相符的 snapshot。
Migration
舊的：
WidgetSnapshots/current-weather.json暫時保留 migration fallback，但只有來源可以被證明與 Widget target 相符時才會顯示。
安全與一致性
sourceIdentifier與 payloadregionCode。sourceIdentifier會拒絕。sourceIdentifier寫入對應 per-location cache。已人工驗證
可同時建立多個 Widget，分別顯示不同地點，例如：
且彼此 snapshot 不會互相覆蓋。
測試涵蓋
不包含
目前 Widget Extension 尚不會自行向天氣 API 取得資料。
若該地點尚未由 DPIP App 載入並發布 snapshot，Widget 不會自行更新。
此 PR 不包含：
Native self-refresh 將由下一支 PR：
feat/ios-widget-weather-refresh處理。
PR Stack
依賴：
feat/ios-widget-configurationfeat/ios-widget-current-weather後續：
feat/ios-widget-weather-refresh檢查清單