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

Unlock and lock product upon rename #560

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

IshavSohal
Copy link
Member

@IshavSohal IshavSohal commented Feb 25, 2025

Related Item(s)

#539

Changes

  • Unlock and lock a product upon a rename, since the existing lock would be corresponding to the old UUID, and there would be no lock corresponding to the new UUID
  • Prevent an attempt to update database if VITE_APP_NET_API_URL is empty or undefined (previously this was only prevented when it was undefined)
  • Commented out the broadcasting of success message upon an unlock in server (for same reason as Load product on page lang change and router param update #555)

Testing

Steps:

  1. Comment out this line in your local server
  2. Load in a product
  3. Try to rename the product
  4. Notice the success toast appear (previously it would hang indefinitely)
  5. Go to editor-main
  6. Make some changes and save them
  7. Notice that the save was successful

This change is Reviewable

@IshavSohal IshavSohal added PR: Active PRs that require a fierce eyeballin. PR: Frontend PR that primarily involves frontend changes. UI experts and CSS Wizards are asked to review. labels Feb 25, 2025
Copy link

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

Copy link
Member

@mohsin-r mohsin-r left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion


src/components/metadata-editor.vue line 1320 at r1 (raw file):

                        // Unlock storyline using old UUID, and lock storyline using new UUID
                        this.lockStore.unlockStoryline();
                        this.lockStore

We should have some error handling here in case locking fails. For example, in the case that someone has created a new product called renamed-product (but has not saved it yet, so they have the lock) and I try renaming my product to renamed-product, the locking fails and you get the same error as before.

To fix this, I would recommend that we check for lock successful before we rename and write a bunch of files to the server.

@IshavSohal
Copy link
Member Author

src/components/metadata-editor.vue line 1320 at r1 (raw file):

Previously, mohsin-r (Mohsin Reza) wrote…

We should have some error handling here in case locking fails. For example, in the case that someone has created a new product called renamed-product (but has not saved it yet, so they have the lock) and I try renaming my product to renamed-product, the locking fails and you get the same error as before.

To fix this, I would recommend that we check for lock successful before we rename and write a bunch of files to the server.

Good idea, donethanks. Now the unlocking/locking occurs prior to calling the /rename endpoint. If the new uuid cannot be locked, then the original uuid is locked again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Active PRs that require a fierce eyeballin. PR: Frontend PR that primarily involves frontend changes. UI experts and CSS Wizards are asked to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants