feat: initial build — weather app with widget, WorkManager, and settings UI#1
Merged
Conversation
Reviewer's GuideInitial implementation of the SimpleWeather Android app: a Compose-based settings UI backed by DataStore and a WeatherRepository that talks to Open-Meteo via Retrofit, a Glance home-screen widget that reads cached temperature, and a WorkManager-powered periodic worker scheduled via a WorkScheduler helper, plus the full Gradle/manifest/theme wiring and Robolectric unit tests. Sequence diagram for enabling device location and saving itsequenceDiagram
actor User
participant MainActivity
participant SettingsScreen
participant SettingsViewModel
participant FusedLocationClient
participant DataStore
participant WorkScheduler
User->>MainActivity: Launch app
MainActivity->>SettingsScreen: setContent SimpleWeatherTheme SettingsScreen
SettingsScreen->>SettingsViewModel: collect uiState
User->>SettingsScreen: Toggle useDeviceLocation ON
SettingsScreen->>SettingsScreen: Request ACCESS_COARSE_LOCATION
User-->>SettingsScreen: Grant permission
SettingsScreen->>SettingsViewModel: setUseDeviceLocation(true)
SettingsScreen->>MainActivity: onLocationPermissionGranted()
MainActivity->>FusedLocationClient: getLastLocation()
FusedLocationClient-->>MainActivity: Location(lat, lon)
MainActivity->>SettingsViewModel: saveDeviceLocation(lat, lon)
SettingsViewModel->>DataStore: edit set LOCATION_LAT, LOCATION_LON, LOCATION_DISPLAY_NAME
SettingsViewModel->>DataStore: read UPDATE_INTERVAL_MINUTES
SettingsViewModel->>WorkScheduler: schedule(context, intervalMinutes)
Sequence diagram for background fetch updating widgetsequenceDiagram
participant WorkManager
participant WeatherFetchWorker
participant DataStore
participant WeatherRepository
participant OpenMeteoAPI
participant WeatherWidget
WorkManager->>WeatherFetchWorker: doWork()
WeatherFetchWorker->>DataStore: read LOCATION_LAT, LOCATION_LON
DataStore-->>WeatherFetchWorker: lat, lon
WeatherFetchWorker->>WeatherRepository: fetchAndCacheWeather(lat, lon)
WeatherRepository->>OpenMeteoAPI: getCurrentWeather(latitude, longitude, current, temperature_unit)
OpenMeteoAPI-->>WeatherRepository: WeatherResponse
WeatherRepository->>DataStore: edit LAST_TEMP_CELSIUS, LAST_UPDATED_EPOCH
WeatherFetchWorker->>WeatherWidget: updateAll(context)
WeatherWidget->>DataStore: read LAST_TEMP_CELSIUS, TEMP_UNIT
DataStore-->>WeatherWidget: tempCelsius, unit
WeatherWidget->>WeatherWidget: compute displayTemp
Class diagram for core SimpleWeather types and data layerclassDiagram
class MainActivity {
+onCreate(savedInstanceState: Bundle): void
+fetchAndSaveDeviceLocation(): void
-viewModel: SettingsViewModel
}
class SimpleWeatherTheme {
+SimpleWeatherTheme(content: () -> Unit): void
}
class SettingsUiState {
+useDeviceLocation: Boolean
+locationDisplayName: String
+tempUnit: String
+updateIntervalMinutes: Int
}
class SettingsViewModel {
-context: Context
-repository: WeatherRepository
-dispatcher: CoroutineDispatcher
+uiState: StateFlow~SettingsUiState~
+setTempUnit(unit: String): void
+setUpdateInterval(minutes: Int): void
+setUseDeviceLocation(use: Boolean): void
+resolveAndSaveLocation(query: String): void
+saveDeviceLocation(lat: Float, lon: Float): void
}
class WeatherRepository {
-context: Context
-weatherService: OpenMeteoService
-geocodingService: OpenMeteoService
+fetchAndCacheWeather(lat: Float, lon: Float): void
+geocodeLocation(query: String): GeocodingResult
}
class RepositoryGeocodingResult {
+lat: Float
+lon: Float
+displayName: String
}
class WeatherDataStore {
+USE_DEVICE_LOCATION: PreferencesKey~Boolean~
+LOCATION_LAT: PreferencesKey~Float~
+LOCATION_LON: PreferencesKey~Float~
+LOCATION_DISPLAY_NAME: PreferencesKey~String~
+TEMP_UNIT: PreferencesKey~String~
+UPDATE_INTERVAL_MINUTES: PreferencesKey~Int~
+LAST_TEMP_CELSIUS: PreferencesKey~Float~
+LAST_UPDATED_EPOCH: PreferencesKey~Long~
}
class WeatherFetchWorker {
-applicationContext: Context
+doWork(): Result
}
class WorkScheduler {
+schedule(context: Context, intervalMinutes: Int): void
+cancel(context: Context): void
}
class WeatherWidget {
+provideGlance(context: Context, id: GlanceId): void
-celsiusToFahrenheit(celsius: Float): Int
}
class WeatherWidgetReceiver {
+glanceAppWidget: GlanceAppWidget
}
class NetworkModule {
+weatherService: OpenMeteoService
+geocodingService: OpenMeteoService
}
class OpenMeteoService {
+getCurrentWeather(latitude: Float, longitude: Float, current: String, temperatureUnit: String): WeatherResponse
+searchLocation(query: String, count: Int, language: String, format: String): GeocodingResponse
}
class WeatherResponse {
+current: CurrentWeather
}
class CurrentWeather {
+time: String
+temperatureCelsius: Float
}
class GeocodingResponse {
+results: List~ApiGeocodingResult~
}
class ApiGeocodingResult {
+name: String
+latitude: Double
+longitude: Double
+country: String
+state: String
}
MainActivity --> SettingsViewModel
MainActivity ..> SimpleWeatherTheme
SettingsViewModel --> SettingsUiState
SettingsViewModel --> WeatherRepository
SettingsViewModel --> WeatherDataStore
SettingsViewModel ..> WorkScheduler
WeatherRepository ..> OpenMeteoService
WeatherRepository --> RepositoryGeocodingResult
WeatherRepository --> WeatherDataStore
WeatherFetchWorker ..> WeatherRepository
WeatherFetchWorker ..> NetworkModule
WeatherFetchWorker --> WeatherDataStore
WorkScheduler ..> WeatherFetchWorker
WeatherWidget --> WeatherDataStore
WeatherWidgetReceiver --> WeatherWidget
NetworkModule --> OpenMeteoService
OpenMeteoService --> WeatherResponse
OpenMeteoService --> GeocodingResponse
WeatherResponse --> CurrentWeather
GeocodingResponse --> ApiGeocodingResult
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
useDeviceLocationtoggle only updates a boolean in DataStore and doesn’t affect the scheduledWorkManagerjob, so background fetches will continue using the last lat/lon even when the user turns location off; consider cancelling or pausing the periodic work when location is disabled or cleared. resolveAndSaveLocationwrites a new lat/lon but doesn’t scheduleWeatherFetchWorker, so manual location entry won’t start background updates until the app is restarted or the interval is changed; you may want to schedule (or reschedule) work immediately after a successful geocode.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `useDeviceLocation` toggle only updates a boolean in DataStore and doesn’t affect the scheduled `WorkManager` job, so background fetches will continue using the last lat/lon even when the user turns location off; consider cancelling or pausing the periodic work when location is disabled or cleared.
- `resolveAndSaveLocation` writes a new lat/lon but doesn’t schedule `WeatherFetchWorker`, so manual location entry won’t start background updates until the app is restarted or the interval is changed; you may want to schedule (or reschedule) work immediately after a successful geocode.
## Individual Comments
### Comment 1
<location path="app/src/main/java/com/example/simpleweather/ui/settings/SettingsScreen.kt" line_range="51-52" />
<code_context>
+ modifier = Modifier.fillMaxWidth()
+ ) {
+ Text("Use device location", modifier = Modifier.weight(1f))
+ Switch(
+ checked = uiState.useDeviceLocation,
+ onCheckedChange = { use ->
+ viewModel.setUseDeviceLocation(use)
</code_context>
<issue_to_address>
**issue:** Handle the case where location permission is denied so the toggle doesn’t get out of sync with reality.
If the user enables "Use device location" but then denies the system permission dialog, `useDeviceLocation` remains `true` in DataStore even though the app still lacks permission. That desynchronizes the toggle from actual capability. Update the `locationPermissionLauncher` callback to revert the switch and preference when `granted` is `false`, or only call `setUseDeviceLocation(true)` after a successful permission result instead of doing it optimistically.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- WeatherRepository is constructed in both MainActivity and WeatherFetchWorker with nearly identical wiring; consider extracting a small provider/factory so the construction is centralized and easier to change.
- WorkScheduler.schedule accepts any intervalMinutes but WorkManager enforces a 15‑minute minimum; it would be safer to validate or clamp the value in schedule() rather than rely on callers to only pass valid values.
- Temperature units ("C"/"F") and default values like 60 minutes are hard-coded in multiple places; defining them as shared constants (e.g., in WeatherDataStore or a Config object) would reduce duplication and the risk of inconsistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- WeatherRepository is constructed in both MainActivity and WeatherFetchWorker with nearly identical wiring; consider extracting a small provider/factory so the construction is centralized and easier to change.
- WorkScheduler.schedule accepts any intervalMinutes but WorkManager enforces a 15‑minute minimum; it would be safer to validate or clamp the value in schedule() rather than rely on callers to only pass valid values.
- Temperature units ("C"/"F") and default values like 60 minutes are hard-coded in multiple places; defining them as shared constants (e.g., in WeatherDataStore or a Config object) would reduce duplication and the risk of inconsistency.
## Individual Comments
### Comment 1
<location path="app/src/main/java/com/example/simpleweather/ui/MainActivity.kt" line_range="65-67" />
<code_context>
+ }
+ }
+
+ /** Called from SettingsScreen when location permission is granted */
+ @SuppressLint("MissingPermission")
+ fun fetchAndSaveDeviceLocation() {
+ val fusedClient = LocationServices.getFusedLocationProviderClient(this)
+ fusedClient.lastLocation.addOnSuccessListener { location ->
</code_context>
<issue_to_address>
**🚨 issue (security):** Consider explicitly checking location permission instead of relying only on the suppress lint annotation.
Because this method is only protected by `@SuppressLint("MissingPermission")`, it assumes `ACCESS_COARSE_LOCATION` is always granted when called. If it’s reused, called from another path, or the permission is revoked, it may hit a `SecurityException` at runtime. Keep the suppress annotation if needed, but also check the permission with `ContextCompat.checkSelfPermission` (or defensively catch `SecurityException`) before calling `lastLocation` to avoid crashes when the permission state changes.
</issue_to_address>
### Comment 2
<location path="app/src/main/java/com/example/simpleweather/ui/settings/SettingsViewModel.kt" line_range="44-47" />
<code_context>
+ }
+ }
+
+ fun setUpdateInterval(minutes: Int) {
+ viewModelScope.launch(dispatcher) {
+ context.dataStore.edit { it[WeatherDataStore.UPDATE_INTERVAL_MINUTES] = minutes }
+ val prefs = context.dataStore.data.first()
+ val lat = prefs[WeatherDataStore.LOCATION_LAT]
+ val lon = prefs[WeatherDataStore.LOCATION_LON]
</code_context>
<issue_to_address>
**suggestion:** Avoid re-reading DataStore and duplicating scheduling logic in each setter.
In `setUpdateInterval` you do an `edit` and then call `dataStore.data.first()` just to immediately read lat/lon for scheduling. The same `dataStore.data.first()` + `WorkScheduler.schedule` pattern appears in `resolveAndSaveLocation` and `saveDeviceLocation`. Extract a suspend helper (e.g. `private suspend fun scheduleIfLocationPresent()`) that reads lat/lon and schedules work, and call it from these methods to avoid duplicate reads and scattered scheduling logic.
Suggested implementation:
```
fun setTempUnit(unit: String) {
viewModelScope.launch(dispatcher) {
context.dataStore.edit { it[WeatherDataStore.TEMP_UNIT] = unit }
}
}
private suspend fun scheduleIfLocationPresent() {
val prefs = context.dataStore.data.first()
val lat = prefs[WeatherDataStore.LOCATION_LAT]
val lon = prefs[WeatherDataStore.LOCATION_LON]
val intervalMinutes = prefs[WeatherDataStore.UPDATE_INTERVAL_MINUTES] ?: 60
if (lat != null && lon != null) {
WorkScheduler.schedule(context, intervalMinutes)
}
}
fun setUpdateInterval(minutes: Int) {
viewModelScope.launch(dispatcher) {
context.dataStore.edit { it[WeatherDataStore.UPDATE_INTERVAL_MINUTES] = minutes }
scheduleIfLocationPresent()
}
}
```
To fully implement your suggestion and avoid duplicated scheduling logic:
1. In `resolveAndSaveLocation`, remove the inline `dataStore.data.first()` + `WorkScheduler.schedule(...)` block and replace it with a call to `scheduleIfLocationPresent()`, invoked from within the same coroutine scope where you perform the `dataStore.edit` calls.
2. In `saveDeviceLocation`, likewise remove the duplicated `dataStore.data.first()` + `WorkScheduler.schedule(...)` logic and replace it with `scheduleIfLocationPresent()`, again ensuring it's called after the relevant `dataStore.edit` completes and still within a coroutine.
3. If either of those methods currently passes an explicit interval value to `WorkScheduler.schedule`, rely instead on the stored `UPDATE_INTERVAL_MINUTES` preference (now read inside `scheduleIfLocationPresent`), or adjust `scheduleIfLocationPresent` to accept an explicit interval parameter if your scheduling semantics require it.
</issue_to_address>
### Comment 3
<location path="app/src/main/java/com/example/simpleweather/data/WeatherRepository.kt" line_range="34-35" />
<code_context>
+ format = "json"
+ )
+ val first = response.results?.firstOrNull() ?: return null
+ val displayName = listOfNotNull(first.name, first.state).joinToString(", ")
+ return GeocodingResult(
+ lat = first.latitude.toFloat(),
</code_context>
<issue_to_address>
**suggestion:** Improve geocoding display name fallback to avoid empty or overly terse labels.
Since `displayName` only uses `name` and `state`, it can end up empty if both are missing, and it also omits `country`, which can help disambiguate locations. Consider including `country` in the composition and/or as a fallback, e.g. `listOfNotNull(first.name, first.state ?: first.country)`, so labels are never empty and are more informative.
```suggestion
val first = response.results?.firstOrNull() ?: return null
val displayName = listOfNotNull(
first.name,
first.state ?: first.country,
first.country
).joinToString(", ").ifBlank { first.country ?: query }
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Kotlin 2.0.21, Compose BOM 2024.12.01, Glance 1.1.1, WorkManager 2.10, DataStore 1.1, Retrofit 2.11 + kotlinx.serialization. Min SDK 26 / Target 35. Includes GitHub Actions CI check and release workflows, lint rules.
- WeatherDataStore: preference keys for location, temp unit, widget color, cache - Open-Meteo API models, Retrofit service, and dual-instance NetworkModule (separate weather + geocoding base URLs, no default params per Retrofit constraint) - WeatherRepository: fetchAndCacheWeather(), geocodeLocation(), reverseGeocodeLocation() (reverse geocoding uses Android Geocoder to resolve lat/lon to city, state)
- WeatherFetchWorker: CoroutineWorker that fetches and caches weather, then triggers WeatherWidget.updateAll(); returns Result.failure() if no location set - WorkScheduler: enqueueUniquePeriodicWork with UPDATE policy, requires CONNECTED network; schedule() guarded to only run when lat/lon are present in DataStore
Widget reads temp, unit, and text color via collectAsState() on the DataStore flow inside provideContent, so any setting change recomposes immediately without waiting for the next scheduled fetch.
- SettingsViewModel: StateFlow<SettingsUiState> backed by DataStore; all setting changes that affect the widget call WeatherWidget.updateAll() - SettingsScreen: location toggle (full-row tappable), manual location entry, F/C segmented button, interval dropdown, widget text color with live preview; stateless SettingsScreenContent composable enables Compose Previews - MainActivity: FusedLocationProviderClient for device location; reverse geocodes lat/lon to city/state display name; WorkManager init on launch - Shared SetButton composable for consistent trailing-icon button styling - String resources follow action_/title_/label_/hint_/error_ naming convention
All tests use RobolectricTestRunner + ApplicationProvider. DataStore tests use @before clearDataStore() to prevent cross-test state pollution. ViewModel tests inject StandardTestDispatcher and use backgroundScope + advanceUntilIdle() to activate WhileSubscribed(5000) and avoid Thread.sleep().
wassupluke
force-pushed
the
feature/initial-build
branch
from
dc7d15a to
31b3004
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Architecture
Two separate Retrofit instances for weather (
api.open-meteo.com) and geocoding (geocoding-api.open-meteo.com).Test Plan
./gradlew :app:testDebugUnitTest— all unit tests pass (Robolectric, no emulator needed)./gradlew assembleDebugsucceeds--°until first fetchSummary by Sourcery
Introduce an initial Android SimpleWeather app with a Compose-based settings UI, a Glance home screen temperature widget, and background weather fetching via WorkManager backed by Open-Meteo APIs and DataStore.
New Features:
Enhancements:
Tests: