re-implement rename feature

[RyanCoulsonCA]

Related Item(s)

#253
#418
#419

Changes

• Added the /rename server endpoint: this is a re-implementation of the rename feature. Instead of relying on the /upload endpoint that simply created a new product folder, the /rename endpoint calls the rename syscall on the existing product folder. It will also delete and re-instantiate the Git repo.
• Added a new /check endpoint to the server to see if a product already exists. This way, we don't need to call the /retrieve endpoint and wait for the entire product to transfer every time the user enters a UUID. It should be much faster now.
• Moved the responseMessages.push() calls in to the logger function to reduce code duplication.

Notes

Testing

This is a big one, so please test thoroughly! If my instructions below are confusing please reach out, or post a comment on this PR to help others if they also have trouble.

⚠️ IMPORTANT! ⚠️ If testing locally, please move the server folder outside of the storylines-editor folder. npm run dev seems to lock the server/public folder and it will result in the rename system call hanging indefinitely otherwise. Alternatively, if you overwrite the server/index.js code with the new code in this PR, you should be able to test using the demo page associated with this PR.

⚠️ If you notice the rename endpoint is hanging at any point, then the product folder is being accessed from somewhere else at the same time. Try closing anything that could be using it (VS Code, File Explorer, notepad, etc.). As far as I understand this should (hopefully) not happen on prod as nothing should be accessing the file system. If anyone knows of something that can be added to the code to fix this, please let me know.

Steps:

1. Pull this PR locally and follow the important step above.
2. Try creating a new product with an existing UUID. Notice that the "UUID is already in use" error should appear much faster than before.
3. Create a new product that doesn't already exist. Save it so it gets uploaded to the server.
4. Return to the home page, and now load this new product.
5. Try to rename it, and ensure it works properly.
6. Now, try to load an existing product.
7. Try to rename it and ensure that everything works here as well. Also, check that the previous editing history has been wiped.
8. Try whatever you want to break this otherwise!

---

This change is 

[github-actions]

Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/rename-refactor

[mohsin-r]

Since we are renaming files on UUID change, we also have the map configs which have default naming in the format uuid-map-#.json. Perhaps its worth it to change those?

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA)

---

server/index.js line 347 at r1 (raw file):

                                fs.rmSync(NEW_PATH + `/.git`, { recursive: true, force: true });
                                const git = simpleGit(NEW_PATH);
                                await git.init();

Curious about the choice of creating a new git repo on rename. If a user just doesn't like their product name and wants a different name, it forces them to nuke the version history. Feel free to let me know if I'm missing something from a technical standpoint.

---

server/index.js line 502 at r1 (raw file):

    // Push to responseMessages.
    responseMessages.push({ type: type, message: message });

Smart coding! 🧠

[RyanCoulsonCA]

Good catch, working on this now.

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @mohsin-r)

---

server/index.js line 347 at r1 (raw file):

Previously, mohsin-r (Mohsin Reza) wrote…

Curious about the choice of creating a new git repo on rename. If a user just doesn't like their product name and wants a different name, it forces them to nuke the version history. Feel free to let me know if I'm missing something from a technical standpoint.

If we don't wipe the Git history, then we'd basically need to add a bunch of new processes to handle reverting a rename:

If a user renames a product and then reverts it, then everything within the product folder will revert back to using the previous UUID (except the folder name itself), which will mean we'd need to add another function to the server to rename the folder back to the previous name. Additionally, I assume we'd also need to fire another request at the database to revert the UUID there as well.

If we really want to keep the Git history we can definitely implement all of this, but I just made the assumption that we'd want to choose the easier option here. We can also add a warning message on the rename screen to let the user know that the edit history will be wiped.

[mohsin-r]

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA)

---

server/index.js line 347 at r1 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

If we don't wipe the Git history, then we'd basically need to add a bunch of new processes to handle reverting a rename:

If a user renames a product and then reverts it, then everything within the product folder will revert back to using the previous UUID (except the folder name itself), which will mean we'd need to add another function to the server to rename the folder back to the previous name. Additionally, I assume we'd also need to fire another request at the database to revert the UUID there as well.

If we really want to keep the Git history we can definitely implement all of this, but I just made the assumption that we'd want to choose the easier option here. We can also add a warning message on the rename screen to let the user know that the edit history will be wiped.

Yeah that totally makes sense to me now, I hadn't considered that before. I think a warning message would be helpful for sure.

[RyanCoulsonCA]

Since we are renaming files on UUID change, we also have the map configs which have default naming in the format uuid-map-#.json. Perhaps its worth it to change those?

Donethanks.

Because the rename endpoint only takes the two renamed config files as opposed to the entire ZIP structure, this had to be implemented on the server-end. I'm not a huge fan of this as it introduces more blocking calls, but I think that since renaming shouldn't be used very often it's not a huge deal.

Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @mohsin-r)

[IshavSohal]

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @mohsin-r and @RyanCoulsonCA)

---

a discussion (no related file):
When a product containing a map has its UUID changed and the product is then loaded again, I noticed that the map fails to render. However upon reloading, the map renders just fine. In the console I get the following error when I open the map panel:

TypeError: this.rampEditorApi.getConfig is not a function

[RyanCoulsonCA]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @IshavSohal and @mohsin-r)

---

a discussion (no related file):

Previously, IshavSohal (Ishav Sohal) wrote…

When a product containing a map has its UUID changed and the product is then loaded again, I noticed that the map fails to render. However upon reloading, the map renders just fine. In the console I get the following error when I open the map panel:

TypeError: this.rampEditorApi.getConfig is not a function

Good catch. A side effect of doing the file renaming on the server-end is that the map configuration files in the ZIP file weren't being renamed until the product was re-fetched from the server.

I've added a new function to rename the files in the ZIP file locally as well to fix this. I think it would be ideal in the future to drop the server-side renaming and just keep the new client side changes, but this would once again require us to send the entire modified ZIP file back to the server when renaming which would make it take a lot longer.

[IshavSohal]

Reviewed 1 of 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[mohsin-r]

Reviewed 1 of 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[szczz]

I pulled this branch locally and moved the server folder outside of storylines-editor, but it still seems to be hanging on rename. It doesn't let me rename a storyline to something that already exists, but pressing rename for a valid uuid results in an infinite loading wheel. I don't see anything in the console and I don't see any requests being made in the network tab.

Edit: Just to clarify, I also tested this with nothing accessing the folders other than the terminals running the express server and vue app

Reviewed 1 of 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[RyanCoulsonCA]

Very interesting. I haven't been able to reproduce the hanging in any case other than a file being in use somehow (even subtly, sometimes even if I close a file in VS Code and then try renaming, it will still block until I close VS Code).

I've done a bunch of research online to see if there's a way to detect if the rename call is being blocked by something, but I can't seem to find anything that will be helpful here.

Either way, I've added in a catch block on the front end to detect 500 errors returned from the server (which probably should've been done in the first place). If one does occur, an error will pop up saying that the rename has failed, and none of the renaming steps will take place. This should resolve the issue with the infinite spinner.

Try to test this again with these new changes and if the server hangs, wait a few minutes to see if an EPERM error pops up in the console (which is the error that usually pops up for me if the rename call is blocked).

For further debugging, I've have a slightly modified index.js with some conveniently placed console.logs so you can see where it's hanging. If you want to try that out, let me know. I originally uploaded it here but Microsoft Defender seems to think it's a virus 👾

Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @IshavSohal, @mohsin-r, and @szczz)

[yileifeng]

Working well for all cases tested 👍

One small issue found is that if you load in a product, modify the UUID input field and proceed to rename, the functionality is missing. Maybe disabling the feature or ignoring the modified input could be a workaround.

Reviewed 1 of 2 files at r2, 1 of 3 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[RyanCoulsonCA]

I've modified the code to use the currently loaded UUID (from this.configFileStructure.uuid) instead of using the value that was typed into the UUID input which should fix this.

Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @yileifeng)

[szczz]

Frontend is still hanging for me and displaying an infinite loading wheel but I can see the rename request in the network tab and the product is successfully renamed... Likely just an issue on my end.

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[yileifeng]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)