-
Notifications
You must be signed in to change notification settings - Fork 0
feature/basis for filtering cards by distance without external maps #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -241,3 +241,67 @@ def delete_study_spot(spot_id): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.session.rollback() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error(f"Error deleting study spot {spot_id}: {str(e)}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return jsonify({'error': 'Failed to delete study spot'}), 500 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @api_bp.route('/study_spots/sort_by_distance', methods=['POST']) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def sort_by_distance(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Sort study spots by distance from user location.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data = request.get_json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not data: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return jsonify({'error': 'No data provided'}), 400 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_lat = data.get('user_lat') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_lng = data.get('user_lng') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if user_lat is None or user_lng is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return jsonify({'error': 'user_lat and user_lng are required'}), 400 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Convert to float | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_lat = float(user_lat) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user_lng = float(user_lng) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except (ValueError, TypeError): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return jsonify({'error': 'Invalid coordinates'}), 400 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return jsonify({'error': 'Invalid coordinates'}), 400 | |
| return jsonify({'error': 'Invalid coordinates'}), 400 | |
| # Validate coordinate bounds | |
| if not (-90 <= user_lat <= 90) or not (-180 <= user_lng <= 180): | |
| return jsonify({'error': 'Coordinates out of range'}), 400 |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StudySpot.query.all() loads every row into memory and then sorts in Python. This won’t scale and can become a latency/memory issue as the table grows. Consider filtering out null coordinates at the DB level, selecting only needed columns, and optionally supporting paging/limits.
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint relies on StudySpot.latitude/longitude, but the create/update JSON handler (_study_spot_from_json) doesn’t accept/populate those fields. As a result, newly created/updated spots will have null coordinates and will be filtered out here, often returning an empty list. Add latitude/longitude handling (with validation) to the create/update flow, or adjust the endpoint’s behavior.
| # Skip spots without coordinates | |
| if not hasattr(spot, 'latitude') or not hasattr(spot, 'longitude'): | |
| continue | |
| if spot.latitude is None or spot.longitude is None: | |
| continue | |
| distance = _haversine_distance(user_lat, user_lng, spot.latitude, spot.longitude) | |
| spot_dict = spot.to_dict() | |
| spot_dict['distance_from_user'] = distance | |
| spots_with_distance.append(spot_dict) | |
| # Sort by distance (closest first) | |
| spots_with_distance.sort(key=lambda x: x['distance_from_user']) | |
| spot_dict = spot.to_dict() | |
| # If the spot doesn't have usable coordinates, include it with no distance | |
| if (not hasattr(spot, 'latitude') or not hasattr(spot, 'longitude') or | |
| spot.latitude is None or spot.longitude is None): | |
| spot_dict['distance_from_user'] = None | |
| spots_with_distance.append(spot_dict) | |
| continue | |
| # Compute distance for spots with valid coordinates | |
| distance = _haversine_distance(user_lat, user_lng, spot.latitude, spot.longitude) | |
| spot_dict['distance_from_user'] = distance | |
| spots_with_distance.append(spot_dict) | |
| # Sort by distance (closest first); spots without distance go last | |
| spots_with_distance.sort( | |
| key=lambda x: ( | |
| x['distance_from_user'] is None, | |
| x['distance_from_user'] if x['distance_from_user'] is not None else float('inf'), | |
| ) | |
| ) |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response adds distance_from_user, but _haversine_distance uses r = 3956, which implies miles. Since the field name doesn’t include units, API consumers won’t know how to display/interpret it. Consider returning an explicit unit (e.g., distance_miles or distance_meters) and documenting it.
| spot_dict['distance_from_user'] = distance | |
| spots_with_distance.append(spot_dict) | |
| # Sort by distance (closest first) | |
| spots_with_distance.sort(key=lambda x: x['distance_from_user']) | |
| # distance is in miles (Earth radius r = 3956 in _haversine_distance) | |
| spot_dict['distance_miles'] = distance | |
| spots_with_distance.append(spot_dict) | |
| # Sort by distance (closest first) | |
| spots_with_distance.sort(key=lambda x: x['distance_miles']) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,8 +6,56 @@ import ParallaxScrollView from '@/components/parallax-scroll-view'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ThemedText } from '@/components/themed-text'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ThemedView } from '@/components/themed-view'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Link } from 'expo-router'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as Location from 'expo-location'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useEffect, useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default function HomeScreen() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [userLat, setUserLat] = useState<number | null>(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [userLng, setUserLng] = useState<number | null>(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [spots, setSpots] = useState<any[]>([]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function getUserLocation() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { status } = await Location.requestForegroundPermissionsAsync(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (status !== 'granted') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const location = await Location.getCurrentPositionAsync({}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setUserLat(location.coords.latitude); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setUserLng(location.coords.longitude); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+26
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { status } = await Location.requestForegroundPermissionsAsync(); | |
| if (status !== 'granted') { | |
| return; | |
| } | |
| const location = await Location.getCurrentPositionAsync({}); | |
| setUserLat(location.coords.latitude); | |
| setUserLng(location.coords.longitude); | |
| try { | |
| const { status } = await Location.requestForegroundPermissionsAsync(); | |
| if (status !== 'granted') { | |
| console.warn('Location permission not granted'); | |
| setUserLat(null); | |
| setUserLng(null); | |
| return; | |
| } | |
| const location = await Location.getCurrentPositionAsync({}); | |
| if (!location || !location.coords) { | |
| console.error('Failed to obtain location coordinates'); | |
| setUserLat(null); | |
| setUserLng(null); | |
| return; | |
| } | |
| setUserLat(location.coords.latitude); | |
| setUserLng(location.coords.longitude); | |
| } catch (error) { | |
| console.error('Error getting user location:', error); | |
| setUserLat(null); | |
| setUserLng(null); | |
| } |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!userLat || !userLng) return; treats 0 as “missing”, which is incorrect for valid coordinates. Use explicit null/undefined checks (e.g., userLat == null || userLng == null) to decide whether coordinates are available.
| if (!userLat || !userLng) return; | |
| if (userLat == null || userLng == null) return; |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coding http://localhost:8000 will fail on physical devices and many emulator/simulator setups because localhost resolves to the device itself. Consider centralizing the API base URL (env/config) and using platform-appropriate host resolution (e.g., dev machine LAN IP / Android emulator 10.0.2.2).
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetchSortedByDistance doesn’t check response.ok before calling response.json(), so non-2xx responses (or non-JSON error bodies) can throw or silently produce confusing UI state. Handle non-OK responses explicitly (and consider surfacing the API error message).
| }); | |
| }); | |
| if (!response.ok) { | |
| const errorText = await response.text().catch(() => ''); | |
| throw new Error( | |
| `Request failed with status ${response.status} ${response.statusText}${ | |
| errorText ? `: ${errorText}` : '' | |
| }`, | |
| ); | |
| } |
Copilot
AI
Mar 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same falsy-check issue here: if (userLat && userLng) will skip when either coordinate is 0. Prefer explicit null/undefined checks before calling fetchSortedByDistance().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding
latitude/longitudecolumns requires a DB schema change. Since the app currently usesdb.create_all()(which won’t alter existing tables), any existing SQLite DB will be incompatible and queries can fail. Consider adding a migration step / schema update script (or ensure the dev DB is recreated) as part of this change.