change: [M3-6546] - Volume drawers and action menu refactored

[dchyrva-akamai]

Description 📝

Created new drawer for managing volume tags, action menu for the volume row updated.

Changes 🔄

• Removed "Show Config" button from the volume table row and added to the actions menu.

• Remove "Edit" button from the volume table row and added to the actions menu .

• Added "Manage Tags" option to the volume table actions menu.

• Added new "Manage Tags" drawer.

• Existing volume tags functionality moved from "Edit Volume" drawer to "Manage Tags" drawer.

[jdamore-linode]

Thanks @dchyrva! Noticed a couple small issues with the E2E test changes but nothing too serious -- happy to help out if you run into any trouble addressing feedback!

[hkhalil-akamai]

Thanks for the contribution! I had a couple of concerns:

1. We have an existing drawer component called TagDrawer. Although the UI is slightly different, would it be possible to use that instead of re-implementing a similar component?
2. The Figma prototype in the attached ticket includes the ability to group volumes by tags. Should this be implemented in this or another ticket (as it may require significant changes)?

[github-actions]

Coverage Report: ❌
Base Coverage: 86.7%
Current Coverage: 86.63%

[jaalah-akamai]

We have an existing drawer component called TagDrawer. Although the UI is slightly different, would it be possible to use that instead of re-implementing a similar component?

@hkhalil-akamai The figma mock is using the version that's currently in place, which is essentially an autocomplete. Let's keep this in place for now until we can ask UX Thursday. Good question 👍

The Figma prototype in the attached ticket includes the ability to group volumes by tags. Should this be implemented in this or another ticket (as it may require significant changes)?

@dchyrva lets add this as part of a new PR - part 2.

[jaalah-akamai]

Let's move forward with the the autocomplete tag implementation. We're eventually going to sunset the TagDrawer where we display each tag visually on the page.

[jaalah-akamai]

Nice job @dchyrva-akamai

[bnussman-akamai]

Getting an unexpected error when clearing tags

Screen.Recording.2024-09-06.at.3.05.24.PM.mov

[dchyrva-akamai]

Hello, @bnussman-akamai

Thank you for catching the error.

Should be fixed now.

[bnussman-akamai]

Looks good! I just have a few comments about the cypress test and form error handling

[bnussman-akamai]

Is there any more safe way we could go about this? I want to make sure we avoid test flake

Could we watch the UI for the status to change from Creating to Active?

@jdamore-linode Might have some better tips on how to avoid cy.wait calls

[jdamore-linode]

@bnussman-akamai Yep, we should be avoiding cy.wait(<number of milliseconds>) if we can (explanation).

I think your suggestion to wait for the UI is what we do in other tests, and it'd look something like this:

cy.defer(() => createVolume(volumeRequest), 'creating volume').then(
      (volume: Volume) => {
        // Volume needs some time to become active after creation.
        // Find the Volume's label in the table, then narrow our selection to that table row.
        cy.findByText(volume.label)
          .should('be.visible')
          .closest('tr')
          .within(() => {
            // Wait for "Creating" status to become "Active".
            // I _think_ the actual DOM text is 'active' and not 'Active' and we use CSS for capitalization here, but
            // you might want to fact check me here.
            cy.findByText('active').should('be.visible');
          });

Edit: Here's an actual working example, except we do it slightly differently. We create the Volume and wait for it to be active by polling its status using the API, and then we visit the Volumes page and proceed with the test.

I'm thinking we might do this because of that events issue you and I looked at a few weeks back where sometimes an event's status doesn't change from started to finished and the UI doesn't automatically update as a result, but I don't remember 100%.

[bnussman-akamai]

Yeah, I think there is some API weirdness causing the status to not always update in the UI. I'll create a separate ticket to investigate that.

[bnussman-akamai]

Created M3-8557 for that bug you mentioned @jdamore-linode

@dchyrva-akamai That example Joe shared using createActiveVolume might be a good option here!

[dchyrva-akamai]

@bnussman-akamai @jdamore-linode Thank you for the comment, I used createActiveVolume instead.

[bnussman-akamai]

@jdamore-linode Is a clean up in the after okay?

[jdamore-linode]

It doesn't hurt anything, but I don't think it should be necessary since we're already doing clean up in the before hook (explanation why). @dchyrva-akamai any reason in particular this was added?

[dchyrva-akamai]

@jdamore-linode I added clean up after in order to remove volumes, created for testing purposes, from the account.
Maybe you could recommend a better way of doing that?

[hkhalil-akamai]

@jdamore-linode can we double check if this is necessary?

[jdamore-linode]

@hkhalil-akamai No, it shouldn't be. @dchyrva-akamai might be able to speak more to his motivation for adding this (e.g. were you getting test failures that this solved?) but broadly speaking we should not be doing any clean up in after hooks (and the link I posted above explains why).

(The obvious elephant in the room is #11028 where I'm doing exactly that; that's an exceptional case that we agreed to for security compliance reasons, unlike this situation where the clean up is being done for the sake of setting up a test's state)

[github-actions]

This PR is stale because it has been open 15 days with no activity. Please attend to this PR or it will be closed in 5 days

[hkhalil-akamai]

Is there anything else blocking this PR, other than the one open question?