Skip to content
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

Supporting moving files from one project to another [INS-3865] #7849

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

yaoweiprc
Copy link
Contributor

@yaoweiprc yaoweiprc commented Aug 21, 2024

Support moving files(collections/documents/mocks/envs) from one project to another.
Due to the cloud synchronization mechanism, the move operation is implemented by first copying the file to new project and then deleting it in previous project. This may lead to some unexpected results:
1. Since the deleted files will not be automatically removed from the clients of other collaborators within the same organization, when User A moves a file from Project X to Project Y, User B will have two identical files in both Project X and Project Y.
2. The IDs of the files and their subfiles will change after the move, so if a mock file is moved, the mock URL will change, and the old mock URL will no longer be available.
image

@yaoweiprc yaoweiprc marked this pull request as draft August 21, 2024 03:19
@yaoweiprc yaoweiprc force-pushed the feat/move-files-between-projects branch 6 times, most recently from 3ae9ebf to 52f822e Compare August 28, 2024 08:16
@yaoweiprc yaoweiprc marked this pull request as ready for review August 28, 2024 09:00
@CurryYangxx
Copy link
Member

Do we need to add some description text to prompt the user that the mock server URL will change after move to another project?

@CurryYangxx CurryYangxx requested a review from a team August 28, 2024 09:14
packages/insomnia/src/ui/index.tsx Outdated Show resolved Hide resolved
const [selectedProjectId, setSelectedProjectId] = useState('');
useEffect(() => {
(async () => {
const organizationProjects = await database.find<Project>(models.project.type, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just leave a comment here and need a discussion:
@gatzjames We want to move db calls out of UI, but it seems unnecessary to create a new loader in this situation. The DB query will invoke the main process logic through IPC.

CurryYangxx
CurryYangxx previously approved these changes Aug 29, 2024
@yaoweiprc yaoweiprc force-pushed the feat/move-files-between-projects branch from a963f75 to 594e6ae Compare September 2, 2024 06:22
filfreire
filfreire previously approved these changes Sep 2, 2024
@filfreire filfreire force-pushed the feat/move-files-between-projects branch from 594e6ae to 5f76e8e Compare September 2, 2024 12:59
@yaoweiprc yaoweiprc dismissed stale reviews from CurryYangxx and filfreire via 5d1fc6b September 4, 2024 07:08
@yaoweiprc
Copy link
Contributor Author

We initially intended to implement the feature to move files to another project, but we discovered that due to issues with cloud synchronization, true file movement couldn't be achieved. Therefore, we have enhanced the existing copy feature, allowing users to now choose the organization and project to which the files will be copied.

Why we can't implement real move:
I had considered implementing a real move operation that keeps the file IDs unchanged, but it introduces several problems in cloud sync:
If we directly change the parentId of the workspace from the old project to the new project, the server currently does not support synchronizing this behavior to other collaborators' clients. Even if the server supports this mechanism, we automatically executing the move operation for users when they receive the move message. It would not work for users using an older version of the client, as they wouldn't be able to achieve this synchronization.
I also tried simply copying the workspace and pointing the parentId of all sub-files of the original workspace to the new workspace, and then deleting the old workspace. However, this method also fails to synchronize on the collaborators' clients because deleting the workspace does not result in the workspace being removed on the collaborators' clients (We do not delete local files for user when the same one is deleted remotely). When the new workspace is synchronized to the collaborators' clients, the synchronization fails due to non-unique IDs of the sub-files.

image

@yaoweiprc yaoweiprc force-pushed the feat/move-files-between-projects branch from 5d1fc6b to 87ba6a0 Compare September 4, 2024 07:22
Add some error tip for user.
Prohibit user from moving mock files.
@yaoweiprc yaoweiprc force-pushed the feat/move-files-between-projects branch from 87ba6a0 to 135f037 Compare September 4, 2024 07:23
@yaoweiprc yaoweiprc merged commit 31daa3d into develop Sep 4, 2024
8 checks passed
@yaoweiprc yaoweiprc deleted the feat/move-files-between-projects branch September 4, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants