feat(project-tree): persist unfiled

frontend/src/layout/navigation-3000/components/ProjectTree/ProjectTree.tsx

@@ -17,6 +17,7 @@ import { FileSystemEntry } from '~/queries/schema/schema-general'
 import { navigation3000Logic } from '../../navigationLogic'
 import { NavbarBottom } from '../NavbarBottom'
 import { projectTreeLogic } from './projectTreeLogic'
+import { joinPath, splitPath } from './utils'
 
 // TODO: Swap out for a better DnD approach
 // Currently you can only drag the title text, and must click on the icon or to the right of it to trigger a click
@@ -72,19 +73,21 @@ export function ProjectTree(): JSX.Element {
                                     }
                                 }
                             } else if (folder === '') {
-                                const oldSplit = oldPath.split('/')
+                                const oldSplit = splitPath(oldPath)
                                 const oldFile = oldSplit.pop()
                                 if (oldFile && oldSplit.length > 0) {
-                                    moveItem(oldPath, oldFile)
+                                    moveItem(oldPath, joinPath([oldFile]))
                                 }
                             } else if (folder) {
                                 const item = viableItems.find((i) => i.path === folder)
                                 if (!item || item.type === 'folder') {
-                                    const oldSplit = oldPath.split('/')
+                                    const oldSplit = splitPath(oldPath)
                                     const oldFile = oldSplit.pop()
-                                    const newFile = folder + '/' + oldFile
-                                    if (newFile !== oldPath) {
-                                        moveItem(oldPath, newFile)
+                                    if (oldFile) {
+                                        const newFile = joinPath([...splitPath(String(folder)), oldFile])
+                                        if (newFile !== oldPath) {
+                                            moveItem(oldPath, newFile)
+                                        }
                                     }
                                 }
                             }
@@ -147,7 +150,10 @@ export function ProjectTree(): JSX.Element {
                                                                 if (folder) {
                                                                     addFolder(
                                                                         record.path
-                                                                            ? record.path + '/' + folder
+                                                                            ? joinPath([
+                                                                                  ...splitPath(record.path),
+                                                                                  folder,
+                                                                              ])
                                                                             : folder
                                                                     )
                                                                 }
@@ -161,9 +167,18 @@ export function ProjectTree(): JSX.Element {
                                                         <LemonButton
                                                             onClick={() => {
                                                                 const oldPath = record.path
-                                                                const folder = prompt('New name?', oldPath)
-                                                                if (folder) {
-                                                                    moveItem(oldPath, folder)
+                                                                const splits = splitPath(oldPath)
+                                                                if (splits.length > 0) {
+                                                                    const folder = prompt(
+                                                                        'New name?',
+                                                                        splits[splits.length - 1]
+                                                                    )
+                                                                    if (folder) {
+                                                                        moveItem(
+                                                                            oldPath,
+                                                                            joinPath([...splits.slice(0, -1), folder])
+                                                                        )
+                                                                    }
                                                                 }
                                                             }}
                                                             fullWidth

frontend/src/layout/navigation-3000/components/ProjectTree/projectTreeLogic.tsx

@@ -16,7 +16,7 @@ import { FileSystemEntry, FileSystemType } from '~/queries/schema/schema-general
 import { getDefaultTree } from './defaultTree'
 import type { projectTreeLogicType } from './projectTreeLogicType'
 import { FileSystemImport, ProjectTreeAction } from './types'
-import { convertFileSystemEntryToTreeDataItem } from './utils'
+import { convertFileSystemEntryToTreeDataItem, joinPath, splitPath } from './utils'
 
 export const projectTreeLogic = kea<projectTreeLogicType>([
     path(['layout', 'navigation-3000', 'components', 'projectTreeLogic']),
@@ -170,9 +170,9 @@ export const projectTreeLogic = kea<projectTreeLogicType>([
                     if (action.type === 'move-create' || action.type === 'move' || action.type === 'create') {
                         if (action.newPath) {
                             unappliedPaths[action.newPath] = true
-                            const split = action.newPath.split('/')
+                            const split = splitPath(action.newPath)
                             for (let i = 1; i < split.length; i++) {
-                                unappliedPaths[split.slice(0, i).join('/')] = true
+                                unappliedPaths[joinPath(split.slice(0, i))] = true
                             }
                         }
                     }

frontend/src/layout/navigation-3000/components/ProjectTree/utils.test.ts

@@ -0,0 +1,41 @@
+import { escapePath, joinPath, splitPath } from './utils'
+
+describe('project tree utils', () => {
+    describe('escapePath', () => {
+        it('escapes paths as expected', () => {
+            expect(escapePath('a/b')).toEqual('a\\/b')
+            expect(escapePath('a/b\\')).toEqual('a\\/b\\\\')
+            expect(escapePath('a/b/c')).toEqual('a\\/b\\/c')
+            expect(escapePath('a\n\t')).toEqual('a\n\t')
+            expect(escapePath('a')).toEqual('a')
+            expect(escapePath('')).toEqual('')
+        })
+    })
+
+    describe('splitPath', () => {
+        it('splits paths as expected', () => {
+            expect(splitPath('a/b')).toEqual(['a', 'b'])
+            expect(splitPath('a\\/b/c')).toEqual(['a/b', 'c'])
+            expect(splitPath('a\\/b\\\\/c')).toEqual(['a/b\\', 'c'])
+            expect(splitPath('a\\/b\\/c')).toEqual(['a/b/c'])
+            expect(splitPath('a\n\t/b')).toEqual(['a\n\t', 'b'])
+            expect(splitPath('a\\n\\t/b')).toEqual(['a\\n\\t', 'b'])
+            expect(splitPath('a\\\\n\\t/b')).toEqual(['a\\n\\t', 'b'])
+            expect(splitPath('a')).toEqual(['a'])
+            expect(splitPath('')).toEqual([])
+        })
+    })
+
+    describe('joinPath', () => {
+        it('joins paths as expected', () => {
+            expect(joinPath(['a', 'b'])).toEqual('a/b')
+            expect(joinPath(['a/b', 'c'])).toEqual('a\\/b/c')
+            expect(joinPath(['a/b\\', 'c'])).toEqual('a\\/b\\\\/c')
+            expect(joinPath(['a/b/c'])).toEqual('a\\/b\\/c')
+            expect(joinPath(['a\n\t', 'b'])).toEqual('a\n\t/b')
+            expect(joinPath(['a\\n\\t', 'b'])).toEqual('a\\\\n\\\\t/b')
+            expect(joinPath(['a'])).toEqual('a')
+            expect(joinPath([])).toEqual('')
+        })
+    })
+})

frontend/src/layout/navigation-3000/components/ProjectTree/utils.ts

@@ -28,21 +28,21 @@ export function convertFileSystemEntryToTreeDataItem(imports: FileSystemImport[]
 
     // Iterate over each raw project item.
     for (const item of imports) {
-        const pathSplit = item.path.split('/').filter(Boolean)
+        const pathSplit = splitPath(item.path)
         const itemName = pathSplit.pop()!
-        const folderPath = pathSplit.join('/')
+        const folderPath = joinPath(pathSplit)
 
         // Split the folder path by "/" (ignoring empty parts).
-        const folderParts = folderPath ? folderPath.split('/').filter(Boolean) : []
+        const folderParts = folderPath ? splitPath(folderPath) : []
 
         // Start at the root level.
         let currentLevel = rootNodes
-        let accumulatedPath = ''
+        const accumulatedPath: string[] = []
 
         // Create (or find) nested folders as needed.
         for (const part of folderParts) {
-            accumulatedPath = accumulatedPath ? accumulatedPath + '/' + part : part
-            const folderNode = findOrCreateFolder(currentLevel, part, accumulatedPath)
+            accumulatedPath.push(part)
+            const folderNode = findOrCreateFolder(currentLevel, part, joinPath(accumulatedPath))
             currentLevel = folderNode.children!
         }
 
@@ -78,3 +78,39 @@ export function convertFileSystemEntryToTreeDataItem(imports: FileSystemImport[]
     sortNodes(rootNodes)
     return rootNodes
 }
+
+/**
+ * Splits `path` by unescaped "/" delimiters.
+ *   - splitPath("a/b")            => ["a", "b"]
+ *   - splitPath("a\\/b/c")        => ["a/b", "c"]
+ *   - splitPath("a\\/b\\\\/c")    => ["a/b\\", "c"]
+ *   - splitPath("a\n\t/b")        => ["a\n\t", "b"]
+ *   - splitPath("a")              => ["a"]
+ *   - splitPath("")               => []
+ */
+export function splitPath(path: string): string[] {
+    const segments: string[] = []
+    let current = ''
+    for (let i = 0; i < path.length; i++) {
+        if (path[i] === '\\' && i < path.length - 1 && (path[i + 1] === '/' || path[i + 1] === '\\')) {
+            current += path[i + 1]
+            i++
+            continue
+        } else if (path[i] === '/') {
+            segments.push(current)
+            current = ''
+        } else {
+            current += path[i]
+        }
+    }
+    segments.push(current)
+    return segments.filter((s) => s !== '')
+}
+
+export function joinPath(path: string[]): string {
+    return path.map(escapePath).join('/')
+}
+
+export function escapePath(path: string): string {
+    return path.replace(/\\/g, '\\\\').replace(/\//g, '\\/')
+}

posthog/api/file_system.py

@@ -8,7 +8,7 @@
 from posthog.api.utils import action
 from posthog.api.routing import TeamAndOrgViewSetMixin
 from posthog.api.shared import UserBasicSerializer
-from posthog.models.file_system import FileSystem, get_unfiled_files
+from posthog.models.file_system import FileSystem, save_unfiled_files
 from posthog.models.user import User
 from posthog.schema import FileSystemType
 
@@ -80,7 +80,7 @@ def unfiled(self, request: Request, *args: Any, **kwargs: Any) -> Response:
         query_serializer = UnfiledFilesQuerySerializer(data=request.query_params)
         query_serializer.is_valid(raise_exception=True)
         file_type = query_serializer.validated_data.get("type")
-        files = get_unfiled_files(self.team, cast(User, request.user), file_type)
+        files = save_unfiled_files(self.team, cast(User, request.user), file_type)
 
         return Response(
             {

posthog/api/test/test_file_system.py

@@ -30,7 +30,7 @@ def test_create_file(self):
         self.assertEqual(response_data["path"], "MyFolder/Document.txt")
         self.assertEqual(response_data["type"], "doc-file")
         self.assertDictEqual(response_data["meta"], {"description": "A test file"})
-        self.assertEqual(response_data["created_by"]["id"], self.user.pk)  # The user who created it
+        self.assertEqual(response_data["created_by"]["id"], self.user.pk)
 
     def test_retrieve_file(self):
         """
@@ -42,7 +42,6 @@ def test_retrieve_file(self):
             type="test-type",
             created_by=self.user,
         )
-
         response = self.client.get(f"/api/projects/{self.team.id}/file_system/{file_obj.pk}/")
         self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
 
@@ -68,7 +67,6 @@ def test_update_file(self):
         self.assertEqual(updated_data["path"], "NewPath/file.txt")
         self.assertEqual(updated_data["type"], "new-type")
 
-        # Verify changes in DB
         file_obj.refresh_from_db()
         self.assertEqual(file_obj.path, "NewPath/file.txt")
         self.assertEqual(file_obj.type, "new-type")
@@ -82,39 +80,38 @@ def test_delete_file(self):
         )
         delete_response = self.client.delete(f"/api/projects/{self.team.id}/file_system/{file_obj.pk}/")
         self.assertEqual(delete_response.status_code, status.HTTP_204_NO_CONTENT)
-
-        # Confirm it's gone
         self.assertFalse(FileSystem.objects.filter(pk=file_obj.pk).exists())
 
     def test_unfiled_endpoint_no_content(self):
         """
-        If there are no relevant FeatureFlags, Experiments, etc. for this team,
-        'unfiled' should return an empty list.
+        If there are no relevant items to create (e.g. no FeatureFlags, Experiments, etc.),
+        'unfiled' should return an empty list and create nothing in the DB.
         """
         response = self.client.get(f"/api/projects/{self.team.id}/file_system/unfiled/")
         self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
         data = response.json()
         self.assertEqual(data["count"], 0)
         self.assertEqual(data["results"], [])
+        self.assertEqual(FileSystem.objects.count(), 0)
 
     def test_unfiled_endpoint_with_content(self):
         """
         If we create some FeatureFlags, Experiments, Dashboards, Insights, or Notebooks,
-        they should show up as ephemeral FileSystem items in the unfiled list.
+        the 'unfiled' endpoint should create them in FileSystem and return them.
         """
         feature_flag = FeatureFlag.objects.create(team=self.team, name="Beta Feature", created_by=self.user)
         Experiment.objects.create(team=self.team, name="Experiment #1", created_by=self.user, feature_flag=feature_flag)
         Dashboard.objects.create(team=self.team, name="User Dashboard", created_by=self.user)
         Insight.objects.create(team=self.team, saved=True, name="Marketing Insight", created_by=self.user)
         Notebook.objects.create(team=self.team, title="Data Exploration", created_by=self.user)
 
-        # Now call the endpoint
         response = self.client.get(f"/api/projects/{self.team.id}/file_system/unfiled/")
         self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())
 
         data = response.json()
         results = data["results"]
-        self.assertGreaterEqual(len(results), 5, results)  # We expect at least 5 ephemeral items
+        self.assertEqual(len(results), 5, results)
+        self.assertEqual(FileSystem.objects.count(), 5)
 
         # Check that each type is present
         types = [item["type"] for item in results]
@@ -126,23 +123,46 @@ def test_unfiled_endpoint_with_content(self):
 
     def test_unfiled_endpoint_with_type_filtering(self):
         """
-        Ensure that the 'type' query parameter works as expected.
+        Ensure that the 'type' query parameter filters creation to a single type.
         """
-        feature_flag = FeatureFlag.objects.create(team=self.team, name="Beta Feature", created_by=self.user)
-        Experiment.objects.create(team=self.team, name="Experiment #1", created_by=self.user, feature_flag=feature_flag)
+        flag = FeatureFlag.objects.create(team=self.team, name="Only Flag", created_by=self.user)
+        Experiment.objects.create(team=self.team, name="Experiment #1", feature_flag=flag, created_by=self.user)
 
-        # Check that the type filtering works
+        # Filter for feature_flag only
         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)
+        data = response.json()
+        self.assertEqual(data["count"], 1)
+        self.assertEqual(FileSystem.objects.count(), 1)
+        self.assertEqual(data["results"][0]["type"], FileSystemType.FEATURE_FLAG)
+
+        # Check that no experiment was created
+        self.assertFalse(FileSystem.objects.filter(type=FileSystemType.EXPERIMENT).exists())
+
+    def test_unfiled_endpoint_is_idempotent(self):
+        """
+        Calling the unfiled endpoint multiple times should not create duplicate FileSystem rows
+        for the same objects.
+        """
+        FeatureFlag.objects.create(team=self.team, name="Beta Feature", created_by=self.user)
+        first_response = self.client.get(f"/api/projects/{self.team.id}/file_system/unfiled/")
+        self.assertEqual(first_response.status_code, status.HTTP_200_OK)
+        self.assertEqual(first_response.json()["count"], 1)
+        self.assertEqual(FileSystem.objects.count(), 1)
+
+        # Second call
+        second_response = self.client.get(f"/api/projects/{self.team.id}/file_system/unfiled/")
+        self.assertEqual(second_response.status_code, status.HTTP_200_OK)
+        self.assertEqual(second_response.json()["count"], 0, second_response.json())  # No new items
+        self.assertEqual(FileSystem.objects.count(), 1)
 
     def test_search_files_by_path(self):
         """
         Ensure the search functionality is working on the 'path' field.
         """
-        FileSystem.objects.create(team=self.team, path="Analytics/Report 1", type="report")
-        FileSystem.objects.create(team=self.team, path="Analytics/Report 2", type="report")
-        FileSystem.objects.create(team=self.team, path="Random/Other File", type="misc")
+        FileSystem.objects.create(team=self.team, path="Analytics/Report 1", type="report", created_by=self.user)
+        FileSystem.objects.create(team=self.team, path="Analytics/Report 2", type="report", created_by=self.user)
+        FileSystem.objects.create(team=self.team, path="Random/Other File", type="misc", created_by=self.user)
 
         response = self.client.get(f"/api/projects/{self.team.id}/file_system/?search=Analytics")
         self.assertEqual(response.status_code, status.HTTP_200_OK, response.json())