Conversation
- Add /api/schema endpoint to expose database structure - Add /api/study_spots/raw endpoint for raw data access - Add /api/study_spots/distinct/<column> endpoint for filter options - Support spot_type, noise_level, tags, access_hours columns - Update SQLAlchemy to 2.0.35 for Python 3.14 compatibility
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
backend/routes.py
Outdated
| inspector = db.inspect(db.engine) | ||
| tables = inspector.get_table_names() |
There was a problem hiding this comment.
db.inspect(db.engine) is not a valid Flask-SQLAlchemy API (the SQLAlchemy() instance in backend/database.py doesn’t expose inspect). This will raise an AttributeError at runtime; use SQLAlchemy’s inspect(db.engine) (import inspect from sqlalchemy) or sqlalchemy.inspect instead.
| @api_bp.route('/schema', methods=['GET']) | ||
| def get_schema(): | ||
| """Return database schema information.""" | ||
| try: |
There was a problem hiding this comment.
The /api/schema endpoint exposes internal database structure (table/column names, types, defaults) and is currently publicly accessible with permissive CORS. This is sensitive information that can aid attackers; consider removing this from the public API or gating it behind admin auth / an environment flag (e.g., only enabled in development).
backend/routes.py
Outdated
| 'name': col['name'], | ||
| 'type': str(col['type']), | ||
| 'nullable': col['nullable'], | ||
| 'default': str(col['default']) if col['default'] else None |
There was a problem hiding this comment.
'default': str(col['default']) if col['default'] else None will incorrectly treat falsy defaults (e.g., 0, False, empty string) as missing. Use an explicit is not None check so real defaults aren’t dropped.
| 'default': str(col['default']) if col['default'] else None | |
| 'default': str(col['default']) if col['default'] is not None else None |
backend/routes.py
Outdated
| @@ -1,5 +1,6 @@ | |||
| from flask import Blueprint, request, jsonify | |||
| from database import db | |||
| from sqlalchemy import or_ | |||
There was a problem hiding this comment.
Unused import: or_ is added but not referenced anywhere in this file. Please remove it to avoid lint issues and keep imports minimal.
| from sqlalchemy import or_ |
| @api_bp.route('/study_spots/raw', methods=['GET']) | ||
| def get_raw_study_spots(): | ||
| """Return raw StudySpot data as-is from database.""" | ||
| try: | ||
| spots = StudySpot.query.all() | ||
| return jsonify([s.to_dict() for s in spots]), 200 |
There was a problem hiding this comment.
/study_spots/raw currently returns the same payload as the existing /study_spots endpoint (to_dict() for all rows), but adds another route that fetches the entire table without pagination. If this is meant for debugging/admin use, consider gating it (auth/env flag) or adding pagination/limits; otherwise it’s redundant and increases maintenance surface.
backend/routes.py
Outdated
| # For open now, return boolean options | ||
| return jsonify(['true', 'false']), 200 |
There was a problem hiding this comment.
/study_spots/distinct/access_hours returns ['true','false'], which doesn’t represent distinct access_hours values and also encodes booleans as strings. If the intent is an “open now” filter, consider using a dedicated route/column name (e.g., open_now) and return JSON booleans (true/false) to avoid confusing API consumers.
| # For open now, return boolean options | |
| return jsonify(['true', 'false']), 200 | |
| # For open now, return boolean options as JSON booleans | |
| return jsonify([True, False]), 200 |
| elif column in {'spot_type', 'tags'}: | ||
| # Handle JSON arrays | ||
| spots = StudySpot.query.all() | ||
| values = set() |
There was a problem hiding this comment.
For spot_type/tags, this endpoint loads all StudySpot rows into memory to compute distinct values. That won’t scale as the table grows; prefer doing this in the database (e.g., via JSON table-valued functions / json_each/jsonb_array_elements depending on the DB) or add a reasonable cap and/or caching strategy.
- Fixed db.inspect(db.engine) to use SQLAlchemy's inspect(db.engine) - Removed unused 'or_' import from sqlalchemy - Other Copilot suggestions will be addressed in future DB architecture changes
|
https://discord.com/channels/743636346727301172/1480319770044928100 Made a thread for your PR, we typically do this with all of ours for UTRP and other projects if u check the history of this channel, feel free to add all your future PRs to here to get them indexed Helps keep have a communication area that is faster |
Uh oh!
There was an error while loading. Please reload this page.