feat(world): map canvas home, first-class activity URLs, and navigation re-root#6980
Closed
wcjord wants to merge 4 commits into
Closed
feat(world): map canvas home, first-class activity URLs, and navigation re-root#6980wcjord wants to merge 4 commits into
wcjord wants to merge 4 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements the world_v2 navigation redesign: creates a route paths foundation (PRoutes), adds AppSection-based URI routing, introduces legacy /rooms redirect shims, builds a world map canvas with location and activity pins, adds first-class activity URLs, centralizes activity session creation, restructures the route tree with new /analytics, /courses, /settings, /profile roots, and wires new routing throughout the navigation UI and auth flows. ChangesPangea World v2 Navigation, Map Canvas, and First-Class Activities
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
lib/pangea/spaces/space_navigation_column.dart (1)
196-207: ⚡ Quick winConsider tightening the 'addcourse' segment check for clarity.
The
segments.contains('addcourse')check at line 201 matches the string in any position within the path segments array. While this provides flexibility, a more explicit positional check would improve clarity and prevent unintended matches.♻️ Proposed refinement
If the intended route structure is
/courses/addcourse/..., consider checking the specific position:case AppSection.courses: // Find/browse flows show the course list in the left column; a // joined course (`/courses/:spaceid`) shows the chat list scoped // to that space until the course-outline relayout lands. if (AppSection.activeSpaceId(uri) == null && - !segments.contains('addcourse')) { + (segments.length < 2 || segments[1] != 'addcourse')) { return const FindCoursePage(); }Alternatively, if multiple creation-flow paths exist (e.g.,
own,preview,addcourse), consider extracting the logic into a helper method for clarity:bool _isCourseBrowsing(List<String> segments, Uri uri) { if (AppSection.activeSpaceId(uri) != null) return false; if (segments.length < 2) return true; // Literal creation/preview flows return false return !const ['addcourse', 'own', 'preview'].contains(segments[1]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/pangea/spaces/space_navigation_column.dart` around lines 196 - 207, The segments.contains('addcourse') check in the AppSection.courses branch is too broad; update the condition to a positional check (e.g., verify segments.length > 1 and compare segments[1] to 'addcourse') or extract a helper (e.g., _isCourseBrowsing(List<String> segments, Uri uri)) and use AppSection.activeSpaceId(uri) plus explicit positional checks for known creation/preview tokens ('addcourse', 'own', 'preview') to avoid accidental matches; modify the branch that returns FindCoursePage() to call the new positional check or helper and return ChatList otherwise so behavior is unchanged but more explicit.lib/widgets/layouts/empty_page.dart (1)
1-46: 💤 Low valueConsider removing commented-out code.
The file contains extensive commented-out code (L2-3, L9-10, L14-15, L20-22, L32-43) from the previous logo-based implementation. If this code is no longer needed and won't be referenced for rollback or comparison, consider removing it to improve maintainability. If it serves as documentation or a fallback, the comments can remain.
🧹 Proposed cleanup to remove dead code
-// `#Pangea` -// import 'dart:math'; -// Pangea# - import 'package:flutter/material.dart'; -// `#Pangea` +// `#Pangea` import 'package:fluffychat/pangea/world/widgets/world_map.dart'; -// import 'package:fluffychat/pangea/common/widgets/pangea_logo_svg.dart'; -// Pangea# +// Pangea# class EmptyPage extends StatelessWidget { - // `#Pangea` - // static const double _width = 400; - // Pangea# const EmptyPage({super.key}); `@override` Widget build(BuildContext context) { - // `#Pangea` - // final width = min(MediaQuery.sizeOf(context).width, EmptyPage._width) / 2; - // final theme = Theme.of(context); - // Pangea# return Scaffold( // Add invisible appbar to make status bar on Android tablets bright. appBar: AppBar( automaticallyImplyLeading: false, elevation: 0, backgroundColor: Colors.transparent, ), extendBodyBehindAppBar: true, // `#Pangea` body: const WorldMap(), - // body: Container( - // alignment: Alignment.center, - // child: Image.asset( - // 'assets/logo_transparent.png', - // color: theme.colorScheme.surfaceContainerHigh, - // width: width, - // height: width, - // filterQuality: FilterQuality.medium, - // ), - // ), // Pangea# ); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/widgets/layouts/empty_page.dart` around lines 1 - 46, This file contains large blocks of dead commented code (old imports, constants, and the previous logo-based body) around the EmptyPage widget; remove the commented-out lines (the commented 'dart:math' import, the commented pangea imports for Pangea logo, the commented static _width and width/theme calculations, and the commented Container body) leaving a clean EmptyPage implementation that uses the const WorldMap() body and the existing AppBar; ensure you only delete commented lines and keep symbols EmptyPage and WorldMap intact so behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/pangea/activity_sessions/launch_activity_session.dart`:
- Around line 50-53: The map literal for the local variable spaces uses invalid
nullable-key syntax; change it to only insert the primarySpace entry when
primarySpace is non-null (i.e., add a conditional entry like "if (primarySpace
!= null) primarySpace.id: primarySpace") and keep the for-loop entry for
matching spaces; locate the variable named spaces and the primarySpace
identifier in launch_activity_session.dart and update the map literal
accordingly.
In `@lib/pangea/world/activity_course_resolver.dart`:
- Around line 59-64: The code currently force-unwraps
planIdByTopicId[topic.uuid] when building matchingPlanIds which can crash if
topic.uuid is absent; change the lookup to a nullable lookup and only add to
matchingPlanIds when the value is non-null (e.g., var planId =
planIdByTopicId[topic.uuid]; if (planId != null) matchingPlanIds.add(planId)).
Ensure this change is applied in the loop over topics.topics.values where
matchingPlanIds is constructed so missing topic.uuid entries are safely ignored.
In `@lib/pangea/world/pages/activity_map_page.dart`:
- Around line 307-309: Replace the hardcoded error string in the _ErrorPopup
widget with a localized string: add a new L10n key (e.g., activityNotFound) to
your ARB/localization files, run codegen, then update the Text call inside
_ErrorPopup to use L10n.of(context).activityNotFound (or the generated getter)
instead of the literal "Activity not found" so the popup uses the i18n resource.
- Around line 271-279: The SnackBar content is hardcoded; add a new localization
key (e.g., joinCourseToPlayActivity) to your L10n files and use it instead of
the literal string—call L10n.of(context).joinCourseToPlayActivity and pass
_activity!.title (or use interpolation pattern in the ARB/strings) when building
the SnackBar in the onPressed branch that checks _matchingSpaces and toggles
_expanded; update all localization resource files (and generated L10n classes)
so the key exists for all locales.
In `@lib/pangea/world/widgets/world_map.dart`:
- Around line 134-151: Wrap the tappable activity marker's GestureDetector in a
Semantics widget providing an explicit accessible name and button role: replace
the direct GestureDetector child with Semantics(label: /* use activity
title/name, e.g. activity.name or activity.title */ , button: true, child:
GestureDetector(...)); keep the existing onTap handler (_openActivity(activity))
and visual decoration unchanged, and ensure the label uses the activity's
human-readable property (fallback to a generic 'Activity' if null) so screen
readers and Playwright can identify the control.
- Around line 46-62: Update the two catch blocks in _loadLocations and
_loadActivities to capture error and stack (catch (e, s)) and call
ErrorHandler.logError(e, s) so failures from
WorldLocationsRepo.mappableLocations() and WorldActivitiesRepo.activityPins()
are reported to Sentry; keep the existing behavior of not rethrowing (map stays
usable) and only add the ErrorHandler.logError call (optionally gated by
production if desired).
In `@lib/pangea/world/world_activities_repo.dart`:
- Around line 47-141: The activityPins() method can throw from calls like
CoursePlansRepo.search, CourseTopicRepo.get, CourseActivityRepo.get, and
WorldLocationsRepo.mappableLocations and currently has no logging; wrap the main
work (everything after the cache validity check) in a try-catch, import and call
Sentry.captureException(error, stackTrace) (and optionally a local logger) in
the catch block to record the error, then rethrow the exception (or return
_setCache([]) if you prefer to swallow but still log); ensure you reference
activityPins, CoursePlansRepo.search, CourseTopicRepo.get,
CourseActivityRepo.get, WorldLocationsRepo.mappableLocations, and use
Sentry.captureException to maintain observability.
In `@lib/pangea/world/world_locations_repo.dart`:
- Around line 14-44: Wrap the body of mappableLocations() in a try-catch that
catches all exceptions thrown by PayloadClient.find (and any other calls
inside), log the error to Sentry using Sentry.captureException with the caught
exception and stack trace, and optionally emit a local log (e.g.,
processLogger.error) with context including the function name and page info;
after logging rethrow the exception (or return an empty list if you prefer
failing-safe) so callers keep current behavior. Ensure you import Sentry and
reference mappableLocations, PayloadClient.find, _cache, _lastFetched, and
_hasUsableCoordinates when updating the method.
---
Nitpick comments:
In `@lib/pangea/spaces/space_navigation_column.dart`:
- Around line 196-207: The segments.contains('addcourse') check in the
AppSection.courses branch is too broad; update the condition to a positional
check (e.g., verify segments.length > 1 and compare segments[1] to 'addcourse')
or extract a helper (e.g., _isCourseBrowsing(List<String> segments, Uri uri))
and use AppSection.activeSpaceId(uri) plus explicit positional checks for known
creation/preview tokens ('addcourse', 'own', 'preview') to avoid accidental
matches; modify the branch that returns FindCoursePage() to call the new
positional check or helper and return ChatList otherwise so behavior is
unchanged but more explicit.
In `@lib/widgets/layouts/empty_page.dart`:
- Around line 1-46: This file contains large blocks of dead commented code (old
imports, constants, and the previous logo-based body) around the EmptyPage
widget; remove the commented-out lines (the commented 'dart:math' import, the
commented pangea imports for Pangea logo, the commented static _width and
width/theme calculations, and the commented Container body) leaving a clean
EmptyPage implementation that uses the const WorldMap() body and the existing
AppBar; ensure you only delete commented lines and keep symbols EmptyPage and
WorldMap intact so behavior is unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 636ad527-c119-4a2a-8c12-7d3ae1d43abb
📒 Files selected for processing (32)
lib/config/routes.dartlib/pages/chat_list/chat_list.dartlib/pages/settings/settings_view.dartlib/pangea/activity_sessions/activity_session_start/activity_sessions_start_view.dartlib/pangea/activity_sessions/activity_session_start/select_role_session_controller.dartlib/pangea/activity_sessions/launch_activity_session.dartlib/pangea/chat_settings/widgets/chat_context_menu_action.dartlib/pangea/common/utils/p_vguard.dartlib/pangea/course_chats/course_chats_page.dartlib/pangea/course_creation/selected_course_page.dartlib/pangea/course_plans/courses/course_plan_room_extension.dartlib/pangea/extensions/room_children_and_parents_extension.dartlib/pangea/join_codes/join_rule_extension.dartlib/pangea/navigation/app_section.dartlib/pangea/navigation/legacy_redirects.dartlib/pangea/navigation/navigation_util.dartlib/pangea/navigation/route_paths.dartlib/pangea/notifications/notification_tap_utils.dartlib/pangea/spaces/space_navigation_column.dartlib/pangea/world/activity_course_resolver.dartlib/pangea/world/pages/activity_map_page.dartlib/pangea/world/widgets/world_map.dartlib/pangea/world/world_activities_repo.dartlib/pangea/world/world_camera_state.dartlib/pangea/world/world_locations_repo.dartlib/widgets/adaptive_dialogs/user_dialog.dartlib/widgets/fluffy_chat_app.dartlib/widgets/layouts/empty_page.dartlib/widgets/matrix.dartlib/widgets/navigation_rail.darttest/pangea/navigation/app_section_test.darttest/pangea/navigation/legacy_redirects_test.dart
Comment on lines
+50
to
+53
| final spaces = <String, Room>{ | ||
| ?primarySpace?.id: ?primarySpace, | ||
| for (final space in matching) space.id: space, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the file compiles by running flutter analyze on this specific file
flutter analyze lib/pangea/activity_sessions/launch_activity_session.dartRepository: pangeachat/client
Length of output: 109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Tool availability =="
command -v flutter >/dev/null 2>&1 && echo "flutter: yes" || echo "flutter: no"
command -v dart >/dev/null 2>&1 && echo "dart: yes" || echo "dart: no"
echo "== File snippet =="
nl -ba lib/pangea/activity_sessions/launch_activity_session.dart | sed -n '35,80p'
echo "== Search for suspicious nullable-key map syntax =="
rg -n --hidden --no-ignore-vcs "\\?\\s*\\w+\\?\\." lib/pangea/activity_sessions/launch_activity_session.dart || trueRepository: pangeachat/client
Length of output: 186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Tool availability =="
command -v python3 >/dev/null 2>&1 && echo "python3: yes" || echo "python3: no"
command -v dart >/dev/null 2>&1 && echo "dart: yes" || echo "dart: no"
command -v flutter >/dev/null 2>&1 && echo "flutter: yes" || echo "flutter: no"
echo "== Print lines 40-70 of target file (via python) =="
python3 - <<'PY'
from pathlib import Path
p = Path("lib/pangea/activity_sessions/launch_activity_session.dart")
txt = p.read_text(encoding="utf-8").splitlines()
start, end = 40, 70
for i in range(start-1, min(end, len(txt))):
print(f"{i+1:4d}: {txt[i]}")
PY
echo "== Search for suspicious pattern in that file =="
rg -n "\\?\\s*primarySpace\\?\\.id\\s*:" lib/pangea/activity_sessions/launch_activity_session.dart || true
rg -n "<String,\\s*Room>\\{[\\s\\S]*" lib/pangea/activity_sessions/launch_activity_session.dart --context 2 || trueRepository: pangeachat/client
Length of output: 1585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find occurrences of primarySpace in the file =="
rg -n "primarySpace" lib/pangea/activity_sessions/launch_activity_session.dart
echo "== Show nearby context around each occurrence (first few) =="
python3 - <<'PY'
from pathlib import Path
p = Path("lib/pangea/activity_sessions/launch_activity_session.dart")
lines = p.read_text(encoding="utf-8").splitlines()
idxs = [i for i,l in enumerate(lines) if "primarySpace" in l]
for i in idxs[:10]:
a=max(0,i-5); b=min(len(lines), i+8)
print("\n--- around line", i+1, "---")
for j in range(a,b):
print(f"{j+1:4d}: {lines[j]}")
PY
echo "== Locate Room.id declaration =="
rg -n "class Room\\b" -S lib | head -n 20 || true
# Try to find `id` field inside Room
rg -n "class Room\\b|\\bid\\b" lib | rg -n "Room" | head -n 50 || trueRepository: pangeachat/client
Length of output: 7918
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for Room class definition in repo =="
rg -n --hidden --no-ignore-vcs "class Room\\b|abstract class Room\\b|mixin Room\\b|interface Room\\b" lib -S || true
echo "== Search for a getter/field named id on Room =="
rg -n --hidden --no-ignore-vcs "(class Room\\b[\\s\\S]{0,2000}?)\\b(id)\\b" lib -S || true
echo "== Find where launch_activity_session.dart imports Room =="
python3 - <<'PY'
from pathlib import Path
p=Path("lib/pangea/activity_sessions/launch_activity_session.dart")
txt=p.read_text(encoding="utf-8").splitlines()
for i,l in enumerate(txt[:80], start=1):
if "import" in l:
print(f"{i:4d}: {l}")
PY
echo "== Search for 'Room?' usage in same file =="
rg -n "\\bRoom\\?" lib/pangea/activity_sessions/launch_activity_session.dartRepository: pangeachat/client
Length of output: 1364
Fix invalid nullable-key syntax in spaces map literal
?primarySpace?.id: ?primarySpace is not valid Dart syntax for map entries. Replace it with a conditional entry when primarySpace is non-null.
🐛 Proposed fix
- final spaces = <String, Room>{
- ?primarySpace?.id: ?primarySpace,
- for (final space in matching) space.id: space,
- };
+ final spaces = <String, Room>{
+ if (primarySpace != null) primarySpace.id: primarySpace,
+ for (final space in matching) space.id: space,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final spaces = <String, Room>{ | |
| ?primarySpace?.id: ?primarySpace, | |
| for (final space in matching) space.id: space, | |
| }; | |
| final spaces = <String, Room>{ | |
| if (primarySpace != null) primarySpace.id: primarySpace, | |
| for (final space in matching) space.id: space, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/activity_sessions/launch_activity_session.dart` around lines 50 -
53, The map literal for the local variable spaces uses invalid nullable-key
syntax; change it to only insert the primarySpace entry when primarySpace is
non-null (i.e., add a conditional entry like "if (primarySpace != null)
primarySpace.id: primarySpace") and keep the for-loop entry for matching spaces;
locate the variable named spaces and the primarySpace identifier in
launch_activity_session.dart and update the map literal accordingly.
Comment on lines
+59
to
+64
| final matchingPlanIds = <String>{}; | ||
| for (final topic in topics.topics.values) { | ||
| if (topic.activityIds.contains(activityId)) { | ||
| matchingPlanIds.add(planIdByTopicId[topic.uuid]!); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify CourseTopicRepo.get contract: does it return only requested topics?
# Search for CourseTopicRepo.get implementation
ast-grep --pattern $'class CourseTopicRepo {
$$$
static $_ get($$$) {
$$$
}
$$$
}'
# Also check the response model for any filtering or extra topics
rg -n -A10 'class.*TopicResponse|class.*TranslateTopicResponse' \
lib/pangea/course_plans/course_topics/Repository: pangeachat/client
Length of output: 1390
Guard against missing topic.uuid in planIdByTopicId lookup (planIdByTopicId[topic.uuid]!)
TranslateTopicResponse.fromJson populates topics directly from json['topics'] without filtering to the requested topicIds, so topics.topics.values may contain a topic.uuid that isn’t present in planIdByTopicId. That makes the non-null assertion (!) potentially crash at runtime; use a nullable lookup + null-check before adding to matchingPlanIds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/activity_course_resolver.dart` around lines 59 - 64, The
code currently force-unwraps planIdByTopicId[topic.uuid] when building
matchingPlanIds which can crash if topic.uuid is absent; change the lookup to a
nullable lookup and only add to matchingPlanIds when the value is non-null
(e.g., var planId = planIdByTopicId[topic.uuid]; if (planId != null)
matchingPlanIds.add(planId)). Ensure this change is applied in the loop over
topics.topics.values where matchingPlanIds is constructed so missing topic.uuid
entries are safely ignored.
Comment on lines
+271
to
+279
| onPressed: _matchingSpaces.isNotEmpty | ||
| ? () => setState(() => _expanded = true) | ||
| : () => ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar( | ||
| content: Text( | ||
| 'Join the course for "${_activity!.title}" to play it!', | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Use L10n for user-facing text.
The SnackBar message at L276 is hardcoded in English. Since the file already imports L10n and uses it elsewhere (L166, L270), this string should also be localized for consistency and i18n compliance.
🌐 Proposed fix to localize the message
Add a new L10n key (e.g., joinCourseToPlayActivity) to the localization files, then:
: () => ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
- 'Join the course for "${_activity!.title}" to play it!',
+ L10n.of(context).joinCourseToPlayActivity(_activity!.title),
),
),
),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/pages/activity_map_page.dart` around lines 271 - 279, The
SnackBar content is hardcoded; add a new localization key (e.g.,
joinCourseToPlayActivity) to your L10n files and use it instead of the literal
string—call L10n.of(context).joinCourseToPlayActivity and pass _activity!.title
(or use interpolation pattern in the ARB/strings) when building the SnackBar in
the onPressed branch that checks _matchingSpaces and toggles _expanded; update
all localization resource files (and generated L10n classes) so the key exists
for all locales.
Comment on lines
+307
to
+309
| const Icon(Icons.error_outline, size: 36), | ||
| const SizedBox(height: 8), | ||
| Text('Activity not found', style: theme.textTheme.titleMedium), |
There was a problem hiding this comment.
Use L10n for user-facing text.
The error message "Activity not found" at L309 is hardcoded in English. Since _ErrorPopup is part of the user-facing UI, this text should be localized using L10n.of(context) for consistency and i18n compliance.
🌐 Proposed fix to localize the error message
Add a new L10n key (e.g., activityNotFound) to the localization files, then:
const Icon(Icons.error_outline, size: 36),
const SizedBox(height: 8),
- Text('Activity not found', style: theme.textTheme.titleMedium),
+ Text(L10n.of(context).activityNotFound, style: theme.textTheme.titleMedium),
const SizedBox(height: 12),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/pages/activity_map_page.dart` around lines 307 - 309,
Replace the hardcoded error string in the _ErrorPopup widget with a localized
string: add a new L10n key (e.g., activityNotFound) to your ARB/localization
files, run codegen, then update the Text call inside _ErrorPopup to use
L10n.of(context).activityNotFound (or the generated getter) instead of the
literal "Activity not found" so the popup uses the i18n resource.
Comment on lines
+46
to
+62
| Future<void> _loadLocations() async { | ||
| try { | ||
| final locations = await WorldLocationsRepo.mappableLocations(); | ||
| if (mounted) setState(() => _locations = locations); | ||
| } catch (_) { | ||
| // Map stays usable without pins; Sentry catches repo errors. | ||
| } | ||
| } | ||
|
|
||
| Future<void> _loadActivities() async { | ||
| try { | ||
| final activities = await WorldActivitiesRepo.activityPins(); | ||
| if (mounted) setState(() => _activities = activities); | ||
| } catch (_) { | ||
| // Map stays usable without activity pins. | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if WorldLocationsRepo and WorldActivitiesRepo log errors to Sentry.
# Search for ErrorHandler.logError or Sentry logging in the repositories
rg -n -A3 'ErrorHandler\.logError|Sentry\.' \
lib/pangea/world/world_locations_repo.dart \
lib/pangea/world/world_activities_repo.dartRepository: pangeachat/client
Length of output: 43
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate the repo implementations
echo "== WorldLocationsRepo implementation(s) =="
fd -e dart -g '*world*locations*repo*.dart' lib/pangea/world || true
rg -n --hidden -S "class\s+WorldLocationsRepo|WorldLocationsRepo" lib/pangea/world -g'*.dart'
echo
echo "== WorldActivitiesRepo implementation(s) =="
fd -e dart -g '*world*activities*repo*.dart' lib/pangea/world || true
rg -n --hidden -S "class\s+WorldActivitiesRepo|WorldActivitiesRepo" lib/pangea/world -g'*.dart'
echo
# 2) Show the current repository files (or key error handling parts)
echo "== Candidate file contents (error handling/logging related) =="
rg -n -S "try\s*\{|catch\s*\(|ErrorHandler|Sentry|captureException|logError|log.*error" \
lib/pangea/world/world_locations_repo.dart \
lib/pangea/world/world_activities_repo.dart || true
echo
# 3) Broader search across lib/pangea/world for sentry/error logging helpers
echo "== Broader search in lib/pangea/world for Sentry/ErrorHandler =="
rg -n -S "Sentry\.|ErrorHandler\.|captureException|logError|captureMessage" lib/pangea/world -g'*.dart' || trueRepository: pangeachat/client
Length of output: 1040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the relevant method bodies
echo "== mappableLocations() implementation =="
rg -n "mappableLocations\s*\(" lib/pangea/world/world_locations_repo.dart
echo
# Show the method (approx range around match)
line=$(rg -n "mappableLocations\s*\(" lib/pangea/world/world_locations_repo.dart | head -n1 | cut -d: -f1)
start=$((line-30)); end=$((line+120))
sed -n "${start},${end}p" lib/pangea/world/world_locations_repo.dart | cat -n
echo
echo "== activityPins() implementation =="
rg -n "activityPins\s*\(" lib/pangea/world/world_activities_repo.dart
echo
line=$(rg -n "activityPins\s*\(" lib/pangea/world/world_activities_repo.dart | head -n1 | cut -d: -f1)
start=$((line-30)); end=$((line+160))
sed -n "${start},${end}p" lib/pangea/world/world_activities_repo.dart | cat -n
echo
echo "== Any ErrorHandler.logError / Sentry usage in these repo files =="
rg -n -S "ErrorHandler\.logError|Sentry\.|captureException|captureMessage|logError" \
lib/pangea/world/world_locations_repo.dart lib/pangea/world/world_activities_repo.dart || true
echo
echo "== Where ErrorHandler.logError is called (to see if UI catches/logs or repos do) =="
rg -n "ErrorHandler\.logError" lib/pangea/world -g'*.dart' || trueRepository: pangeachat/client
Length of output: 2025
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file1="lib/pangea/world/world_locations_repo.dart"
file2="lib/pangea/world/world_activities_repo.dart"
echo "== Searching logging usage in repo files =="
rg -n -S "ErrorHandler\.logError|Sentry\.|captureException|captureMessage|logError" "$file1" "$file2" || true
echo
echo "== mappableLocations() context =="
line1=$(rg -n "mappableLocations\s*\(" "$file1" | head -n1 | cut -d: -f1)
start1=$((line1-30)); [ "$start1" -lt 1 ] && start1=1
end1=$((line1+140))
nl -ba "$file1" | sed -n "${start1},${end1}p"
echo
echo "== activityPins() context =="
line2=$(rg -n "activityPins\s*\(" "$file2" | head -n1 | cut -d: -f1)
start2=$((line2-30)); [ "$start2" -lt 1 ] && start2=1
end2=$((line2+220))
nl -ba "$file2" | sed -n "${start2},${end2}p"Repository: pangeachat/client
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f1="lib/pangea/world/world_locations_repo.dart"
f2="lib/pangea/world/world_activities_repo.dart"
echo "== Search for ErrorHandler/Sentry/logging inside the repo files =="
rg -n -S "ErrorHandler|Sentry\.|captureException|captureMessage|logError" "$f1" "$f2" || true
echo
echo "== mappableLocations() body (with nearby lines) =="
start=$(rg -n "static Future<.*>\s*mappableLocations\s*\(" "$f1" | head -n1 | cut -d: -f1)
# show a window (safe bounds)
win_pre=25; win_post=180
awk -v s="$((start-win_pre))" -v e="$((start+win_post))" '
NR>=1 && NR>=s && NR<=e { printf "%5d:%s\n", NR, $0 }
' "$f1"
echo
echo "== activityPins() body (with nearby lines) =="
start2=$(rg -n "static Future<.*>\s*activityPins\s*\(" "$f2" | head -n1 | cut -d: -f1 || true)
if [ -z "${start2:-}" ]; then
# fallback: match without "static"
start2=$(rg -n "\bactivityPins\s*\(" "$f2" | head -n1 | cut -d: -f1)
fi
win_pre2=25; win_post2=240
awk -v s="$((start2-win_pre2))" -v e="$((start2+win_post2))" '
NR>=1 && NR>=s && NR<=e { printf "%5d:%s\n", NR, $0 }
' "$f2"Repository: pangeachat/client
Length of output: 7461
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find ErrorHandler.logError definition/usages =="
rg -n "ErrorHandler\.logError" -S lib || true
rg -n "class\s+ErrorHandler|logError\s*\(" -S lib || true
rg -n -S "Sentry\." lib || true
rg -n -S "captureException|captureMessage|captureEvent" lib || true
echo
echo "== Inspect PayloadClient implementation (error handling / sentry) =="
payload_file=$(fd -e dart PayloadClient lib -a 2>/dev/null | head -n1 || true)
echo "PayloadClient file candidates: $(fd -e dart '*payload*client*.dart' lib -a 2>/dev/null | head -n5 | tr '\n' ' ')"
rg -n "class\s+PayloadClient" -S lib || true
# If we found a plausible file, show error-handling area around its request methods
rg -n "Future<.*find\s*\(|\bfind\s*\(" -S lib/pangea/payload_client lib || trueRepository: pangeachat/client
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect ErrorHandler implementation (to see if it logs to Sentry)
echo "== error_handler.dart =="
f="lib/pangea/common/utils/error_handler.dart"
rg -n "class\s+ErrorHandler|logError|Sentry|captureException|captureMessage" "$f" || true
start=$(rg -n "class\s+ErrorHandler" "$f" | head -n1 | cut -d: -f1)
# print a chunk covering logError + internals
end=$((start+140))
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$f"
echo
# 2) Locate PayloadClient implementation(s)
echo "== Locate PayloadClient files =="
payload_files=$(fd -e dart -g '*payload*' lib | head -n50 || true)
echo "$payload_files" | head -n50
echo
echo "== Search PayloadClient for ErrorHandler/Sentry/logging =="
# Try to find the actual PayloadClient class file(s)
payload_class_files=$(rg -l "class\s+PayloadClient" lib -g'*.dart' || true)
echo "PayloadClient class files:"
echo "$payload_class_files"
for pf in $payload_class_files; do
echo
echo "== Inspect $pf =="
rg -n "Future<.*\b(find|get)\b|ErrorHandler|Sentry\.|captureException|captureMessage" "$pf" || true
# show likely request methods area if present
loc_line=$(rg -n "\bFuture<.*\bfind\b|\bFuture<.*\bget\b|\bfind\s*\(" "$pf" | head -n1 | cut -d: -f1 || true)
if [ -n "${loc_line:-}" ]; then
s=$((loc_line-20)); [ "$s" -lt 1 ] && s=1
e=$((loc_line+200))
awk -v s="$s" -v e="$e" 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$pf"
fi
doneRepository: pangeachat/client
Length of output: 9552
Add Sentry logging to world map location/activity load failures
WorldLocationsRepo.mappableLocations()andWorldActivitiesRepo.activityPins()contain noErrorHandler.logError/Sentrycalls and rely onPayloadClientwhich throws on HTTP errors without logging.- As a result, the
catch (_) { ... }blocks inlib/pangea/world/widgets/world_map.dart(L46-62) swallow exceptions without reporting to Sentry; update them to log viaErrorHandler.logError(catch(e, s)and call it) in production.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/widgets/world_map.dart` around lines 46 - 62, Update the two
catch blocks in _loadLocations and _loadActivities to capture error and stack
(catch (e, s)) and call ErrorHandler.logError(e, s) so failures from
WorldLocationsRepo.mappableLocations() and WorldActivitiesRepo.activityPins()
are reported to Sentry; keep the existing behavior of not rethrowing (map stays
usable) and only add the ErrorHandler.logError call (optionally gated by
production if desired).
Comment on lines
+134
to
+151
| child: GestureDetector( | ||
| onTap: () => _openActivity(activity), | ||
| child: Container( | ||
| decoration: BoxDecoration( | ||
| color: AppConfig.gold, | ||
| shape: BoxShape.circle, | ||
| border: Border.all(color: Colors.white, width: 2), | ||
| boxShadow: const [ | ||
| BoxShadow(blurRadius: 4, color: Colors.black38), | ||
| ], | ||
| ), | ||
| child: const Icon( | ||
| Icons.star, | ||
| size: 20, | ||
| color: Colors.white, | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
GestureDetector must be wrapped in Semantics for accessibility.
The activity marker's GestureDetector (L134-135) is used as a tappable button but lacks an accessible name for screen readers and Playwright. As per coding guidelines, GestureDetector and InkWell widgets used as buttons must be wrapped in Semantics(label: '...', button: true, child: ...).
♿ Proposed fix to wrap GestureDetector in Semantics
child: Tooltip(
message: '${activity.title}\n${activity.locationName}',
- child: GestureDetector(
- onTap: () => _openActivity(activity),
- child: Container(
+ child: Semantics(
+ label: 'Activity: ${activity.title} at ${activity.locationName}',
+ button: true,
+ child: GestureDetector(
+ onTap: () => _openActivity(activity),
+ child: Container(
decoration: BoxDecoration(
color: AppConfig.gold,
shape: BoxShape.circle,
border: Border.all(color: Colors.white, width: 2),
boxShadow: const [
BoxShadow(blurRadius: 4, color: Colors.black38),
],
),
child: const Icon(
Icons.star,
size: 20,
color: Colors.white,
),
+ ),
),
),
),As per coding guidelines: "GestureDetector and InkWell widgets used as buttons must be wrapped in Semantics(label: '...', button: true, child: ...) to surface an accessible name"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/widgets/world_map.dart` around lines 134 - 151, Wrap the
tappable activity marker's GestureDetector in a Semantics widget providing an
explicit accessible name and button role: replace the direct GestureDetector
child with Semantics(label: /* use activity title/name, e.g. activity.name or
activity.title */ , button: true, child: GestureDetector(...)); keep the
existing onTap handler (_openActivity(activity)) and visual decoration
unchanged, and ensure the label uses the activity's human-readable property
(fallback to a generic 'Activity' if null) so screen readers and Playwright can
identify the control.
Source: Coding guidelines
Comment on lines
+47
to
+141
| static Future<List<WorldActivityPin>> activityPins() async { | ||
| if (_cache != null && | ||
| _lastFetched != null && | ||
| DateTime.now().difference(_lastFetched!) < _cacheDuration) { | ||
| return _cache!; | ||
| } | ||
|
|
||
| final l1 = MatrixState.pangeaController.userController.userL1Code ?? 'en'; | ||
|
|
||
| // 1. All course plan ids (public CMS read). | ||
| final payload = PayloadClient( | ||
| baseUrl: Environment.cmsApi, | ||
| accessToken: MatrixState.pangeaController.userController.accessToken, | ||
| ); | ||
| final planIdsResp = await payload.find( | ||
| 'course-plans', | ||
| (json) => json['id'] as String, | ||
| limit: 100, | ||
| select: {'id': true}, | ||
| ); | ||
|
|
||
| // 2. Localized course plans -> topic ids. | ||
| final coursesResp = await CoursePlansRepo.search( | ||
| GetLocalizedCoursesRequest(coursePlanIds: planIdsResp.docs, l1: l1), | ||
| ); | ||
| final planIdByTopicId = <String, String>{}; | ||
| for (final entry in coursesResp.coursePlans.entries) { | ||
| for (final topicId in entry.value.topicIds) { | ||
| planIdByTopicId[topicId] = entry.key; | ||
| } | ||
| } | ||
| final topicIds = planIdByTopicId.keys.toList(); | ||
| if (topicIds.isEmpty) return _setCache([]); | ||
|
|
||
| // 3. Topics -> activity ids + location ids. | ||
| final topicsResp = await CourseTopicRepo.get( | ||
| TranslateTopicRequest(topicIds: topicIds, l1: l1), | ||
| _worldBatchId, | ||
| ); | ||
|
|
||
| // 4. Coordinates per location id (raw CMS read keeps coordinates). | ||
| final locations = await WorldLocationsRepo.mappableLocations(); | ||
| final coordsById = { | ||
| for (final location in locations) location.id: location, | ||
| }; | ||
|
|
||
| // 5. Activity details for topics that can be placed on the map. | ||
| final mappableTopics = topicsResp.topics.values.where( | ||
| (topic) => topic.locationIds.any(coordsById.containsKey), | ||
| ); | ||
| final activityIds = mappableTopics | ||
| .expand((topic) => topic.activityIds) | ||
| .toSet() | ||
| .toList(); | ||
| if (activityIds.isEmpty) return _setCache([]); | ||
| final activitiesResp = await CourseActivityRepo.get( | ||
| TranslateActivityRequest(activityIds: activityIds, l1: l1), | ||
| _worldBatchId, | ||
| ); | ||
|
|
||
| // 6. Pins, fanned out around each topic's location. | ||
| const distance = Distance(); | ||
| final pins = <WorldActivityPin>[]; | ||
| for (final topic in mappableTopics) { | ||
| final coursePlanId = planIdByTopicId[topic.uuid]; | ||
| if (coursePlanId == null) continue; | ||
| final location = topic.locationIds | ||
| .where(coordsById.containsKey) | ||
| .map((id) => coordsById[id]!) | ||
| .first; | ||
| final center = LatLng( | ||
| location.coordinates![1], | ||
| location.coordinates![0], | ||
| ); | ||
| final placeable = topic.activityIds | ||
| .where(activitiesResp.plans.containsKey) | ||
| .toList(); | ||
| for (var i = 0; i < placeable.length; i++) { | ||
| final plan = activitiesResp.plans[placeable[i]]!; | ||
| // Ring of ~25km around the location; single activities sit on it | ||
| // too so the location pin itself stays visible underneath. | ||
| final point = distance.offset(center, 25000, (360 / placeable.length) * i); | ||
| pins.add( | ||
| WorldActivityPin( | ||
| activityId: placeable[i], | ||
| title: plan.title, | ||
| locationName: location.name, | ||
| point: point, | ||
| coursePlanId: coursePlanId, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
| return _setCache(pins); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add error logging to maintain observability.
The activityPins() method has no try-catch block and will propagate exceptions from repository calls (CoursePlansRepo.search, CourseTopicRepo.get, CourseActivityRepo.get, WorldLocationsRepo.mappableLocations) to the caller. As per coding guidelines, ensure all error handling includes logging to Sentry in production code to maintain error observability, especially for hotfixes. The caller (WorldMap._loadActivities) swallows errors without logging, relying on this repo to log them.
🛡️ Proposed fix to add Sentry error logging
Add import:
+import 'package:fluffychat/pangea/common/utils/error_handler.dart';Wrap the method body (after cache check):
static Future<List<WorldActivityPin>> activityPins() async {
if (_cache != null &&
_lastFetched != null &&
DateTime.now().difference(_lastFetched!) < _cacheDuration) {
return _cache!;
}
+ try {
final l1 = MatrixState.pangeaController.userController.userL1Code ?? 'en';
// 1. All course plan ids (public CMS read).
// ... rest of the method ...
return _setCache(pins);
+ } catch (e, s) {
+ ErrorHandler.logError(e: e, s: s);
+ rethrow;
+ }
}As per coding guidelines: "Ensure all error handling includes logging to Sentry in production code to maintain error observability, especially for hotfixes"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static Future<List<WorldActivityPin>> activityPins() async { | |
| if (_cache != null && | |
| _lastFetched != null && | |
| DateTime.now().difference(_lastFetched!) < _cacheDuration) { | |
| return _cache!; | |
| } | |
| final l1 = MatrixState.pangeaController.userController.userL1Code ?? 'en'; | |
| // 1. All course plan ids (public CMS read). | |
| final payload = PayloadClient( | |
| baseUrl: Environment.cmsApi, | |
| accessToken: MatrixState.pangeaController.userController.accessToken, | |
| ); | |
| final planIdsResp = await payload.find( | |
| 'course-plans', | |
| (json) => json['id'] as String, | |
| limit: 100, | |
| select: {'id': true}, | |
| ); | |
| // 2. Localized course plans -> topic ids. | |
| final coursesResp = await CoursePlansRepo.search( | |
| GetLocalizedCoursesRequest(coursePlanIds: planIdsResp.docs, l1: l1), | |
| ); | |
| final planIdByTopicId = <String, String>{}; | |
| for (final entry in coursesResp.coursePlans.entries) { | |
| for (final topicId in entry.value.topicIds) { | |
| planIdByTopicId[topicId] = entry.key; | |
| } | |
| } | |
| final topicIds = planIdByTopicId.keys.toList(); | |
| if (topicIds.isEmpty) return _setCache([]); | |
| // 3. Topics -> activity ids + location ids. | |
| final topicsResp = await CourseTopicRepo.get( | |
| TranslateTopicRequest(topicIds: topicIds, l1: l1), | |
| _worldBatchId, | |
| ); | |
| // 4. Coordinates per location id (raw CMS read keeps coordinates). | |
| final locations = await WorldLocationsRepo.mappableLocations(); | |
| final coordsById = { | |
| for (final location in locations) location.id: location, | |
| }; | |
| // 5. Activity details for topics that can be placed on the map. | |
| final mappableTopics = topicsResp.topics.values.where( | |
| (topic) => topic.locationIds.any(coordsById.containsKey), | |
| ); | |
| final activityIds = mappableTopics | |
| .expand((topic) => topic.activityIds) | |
| .toSet() | |
| .toList(); | |
| if (activityIds.isEmpty) return _setCache([]); | |
| final activitiesResp = await CourseActivityRepo.get( | |
| TranslateActivityRequest(activityIds: activityIds, l1: l1), | |
| _worldBatchId, | |
| ); | |
| // 6. Pins, fanned out around each topic's location. | |
| const distance = Distance(); | |
| final pins = <WorldActivityPin>[]; | |
| for (final topic in mappableTopics) { | |
| final coursePlanId = planIdByTopicId[topic.uuid]; | |
| if (coursePlanId == null) continue; | |
| final location = topic.locationIds | |
| .where(coordsById.containsKey) | |
| .map((id) => coordsById[id]!) | |
| .first; | |
| final center = LatLng( | |
| location.coordinates![1], | |
| location.coordinates![0], | |
| ); | |
| final placeable = topic.activityIds | |
| .where(activitiesResp.plans.containsKey) | |
| .toList(); | |
| for (var i = 0; i < placeable.length; i++) { | |
| final plan = activitiesResp.plans[placeable[i]]!; | |
| // Ring of ~25km around the location; single activities sit on it | |
| // too so the location pin itself stays visible underneath. | |
| final point = distance.offset(center, 25000, (360 / placeable.length) * i); | |
| pins.add( | |
| WorldActivityPin( | |
| activityId: placeable[i], | |
| title: plan.title, | |
| locationName: location.name, | |
| point: point, | |
| coursePlanId: coursePlanId, | |
| ), | |
| ); | |
| } | |
| } | |
| return _setCache(pins); | |
| } | |
| static Future<List<WorldActivityPin>> activityPins() async { | |
| if (_cache != null && | |
| _lastFetched != null && | |
| DateTime.now().difference(_lastFetched!) < _cacheDuration) { | |
| return _cache!; | |
| } | |
| try { | |
| final l1 = MatrixState.pangeaController.userController.userL1Code ?? 'en'; | |
| // 1. All course plan ids (public CMS read). | |
| final payload = PayloadClient( | |
| baseUrl: Environment.cmsApi, | |
| accessToken: MatrixState.pangeaController.userController.accessToken, | |
| ); | |
| final planIdsResp = await payload.find( | |
| 'course-plans', | |
| (json) => json['id'] as String, | |
| limit: 100, | |
| select: {'id': true}, | |
| ); | |
| // 2. Localized course plans -> topic ids. | |
| final coursesResp = await CoursePlansRepo.search( | |
| GetLocalizedCoursesRequest(coursePlanIds: planIdsResp.docs, l1: l1), | |
| ); | |
| final planIdByTopicId = <String, String>{}; | |
| for (final entry in coursesResp.coursePlans.entries) { | |
| for (final topicId in entry.value.topicIds) { | |
| planIdByTopicId[topicId] = entry.key; | |
| } | |
| } | |
| final topicIds = planIdByTopicId.keys.toList(); | |
| if (topicIds.isEmpty) return _setCache([]); | |
| // 3. Topics -> activity ids + location ids. | |
| final topicsResp = await CourseTopicRepo.get( | |
| TranslateTopicRequest(topicIds: topicIds, l1: l1), | |
| _worldBatchId, | |
| ); | |
| // 4. Coordinates per location id (raw CMS read keeps coordinates). | |
| final locations = await WorldLocationsRepo.mappableLocations(); | |
| final coordsById = { | |
| for (final location in locations) location.id: location, | |
| }; | |
| // 5. Activity details for topics that can be placed on the map. | |
| final mappableTopics = topicsResp.topics.values.where( | |
| (topic) => topic.locationIds.any(coordsById.containsKey), | |
| ); | |
| final activityIds = mappableTopics | |
| .expand((topic) => topic.activityIds) | |
| .toSet() | |
| .toList(); | |
| if (activityIds.isEmpty) return _setCache([]); | |
| final activitiesResp = await CourseActivityRepo.get( | |
| TranslateActivityRequest(activityIds: activityIds, l1: l1), | |
| _worldBatchId, | |
| ); | |
| // 6. Pins, fanned out around each topic's location. | |
| const distance = Distance(); | |
| final pins = <WorldActivityPin>[]; | |
| for (final topic in mappableTopics) { | |
| final coursePlanId = planIdByTopicId[topic.uuid]; | |
| if (coursePlanId == null) continue; | |
| final location = topic.locationIds | |
| .where(coordsById.containsKey) | |
| .map((id) => coordsById[id]!) | |
| .first; | |
| final center = LatLng( | |
| location.coordinates![1], | |
| location.coordinates![0], | |
| ); | |
| final placeable = topic.activityIds | |
| .where(activitiesResp.plans.containsKey) | |
| .toList(); | |
| for (var i = 0; i < placeable.length; i++) { | |
| final plan = activitiesResp.plans[placeable[i]]!; | |
| // Ring of ~25km around the location; single activities sit on it | |
| // too so the location pin itself stays visible underneath. | |
| final point = distance.offset(center, 25000, (360 / placeable.length) * i); | |
| pins.add( | |
| WorldActivityPin( | |
| activityId: placeable[i], | |
| title: plan.title, | |
| locationName: location.name, | |
| point: point, | |
| coursePlanId: coursePlanId, | |
| ), | |
| ); | |
| } | |
| } | |
| return _setCache(pins); | |
| } catch (e, s) { | |
| ErrorHandler.logError(e: e, s: s); | |
| rethrow; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/world_activities_repo.dart` around lines 47 - 141, The
activityPins() method can throw from calls like CoursePlansRepo.search,
CourseTopicRepo.get, CourseActivityRepo.get, and
WorldLocationsRepo.mappableLocations and currently has no logging; wrap the main
work (everything after the cache validity check) in a try-catch, import and call
Sentry.captureException(error, stackTrace) (and optionally a local logger) in
the catch block to record the error, then rethrow the exception (or return
_setCache([]) if you prefer to swallow but still log); ensure you reference
activityPins, CoursePlansRepo.search, CourseTopicRepo.get,
CourseActivityRepo.get, WorldLocationsRepo.mappableLocations, and use
Sentry.captureException to maintain observability.
Source: Coding guidelines
Comment on lines
+14
to
+44
| static Future<List<CmsCoursePlanTopicLocation>> mappableLocations() async { | ||
| if (_cache != null && | ||
| _lastFetched != null && | ||
| DateTime.now().difference(_lastFetched!) < _cacheDuration) { | ||
| return _cache!; | ||
| } | ||
|
|
||
| final payload = PayloadClient( | ||
| baseUrl: Environment.cmsApi, | ||
| accessToken: MatrixState.pangeaController.userController.accessToken, | ||
| ); | ||
|
|
||
| final List<CmsCoursePlanTopicLocation> locations = []; | ||
| int page = 1; | ||
| bool hasNext = true; | ||
| while (hasNext && page <= 10) { | ||
| final resp = await payload.find( | ||
| CmsCoursePlanTopicLocation.slug, | ||
| CmsCoursePlanTopicLocation.fromJson, | ||
| page: page, | ||
| limit: 100, | ||
| ); | ||
| locations.addAll(resp.docs); | ||
| hasNext = resp.hasNextPage; | ||
| page++; | ||
| } | ||
|
|
||
| _cache = locations.where(_hasUsableCoordinates).toList(); | ||
| _lastFetched = DateTime.now(); | ||
| return _cache!; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add error logging to maintain observability.
The mappableLocations() method has no try-catch block and will propagate exceptions from PayloadClient.find to the caller. As per coding guidelines, ensure all error handling includes logging to Sentry in production code to maintain error observability, especially for hotfixes. The caller (WorldMap._loadLocations) swallows errors without logging, relying on this repo to log them.
🛡️ Proposed fix to add Sentry error logging
Add import:
+import 'package:fluffychat/pangea/common/utils/error_handler.dart';Wrap the method body:
static Future<List<CmsCoursePlanTopicLocation>> mappableLocations() async {
if (_cache != null &&
_lastFetched != null &&
DateTime.now().difference(_lastFetched!) < _cacheDuration) {
return _cache!;
}
+ try {
final payload = PayloadClient(
baseUrl: Environment.cmsApi,
accessToken: MatrixState.pangeaController.userController.accessToken,
);
final List<CmsCoursePlanTopicLocation> locations = [];
int page = 1;
bool hasNext = true;
while (hasNext && page <= 10) {
final resp = await payload.find(
CmsCoursePlanTopicLocation.slug,
CmsCoursePlanTopicLocation.fromJson,
page: page,
limit: 100,
);
locations.addAll(resp.docs);
hasNext = resp.hasNextPage;
page++;
}
_cache = locations.where(_hasUsableCoordinates).toList();
_lastFetched = DateTime.now();
return _cache!;
+ } catch (e, s) {
+ ErrorHandler.logError(e: e, s: s);
+ rethrow;
+ }
}As per coding guidelines: "Ensure all error handling includes logging to Sentry in production code to maintain error observability, especially for hotfixes"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/pangea/world/world_locations_repo.dart` around lines 14 - 44, Wrap the
body of mappableLocations() in a try-catch that catches all exceptions thrown by
PayloadClient.find (and any other calls inside), log the error to Sentry using
Sentry.captureException with the caught exception and stack trace, and
optionally emit a local log (e.g., processLogger.error) with context including
the function name and page info; after logging rethrow the exception (or return
an empty list if you prefer failing-safe) so callers keep current behavior.
Ensure you import Sentry and reference mappableLocations, PayloadClient.find,
_cache, _lastFetched, and _hasUsableCoordinates when updating the method.
Source: Coding guidelines
Collaborator
|
Looks like this all got moved into #7028. Closing this. |
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
2 participants
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.
What
World v2 slice 1: the world map becomes the app's canvas (every section root renders it), activities get shareable first-class URLs with a map popup, sessions share into all matching course spaces, and Pangea sections move out of
/roomsbehind permanent redirect shims.Why
Closes #6979. Addresses #5775.
Design doc:
pangeachat/.github→vision/world_v2.md.Testing
flutter analyze lib/clean; fullflutter testsuite green (595/595), including new unit tests forLegacyRedirectsandAppSection(15)./, analytics/settings/courses roots with map canvas + correct left columns and rail selection; legacy-path redirects; cold activity-URL deep link → popup → expand → close; activity session creation shared into the matching course space; settings category highlight; camera persistence across section switches./coursesauth gate restored,extra-carrying navigations migrated off the shim (go_router dropsextraon top-level redirects),MatrixState.activeRoomIdrecognizes/courseschats (push suppression), shim re-encodes path segments, bounded resolver timeout inside the session-launch dialog.Notable for reviewers:
lib/pangea/navigation/is the new single source of truth:PRoutes(typed paths),AppSection(exact-segment section resolution — replacesfullPath.contains()hacks),LegacyRedirects(one router-level shim; old URLs work forever)./rooms/:roomid, archive, new-chat) intentionally stay on upstream paths — push notification and matrix.to handling untouched.PRoutesincrementally.extra-carrying callsites were all migrated now (the shim cannot preserveextra)./courses/preview/:idto avoid colliding with/courses/:spaceid.Deploy Notes
None.
Summary by CodeRabbit
Release Notes
New Features
Improvements