Ban overwriting of UUIDs on new product creation, modify "edit metadata" modal design

[gordlin]

Related Item(s)

Issue #445

Changes

• [FEATURE] Changes the This UUID already exists warning on the Create New Storylines Product page into an error, and block the user from continuing until they choose another one.
• [REFACTOR] Makes some minor changes to the Edit Metadata modal. (See comment)
  • Moves "Done" button to header
  • Makes header sticky
  • Adds bottom border to header for visual separation

Testing

Steps:

1. Open the Create New Storylines Product page.
2. Enter the UUID of an already existing product (e.g. 00000000-0000-0000-0000-000000000000). See the red error that pops up instead of the previous orange warning.
3. Scroll to the bottom of the page. Notice the Next button is grayed out. The user must change the UUID to one that doesn't conflict with an existing product in order to continue.
4. Change the UUID to a new, unique one. The error disappears, and the Next button can be clicked.
5. Once in the editor, click Edit Project Metadata and play with the modified metadata modal.

---

This change is 

[github-actions]

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

[RyanCoulsonCA]

I've found that if you enter a UUID that is currently in use and then quickly click "Next" before the "in use" warning pops up, you can still overwrite a product. Maybe we also need to disable the "Next" button while the check is in progress?

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gordlin)

[gordlin]

Good idea, Donethanks!

Also disabled the "Next" button when the UUID field is blank.

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[RyanCoulsonCA]

Looks good! One more quick thing (should've included this in my original review, sorry): when renaming, we show this little spinner next to the "Rename" button while the UUID is being checked:

I'm thinking it would probably be beneficial to include this spinner next to the UUID input box while it's checking the UUID just so that the user knows that something is happening and isn't confused about why the "Next" button is disabled.

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @gordlin)

[gordlin]

Donethanks! Also added a spinner to the "Next" button itself to indicate we're loading the editor (although it only seems to show up sometimes, and is frozen when it does appear).

In addition, made some minor modification to the edit metadata modal:

• Moved "Done" button to header
• Made header sticky
• Added bottom border to header for visual separation

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[gordlin]

Managed to fix the "Next" button spinner "only showing up sometimes" by adding a slightly longer setTimeout delay. It's still frozen (no spinning), though.

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @RyanCoulsonCA)

[IshavSohal]

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @gordlin and @RyanCoulsonCA)

---

a discussion (no related file):
I noticed that when you remove the UUID provided, the spinner will spin forever, and the Next button remains disabled forever. Entering a new UUID doesn't seem to fix it.

[gordlin]

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @IshavSohal and @RyanCoulsonCA)

---

a discussion (no related file):

Previously, IshavSohal (Ishav Sohal) wrote…

I noticed that when you remove the UUID provided, the spinner will spin forever, and the Next button remains disabled forever. Entering a new UUID doesn't seem to fix it.

Donethanks! Also added an error message when the UUID field on the Create New Storylines Product page is left blank:

[mohsin-r]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @IshavSohal)

[gordlin]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @IshavSohal, @mohsin-r, and @RyanCoulsonCA)

---

a discussion (no related file):
Rebased with main. No functional changes.

[yileifeng]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @IshavSohal, @mohsin-r, and @RyanCoulsonCA)

[IshavSohal]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mohsin-r, @RyanCoulsonCA, and @yileifeng)

[RyanCoulsonCA]

Changes to metadata modal look great 👍

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gordlin, @IshavSohal, @mohsin-r, and @yileifeng)

---

src/components/metadata-editor.vue line 1767 at r5 (raw file):

        // Needed in order to show the loading spinner at all
        // Although it shows, it's still frozen (since app's really just lagging until editor's in)
        setTimeout(() => {

This is good for now, but I think we should create an issue to look in to removing all of the setTimeout calls we've added to the project for UI purposes recently. I could be totally wrong but I feel like there's got to be a better solution considering Vue is reactive? May just require a bit of investigation.

[gordlin]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @IshavSohal, @mohsin-r, @RyanCoulsonCA, and @yileifeng)

---

a discussion (no related file):
Rebased with main, resolved merge conflicts.

Some of the recent commits to main seem to have broken the existing UUID checking functionality. I think the problem involves the /check/:id API endpoint, which isn't detecting existing UUIDs as existing (I used the 00000000-0000-0000-0000-000000000000 product), and so the UUID inputs weren't showing the "Product already exists" error. I've rolled back a few lines of checkUUID() so it uses the previous /retrieve/:id/:lang endpoint, and checking seems to work now.

[RyanCoulsonCA]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @gordlin, @IshavSohal, @mohsin-r, and @yileifeng)

---

a discussion (no related file):

Previously, gordlin (Gordon Lin) wrote…

Rebased with main, resolved merge conflicts.

Some of the recent commits to main seem to have broken the existing UUID checking functionality. I think the problem involves the /check/:id API endpoint, which isn't detecting existing UUIDs as existing (I used the 00000000-0000-0000-0000-000000000000 product), and so the UUID inputs weren't showing the "Product already exists" error. I've rolled back a few lines of checkUUID() so it uses the previous /retrieve/:id/:lang endpoint, and checking seems to work now.

That's interesting... the /check/ endpoint seems to be working for me after pulling the latest changes. Is anyone else experiencing this?

[gordlin]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @IshavSohal, @mohsin-r, @RyanCoulsonCA, and @yileifeng)

---

a discussion (no related file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

That's interesting... the /check/ endpoint seems to be working for me after pulling the latest changes. Is anyone else experiencing this?

Surprisingly it does seem to work now, despite not working yesterday. I've changed it back.

If anyone notices any problems with the "UUID already exists" error not showing up in the rename or new product inputs, please let me know ASAP.

[szczz]

Reviewed 1 of 2 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gordlin, @IshavSohal, @mohsin-r, @RyanCoulsonCA, and @yileifeng)

---

a discussion (no related file):

Previously, gordlin (Gordon Lin) wrote…

Surprisingly it does seem to work now, despite not working yesterday. I've changed it back.

If anyone notices any problems with the "UUID already exists" error not showing up in the rename or new product inputs, please let me know ASAP.

Worked great for me.

[yileifeng]

Reviewed 1 of 2 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gordlin, @IshavSohal, @mohsin-r, @RyanCoulsonCA, and @szczz)

---

a discussion (no related file):

Previously, szczz (Matt Szczerba) wrote…

Worked great for me.

WOMM

---

src/components/metadata-editor.vue line 1767 at r5 (raw file):

Previously, RyanCoulsonCA (Ryan Coulson) wrote…

This is good for now, but I think we should create an issue to look in to removing all of the setTimeout calls we've added to the project for UI purposes recently. I could be totally wrong but I feel like there's got to be a better solution considering Vue is reactive? May just require a bit of investigation.

#515

[szczz]

Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @IshavSohal, @mohsin-r, @RyanCoulsonCA, and @yileifeng)