diff --git a/app/templates/admin/users.html b/app/templates/admin/users.html index f518df5..8ed4420 100644 --- a/app/templates/admin/users.html +++ b/app/templates/admin/users.html @@ -62,6 +62,12 @@
{{ _('Edit') }} {{ _('Roles') }} + {% if user.id != current_user.id %} + + + {% endif %}
@@ -73,4 +79,43 @@ + + {% endblock %} diff --git a/docs/features/USER_DELETION.md b/docs/features/USER_DELETION.md new file mode 100644 index 0000000..cf1812b --- /dev/null +++ b/docs/features/USER_DELETION.md @@ -0,0 +1,327 @@ +# User Deletion Feature + +## Overview + +The user deletion feature allows administrators to permanently delete user accounts from the system. This feature includes comprehensive safety checks to prevent accidental deletion of critical data or system administrators. + +## Feature Implementation Date + +**Date**: October 29, 2025 +**Version**: Latest +**Status**: ✅ Complete + +## Access Control + +### Who Can Delete Users? + +- **Admin users**: Full access to delete any user (except themselves if they're the last admin) +- **Regular users**: No access to user deletion functionality +- **Permissions**: Requires `delete_users` permission + +### Who Cannot Be Deleted? + +1. **The last active administrator**: The system prevents deletion of the last active admin to ensure the system remains manageable +2. **Users with time entries**: Users who have logged time entries cannot be deleted to preserve data integrity +3. **Current logged-in user**: Users cannot delete their own account from the user list view + +## User Interface + +### Location + +The delete functionality is accessible from the **Admin Panel → Manage Users** page: + +``` +/admin/users +``` + +### UI Elements + +1. **Delete Button**: Appears next to each user (except current user) in the user list +2. **Confirmation Dialog**: Shows before deletion with appropriate warnings +3. **Error Messages**: Clear feedback when deletion is not allowed + +### Delete Button Behavior + +- **Visible**: For all users except the currently logged-in admin +- **Click Action**: Opens a confirmation dialog +- **Confirmation**: Shows user's name and warning about permanent deletion +- **With Time Entries**: Shows a special warning that the user cannot be deleted + +## Safety Checks + +### Pre-Deletion Validation + +The system performs the following checks before allowing deletion: + +#### 1. Admin Protection +```python +# Don't allow deleting the last admin +if user.is_admin: + admin_count = User.query.filter_by(role='admin', is_active=True).count() + if admin_count <= 1: + flash('Cannot delete the last administrator', 'error') + return redirect(url_for('admin.list_users')) +``` + +#### 2. Data Integrity Protection +```python +# Don't allow deleting users with time entries +if user.time_entries.count() > 0: + flash('Cannot delete user with existing time entries', 'error') + return redirect(url_for('admin.list_users')) +``` + +### Frontend Validation + +JavaScript validation checks time entry count before submitting the form: + +```javascript +function confirmDeleteUser(userId, username, timeEntriesCount) { + // Check if user has time entries + if (timeEntriesCount > 0) { + // Show warning dialog (cannot delete) + showConfirm('Cannot delete user...', { + variant: 'warning', + showCancel: false + }); + return false; + } + + // Show confirmation dialog + showConfirm('Are you sure...', { + variant: 'danger' + }).then(function(ok) { + if (ok) { + // Submit delete form + } + }); +} +``` + +## Database Cascading Behavior + +When a user is deleted, the following related data is automatically handled: + +### ✅ Cascaded (Deleted) + +1. **Time Entries**: All time entries are deleted (but deletion is blocked if any exist) +2. **Project Costs**: User-specific project cost records are deleted +3. **Favorite Projects**: User's favorite project associations are removed + +### ⚠️ Nullified (Set to NULL) + +1. **Task Assignments**: Tasks assigned to the user have `assigned_to` set to NULL +2. **User Roles**: Many-to-many role associations are removed + +### ❌ Protected (Prevents Deletion) + +1. **Created Tasks**: Users who created tasks cannot be deleted (enforced by database constraint) +2. **Time Entries**: Users with time entries cannot be deleted (enforced by application logic) + +## API Endpoints + +### Delete User + +**Endpoint**: `POST /admin/users//delete` + +**Authentication**: Required (Admin only) + +**Parameters**: +- `user_id` (path parameter): ID of the user to delete + +**Response Codes**: +- `200`: Success (redirects to user list with success message) +- `302`: Redirects with error message if deletion is not allowed +- `404`: User not found +- `403`: Insufficient permissions + +**Example Usage**: +```python +# Via route +url_for('admin.delete_user', user_id=123) + +# Expected redirect +→ /admin/users (with flash message) +``` + +## Testing + +The feature includes comprehensive tests: + +### Unit Tests (`tests/test_admin_users.py`) + +- ✅ Test successful user deletion +- ✅ Test deletion with time entries (should fail) +- ✅ Test deletion of last admin (should fail) +- ✅ Test deletion by non-admin (should be denied) +- ✅ Test deletion of non-existent user (404) +- ✅ Test UI shows/hides delete buttons appropriately + +### Model Tests (`tests/test_models_comprehensive.py`) + +- ✅ Test user deletion without relationships +- ✅ Test cascading to project costs +- ✅ Test cascading to time entries +- ✅ Test removal from favorite projects +- ✅ Test task assignment nullification +- ✅ Test protection for task creators + +### Smoke Tests (`tests/test_admin_users.py`) + +- ✅ End-to-end deletion workflow +- ✅ Critical safety checks +- ✅ UI accessibility tests +- ✅ Permission enforcement + +### Running Tests + +```bash +# Run all admin user tests +pytest tests/test_admin_users.py -v + +# Run only smoke tests +pytest tests/test_admin_users.py -v -m smoke + +# Run all user deletion model tests +pytest tests/test_models_comprehensive.py::test_user_deletion -v +``` + +## Error Messages + +| Scenario | Message | Action | +|----------|---------|--------| +| User has time entries | "Cannot delete user with existing time entries" | Show error, prevent deletion | +| Last administrator | "Cannot delete the last administrator" | Show error, prevent deletion | +| User not found | 404 error page | Show not found | +| No permission | Redirect to dashboard | Show access denied | +| Success | "User '[username]' deleted successfully" | Redirect to user list | + +## Implementation Details + +### Backend Route + +**File**: `app/routes/admin.py` + +**Function**: `delete_user(user_id)` + +**Decorators**: +- `@admin_bp.route('/admin/users//delete', methods=['POST'])` +- `@login_required` +- `@admin_or_permission_required('delete_users')` + +### Template + +**File**: `app/templates/admin/users.html` + +**Components**: +1. Delete button (conditional rendering) +2. Hidden form for DELETE request +3. JavaScript confirmation handler +4. Internationalized error messages + +## Security Considerations + +### CSRF Protection + +- All delete requests use POST method +- CSRF tokens are required (Flask-WTF) +- Forms include CSRF token validation + +### Permission Checks + +- Route-level permission enforcement via `@admin_or_permission_required` +- Additional checks in function body for special cases +- Session-based authentication required + +### Data Integrity + +- Database-level foreign key constraints +- Application-level validation before deletion +- Transaction rollback on errors + +## Best Practices for Administrators + +### Before Deleting a User + +1. **Check Time Entries**: Verify if the user has logged any time +2. **Transfer Data**: If needed, reassign tasks to other users +3. **Export Data**: Consider exporting user's data before deletion +4. **Notify Stakeholders**: Inform team members if the user was involved in active projects + +### When Deletion Fails + +1. **Time Entries Present**: + - Option 1: Keep the user as inactive instead of deleting + - Option 2: Archive time entries if appropriate + +2. **Last Admin**: + - Promote another user to admin role first + - Then delete the admin if still needed + +### Alternative to Deletion + +Instead of deleting users, consider: + +1. **Deactivate User**: Set `is_active = False` + - Preserves all data and relationships + - User cannot log in + - Can be reactivated if needed + +2. **Archive Projects**: Archive or complete any active projects first + +## Future Enhancements + +Potential improvements for this feature: + +- [ ] Soft delete option (mark as deleted but keep in database) +- [ ] Bulk user deletion +- [ ] User deletion audit log +- [ ] Export user data before deletion +- [ ] Reassign user's data to another user +- [ ] Deletion confirmation via email +- [ ] Admin approval workflow for user deletion + +## Troubleshooting + +### Issue: Cannot delete user + +**Cause**: User has time entries or is the last admin + +**Solution**: +1. Check error message for specific reason +2. For time entries: Consider deactivating instead +3. For last admin: Create another admin first + +### Issue: Delete button not showing + +**Cause**: May be the current logged-in user or permission issue + +**Solution**: +1. Verify you're logged in as admin +2. Check if trying to delete your own account +3. Verify `delete_users` permission + +### Issue: Permission denied + +**Cause**: User doesn't have admin rights or `delete_users` permission + +**Solution**: +1. Log in as an administrator +2. Check role assignments in permission system + +## Related Documentation + +- [User Management](../admin/USER_MANAGEMENT.md) +- [Permissions System](../security/PERMISSIONS.md) +- [Admin Panel](../admin/ADMIN_PANEL.md) +- [Testing Guide](../development/TESTING.md) + +## Changelog + +### Version 1.0 (October 29, 2025) +- ✅ Initial implementation of user deletion feature +- ✅ UI integration with user list page +- ✅ Safety checks for admin and data protection +- ✅ Comprehensive test coverage +- ✅ Documentation completed + diff --git a/tests/test_admin_users.py b/tests/test_admin_users.py new file mode 100644 index 0000000..fcefe77 --- /dev/null +++ b/tests/test_admin_users.py @@ -0,0 +1,624 @@ +""" +Tests for admin user management routes including user deletion. + +These tests cover: +- User listing +- User creation +- User editing +- User deletion (with various edge cases) +- Smoke tests for critical user deletion workflows +""" + +import pytest +from flask import url_for +from app.models import User, TimeEntry, Project, Client +from datetime import datetime, timedelta +from decimal import Decimal + + +class TestAdminUserList: + """Tests for listing users in admin panel.""" + + def test_list_users_as_admin(self, client, admin_user): + """Test that admin can view user list.""" + with client: + # Login as admin + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.list_users')) + assert response.status_code == 200 + assert b'Manage Users' in response.data + assert admin_user.username.encode() in response.data + + def test_list_users_as_regular_user_denied(self, client, user): + """Test that regular users cannot access user list.""" + with client: + # Login as regular user + with client.session_transaction() as sess: + sess['_user_id'] = str(user.id) + + response = client.get(url_for('admin.list_users')) + # Should redirect or show error + assert response.status_code in [302, 403] + + def test_list_users_unauthenticated(self, client): + """Test that unauthenticated users cannot access user list.""" + response = client.get(url_for('admin.list_users'), follow_redirects=False) + assert response.status_code == 302 # Redirect to login + + +class TestAdminUserCreation: + """Tests for creating users via admin panel.""" + + def test_create_user_get_form(self, client, admin_user): + """Test that admin can access user creation form.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.create_user')) + assert response.status_code == 200 + + def test_create_user_success(self, client, admin_user, app): + """Test successful user creation.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.create_user'), + data={ + 'username': 'newuser', + 'role': 'user' + }, + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'created successfully' in response.data + + # Verify user was created + with app.app_context(): + new_user = User.query.filter_by(username='newuser').first() + assert new_user is not None + assert new_user.role == 'user' + + def test_create_user_duplicate_username(self, client, admin_user, user): + """Test that creating a user with duplicate username fails.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.create_user'), + data={ + 'username': user.username, # Duplicate + 'role': 'user' + }, + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'already exists' in response.data + + def test_create_user_missing_username(self, client, admin_user): + """Test that creating a user without username fails.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.create_user'), + data={'role': 'user'}, + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'required' in response.data + + +class TestAdminUserEditing: + """Tests for editing users via admin panel.""" + + def test_edit_user_get_form(self, client, admin_user, user): + """Test that admin can access user edit form.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.edit_user', user_id=user.id)) + assert response.status_code == 200 + assert user.username.encode() in response.data + + def test_edit_user_success(self, client, admin_user, user, app): + """Test successful user editing.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.edit_user', user_id=user.id), + data={ + 'username': 'updateduser', + 'role': 'admin', + 'is_active': 'on' + }, + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'updated successfully' in response.data + + # Verify user was updated + with app.app_context(): + updated_user = User.query.get(user.id) + assert updated_user.username == 'updateduser' + assert updated_user.role == 'admin' + + def test_edit_user_deactivate(self, client, admin_user, user, app): + """Test deactivating a user.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.edit_user', user_id=user.id), + data={ + 'username': user.username, + 'role': user.role + # is_active is not checked, so user will be deactivated + }, + follow_redirects=True + ) + + assert response.status_code == 200 + + # Verify user was deactivated + with app.app_context(): + updated_user = User.query.get(user.id) + assert not updated_user.is_active + + +class TestAdminUserDeletion: + """Tests for deleting users via admin panel.""" + + def test_delete_user_success(self, client, admin_user, app): + """Test successful user deletion.""" + with app.app_context(): + # Create a user to delete + delete_user = User(username='deleteme', role='user') + delete_user.is_active = True + from app import db + db.session.add(delete_user) + db.session.commit() + user_id = delete_user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'deleted successfully' in response.data + + # Verify user was deleted + with app.app_context(): + deleted_user = User.query.get(user_id) + assert deleted_user is None + + def test_delete_user_with_time_entries_fails(self, client, admin_user, user, test_client, test_project, app): + """Test that deleting a user with time entries fails.""" + with app.app_context(): + # Create a time entry for the user + from app import db + time_entry = TimeEntry( + user_id=user.id, + project_id=test_project.id, + start_time=datetime.utcnow(), + end_time=datetime.utcnow() + timedelta(hours=1), + description='Test entry' + ) + db.session.add(time_entry) + db.session.commit() + user_id = user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'Cannot delete user with existing time entries' in response.data + + # Verify user was NOT deleted + with app.app_context(): + still_exists = User.query.get(user_id) + assert still_exists is not None + + def test_delete_last_admin_fails(self, client, admin_user, app): + """Test that deleting the last admin fails.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Try to delete the only admin + response = client.post( + url_for('admin.delete_user', user_id=admin_user.id), + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'Cannot delete the last administrator' in response.data + + # Verify admin was NOT deleted + with app.app_context(): + still_exists = User.query.get(admin_user.id) + assert still_exists is not None + + def test_delete_admin_with_multiple_admins_success(self, client, admin_user, app): + """Test that deleting an admin succeeds when there are multiple admins.""" + with app.app_context(): + # Create another admin + from app import db + admin2 = User(username='admin2', role='admin') + admin2.is_active = True + db.session.add(admin2) + db.session.commit() + admin2_id = admin2.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Delete the second admin + response = client.post( + url_for('admin.delete_user', user_id=admin2_id), + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'deleted successfully' in response.data + + # Verify admin2 was deleted + with app.app_context(): + deleted = User.query.get(admin2_id) + assert deleted is None + + def test_delete_user_as_regular_user_denied(self, client, user, app): + """Test that regular users cannot delete other users.""" + with app.app_context(): + # Create a user to delete + from app import db + delete_user = User(username='deleteme2', role='user') + delete_user.is_active = True + db.session.add(delete_user) + db.session.commit() + user_id = delete_user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(user.id) + + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=False + ) + + # Should be denied + assert response.status_code in [302, 403] + + # Verify user was NOT deleted + with app.app_context(): + still_exists = User.query.get(user_id) + assert still_exists is not None + + def test_delete_nonexistent_user_404(self, client, admin_user): + """Test that deleting a non-existent user returns 404.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.post( + url_for('admin.delete_user', user_id=99999), + follow_redirects=False + ) + + assert response.status_code == 404 + + def test_delete_user_unauthenticated(self, client, user): + """Test that unauthenticated users cannot delete users.""" + response = client.post( + url_for('admin.delete_user', user_id=user.id), + follow_redirects=False + ) + + assert response.status_code == 302 # Redirect to login + + def test_delete_inactive_admin_with_one_active_admin_fails(self, client, admin_user, app): + """Test that deleting an inactive admin when there's only one active admin fails.""" + with app.app_context(): + # Create an inactive admin + from app import db + inactive_admin = User(username='inactive_admin', role='admin') + inactive_admin.is_active = False + db.session.add(inactive_admin) + db.session.commit() + inactive_admin_id = inactive_admin.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Try to delete the active admin (only active admin) + response = client.post( + url_for('admin.delete_user', user_id=admin_user.id), + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'Cannot delete the last administrator' in response.data + + +class TestAdminUserDeletionCascading: + """Tests for cascading effects when deleting users.""" + + def test_delete_user_cascades_to_project_costs(self, client, admin_user, user, test_client, test_project, app): + """Test that deleting a user cascades to project costs.""" + with app.app_context(): + # Create a project cost for the user + from app.models import ProjectCost + from app import db + from datetime import date + project_cost = ProjectCost( + project_id=test_project.id, + user_id=user.id, + description='Test expense for user', + category='services', + amount=Decimal('75.00'), + cost_date=date.today() + ) + db.session.add(project_cost) + db.session.commit() + user_id = user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # User has no time entries, so deletion should succeed + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=True + ) + + assert response.status_code == 200 + assert b'deleted successfully' in response.data + + # Verify user and project costs were deleted + with app.app_context(): + from app.models import ProjectCost + deleted_user = User.query.get(user_id) + assert deleted_user is None + + # Project costs should be cascaded (deleted) + remaining_costs = ProjectCost.query.filter_by(user_id=user_id).all() + assert len(remaining_costs) == 0 + + def test_user_list_shows_delete_button_for_other_users(self, client, admin_user, user): + """Test that the user list shows delete button for other users.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.list_users')) + assert response.status_code == 200 + + # Should show delete button for the regular user + assert b'Delete' in response.data + assert f'confirmDeleteUser'.encode() in response.data + + def test_user_list_hides_delete_button_for_current_user(self, client, admin_user): + """Test that the user list doesn't show delete button for current user.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.list_users')) + assert response.status_code == 200 + + # Check that the JavaScript function exists + assert b'confirmDeleteUser' in response.data + + +# ============================================================================ +# Smoke Tests - Critical User Deletion Workflows +# ============================================================================ + +class TestUserDeletionSmokeTests: + """Smoke tests for critical user deletion workflows.""" + + @pytest.mark.smoke + def test_admin_can_delete_user_without_data(self, client, admin_user, app): + """SMOKE: Admin can successfully delete a user without any data.""" + with app.app_context(): + # Create a clean user + from app import db + clean_user = User(username='cleanuser', role='user') + clean_user.is_active = True + db.session.add(clean_user) + db.session.commit() + user_id = clean_user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Delete the user + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=True + ) + + # Should succeed + assert response.status_code == 200 + assert b'deleted successfully' in response.data + + # Verify deletion + with app.app_context(): + assert User.query.get(user_id) is None + + @pytest.mark.smoke + def test_cannot_delete_user_with_time_entries(self, client, admin_user, user, test_client, test_project, app): + """SMOKE: System prevents deletion of user with time entries.""" + with app.app_context(): + # Create time entry + from app import db + entry = TimeEntry( + user_id=user.id, + project_id=test_project.id, + start_time=datetime.utcnow(), + end_time=datetime.utcnow() + timedelta(hours=1), + description='Important work' + ) + db.session.add(entry) + db.session.commit() + user_id = user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Try to delete + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=True + ) + + # Should fail with appropriate message + assert response.status_code == 200 + assert b'Cannot delete user with existing time entries' in response.data + + # User should still exist + with app.app_context(): + assert User.query.get(user_id) is not None + + @pytest.mark.smoke + def test_cannot_delete_last_admin(self, client, admin_user, app): + """SMOKE: System prevents deletion of the last administrator.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Try to delete the only admin + response = client.post( + url_for('admin.delete_user', user_id=admin_user.id), + follow_redirects=True + ) + + # Should fail + assert response.status_code == 200 + assert b'Cannot delete the last administrator' in response.data + + # Admin should still exist + with app.app_context(): + assert User.query.get(admin_user.id) is not None + + @pytest.mark.smoke + def test_user_list_accessible_to_admin(self, client, admin_user): + """SMOKE: Admin can access user list page.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.list_users')) + + # Should succeed + assert response.status_code == 200 + assert b'Manage Users' in response.data + + @pytest.mark.smoke + def test_regular_user_cannot_access_user_deletion(self, client, user, app): + """SMOKE: Regular users cannot access user deletion functionality.""" + with app.app_context(): + # Create another user + from app import db + other_user = User(username='otheruser', role='user') + other_user.is_active = True + db.session.add(other_user) + db.session.commit() + other_user_id = other_user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(user.id) + + # Try to delete + response = client.post( + url_for('admin.delete_user', user_id=other_user_id), + follow_redirects=False + ) + + # Should be denied + assert response.status_code in [302, 403] + + @pytest.mark.smoke + def test_delete_button_appears_in_ui(self, client, admin_user, user): + """SMOKE: Delete button appears in user list UI.""" + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + response = client.get(url_for('admin.list_users')) + + # Should show delete functionality + assert response.status_code == 200 + assert b'Delete' in response.data + assert b'confirmDeleteUser' in response.data + + @pytest.mark.smoke + def test_complete_user_deletion_workflow(self, client, admin_user, app): + """SMOKE: Complete end-to-end user deletion workflow.""" + with app.app_context(): + # Step 1: Create user + from app import db + new_user = User(username='workflowuser', role='user') + new_user.is_active = True + db.session.add(new_user) + db.session.commit() + user_id = new_user.id + + with client: + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + + # Step 2: View user list (should show user) + response = client.get(url_for('admin.list_users')) + assert response.status_code == 200 + assert b'workflowuser' in response.data + + # Step 3: Delete user + response = client.post( + url_for('admin.delete_user', user_id=user_id), + follow_redirects=True + ) + assert response.status_code == 200 + assert b'deleted successfully' in response.data + + # Step 4: Verify user list no longer shows user + response = client.get(url_for('admin.list_users')) + assert response.status_code == 200 + assert b'workflowuser' not in response.data + + # Step 5: Verify user is actually deleted + with app.app_context(): + assert User.query.get(user_id) is None + diff --git a/tests/test_models_comprehensive.py b/tests/test_models_comprehensive.py index 7c3b44d..ad77765 100644 --- a/tests/test_models_comprehensive.py +++ b/tests/test_models_comprehensive.py @@ -513,3 +513,282 @@ def test_time_entry_requires_start_time(app, user, project): db.session.add(entry) db.session.commit() + +# ============================================================================ +# User Deletion and Cascading Tests +# ============================================================================ + +@pytest.mark.unit +@pytest.mark.models +def test_user_deletion_without_relationships(app): + """Test that a user without relationships can be deleted.""" + with app.app_context(): + # Create a user with no relationships + delete_user = User(username='deletable', role='user') + delete_user.is_active = True + db.session.add(delete_user) + db.session.commit() + user_id = delete_user.id + + # Delete the user + db.session.delete(delete_user) + db.session.commit() + + # Verify deletion + deleted = User.query.get(user_id) + assert deleted is None + + +@pytest.mark.unit +@pytest.mark.models +def test_user_deletion_cascades_project_costs(app, test_client): + """Test that deleting a user cascades to project costs.""" + from app.models import ProjectCost + from datetime import date + + with app.app_context(): + # Create user and project + user = User(username='costuser', role='user') + user.is_active = True + db.session.add(user) + + project = Project( + name='Cost Test Project', + client_id=test_client.id, + billable=True + ) + db.session.add(project) + db.session.commit() + + # Create project cost + cost = ProjectCost( + project_id=project.id, + user_id=user.id, + description='Test expense', + category='materials', + amount=Decimal('100.00'), + cost_date=date.today() + ) + db.session.add(cost) + db.session.commit() + + user_id = user.id + cost_id = cost.id + + # Delete user + db.session.delete(user) + db.session.commit() + + # Verify user is deleted + deleted_user = User.query.get(user_id) + assert deleted_user is None + + # Verify project cost is cascaded (deleted) + deleted_cost = ProjectCost.query.get(cost_id) + assert deleted_cost is None + + +@pytest.mark.unit +@pytest.mark.models +def test_user_deletion_cascades_time_entries(app, test_client): + """Test that deleting a user cascades to time entries.""" + with app.app_context(): + # Create user and project + user = User(username='entryuser', role='user') + user.is_active = True + db.session.add(user) + + project = Project( + name='Entry Test Project', + client_id=test_client.id, + billable=True + ) + db.session.add(project) + db.session.commit() + + # Create time entry + entry = TimeEntry( + user_id=user.id, + project_id=project.id, + start_time=datetime.utcnow(), + end_time=datetime.utcnow() + timedelta(hours=1), + description='Test entry' + ) + db.session.add(entry) + db.session.commit() + + user_id = user.id + entry_id = entry.id + + # Delete user + db.session.delete(user) + db.session.commit() + + # Verify user is deleted + deleted_user = User.query.get(user_id) + assert deleted_user is None + + # Verify time entry is cascaded (deleted) + deleted_entry = TimeEntry.query.get(entry_id) + assert deleted_entry is None + + +@pytest.mark.unit +@pytest.mark.models +def test_user_deletion_removes_from_favorite_projects(app, test_client): + """Test that deleting a user removes them from favorite projects.""" + with app.app_context(): + # Create user and project + user = User(username='favuser', role='user') + user.is_active = True + db.session.add(user) + + project = Project( + name='Favorite Test Project', + client_id=test_client.id, + billable=True + ) + db.session.add(project) + db.session.commit() + + # Add project to favorites + user.favorite_projects.append(project) + db.session.commit() + + # Verify favorite was added + assert project in user.favorite_projects.all() + + user_id = user.id + project_id = project.id + + # Delete user + db.session.delete(user) + db.session.commit() + + # Verify user is deleted + deleted_user = User.query.get(user_id) + assert deleted_user is None + + # Verify project still exists (favorites are many-to-many) + remaining_project = Project.query.get(project_id) + assert remaining_project is not None + + # Verify user is not in project's favorited_by + assert user_id not in [u.id for u in remaining_project.favorited_by.all()] + + +@pytest.mark.unit +@pytest.mark.models +def test_user_deletion_preserves_tasks_assigned_to_them(app, test_client): + """Test that deleting a user preserves tasks but nullifies assigned_to.""" + with app.app_context(): + # Create users and project + creator = User(username='creator', role='user') + creator.is_active = True + assignee = User(username='assignee', role='user') + assignee.is_active = True + db.session.add_all([creator, assignee]) + + project = Project( + name='Task Test Project', + client_id=test_client.id, + billable=True + ) + db.session.add(project) + db.session.commit() + + # Create task + task = Task( + project_id=project.id, + name='Test Task', + description='Test description', + created_by=creator.id, + assigned_to=assignee.id + ) + db.session.add(task) + db.session.commit() + + assignee_id = assignee.id + task_id = task.id + + # Delete assignee + db.session.delete(assignee) + db.session.commit() + + # Verify assignee is deleted + deleted_user = User.query.get(assignee_id) + assert deleted_user is None + + # Verify task still exists but assigned_to is nullified + remaining_task = Task.query.get(task_id) + assert remaining_task is not None + assert remaining_task.assigned_to is None + + +@pytest.mark.unit +@pytest.mark.models +def test_user_cannot_be_deleted_if_has_created_tasks(app, test_client): + """Test that deleting a user who created tasks cascades properly.""" + from sqlalchemy.exc import IntegrityError + + with app.app_context(): + # Create user and project + creator = User(username='taskcreator', role='user') + creator.is_active = True + db.session.add(creator) + + project = Project( + name='Task Creator Project', + client_id=test_client.id, + billable=True + ) + db.session.add(project) + db.session.commit() + + # Create task + task = Task( + project_id=project.id, + name='Created Task', + description='Test description', + created_by=creator.id + ) + db.session.add(task) + db.session.commit() + + creator_id = creator.id + + # Try to delete creator - should raise IntegrityError because created_by is NOT NULL + with pytest.raises(IntegrityError): + db.session.delete(creator) + db.session.commit() + + db.session.rollback() + + # Verify creator still exists + still_exists = User.query.get(creator_id) + assert still_exists is not None + + +@pytest.mark.unit +@pytest.mark.models +def test_user_deletion_count_check(app): + """Test that we can query user count before and after deletion.""" + with app.app_context(): + # Get initial count + initial_count = User.query.count() + + # Create and delete a user + temp_user = User(username='tempuser', role='user') + temp_user.is_active = True + db.session.add(temp_user) + db.session.commit() + + # Verify count increased + assert User.query.count() == initial_count + 1 + + # Delete user + db.session.delete(temp_user) + db.session.commit() + + # Verify count back to initial + assert User.query.count() == initial_count \ No newline at end of file