Cleanup #320
Cleanup #320
Conversation
- Bump changelog version from 2.115 to 2.117. - Update iOS project version from 2.116 to 2.117 and increment CURRENT_PROJECT_VERSION to 168. - Upgrade Angular dependencies to version 21.1.1 and adjust devDependencies accordingly.
📝 WalkthroughWalkthroughVersion 2.117 updates Android and iOS app versions, upgrades Angular and Capacitor dependencies, increases geolocation accuracy and adds a 20s timeout with improved error handling, and updates the iOS CI runner/Xcode versions. Changes
Sequence Diagram(s)sequenceDiagram
participant BroadcastPage as Broadcast Page
participant GeoService as Geo Service
participant GeolocationAPI as Device Geolocation API
participant Processor as Position Processor
BroadcastPage->>GeoService: startWatch({enableHighAccuracy:true, timeout:20000})
GeoService->>GeolocationAPI: watchPosition(callback)
BroadcastPage->>GeoService: getCurrentPosition({enableHighAccuracy:true, timeout:20000})
GeolocationAPI-->>GeoService: position / error
alt position received
GeoService->>BroadcastPage: emit(position)
BroadcastPage->>Processor: processPosition(position)
else error or timeout
GeoService->>BroadcastPage: emit(error state)
BroadcastPage->>Processor: skip processing / log warning
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/geolocation/geo.service.ts`:
- Around line 129-143: The getPosition function currently returns
this.gpsPosition() immediately before Geolocation.getCurrentPosition resolves,
causing callers to receive stale data; change getPosition to return a Promise
that awaits Geolocation.getCurrentPosition (or its failure) and only resolve
with the fresh coordinate after updating this.gpsPosition. Concretely: make
getPosition async (or explicitly return a new Promise), set this.gpsBusy
appropriately, await Geolocation.getCurrentPosition({enableHighAccuracy:true,
timeout:20000}), on success apply db.offsetGPS, timeStampGPS, set
this.lastGpsUpdate/this.gpsPosition/this.hasPermission and then resolve with the
new value (or the signal value), and on catch set this.gpsBusy=false and reject
or resolve with last-known/NoGPSCoord; keep references to this.gpsBusy,
timeStampGPS, this.db.offsetGPS, this.gpsPosition, and this.hasPermission so
callers using await this.geo.getPosition() receive up-to-date coordinates.
🧹 Nitpick comments (3)
package.json (2)
30-39: Consider consistent version pinning for Angular packages.The version specifiers are inconsistent:
@angular/animations: exact"21.1.1"@angular/cdk: major only"21"@angular/commonand others: caret"^21.1.1"This inconsistency can cause version drift between environments. Consider aligning all Angular packages to the same version specifier (e.g.,
"^21.1.1"or exact"21.1.1").
80-88: Angular devDependencies also have inconsistent pinning.Similar to dependencies,
@angular-eslint/*packages use major-only"21"while@angular/cliuses exact"21.1.1"and@angular/compiler-cliuses"^21.1.1". Consider aligning these for consistency.src/app/geolocation/geo.service.ts (1)
138-142: Misleading comment in catch block.The comment says "Return last known position or NoGPSCoord if none exists" but the
returnstatement is outside the catch block at line 143. The catch block itself doesn't return anything—it just clearsgpsBusyand logs. Consider removing or rewording this comment for clarity.Suggested fix
.catch((err) => { this.gpsBusy.set(false); console.warn('Failed to obtain GPS position:', err?.message); - // Return last known position or NoGPSCoord if none exists + // Preserve existing gpsPosition signal value as fallback });
| Geolocation.getCurrentPosition({ enableHighAccuracy: true, timeout: 20000 }) | ||
| .then((position) => { | ||
| this.gpsBusy.set(false); | ||
| let gps = { lat: position.coords.latitude, lng: position.coords.longitude }; | ||
| gps = this.db.offsetGPS(gps); | ||
| this.lastGpsUpdate = new Date(); | ||
| this.gpsPosition.set(timeStampGPS(gps)); | ||
| this.hasPermission = true; | ||
| }) | ||
| .catch((err) => { | ||
| this.gpsBusy.set(false); | ||
| console.warn('Failed to obtain GPS position:', err?.message); | ||
| // Return last known position or NoGPSCoord if none exists | ||
| }); | ||
| return this.gpsPosition(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts -C3 'getPosition\s*\(' -g '!**/geo.service.ts'Repository: damiant/dust
Length of output: 4293
🏁 Script executed:
cat -n src/app/geolocation/geo.service.ts | head -160 | tail -50Repository: damiant/dust
Length of output: 1987
Critical bug: getPosition() returns stale position data despite callers using await.
The function returns this.gpsPosition() immediately (line 143) before the GPS fetch Promise resolves. Since gpsPosition() is a signal, callers awaiting this return get the previous cached position, not fresh GPS data.
All 8+ callers expect fresh coordinates:
events.page.ts:355,map.component.ts:347,favs.page.ts:322,search.page.ts:85,rsl.page.ts:159,pin-map.page.ts:226/334,camps.page.ts:169,art.page.ts:201
They all use await this.geo.getPosition() expecting current GPS, but receive stale data instead. The GPS fetch runs asynchronously in the background (lines 129–142), but callers don't actually await it.
Fix required: Either return a Promise that resolves when GPS is actually obtained, or refactor callers to reactively observe the gpsPosition() signal changes rather than awaiting a stale return value.
🤖 Prompt for AI Agents
In `@src/app/geolocation/geo.service.ts` around lines 129 - 143, The getPosition
function currently returns this.gpsPosition() immediately before
Geolocation.getCurrentPosition resolves, causing callers to receive stale data;
change getPosition to return a Promise that awaits
Geolocation.getCurrentPosition (or its failure) and only resolve with the fresh
coordinate after updating this.gpsPosition. Concretely: make getPosition async
(or explicitly return a new Promise), set this.gpsBusy appropriately, await
Geolocation.getCurrentPosition({enableHighAccuracy:true, timeout:20000}), on
success apply db.offsetGPS, timeStampGPS, set
this.lastGpsUpdate/this.gpsPosition/this.hasPermission and then resolve with the
new value (or the signal value), and on catch set this.gpsBusy=false and reject
or resolve with last-known/NoGPSCoord; keep references to this.gpsBusy,
timeStampGPS, this.db.offsetGPS, this.gpsPosition, and this.hasPermission so
callers using await this.geo.getPosition() receive up-to-date coordinates.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/app-ios.yml:
- Line 49: The workflow currently installs Xcode 26.2 via the setup-xcode action
but also contains a "Select Xcode Version" step that runs xcode-select pointing
to /Applications/Xcode_16.4.app (which doesn't exist on macos-26); remove that
manual xcode-select step or update it to point to an available Xcode path such
as /Applications/Xcode_26.2.app so it matches the setup-xcode xcode-version:
26.2 configuration and avoid the xcode-select failure.
🧹 Nitpick comments (2)
package.json (2)
81-85: Consider more specific version pins for@angular-eslintpackages.Using major-only version
"21"for all@angular-eslint/*packages is quite loose and could introduce unexpected breaking changes duringnpm install. Since these are dev dependencies, the impact is limited to the development environment, but tighter pins would improve reproducibility.♻️ Suggested fix
- "@angular-eslint/builder": "21", - "@angular-eslint/eslint-plugin": "21", - "@angular-eslint/eslint-plugin-template": "21", - "@angular-eslint/schematics": "21", - "@angular-eslint/template-parser": "21", + "@angular-eslint/builder": "^21.0.0", + "@angular-eslint/eslint-plugin": "^21.0.0", + "@angular-eslint/eslint-plugin-template": "^21.0.0", + "@angular-eslint/schematics": "^21.0.0", + "@angular-eslint/template-parser": "^21.0.0",
30-39: Inconsistent version pinning across Angular packages.The Angular dependencies use three different pinning strategies:
@angular/animations: exact (21.1.1)@angular/cdk: major-only (21) — this is very loose and could pull any 21.x.x version- Others: caret (
^21.1.1)Angular packages should stay in sync to avoid potential compatibility issues. Consider aligning all Angular packages to the same pinning strategy, such as caret notation for all.
♻️ Suggested fix to align version pins
- "@angular/animations": "21.1.1", - "@angular/cdk": "21", - "@angular/common": "^21.1.1", - "@angular/compiler": "^21.1.1", - "@angular/core": "^21.1.1", - "@angular/forms": "^21.1.1", - "@angular/platform-browser": "^21.1.1", - "@angular/platform-browser-dynamic": "^21.1.1", - "@angular/router": "^21.1.1", - "@angular/service-worker": "^21.1.1", + "@angular/animations": "^21.1.1", + "@angular/cdk": "^21.1.1", + "@angular/common": "^21.1.1", + "@angular/compiler": "^21.1.1", + "@angular/core": "^21.1.1", + "@angular/forms": "^21.1.1", + "@angular/platform-browser": "^21.1.1", + "@angular/platform-browser-dynamic": "^21.1.1", + "@angular/router": "^21.1.1", + "@angular/service-worker": "^21.1.1",
Summary by CodeRabbit
Bug Fixes
Chores
Infrastructure
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.