feat(coral): Allow users to delete clusters

[mathieu-anderson]

Linked issue

Resolves: #2282

What kind of change does this PR introduce?

• Bug fix
• New feature
• Refactor
• Docs update
• CI update

What is the current behavior?

There is not way to delete clusters from Klaw currently

What is the new behavior?

• Add context menu to Clusters table row with Delete option (will also have Edit option later)
• Add ability to delete clusters
• Do not show if user does not have the permission addDeleteEditClusters

Screen.Recording.2024-03-15.at.11.49.24.mov

Other information

• Added the possibilty to pass React.ReactElement as children to Dialog
• The copy is still being worked on by @harshini-rangaswamy

Requirements (all must be checked before review)

• The pull request title follows our guidelines
• Tests for the changes have been added (if relevant)
• The latest changes from the main branch have been pulled
• pnpm lint has been run successfully

[harshini-rangaswamy]

@mathieu-anderson and @programmiri

For the copy, let's change Delete to Remove. "Remove" is better here as we are clearing or removing the cluster association in Klaw. For ref: https://aquarium.aiven.io/43ae72f19/p/925433-word-list/t/678211

Also, I suggest updating the label next to the delete icon to "Remove" instead of "Delete." We have something similar in the Aiven Console, for example.

Scenario 1: When users cannot remove cluster as it is associated with an env

Remove cluster
'DEV' is linked to an environment and cannot be removed from Klaw. Unlink the environment to proceed.

Scenario 2: When users can remove clusters

Remove cluster
Confirm removal of 'TEST2'. The cluster will be excluded from Klaw.

Buttons: Cancel, Remove

[mathieu-anderson]

@harshini-rangaswamy implemented the changes here: 35d9e5e

[harshini-rangaswamy]

@harshini-rangaswamy implemented the changes here: 35d9e5e

Looks good, thanks!

[programmiri]

Left a few small comments.

One bigger question -> I think for superadmin, the "connect" action should also be in the Dropdown? 🤔 It does not make sense to have one directly accessible button and one other button hidden behind a menu. Also, the contrast for which dropdown menu is active is already very bad accessibility wise, but if there's only one, that is in focus from the beginning, but it's not a focus style, so it's not really to see, which makes keyboard navigation hard.

[programmiri]

I would add a test case for that, too, so we make sure the "Remove" functionality is only available for admins.

[mathieu-anderson]

I think this is covered by this test? https://github.com/Aiven-Open/klaw/pull/2354/files#diff-e6826bf48eff7da7a98e47263d677fdeaefe63c1e1bb14a9828e59edd911541bR440-R467

[programmiri]

Sorry for not being more precise -> I meant explicitly confirming that there is no delete action available for the user without the permission (e.g. in the block shows all clusters as a table (user without permissions.addDeleteEditClusters)). It's can get a bit trick to decide how much of "testing that something is NOT there" makes sense 😅 but in a case of this kind of functionality I would do it.

[mathieu-anderson]

Aaah of course!

Added here ^^ 62dd1c7

[mathieu-anderson]

@programmiri regarding putting the connect button in the dropdown, I considered it, but I decided against because:

• it is different in nature: in the dropdown, it is actions on the cluster: delete and dedit. In the button, it's an informational help.
• I believe that the connect information would be something that users would want easy access to, as opposed to operation which seems unusual in a normal flow (delete and edit)

Regarding accessibility, yeah, this is for sure an issue :/ But I think it should probably be acted on at the DS level?

At any rate, I think we can modify this later, as it's not central to the functionality. Well, except for the accessibility issues, which are a little tricky to address...

[programmiri]

Yes, your thinking with the button makes sense 🤔 I still would verify with Kate (but for a later PR), as I remember a discussion about this (with Mustafa maybe?) and the "rule" was: if there more than one action, it should be a menu. But that may not be applicable for this case.

But I think it should probably be acted on at the DS level?

It's a colour and focus visibility issue, that's nothing that is planned in the near future to be looked into. We could maybe check if we can apply a simple css fix for this, but that's out of scope for this PR as you said!

[mathieu-anderson]

Kate has seen the UI, as I asked here if a modal worked in the context of deleting the cluster, so I think she doesn't have any opposition to what is currently implemented. But I can ask another confirmation ^^

[programmiri]

No if she has seen it that's perfect! :D