AAP-30986: Support deletion of WCA API_KEY from Admin Portal (Frontend)

[mabulgu]

Jira Issue: https://issues.redhat.com/browse/AAP-30986

Description

Please see the Jira issue for details. Note that this issue splitted into two: frontend & backend. This PR includes the FE changes work.

Testing

Follow the following steps. Do not commit anything by mistake as you will need to de-comment some places in the code.

Steps to test

1. Go to ansible_ai_connect_admin_portal/src/ModelSettingsOverview.tsx:288
2. Comment the related part (the delete button)
3. Run npm start Open the link in browser tab. This is the mocked mode.
4. Test out the deletion. You might need to play with the mock API here (ansible_ai_connect_admin_portal/config/webpack.mock.api.keys.ts:66) to simulate an error.
5. Revert your changes and dont commit/push them by mistake.
6. Run the npm run test command to make sure all tests pass.

Scenarios tested

• Deletion with success
• Deletion with failure

Production deployment

• This code change is ready for production on its own
• This code change requires the following considerations before going to production: The backend part of this code should be completed and done. See AAP-32688.

[github-actions]

# npm audit report

rollup  <2.79.2
Severity: high
DOM Clobbering Gadget found in rollup bundled scripts that leads to XSS - https://github.com/advisories/GHSA-gcx4-mw62-g8wm
fix available via `npm audit fix`
node_modules/rollup

1 high severity vulnerability

To address all issues, run:
  npm audit fix

[mabulgu]

I am going to ask about this text to Marty once I start the backend part. For now since we dont see it, pls ignore the product owner approval. I will get the related approvals.

[manstis]

Hi @mabulgu

I had a quick look. Was you planning on writing tests too?

I also note that the JIRA states:

Deleting the API_KEY should also delete the MODEL_ID as having one set without the other is an indeterminate system state.

I don't see anything in your mocked backend to support this... it might be that you need to combine the two mock backends into a single file so that "Key deletion" can access "Model" for the origanisation:

• https://github.com/ansible/ansible-ai-connect-service/blob/main/ansible_ai_connect_admin_portal/config/webpack.mock.api.keys.ts
• https://github.com/ansible/ansible-ai-connect-service/blob/main/ansible_ai_connect_admin_portal/config/webpack.mock.api.modelids.ts

Let me know your answers then I'll review this PR.

[mabulgu]

Hi @mabulgu

I had a quick look. Was you planning on writing tests too?

I also note that the JIRA states:

Deleting the API_KEY should also delete the MODEL_ID as having one set without the other is an indeterminate system state.

I don't see anything in your mocked backend to support this... it might be that you need to combine the two mock backends into a single file so that "Key deletion" can access "Model" for the origanisation:

• https://github.com/ansible/ansible-ai-connect-service/blob/main/ansible_ai_connect_admin_portal/config/webpack.mock.api.keys.ts
• https://github.com/ansible/ansible-ai-connect-service/blob/main/ansible_ai_connect_admin_portal/config/webpack.mock.api.modelids.ts

Let me know your answers then I'll review this PR.

@manstis I implemented the key -model id deletion relation

[mabulgu]

Code coverage complains because there is no tests for the parts that I not fully comemnted. I changed the strategy by making the delete button hidden so that I can make it visible and run tests against it. Working on this right now. jfyi

[github-actions]

# npm audit report

cookie  <0.7.0
cookie accepts cookie name, path, and domain with out of bounds characters - https://github.com/advisories/GHSA-pxg6-pf52-xh8x
fix available via `npm audit fix`
node_modules/cookie
  express  3.0.0-alpha1 - 4.21.0 || 5.0.0-alpha.1 - 5.0.0
  Depends on vulnerable versions of cookie
  node_modules/express

2 low severity vulnerabilities

To address all issues, run:
  npm audit fix

[mabulgu]

@manstis ready for another round of review

[mabulgu]

My Failure case aims a 404 error but I realized in the backend we do return 400 for those. Those changes will be included in my backend PR so please consider them as 404 for now. JFYI.

[manstis]

Looking good @mabulgu

A few comments that should probably be addressed.

[manstis]

@mabulgu

My Failure case aims a 404 error but I realized in the backend we do return 400 for those. Those changes will be included in my backend PR so please consider them as 404 for now. JFYI.

For a "successful" operation with no content (i.e. DELETE) a 204 may be best.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[manstis]

LGTM 👍

Thanks @mabulgu

[mabulgu]

Thanks @manstis for your help and reviews!