Refactor common code to use full paths #4393
Conversation
AdityaHegde
force-pushed
the
adityahegde/folder-structure-commons-refactor
branch
from
90db1d2
to
47baed1
Compare
| import NavigationMenuItem from "@rilldata/web-common/layout/navigation/NavigationMenuItem.svelte"; | ||
| import { runtime } from "../../runtime-client/runtime-store"; | ||
| import { deleteFileArtifact } from "../entity-management/actions"; | ||
| import { EntityType } from "../entity-management/types"; | ||
| import { useChartFileNames } from "./selectors"; | ||
| import { useChartRoutes } from "./selectors"; | ||
|
|
||
| export let chartName: string; |
There was a problem hiding this comment.
Rather than chartName, this should now be chartPath. Then we can use the path directly in the deleteFileArtifact function.
There was a problem hiding this comment.
Agree. This PR makes an effort to not touch the components to update such params.
| getFileAPIPathFromNameAndType(currentAssetName, entityType), | ||
| getFileAPIPathFromNameAndType(values.newName, entityType), |
There was a problem hiding this comment.
When we migrate, the modal should take a fileName as prop, not an assetName
| const route = getRouteFromName(getNextEntityName(names, name), type); | ||
|
|
||
| await goto(route); | ||
| await goto(getNextEntityName(allPaths, name)); |
There was a problem hiding this comment.
When we migrate, navigation to the "next entity name" will be confusing in cases where users do not have a sources/, models/, dashboards/ directory structure. So instead, to avoid too much magic in the code, I think we should navigate to the root / in all cases.
There was a problem hiding this comment.
Hmm true. I thought we could navigate to the next file in the folder. But i havent seen any editor do that.
| import { | ||
| extractFileExtension, | ||
| sanitizeEntityName, | ||
| } from "web-common/src/features/sources/extract-file-name"; |
There was a problem hiding this comment.
This import looks off – shouldn't it include "@rilldata/web-common/..."?
| @@ -7,6 +7,7 @@ import { createRuntimeServiceListFiles } from "@rilldata/web-common/runtime-clie | |||
| export function useMainEntityFiles( | |||
| instanceId: string, | |||
| prefix: "sources" | "models" | "dashboards" | "charts" | "custom-dashboards", | |||
| transform = (name: string) => name, | |||
There was a problem hiding this comment.
Rather than this transform parameter, can we instead just edit the createRuntimeServiceListFiles selector function to return file paths not file names?
There was a problem hiding this comment.
Yes, but it is still used in a lot of places without the fill path. Lets do that refactor once we get to different entities.
To replace the left nav with a free form folder structure we need to make sure our code uses paths everywhere instead of mapping name and type to a path. This PR is the 1st step where we update common methods to use paths.