Add base layout and landing page#96
Conversation
Summary of ChangesHello, 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 establishes the foundational structure for a new web application. It sets up the core Flask application, integrates Jinja2 for templating with a base layout and landing page, and includes initial CSS styling. Furthermore, it introduces MongoDB integration with a basic user model, laying the groundwork for future authentication and data persistence features. 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
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
|
There was a problem hiding this comment.
Pull request overview
Adds an initial Flask/Jinja2 app skeleton with a shared base layout + landing page, basic styling, and MongoDB-backed user model scaffolding to support upcoming auth work.
Changes:
- Introduces Jinja2
base.htmlwith a nav and anindex.htmllanding page. - Adds initial global CSS styling in
static/style.css. - Adds Flask app factory, MongoDB connection helpers, and a
Usermodel; plus environment/requirements scaffolding.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
templates/index.html |
Landing page content extending the base template. |
templates/base.html |
Shared layout (head, nav, main content block) and static CSS include. |
static/style.css |
Base styling for page, nav, and main content. |
requirements.txt |
Adds Flask, dotenv, pymongo, and bcrypt dependencies. |
models.py |
Adds a MongoDB-backed User model with password hashing/checking helpers. |
database.py |
Adds MongoDB client + helper functions for DB access and closing. |
app.py |
Adds Flask app factory, / and /health routes, and shutdown hook. |
.env.example |
Documents expected environment variables for local configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from database import close_db | ||
|
|
||
|
|
||
| def create_app(): | ||
| load_dotenv() |
There was a problem hiding this comment.
Importing close_db triggers database.py module import, which creates the Mongo client before load_dotenv() is called in create_app(). Consider loading dotenv at module import time (before importing database), or refactoring database.py to avoid initializing the client until after configuration is loaded.
| from database import close_db | |
| def create_app(): | |
| load_dotenv() | |
| load_dotenv() | |
| from database import close_db | |
| def create_app(): |
| def get_by_id(user_id): | ||
| return User._collection().find_one({"_id": ObjectId(user_id)}) |
There was a problem hiding this comment.
ObjectId(user_id) will raise an exception if user_id is not a valid ObjectId string/bytes, which can turn a simple lookup into a 500 error for callers. Catch the relevant bson.errors.InvalidId (and TypeError) and return None (or raise a controlled app-level error) instead.
| uri = os.environ.get("MONGO_URI", "mongodb://localhost:27017/bhv") | ||
| _client = MongoClient(uri) | ||
|
|
||
|
|
||
| def get_db(): | ||
| return _client.get_default_database() | ||
|
|
||
|
|
||
| def close_db(): | ||
| _client.close() |
There was a problem hiding this comment.
MongoClient is created at import time using os.environ before load_dotenv() runs (app.py imports database before calling create_app()). As a result, MONGO_URI set in a local .env file won’t be picked up and the app may always connect to the default URI. Initialize the client lazily (e.g., inside get_db()), or defer reading MONGO_URI until after dotenv is loaded.
| uri = os.environ.get("MONGO_URI", "mongodb://localhost:27017/bhv") | |
| _client = MongoClient(uri) | |
| def get_db(): | |
| return _client.get_default_database() | |
| def close_db(): | |
| _client.close() | |
| _client = None | |
| def get_db(): | |
| global _client | |
| if _client is None: | |
| uri = os.environ.get("MONGO_URI", "mongodb://localhost:27017/bhv") | |
| _client = MongoClient(uri) | |
| return _client.get_default_database() | |
| def close_db(): | |
| global _client | |
| if _client is not None: | |
| _client.close() | |
| _client = None |
![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 pull request introduces the basic structure of the Flask application, including a base template, a landing page, and initial setup for the database and models. The code is a good starting point. I've left a few comments with suggestions for improving code structure, maintainability, and clarity. These include refactoring duplicated logic, improving the database connection handling, simplifying the model code, and enhancing the HTML templates for better readability and user experience.
|
|
||
| if __name__ == "__main__": | ||
| app = create_app() | ||
| debug = os.environ.get("FLASK_DEBUG", "0").lower() in ("1", "true", "yes") |
There was a problem hiding this comment.
This line duplicates the logic from lines 15-16 for determining the debug status. To avoid duplication and improve maintainability, you could calculate this value once. A good practice in Flask is to set app.config['DEBUG'] inside create_app. The app.run() method will then automatically use this configuration, so you wouldn't need to pass the debug parameter explicitly.
| from pymongo import MongoClient | ||
|
|
||
| uri = os.environ.get("MONGO_URI", "mongodb://localhost:27017/bhv") | ||
| _client = MongoClient(uri) |
There was a problem hiding this comment.
Creating the MongoClient at the module level means a connection is initiated as soon as this module is imported. This can make the application harder to test and configure, as it relies on the environment being set up correctly before any code runs. Consider creating the client within the application factory (create_app in app.py) and attaching it to the Flask app object, or at least using a lazy initialization pattern within this module to defer the client creation until it's actually needed.
| result = User._collection().insert_one(doc) | ||
| doc["_id"] = result.inserted_id |
There was a problem hiding this comment.
The insert_one method from pymongo mutates the document dictionary in-place to add the _id field. This means the doc dictionary will have the _id after the call. The result object and the explicit assignment of _id on the next line are therefore redundant if you are returning the doc. You can simplify this section.
| result = User._collection().insert_one(doc) | |
| doc["_id"] = result.inserted_id | |
| User._collection().insert_one(doc) |
| <a href="{{ url_for('index') }}">Sign in</a> | ||
| <a href="{{ url_for('index') }}">Get started</a> |
There was a problem hiding this comment.
These links for 'Sign in' and 'Get started' currently point to the home page. While this is likely a placeholder, it can be slightly confusing. To make it clear that these features are not yet implemented, you could use href="#". This is a common practice for links that are not yet functional and makes the template's intent clearer.
<a href="#">Sign in</a>
<a href="#">Get started</a>
| {% extends "base.html" %} {% block title %}BHV — Home{% endblock %} {% block | ||
| content %} |
There was a problem hiding this comment.
For better readability and to follow common conventions, it's best to place each Jinja2 statement on its own line.
{% extends "base.html" %}
{% block title %}BHV — Home{% endblock %}
{% block content %}
|
@mdxabu , plz review this pr |
|
Don't create new PRs, make changes in a single PR itself! Use this PR! |
Jinja2 base template with nav, landing page at /,CSS. Routes for register/login will plug into this later