Feat/added frontend UI and backend #82
Conversation
…nd templated UI - Add run.py: single-command app launcher - Implement bhv/db.py: MongoDB + TinyDB fallback DB abstraction - Implement bhv/full_app.py: minimal Flask app with routes for signup, login, logout, upload, my_entries, admin, history, diff, file_version - Add templates: base.html (header/nav/footer), index.html, signup.html, login.html, upload.html, patient.html (my entries), admin.html, history.html, diff.html - Add static/css/alaska.css: Alaska-themed design (teal accents, minimal cards, responsive layout) - Update requirements.txt: add TinyDB for embedded DB fallback - Templates support Bootstrap-like card layout with forms and file listings - Routes support role-based access control (patients view own entries, admins see all) - Storage integration: uses GitAdapter for versioned commits per file save
- Add flask-wtf CSRF protection to all forms - Implement email validation and password strength check (min 6 chars) - Add Google OAuth endpoint (/auth/google) for user auto-creation and login - Add Google Sign-In button to login and signup pages using Google Identity Services - Set secure session cookie flags (HttpOnly, SameSite, Secure in production) - Add persistent session management (7-day lifetime) - Update signup/login to handle OAuth and email-based auth seamlessly - Add .env.example with documentation for GOOGLE_CLIENT_ID and MONGO_URI
Summary of ChangesHello @Prateekiiitg56, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant upgrade to the application by introducing a robust Git-backed storage system for managing immutable, versioned patient records. It integrates this new storage into a fully functional Flask web application, complete with user authentication, file upload capabilities, and detailed version history and diff views. The user interface has been completely revamped with a modern, consistent theme, and several security enhancements have been implemented to ensure data integrity and user protection. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a substantial pull request that introduces a complete backend with a Git-based storage adapter and a new frontend UI. The overall architecture is well-thought-out, with a clear separation of concerns between the web app, database layer, and storage adapter. The use of a database abstraction layer and an abstract storage adapter class are great design choices. My review focuses on improving robustness, security, and maintainability. I've identified a few critical issues, such as a security risk with a committed data file, a CSS syntax error, and a missing import that would cause a runtime error. I've also provided suggestions to handle exceptions more gracefully, clean up dependencies, and improve the clarity of tests and documentation. Overall, this is a very impressive feature addition. Addressing these points will help make the application more secure and reliable.
| @@ -0,0 +1 @@ | |||
| {"users": {"1": {"email": "test@example.com", "password": "scrypt:32768:8:1$rzJQOnK6yDTD29gZ$6d4e6acea9e3f7a746a3778c056ea7a8f2af90b20dd2723c99ae28142a062c3503b1e04d407c5280b847d35b57007ceef053898c92852f135d901c9d14076ea9", "role": "patient"}, "2": {"email": "patient@example.com", "password": "scrypt:32768:8:1$PkIL3lc7IsiuD59w$3e961f57b0a0a75a4f17e20dd05642d867f39d8cb992fd717c77fe80a1e3c8677b8132510edd1de913675bc76ccfb6411f9c3c38b729f4ddb4f3b6f6738d6977", "role": "patient"}, "3": {"email": "version@example.com", "password": "scrypt:32768:8:1$EWcPmYC38oGXFHBO$e303115b424697d6efe369b640af249bf8d66581041142a7ddeb686fc9af03b47d0c576e689542b4a1f8b6fd71a562afdd70948642c49a8102d506a252a17d6b", "role": "patient"}, "4": {"email": "download@example.com", "password": "scrypt:32768:8:1$mml5O8EzrxcBgURY$172bfeb3ac63eee926dc0597f7632f40341968dc9bd90a004b631b4bda3e2aba6bb90cd071d80af91c9c17ce77d0e497d73b839a35244c28db8c3aa56e7bd1e0", "role": "patient"}, "5": {"email": "admin@example.com", "password": "scrypt:32768:8:1$G8pXchlCpNjOySdM$d045220f7c8e2289b8749008a84bf44125049cbcea7933a39d25dbb549d103639df3ac2c0dae29e941d091dbb302b16d02afa8d90bb7a9a05f3934b3c33e43bd", "role": "admin"}, "6": {"email": "patient1@example.com", "password": "scrypt:32768:8:1$vWFvVhBvXKMgg6ig$5c88706d61af51be5bddcc5fe832573b8602a59163f6e60555522a18478a5c95b6e3b305820273d69e64e6508bb64ece90420323d5220cdeca22171efcc7c853", "role": "patient"}, "7": {"email": "patient2@example.com", "password": "scrypt:32768:8:1$CQFRHKENYhy8fBq1$a633880009710c203eaf4dfb783a1da44ff54a24bba23277be40fdbdedb96e7350e3b4a98ebd6c823355feed7a959bab51a0879762346fa287d6e9691298401c", "role": "patient"}, "8": {"email": "ps332927@gmail.com", "password": "", "role": "patient"}, "9": {"email": "prateek.singh23b@iiitg.ac.in", "password": "", "role": "patient"}, "10": {"email": "smoke@example.com", "password": "scrypt:32768:8:1$jaupQ6ucydydcpHJ$4c0345798909ddbef3d89ac9029e694cb17bb53c97baac8a7172893d99a0dc823292e5d97d0e7adf447b2ba12a507341a5cef088a57f67e256685600c940ffa6", "role": "patient"}}} No newline at end of file | |||
There was a problem hiding this comment.
This file contains sensitive data, including user emails and hashed passwords. Committing such a file to version control, even with test data, is a significant security risk. It exposes user information and password hashes to anyone with access to the repository's history. This file should be removed from the repository and added to .gitignore to prevent accidental commits in the future.
| color: rgba(255,255,255,0.88); | ||
| color: var(--text-secondary); | ||
| text-decoration: none; | ||
| padding: 6px 0; | ||
| border-bottom: 2px solid transparent; | ||
| transition: all 0.2s ease; | ||
| } | ||
|
|
||
| color: rgba(255,255,255,0.98); | ||
| border-bottom-color: rgba(255,255,255,0.55); | ||
| border-bottom-color: var(--accent-teal); |
There was a problem hiding this comment.
There are a couple of issues in this block:
- The
colorproperty is set twice (lines 141 and 142). The second one will always override the first. - The rules on lines 149-151 are outside of any CSS selector, which is a syntax error. They will be ignored by the browser. These rules might belong inside a pseudo-class selector like
:hoverfor.site-nav a. border-bottom-coloris declared twice on lines 150 and 151.
| @@ -0,0 +1,350 @@ | |||
| import os | |||
| import re | |||
| from flask import Flask, render_template, request, redirect, url_for, session, send_from_directory, flash | |||
There was a problem hiding this comment.
The Response class is used on line 347 but is not imported from flask. This will cause a NameError at runtime. Please add it to the import statement.
| from flask import Flask, render_template, request, redirect, url_for, session, send_from_directory, flash | |
| from flask import Flask, render_template, request, redirect, url_for, session, send_from_directory, flash, Response |
| return list(db.entries.find()) | ||
|
|
||
| def get_entry(entry_id): | ||
| from bson import ObjectId |
There was a problem hiding this comment.
Importing ObjectId within a function is inefficient and not standard practice. This import should be moved to the module level, inside the if MONGO_URI: block (e.g., at line 7), to be imported only once. This applies to the same import on lines 40 and 44.
| flash('File required') | ||
| return redirect(url_for('upload')) | ||
| filename = secure_filename(f.filename) | ||
| patient_id = user.get('email') if user.get('role')=='patient' else request.form.get('patient_id') |
There was a problem hiding this comment.
If an admin user submits the upload form without providing a patient_id, request.form.get('patient_id') will return None. This will cause os.path.join(patient_id, filename) on the next line to raise a TypeError. You should add a check to ensure patient_id is present for admin uploads.
| patient_id = user.get('email') if user.get('role')=='patient' else request.form.get('patient_id') | |
| patient_id = user.get('email') if user.get('role')=='patient' else request.form.get('patient_id') | |
| if user.get('role') == 'admin' and not patient_id: | |
| flash('Patient ID is required for admin uploads.') | |
| return redirect(url_for('upload')) |
| session.permanent = True | ||
| session['user_email'] = email | ||
| return redirect(url_for('index')) | ||
| except Exception as e: |
There was a problem hiding this comment.
Catching a broad Exception can hide underlying issues and make debugging difficult. It's better to catch the specific exceptions that id_token.verify_oauth2_token can raise, such as ValueError for an invalid token.
| except Exception as e: | |
| except ValueError as e: |
| new_bytes = storage.get(rel, new_sha) | ||
| try: | ||
| old_text = old_bytes.decode('utf-8').splitlines() | ||
| except Exception: |
There was a problem hiding this comment.
Catching a broad Exception here can mask the actual problem. It's better to catch the specific UnicodeDecodeError that can occur during decoding. This would make the error handling more precise.
| except Exception: | |
| except UnicodeDecodeError: |
| # HEAD references it. | ||
| try: | ||
| _ = repo.head.commit.hexsha | ||
| except Exception: |
There was a problem hiding this comment.
Catching a broad Exception can hide unexpected errors and make debugging more difficult. It's better to catch more specific exceptions that GitPython might raise, such as git.exc.GitCommandError. This applies to other broad except blocks in this file as well (e.g., lines 59, 84, 117, 139).
| pymongo>=4.0 | ||
| python-dotenv>=0.21 | ||
| flask-wtf>=1.0 | ||
| flask-oauthlib>=0.9 |
There was a problem hiding this comment.
The flask-oauthlib library is included as a dependency, but it appears to be unused in the project. This library is also considered outdated and is unmaintained. It should be removed to keep the project's dependencies clean and avoid potential security vulnerabilities from unmaintained packages.
| try: | ||
| adapter.save_with_parent(rel, b'three', user_id='u3', action='edit', parent='deadbeef') | ||
| assert False, "Expected conflict" | ||
| except Conflict: | ||
| pass |
There was a problem hiding this comment.
While this try/except block works for testing that an exception is raised, the idiomatic way to test for exceptions with pytest is to use the pytest.raises context manager. This makes the test more concise and clearly states the intent.
| try: | |
| adapter.save_with_parent(rel, b'three', user_id='u3', action='edit', parent='deadbeef') | |
| assert False, "Expected conflict" | |
| except Conflict: | |
| pass | |
| from bhv.storage.errors import Conflict | |
| import pytest | |
| with pytest.raises(Conflict): | |
| adapter.save_with_parent(rel, b'three', user_id='u3', action='edit', parent='deadbeef') |
There was a problem hiding this comment.
Pull request overview
Adds a Git-backed, commit-per-save storage adapter and integrates it into a Flask “minimalist monolith” with upload/history/diff views, along with a new Alaska-themed UI and initial test coverage.
Changes:
- Introduce
GitAdapter+ storage interface/exceptions for versioned file storage. - Add/expand Flask app routes for auth, upload, entries, history, diff, and download; add templates + Alaska CSS/theme assets.
- Add pytest config and multiple unit/integration tests; add demo/screenshot scripts and documentation.
Reviewed changes
Copilot reviewed 35 out of 44 changed files in this pull request and generated 31 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_git_adapter.py | Unit tests for GitAdapter save/history/conflict behavior |
| tests/test_full_app.py | Integration tests for Flask full app flows |
| tests/test_app_integration.py | Integration tests for the legacy/demo API app (bhv.app) |
| templates/base.html | Global layout + navbar/footer |
| templates/home.html | Themed homepage hero + highlights section |
| templates/index.html | Simple landing page template (may be legacy) |
| templates/signup.html | Signup page + Google sign-in |
| templates/login.html | Login page + Google sign-in |
| templates/upload.html | Upload form with CSRF token hidden input |
| templates/patient.html | Patient entries list + download/delete actions |
| templates/admin.html | Admin entries list + download/delete actions |
| templates/profile.html | Profile view with recent files |
| templates/history.html | History view with per-commit download + diff links |
| templates/diff.html | Unified diff display |
| templates/features.html | Informational page |
| templates/how_it_works.html | Informational page |
| templates/testimonials.html | Informational page |
| templates/contact.html | Informational page |
| static/css/alaska.css | New Alaska/aurora theme styling |
| static/img/favicon.svg | App favicon |
| static/img/alaska-logo.svg | Navbar/logo asset |
| static/img/alaska-3d.svg | Decorative asset |
| static/img/hospital-icon.svg | Decorative icon asset |
| scripts/demo_run.py | Demo script to exercise upload/history/diff |
| scripts/make_screenshots.py | Generates screenshots from demo output |
| scripts/check_cffi.py | Diagnostic script for cffi/cryptography environment |
| scripts/screenshots/initial_and_edit.png | Generated screenshot artifact |
| scripts/screenshots/history.png | Generated screenshot artifact |
| scripts/screenshots/diff.png | Generated screenshot artifact |
| bhv/storage/base.py | StorageAdapter interface |
| bhv/storage/errors.py | Conflict exception type |
| bhv/storage/git_adapter.py | Git-backed storage implementation |
| bhv/db.py | MongoDB/TinyDB abstraction for users/entries |
| bhv/app.py | Legacy/demo API app using GitAdapter |
| bhv/full_app.py | Main Flask app factory + routes (auth/upload/history/diff/download) |
| bhv/init.py | Package marker/exports |
| run.py | Entry point to run the full app |
| requirements.txt | Runtime + test dependencies |
| pytest.ini | Pytest configuration |
| data/db.json | TinyDB data file (currently populated) |
| README_GIT_ADAPTER.md | Git adapter documentation |
| README_APP.md | Full app documentation |
| .env.example | Environment variable template |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| patient_id = parts[0] | ||
| repo = self._ensure_repo(patient_id) | ||
| lock = self._locks[patient_id] | ||
| full_path = os.path.join(repo.working_tree_dir, *parts[1:]) | ||
| os.makedirs(os.path.dirname(full_path), exist_ok=True) | ||
|
|
||
| # Default save uses no parent check |
There was a problem hiding this comment.
In save(), repo, lock, full_path, and the os.makedirs(...) call are computed but then ignored because the method immediately delegates to save_with_parent, which repeats the same work. This duplication adds maintenance cost and makes it easier for the two paths to diverge—consider removing the unused setup in save() and just delegating after validating the path.
| patient_id = parts[0] | |
| repo = self._ensure_repo(patient_id) | |
| lock = self._locks[patient_id] | |
| full_path = os.path.join(repo.working_tree_dir, *parts[1:]) | |
| os.makedirs(os.path.dirname(full_path), exist_ok=True) | |
| # Default save uses no parent check | |
| # Default save uses no parent check; repository and path setup are handled by save_with_parent |
| # Upload v1 | ||
| client.post('/upload', data={ | ||
| 'file': (b'initial content\nline 2\n', 'notes.txt'), | ||
| 'narrative': 'Version 1' | ||
| }) | ||
|
|
||
| # Upload v2 (same filename) | ||
| client.post('/upload', data={ | ||
| 'file': (b'initial content\nmodified line 2\nline 3\n', 'notes.txt'), | ||
| 'narrative': 'Version 2' | ||
| }) |
There was a problem hiding this comment.
Same issue as above: these /upload calls are missing a multipart file object + content_type='multipart/form-data', so they won't populate request.files and won't actually test versioning/diff behavior.
| <form method="post"> | ||
| {{ csrf_token() }} | ||
| <label>Email</label> |
There was a problem hiding this comment.
CSRF token is currently rendered as plain text ({{ csrf_token() }}) instead of being submitted as a form field. With CSRF enabled, this form will fail validation. Render it as a hidden input (or use Flask-WTF form helpers like form.hidden_tag()).
| delete_entry(entry_id) | ||
| flash('Deleted') | ||
| return redirect(url_for('admin') if user.get('role')=='admin' else url_for('my_entries')) |
There was a problem hiding this comment.
entry_delete deletes by entry_id with no server-side authorization beyond “logged in”. A patient can craft a POST to delete other patients’ entries. Add an ownership/admin check (e.g., load the entry and ensure user.role == 'admin' or entry.patient_id == user.email) before calling delete_entry.
| delete_entry(entry_id) | |
| flash('Deleted') | |
| return redirect(url_for('admin') if user.get('role')=='admin' else url_for('my_entries')) | |
| entry = get_entry(entry_id) | |
| if not entry: | |
| flash('Entry not found') | |
| return redirect(url_for('admin') if user.get('role') == 'admin' else url_for('my_entries')) | |
| # authorization: only admins or the owning patient may delete | |
| if user.get('role') != 'admin' and entry.get('patient_id') != user.get('email'): | |
| flash('Forbidden') | |
| return redirect(url_for('index')) | |
| delete_entry(entry_id) | |
| flash('Deleted') | |
| return redirect(url_for('admin') if user.get('role') == 'admin' else url_for('my_entries')) |
| # attempt save with wrong parent should raise | ||
| from bhv.storage.errors import Conflict | ||
| try: | ||
| adapter.save_with_parent(rel, b'three', user_id='u3', action='edit', parent='deadbeef') | ||
| assert False, "Expected conflict" | ||
| except Conflict: | ||
| pass |
There was a problem hiding this comment.
Use pytest.raises(Conflict) instead of try/except + assert False; the current pattern can hide assertion failures and produces less helpful errors when the exception isn't raised.
|
|
||
| try: | ||
| import cryptography | ||
| print("cryptography OK") |
There was a problem hiding this comment.
Import of 'cryptography' is not used.
| print("cryptography OK") | |
| print("cryptography:", getattr(cryptography, "__file__", "no file"), getattr(cryptography, "__version__", "n/a")) |
| print("cryptography import error:", repr(e)) | ||
|
|
||
| try: | ||
| import _cffi_backend |
There was a problem hiding this comment.
Import of '_cffi_backend' is not used.
| from google.oauth2 import id_token | ||
| from google.auth.transport import requests as google_requests | ||
|
|
||
| from .db import init_db, create_user, get_user_by_email, create_entry, list_entries_for_patient, list_all_entries, get_entry, delete_entry, update_entry |
There was a problem hiding this comment.
Import of 'get_entry' is not used.
Import of 'update_entry' is not used.
| from .db import init_db, create_user, get_user_by_email, create_entry, list_entries_for_patient, list_all_entries, get_entry, delete_entry, update_entry | |
| from .db import init_db, create_user, get_user_by_email, create_entry, list_entries_for_patient, list_all_entries, delete_entry |
| @@ -0,0 +1,77 @@ | |||
| import tempfile | |||
| import io | |||
| import os | |||
There was a problem hiding this comment.
Import of 'os' is not used.
| import shutil | ||
| import pytest | ||
| from bhv.full_app import create_app | ||
| from bhv.db import init_db |
There was a problem hiding this comment.
Import of 'init_db' is not used.
1 participant
PR Description
Summary
This PR delivers a Git-based storage adapter for immutable, versioned patient records (commit-per-save) and integrates it into a Flask “minimalist monolith” web app with upload/history/diff views. It also updates the UI to a consistent Alaska/aurora theme across all pages with a project-relevant navbar and this is just prototype like a quick view of how the overall theme will gonna be.Every feature is only listed for now ,they can be enhanced and be for efficient later.
Key Features
Git-backed storage (
bhv/storage/git_adapter.py)Flask app flow (
bhv/full_app.py)/always renders the homepage (no redirect after login)/dashboardroutes signed-in users to entries/admin appropriatelyBHV_REQUEST_DEBUG=1Security / Auth
app.config['GOOGLE_CLIENT_ID']SESSION_COOKIE_SECUREUI/UX (
templates/*+static/css/alaska.css)