feat(project-tree): activity log #33303
Conversation
There was a problem hiding this comment.
PR Summary
Added activity logging functionality for file system entries, introducing tracking for create, update, and delete operations across the backend infrastructure.
- Implemented
log_file_system_activity()helper inposthog/api/file_system/file_system.pyto track file operations consistently - Added activity tracking points in key file system operations (create, update, destroy, move, link) with appropriate scope and detail logging
- Extended
ActivityScopeenum infrontend/src/types.tsto include 'FILE_SYSTEM' for proper type support - Comprehensive test coverage in
test_file_system.pyvalidates activity logging for all file system operations
While the core functionality is solid, recommend adding error handling for the activity logging to prevent operation failures if logging fails.
4 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
| create_resp = self.client.post( | ||
| f"/api/projects/{self.team.id}/file_system/", | ||
| {"path": "Logged/File.txt", "type": "doc"}, | ||
| ) | ||
| self.assertEqual(create_resp.status_code, status.HTTP_201_CREATED) | ||
| from posthog.models import ActivityLog | ||
|
|
||
| activity = ActivityLog.objects.filter(scope="FileSystem", activity="created").first() | ||
| assert activity is not None |
There was a problem hiding this comment.
style: Import ActivityLog at the top of the file with other imports rather than inside the test method
| create_resp = self.client.post( | |
| f"/api/projects/{self.team.id}/file_system/", | |
| {"path": "Logged/File.txt", "type": "doc"}, | |
| ) | |
| self.assertEqual(create_resp.status_code, status.HTTP_201_CREATED) | |
| from posthog.models import ActivityLog | |
| activity = ActivityLog.objects.filter(scope="FileSystem", activity="created").first() | |
| assert activity is not None | |
| import pytest | |
| from freezegun import freeze_time | |
| from rest_framework import status | |
| from rest_framework.test import APIRequestFactory | |
| from posthog.test.base import APIBaseTest | |
| from posthog.models import User, FeatureFlag, Dashboard, Experiment, Insight, Notebook, Team, Project, ActivityLog | |
| from posthog.models.file_system.file_system import FileSystem | |
| from unittest.mock import patch | |
| from ee.models.rbac.access_control import AccessControl | |
| from datetime import UTC |
|
|
||
| activity = ActivityLog.objects.filter(scope="FileSystem", activity="created").first() | ||
| assert activity is not None |
There was a problem hiding this comment.
style: Consider using assertIsNotNone() instead of assert activity is not None for consistency with other assertions in the test class
| log_file_system_activity( | ||
| activity="updated", | ||
| file_system=instance, | ||
| user=request.user, | ||
| was_impersonated=is_impersonated_session(request), | ||
| changes=[ | ||
| Change( | ||
| type="FileSystem", | ||
| field="path", | ||
| action="changed", | ||
| before=old_path, | ||
| after=instance.path, | ||
| ) | ||
| ], | ||
| ) |
There was a problem hiding this comment.
logic: Potential bug: logging activity with instance that has no ID (pk=None was just called). Use original instance for logging.
|
Size Change: -98 B (0%) Total Size: 3.76 MB ℹ️ View Unchanged
|
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
Problem
File system entries don't have activity logs.
Changes
Add 'em. Only storing the data in the backend now, not displaying it anywhere.
How did you test this code?
There's a test