Revert "SANDBOX-1755: OpenShift AI temporarily disabled"#58
Revert "SANDBOX-1755: OpenShift AI temporarily disabled"#58MikelAlejoBR merged 1 commit intomasterfrom
Conversation
This reverts commit 4e50a5c.
WalkthroughRe-enabled OpenShift AI product support previously commented out in test cases and runtime code. Changes include activating test assertions for analytics tracking, product data validation, and URL mapping configuration across test and implementation files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/sandbox/src/components/SandboxCatalog/__tests__/productData.test.tsx (1)
71-71: Prefer ID-based lookup over fixed index in test setup.At Line 71, using
productData[3]couples this test to array order; a future reorder will cause unrelated failures. Resolve the AAP row byProduct.AAPinstead.Suggested refactor
- const warningIcon = productData[3].description[4].icon; // AAP's warning icon + const aapProduct = productData.find(p => p.id === Product.AAP); + expect(aapProduct).toBeDefined(); + const warningIcon = aapProduct!.description[4].icon; // AAP's warning icon🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/sandbox/src/components/SandboxCatalog/__tests__/productData.test.tsx` at line 71, The test currently picks AAP by array index (productData[3]) which is brittle; change the setup to locate the AAP product by ID using productData.find(p => p.id === Product.AAP) (or equivalent) and then read its description[4].icon into warningIcon, guarding against undefined (throw or assert) so the test fails clearly if AAP is absent. This replaces the fixed-index lookup with an ID-based lookup while keeping the same description index access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/sandbox/src/hooks/useProductURLs.ts`:
- Around line 65-68: The current mapping for Product.OPENSHIFT_AI uses a
template literal which turns undefined into the string "undefined"; change the
url assignment in useProductURLs.ts so it does not stringify optional values —
use a direct conditional or nullish coalescing to set url to
userData.rhodsMemberURL only when isProvisioned and the property is defined,
otherwise set it to an empty string (update the object for Product.OPENSHIFT_AI
to reference userData?.rhodsMemberURL directly instead of using a template
literal).
---
Nitpick comments:
In
`@plugins/sandbox/src/components/SandboxCatalog/__tests__/productData.test.tsx`:
- Line 71: The test currently picks AAP by array index (productData[3]) which is
brittle; change the setup to locate the AAP product by ID using
productData.find(p => p.id === Product.AAP) (or equivalent) and then read its
description[4].icon into warningIcon, guarding against undefined (throw or
assert) so the test fails clearly if AAP is absent. This replaces the
fixed-index lookup with an ID-based lookup while keeping the same description
index access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 240405c8-4cbc-4fa1-8f56-aac00beef206
📒 Files selected for processing (4)
plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogCardButton.test.tsxplugins/sandbox/src/components/SandboxCatalog/__tests__/productData.test.tsxplugins/sandbox/src/components/SandboxCatalog/productData.tsxplugins/sandbox/src/hooks/useProductURLs.ts
| { | ||
| id: Product.OPENSHIFT_AI, | ||
| url: isProvisioned ? `${userData?.rhodsMemberURL}` : '', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify optional URL fields are not wrapped in template literals.
# Expected: avoid `${userData?.<optionalUrl>}` for URL values.
rg -nP --type=ts '\$\{userData\?\.(rhodsMemberURL|cheDashboardURL|consoleURL)\}' plugins/sandbox/src/hooks/useProductURLs.tsRepository: codeready-toolchain/devsandbox-dashboard
Length of output: 396
🏁 Script executed:
cat -n plugins/sandbox/src/types/registration.ts | sed -n '30,50p'Repository: codeready-toolchain/devsandbox-dashboard
Length of output: 679
🏁 Script executed:
cat -n plugins/sandbox/src/hooks/useProductURLs.ts | sed -n '55,85p'Repository: codeready-toolchain/devsandbox-dashboard
Length of output: 1193
Avoid stringifying an optional URL into "undefined".
At line 67, the template literal converts undefined to a literal string when provisioned users lack rhodsMemberURL, producing an invalid URL.
Suggested fix
{
id: Product.OPENSHIFT_AI,
- url: isProvisioned ? `${userData?.rhodsMemberURL}` : '',
+ url: isProvisioned ? userData?.rhodsMemberURL ?? '' : '',
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| id: Product.OPENSHIFT_AI, | |
| url: isProvisioned ? `${userData?.rhodsMemberURL}` : '', | |
| }, | |
| { | |
| id: Product.OPENSHIFT_AI, | |
| url: isProvisioned ? userData?.rhodsMemberURL ?? '' : '', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/sandbox/src/hooks/useProductURLs.ts` around lines 65 - 68, The
current mapping for Product.OPENSHIFT_AI uses a template literal which turns
undefined into the string "undefined"; change the url assignment in
useProductURLs.ts so it does not stringify optional values — use a direct
conditional or nullish coalescing to set url to userData.rhodsMemberURL only
when isProvisioned and the property is defined, otherwise set it to an empty
string (update the object for Product.OPENSHIFT_AI to reference
userData?.rhodsMemberURL directly instead of using a template literal).
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MikelAlejoBR The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
2 participants
Reverts #52
Summary by CodeRabbit
Release Notes
New Features
Tests