-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chore/allow collections move #1001
base: main
Are you sure you want to change the base?
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 257 Passed, 36 Skipped, 44.97s Total Time |
@@ -59,7 +60,7 @@ const SuspendableMoveItem = ({ | |||
pl="2.25rem" | |||
size="xs" | |||
onClick={handleOnClick} | |||
leftIcon={<BiFolder />} | |||
leftIcon={type === "Collection" ? <BiData /> : <BiFolder />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think I've seen this if-else a couple of times, might be good to standardise to use a common function that determines the icon to use
For example, ICON_MAPPINGS
inside apps/studio/src/features/dashboard/components/DirectorySidebar/constants.ts
has this mapping for all the resources currently.
@@ -71,7 +72,7 @@ const MoveResourceContent = withSuspense( | |||
const parentDest = resourceStack[resourceStack.length - 2] | |||
const curResourceId = moveDest?.resourceId | |||
const { data, fetchNextPage, hasNextPage, isFetchingNextPage } = | |||
trpc.resource.getFolderChildrenOf.useInfiniteQuery( | |||
trpc.resource.getChildrenOf.useInfiniteQuery( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue(perf): I think we should still continue using getFolderChildrenOf
, otherwise it seems a bit heavy for the client browser to perform the filtering on the entire list of resources, rather than letting the backend perform an initial filtering to folder-like resources.
parent.type !== ResourceType.Folder && | ||
parent.type !== ResourceType.Collection) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: might be good to add a comment about the frontend-side filtering, which makes moving items from Folder to Collection (and vice versa) not possible to a normal user.
Problem
previously we disallowed users from shifting items inside collections. this pr allows shifting of collection items between collections.
Solution
getChildrenOf
and filterpost
directly and are verifiedTests