Update AHN endpoint and improve caching stability#395
Conversation
…path, implementing robust localStorage error handling, and adding a 'fallback-to-stale' strategy for offline resilience
|
|
👋 Hotfix Branch PR Detected! Before merging this Pull Request into This will:
You can trigger the workflow from the 'Actions' tab, selecting the 'Release' workflow, and choosing this |
|
Caution Review failedThe pull request is closed. WalkthroughThis PR enhances the elevation map's cache handling and network resilience by implementing a "fallback-to-stale" pattern. It adds robust localStorage validation with 24-hour TTL, corrects the AHN index endpoint path, and introduces comprehensive error handling for offline scenarios. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Cache as localStorage Cache
participant Server as AHN API
Client->>Cache: Check for cached index
alt Cache Accessible & Valid
Cache-->>Client: Return fresh cached data
else Cache Missing or Expired
Client->>Server: Fetch fresh AHN index from /atlas/ahn-index
alt Fetch Successful
Server-->>Client: Return index with features
rect rgba(76, 175, 80, 0.1)
note right of Client: Validate features & store<br/>with timestamp in cache
end
Client->>Cache: Store fresh data + timestamp
Cache-->>Client: ✓ Cached
else Fetch Failed (Network Error)
rect rgba(244, 67, 54, 0.1)
note right of Client: Network unavailable
end
Client->>Cache: Attempt fallback to stale data
alt Stale Cache Available
Cache-->>Client: Return expired cached data
note right of Client: Use stale data for rendering
else No Cache Available
Client-->>Client: Signal error — no data available
end
end
else Cache Access Error (e.g., Private Mode)
Client->>Server: Proceed to fetch (skip cache)
Server-->>Client: Return index
Client->>Cache: Attempt to store (logged if fails)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #395 +/- ##
=======================================
Coverage 87.47% 87.47%
=======================================
Files 91 91
Lines 4559 4559
Branches 1391 1391
=======================================
Hits 3988 3988
Misses 571 571
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
coderabbitai
Bot
changed the title
coderabbitai
Bot
added
branch:main
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (1)
277-296: Solid fallback-to-stale implementation for offline resilience.The strategy correctly prioritizes: fresh cache → network fetch → expired cache → error state. This aligns well with the PR objectives.
One consideration: setting
networkStatusto"idle"when serving stale data (line 289) means users won't know they're viewing potentially outdated information. The console warning is helpful for debugging, but you might consider adding a"stale"status or displaying a subtle UI indicator. This is optional if the UX decision is to silently degrade.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-mobile-elevation-fallback.mdfdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx.changeset/fix-mobile-elevation-fallback.md
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Applied to files:
.changeset/fix-mobile-elevation-fallback.md
🔇 Additional comments (4)
.changeset/fix-mobile-elevation-fallback.md (1)
1-5: LGTM!The changeset accurately describes the fix with appropriate patch-level versioning. The description aligns well with the PR objectives and the actual code changes.
fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx (3)
224-250: Robust cache lookup with proper error handling.The implementation correctly:
- Guards localStorage access with
typeofcheck for SSR compatibility- Wraps parsing in try-catch to handle malformed cache entries
- Validates both freshness (24h TTL) and data integrity before using cache
- Clears corrupted cache entries on failure
The empty catch on line 249 is acceptable here—localStorage cleanup is best-effort and shouldn't block the fetch fallback.
220-300: Well-structured caching strategy addressing mobile stability concerns.The
fetchIndeximplementation correctly addresses the issue requirements:
- ✅ localStorage access wrapped in try-catch to prevent crashes
- ✅ 24-hour cache TTL aligned with server
- ✅ Fresh cache → network → stale fallback flow
- ✅ Guards for undefined localStorage (SSR/mobile edge cases)
The double localStorage read (lines 228 and 281) is intentional and appropriate since the first block may clear corrupted entries before the fallback block runs.
252-274: Good fetch logic with proper validation and caching.The implementation correctly validates the response structure before using/caching it, preventing invalid data from propagating. Storage failures are gracefully handled. The
/atlas/ahn-indexendpoint is properly configured infdm-app/app/routes/atlas.ahn-index.tsxwith appropriate caching headers (86400 seconds) and server-side in-memory caching with a 24-hour TTL.
1 participant
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.
Closes #394