fix(project-tree): split unfiled loading #28761
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR improves project tree loading performance by splitting unfiled item loading and filtering out unsaved insights, particularly addressing issues with large datasets.
- Added type filtering in
/posthog/api/file_system.pywith newUnfiledFilesQuerySerializerto load unfiled items by type - Modified
/posthog/models/file_system.pyto filter insights withsaved=Trueand support type-specific collection - Updated
/frontend/src/layout/navigation-3000/components/projectTreeLogic.tsxto load unfiled items separately by type with concurrent loading states - Added
test_unfiled_endpoint_with_type_filteringin/posthog/api/test/test_file_system.pyto verify type filtering functionality
5 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| loadSavedItems: async () => { | ||
| const response = await api.fileSystem.list() | ||
| return response.results | ||
| return [...values.savedItems, ...response.results] | ||
| }, |
There was a problem hiding this comment.
logic: Potential issue with duplicate items since existing items are concatenated with new results without deduplication
| loadSavedItems: async () => { | |
| const response = await api.fileSystem.list() | |
| return response.results | |
| return [...values.savedItems, ...response.results] | |
| }, | |
| loadSavedItems: async () => { | |
| const response = await api.fileSystem.list() | |
| return response.results | |
| }, |
| const response = await api.fileSystem.unfiled(type) | ||
| return [...values.allUnfiledItems, ...response.results] | ||
| }, |
There was a problem hiding this comment.
logic: Same duplication issue as above - concatenating results without deduplication could lead to duplicate entries in allUnfiledItems
| const response = await api.fileSystem.unfiled(type) | |
| return [...values.allUnfiledItems, ...response.results] | |
| }, | |
| const response = await api.fileSystem.unfiled(type) | |
| return response.results | |
| }, |
| # Check that the type filtering works | ||
| response = self.client.get(f"/api/projects/{self.team.id}/file_system/unfiled/?type=feature_flag") | ||
| self.assertEqual(response.status_code, status.HTTP_200_OK, response.json()) | ||
| self.assertEqual(response.json()["count"], 1) |
There was a problem hiding this comment.
logic: test should also verify that other types return 0 results and that invalid types return an appropriate error response
| type = serializers.ChoiceField( | ||
| choices=[(choice.value, choice.value) for choice in FileSystemType], required=False, allow_blank=True | ||
| ) |
There was a problem hiding this comment.
style: choices should be defined as a class constant to avoid recreating the list on every serializer instantiation
| def collect(self, file_type: FileSystemType) -> list[FileSystem]: | ||
| if file_type == FileSystemType.FEATURE_FLAG: | ||
| return self.collect_feature_flags() | ||
| elif file_type == FileSystemType.EXPERIMENT: | ||
| return self.collect_experiments() | ||
| elif file_type == FileSystemType.INSIGHT: | ||
| return self.collect_insights() | ||
| elif file_type == FileSystemType.DASHBOARD: | ||
| return self.collect_dashboards() | ||
| elif file_type == FileSystemType.NOTEBOOK: | ||
| return self.collect_notebooks() | ||
| return [] |
There was a problem hiding this comment.
style: no error handling for invalid FileSystemType values - could return unexpected empty list
|
Size Change: +48 B (0%) Total Size: 1.21 MB ℹ️ View Unchanged
|
Problem
The
tree-viewflag doesn't load for our team as we have too many unsaved insights.Changes
Even this new approach is a temporary one. We should build this tree out beforehand, and integrate into the products.
This should however make something load for our team in production.
Does this work well for both Cloud and self-hosted?
Yep
How did you test this code?
Added a test for API filtering, tested locally in the browser