Feature/add login #2

cantis · 2025-11-10T15:42:35Z

Login now working nicely, cleaned up tests and added versioning to docker image generation.

…ML templates for better presentation, and add JavaScript for date selection.

…t, and enhance accessibility with consistent casing for text elements.

…dalone.py, and test_time_formatting.py to clean up the repository.

Implement weekly report generation and improve templates

- Added Flask-Login for user session management. - Created User model with password hashing and role management. - Developed user service for creating, updating, and deleting users. - Implemented authentication routes for login, logout, and profile management. - Added admin routes for user management, including adding, editing, and deleting users. - Enhanced templates for user management with forms and user lists. - Updated header template to include user authentication status and admin links. - Created default admin user if no users exist. - Added client-side confirmation for user deletion.

- Updated create_app function to accept configuration overrides for testing. - Modified Config class to use an in-memory database during tests. - Renamed 'is_active' to 'user_active' in User model for clarity and added compatibility property for Flask-Login. - Adjusted user creation and update logic to reflect the new 'user_active' field. - Added comprehensive tests for UserService, covering user creation, retrieval, updating, and deletion scenarios.

…nment variables

…entication and data handling

…y; add PostgreSQL support and deployment guide

… and adjust versioning

…vice functionality - Implement tests for authentication routes including profile and password change functionalities. - Create tests for home routes to validate time entry display and CRUD operations. - Develop extensive tests for user service functions, covering user creation, retrieval, updating, and deletion with exception handling. - Ensure all tests validate expected outcomes and handle edge cases effectively.

…n bump process in build_image.py

…d Render deployment; consolidate information in README.md

…ptions

…atabase and user creation scripts for PostgreSQL

… 'is_active'; enhance profile update functionality with email validation

…er_active'; adjust related tests and versioning

…; include version tagging and requirements update

Copilot

Pull Request Overview

This PR adds comprehensive user authentication and authorization functionality to the TimeTracker application, along with PostgreSQL support for production deployment. The changes implement Flask-Login for session management, user administration features, and enhanced weekly reporting capabilities.

Key changes:

Added user authentication system with login/logout, profile management, and password changes
Implemented role-based access control with admin privileges and user management
Added PostgreSQL database support while maintaining SQLite for development
Enhanced weekly report generation with detailed statistics and improved UI

Reviewed Changes

Copilot reviewed 39 out of 42 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
uv.lock, requirements.txt, pyproject.toml	Added flask-login and psycopg2-binary dependencies for auth and PostgreSQL support
tests/test_user_service.py	New comprehensive test suite for user service operations (586 lines)
tests/test_auth.py	New tests for authentication routes (213 lines)
tests/test_home.py, test_reports.py	Updated tests to include authentication requirements
tests/conftest.py	Enhanced test fixtures to skip default admin creation and support authentication
app/models.py	Added User model with password hashing and Flask-Login integration; changed time_out from int to boolean
app/service/user_service.py	New service module for user CRUD operations with custom exceptions
app/service/weekly_report_service.py	New service for generating comprehensive weekly reports with statistics
app/routes/auth.py	New authentication blueprint with login, logout, profile, and password change routes
app/routes/admin.py	Enhanced admin routes with user management (list, add, edit, delete)
app/routes/home.py, reports.py	Added @login_required decorators and refactored to use new services
app/templates/auth/*	New templates for login, profile, and password change pages
app/templates/admin/*	New templates for user management UI
app/templates/reports/*	Enhanced weekly report templates with detailed statistics and improved UX
app/templates/header.html	Added user menu with authentication state display
app/config.py	Updated to support DATABASE_URL (Render.com) and PostgreSQL configuration
app/app.py	Integrated Flask-Login, added user loader, and default admin creation
scripts/create_db.py	New standalone script for PostgreSQL database setup
docs/*	Added comprehensive documentation for deployment and PostgreSQL setup
render.yaml	New Render.com deployment configuration
build_image.py, build_current.py	Updated build scripts with proper version management

Copilot · 2025-11-10T15:47:02Z

scripts/create_db.py

+
+DB_NAME = 'timetracker_db'
+DB_USER = 'timetracker_user'
+DB_PASS = 'your_password'


Security issue: The database password is hardcoded as 'your_password' in plaintext. This script creates a PostgreSQL user with this weak, predictable password.

Recommendation: Either:

Prompt the user for the database password during script execution (similar to how the superuser password is prompted on line 30), or

Read it from an environment variable, or

Add a comment/documentation warning that this must be changed before use

Copilot · 2025-11-10T15:47:02Z

scripts/create_db.py

+    finally:
+        if conn:
+            conn.close()


Uninitialized variable conn may cause UnboundLocalError: If the psycopg2.connect() call on line 33 raises an exception, the conn variable will never be assigned. The finally block on line 68 then attempts to check if conn:, which will raise UnboundLocalError: local variable 'conn' referenced before assignment.

Fix: Initialize conn = None before the try block.

Copilot · 2025-11-10T15:47:03Z

app/routes/admin.py

+def admin_required(f: Callable[..., Any]) -> Callable[..., Any]:
+    """Decorator to require admin privileges."""
+
+    def decorated_function(*args, **kwargs):
+        if not current_user.is_authenticated or not current_user.is_admin:
+            flash('Admin privileges required.', 'error')
+            return redirect(url_for('home.index'))
+        return f(*args, **kwargs)
+
+    decorated_function.__name__ = f.__name__
+    return decorated_function


The decorator admin_required should use @wraps(f) from functools to preserve the original function's metadata (name, docstring, etc.). Without this, Flask's routing system may have issues with multiple routes using this decorator, and debugging/introspection will be harder.

scripts/create_db.py

Copilot · 2025-11-10T15:47:03Z

tests/test_user_service.py

+        @patch.dict(
+            os.environ,
+            {
+                'DEFAULT_ADMIN_USERNAME': 'customadmin',
+                'DEFAULT_ADMIN_EMAIL': 'custom@admin.com',
+                'DEFAULT_ADMIN_PASSWORD': 'custompass123',
+            },
+        )
+        def test_create_default_admin_with_custom_env_vars(self, app):
+            """Test creating default admin with custom environment variables."""
+            with app.app_context():
+                # Act
+                admin_user = create_default_admin()
+
+                # Assert
+                assert admin_user is not None
+                assert admin_user.username == 'customadmin'
+                assert admin_user.email == 'custom@admin.com'
+                assert admin_user.check_password('custompass123')
+                assert admin_user.is_admin is True
+


The test function is incorrectly indented inside another test function's scope. Lines 565-585 define test_create_default_admin_with_custom_env_vars but it's indented under test_create_default_admin_when_users_exist (which ends at line 563). This will cause the nested function to never be executed as a test.

The @patch.dict decorator on line 565 should be at the class level, and the function should be dedented to be a proper class method.

Copilot · 2025-11-10T15:47:03Z

scripts/create_db.py

+# /// script
+# requires-python = ">=3.11"
+# dependencies = [
+#     "getpass",


The getpass module is a standard library module and should not be listed in the dependencies. The PEP 723 inline script metadata at line 6 incorrectly lists "getpass" as a dependency, which will cause an error when uv tries to install it.

Copilot · 2025-11-10T15:47:04Z

app/service/user_service.py

+    is_admin: Optional[bool] = None,
+    is_active: Optional[bool] = None,
+) -> User:
+    """Update user detail"""


Missing docstring: The update_user function is missing a proper docstring. While there's a brief one-liner on line 98, it should document the parameters, return value, and exceptions raised (especially the custom UpdateUserError), following the pattern used in other functions like delete_user.

Suggested change

"""Update user detail"""

"""

Update the details of an existing user.

Args:

user_id (int): The ID of the user to update.

username (Optional[str], optional): New username. Must be unique and at least 3 characters.

email (Optional[str], optional): New email address. Must be unique and valid.

password (Optional[str], optional): New password. Must be at least 6 characters.

is_admin (Optional[bool], optional): Whether the user is an admin.

is_active (Optional[bool], optional): Whether the user is active.

Returns:

User: The updated User object.

Raises:

UpdateUserError: If the user is not found, if username/email conflicts occur,

if validation fails, or if a database error occurs.

"""

Copilot · 2025-11-10T15:47:04Z

app/service/user_service.py

+            user.is_admin = is_admin
+
+        if is_active is not None:
+            user.user_active = is_active


Inconsistency between property name and database column: The model defines a column named user_active (line 22), and the is_active property (lines 53-56) reads from user_active. However, in the update_user function, line 131 assigns to user.user_active, while line 329 in the test expects user.is_active to be updated. This creates a mismatch.

The __init__ method (line 39) also assigns to self.user_active when the parameter is named is_active, which is correct for the database column but inconsistent with the property name used elsewhere.

Consider standardizing on either:

Use user_active consistently as the attribute name, OR

Add a setter for the is_active property to handle assignments properly

Suggested change

user.user_active = is_active

user.is_active = is_active

Copilot · 2025-11-10T15:47:04Z

tests/test_user_service.py

+            updated_user = update_user(user.id, is_active=False)
+
+            # Assert
+            assert updated_user is not None
+            assert updated_user.user_active is False
+
+    def test_update_user_multiple_fields_success(self, app):
+        """Test updating multiple user fields at once."""
+        with app.app_context():
+            # Arrange
+            user = create_user('oldname', 'old@example.com', 'oldpassword', is_admin=False, is_active=True)
+
+            # Act
+            updated_user = update_user(
+                user.id,
+                username='newname',
+                email='new@example.com',
+                password='newpassword123',
+                is_admin=True,
+                is_active=False,
+            )
+
+            # Assert
+            assert updated_user is not None
+            assert updated_user.username == 'newname'
+            assert updated_user.email == 'new@example.com'
+            assert updated_user.check_password('newpassword123')
+            assert updated_user.is_admin is True
+            assert updated_user.is_active is False


The test expects updated_user.is_active to be False, but the update_user function assigns to user.user_active. Since is_active is a read-only property (no setter defined) that returns bool(self.user_active), this test should pass. However, for clarity and to prevent future issues, consider adding a setter for the is_active property in the User model, or consistently use user_active throughout the codebase.

Copilot · 2025-11-10T15:47:05Z

tests/test_auth.py

+        with app.app_context():
+            # Create two users
+            user1 = create_user('user1', 'user1@example.com', 'password123')
+            _user2 = create_user('user2', 'user2@example.com', 'password123')


Variable _user2 is not used.

cantis added 22 commits August 2, 2025 10:13

Implement weekly report generation: refactor report logic, enhance HT…

ec0860d

…ML templates for better presentation, and add JavaScript for date selection.

Refactor weekly report templates: standardize headings, improve layou…

4489a77

…t, and enhance accessibility with consistent casing for text elements.

Remove unused test files: delete test_time_fix.py, test_time_fix_stan…

f98ce99

…dalone.py, and test_time_formatting.py to clean up the repository.

Merge pull request #1 from cantis:feature/add-weekly-report

985d55b

Implement weekly report generation and improve templates

feat: Add user creation and login in tests for improved test reliability

ce0512e

feat: Implement configurable default admin user creation using enviro…

0f84a17

…nment variables

feat: Add change password and profile pages with validation and tests

9e87848

feat: Refactor tests for home and reports routes to improve user auth…

43fd322

…entication and data handling

feat: Update Dockerfile and configuration for Render.com compatibilit…

adc51dc

…y; add PostgreSQL support and deployment guide

feat: Update dependencies for PostgreSQL support; add psycopg2-binary…

50e0c79

… and adjust versioning

Refactoring user service to remove tuple retgurn code

37fe087

feat: Update version in pyproject.toml and uv.lock; streamline versio…

f5cc35b

…n bump process in build_image.py

chore: Update README.md to reflect project status and build instructions

6f6acb2

chore: Remove outdated documentation files for Docker, PostgreSQL, an…

6a8b097

…d Render deployment; consolidate information in README.md

refactor: Enhance time entry form layout and improve time selection o…

917db5f

…ptions

feat: Update TimeEntry model to use boolean for time_out field; add d…

2f6bd6f

…atabase and user creation scripts for PostgreSQL

feat: Refactor user model and service to use 'user_active' instead of…

8807d99

… 'is_active'; enhance profile update functionality with email validation

feat: Update user model and service to use 'is_active' instead of 'us…

e57fa48

…er_active'; adjust related tests and versioning

feat: Update README and scripts to enhance Docker image build process…

2aaf8af

…; include version tagging and requirements update

cantis requested a review from Copilot November 10, 2025 15:42

Copilot AI reviewed Nov 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/add login #2

Feature/add login #2

Uh oh!

cantis commented Nov 10, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Copilot AI Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    """Update user detail"""
+    """
+    Update the details of an existing user.
+    Args:
+        user_id (int): The ID of the user to update.
+        username (Optional[str], optional): New username. Must be unique and at least 3 characters.
+        email (Optional[str], optional): New email address. Must be unique and valid.
+        password (Optional[str], optional): New password. Must be at least 6 characters.
+        is_admin (Optional[bool], optional): Whether the user is an admin.
+        is_active (Optional[bool], optional): Whether the user is active.
+    Returns:
+        User: The updated User object.
+    Raises:
+        UpdateUserError: If the user is not found, if username/email conflicts occur,
+            if validation fails, or if a database error occurs.
+    """

Feature/add login #2

Are you sure you want to change the base?

Feature/add login #2

Uh oh!

Conversation

cantis commented Nov 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants