SANDBOX-1893 | Show alerts for user signup errors#102
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (3)**/__tests__/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
plugins/sandbox/src/api/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
🔀 Multi-repo context codeready-toolchain/registration-service, codeready-toolchain/toolchain-common, codeready-toolchain/api, codeready-toolchain/host-operatorLinked repositories findingscodeready-toolchain/registration-service
codeready-toolchain/toolchain-common
codeready-toolchain/api
codeready-toolchain/host-operator
🔇 Additional comments (1)
WalkthroughAdds ChangesSignup error flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/sandbox/src/api/errors/UserSignupError.ts`:
- Around line 36-40: In the switch case that handles the 'invalid code' scenario
within the UserSignupError class, guard against undefined or null values for
body?.details when constructing the error message. Instead of directly
interpolating body?.details into the template string, use a nullish coalescing
operator or conditional logic to provide a fallback message (such as a generic
description or empty string) when details is undefined, so users don't see the
literal text "undefined" in the error message.
In `@plugins/sandbox/src/hooks/useSandboxContext.tsx`:
- Around line 192-195: In the useSandboxContext hook, the signupError is set
when a UserSignupError is caught, but it's never cleared on subsequent
successful refetches. Add logic to clear the signupError by calling
setSignupError with an empty or null value when the fetch succeeds and userData
is successfully populated, ensuring stale error messages don't persist after the
user regains 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5d3a877e-f33a-45d4-9d34-6398b06f4a64
📒 Files selected for processing (7)
plugins/sandbox/src/api/RegistrationBackendClient.tsxplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsxplugins/sandbox/src/api/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/errors/UserSignupError.tsplugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsxplugins/sandbox/src/hooks/useSandboxContext.tsxplugins/sandbox/src/test-utils/mockResponse.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/registration-service(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ci
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (4)
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/__tests__/**/*.{ts,tsx}: Use Jest viabackstage-cli,@testing-library/react, and MSW for API mocking in unit tests
Place test files in__tests__/directories next to the code they test
Files:
plugins/sandbox/src/api/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx
plugins/sandbox/src/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Authenticate backend service clients via
SecureFetchClientwhich wraps OAuth2 tokens from Keycloak
Files:
plugins/sandbox/src/api/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/errors/UserSignupError.tsplugins/sandbox/src/api/RegistrationBackendClient.tsxplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
plugins/sandbox/src/api/__tests__/UserSignupError.test.tsplugins/sandbox/src/test-utils/mockResponse.tsplugins/sandbox/src/api/errors/UserSignupError.tsplugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsxplugins/sandbox/src/api/RegistrationBackendClient.tsxplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsxplugins/sandbox/src/hooks/useSandboxContext.tsx
plugins/sandbox/src/hooks/useSandboxContext.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use
SandboxProvideras the central React context for state management, managing user signup status and AAP instance state with configurable polling intervals
Files:
plugins/sandbox/src/hooks/useSandboxContext.tsx
🔇 Additional comments (5)
plugins/sandbox/src/test-utils/mockResponse.ts (1)
17-43: LGTM!plugins/sandbox/src/api/__tests__/UserSignupError.test.ts (1)
1-165: LGTM!plugins/sandbox/src/api/RegistrationBackendClient.tsx (1)
76-80: LGTM!plugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx (1)
20-20: LGTM!plugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsx (1)
63-68: LGTM!
MikelAlejoBR
force-pushed
the
SANDBOX-1893-feedback-usersignup-errors
branch
from
b819c80 to
28ed521
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx (1)
99-104: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd signupError to Provider value for consistency.
The mocked
useSandboxContextreturn value includessignupError(line 71), but theSandboxContext.Providervalue does not. While the component likely uses the hook, this inconsistency creates a confusing test setup and could cause issues if any code accesses the context directly.🔧 Suggested fix
<SandboxContext.Provider value={{ loading: false, userData: undefined, verificationRequired: false, pendingApproval: false, + signupError: undefined, ...contextValue, }} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx` around lines 99 - 104, The SandboxContext.Provider value object is missing the signupError property that exists in the mocked useSandboxContext hook return value at line 71, creating an inconsistency in the test setup. Add the signupError property to the Provider value object (in the context value assignment around lines 99-104) with an appropriate default value such as undefined to ensure consistency between the hook mock and the actual context provider value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx`:
- Line 71: The test file SandboxCatalogBanner.test.tsx currently sets up
signupError as undefined but lacks a test case verifying the component's
behavior when signupError contains an actual error message. Add a new test case
in the test suite that creates a component instance with signupError set to a
sample error message (instead of undefined), then verify that the alertApi.post
method is called with the correct parameters including error severity and the
error message content, to ensure the component properly handles and communicates
signup errors to the user.
---
Outside diff comments:
In
`@plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx`:
- Around line 99-104: The SandboxContext.Provider value object is missing the
signupError property that exists in the mocked useSandboxContext hook return
value at line 71, creating an inconsistency in the test setup. Add the
signupError property to the Provider value object (in the context value
assignment around lines 99-104) with an appropriate default value such as
undefined to ensure consistency between the hook mock and the actual context
provider value.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 684f01e2-b5bd-43f7-bf86-fb6a6e40fd6b
📒 Files selected for processing (8)
plugins/sandbox/src/api/RegistrationBackendClient.tsxplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsxplugins/sandbox/src/api/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/errors/UserSignupError.tsplugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsxplugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsxplugins/sandbox/src/hooks/useSandboxContext.tsxplugins/sandbox/src/test-utils/mockResponse.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/registration-service(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
🚧 Files skipped from review as they are similar to previous changes (7)
- plugins/sandbox/src/api/errors/UserSignupError.ts
- plugins/sandbox/src/test-utils/mockResponse.ts
- plugins/sandbox/src/api/RegistrationBackendClient.tsx
- plugins/sandbox/src/api/tests/UserSignupError.test.ts
- plugins/sandbox/src/api/tests/RegistrationBackendClient.test.tsx
- plugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsx
- plugins/sandbox/src/hooks/useSandboxContext.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ci
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (2)
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/__tests__/**/*.{ts,tsx}: Use Jest viabackstage-cli,@testing-library/react, and MSW for API mocking in unit tests
Place test files in__tests__/directories next to the code they test
Files:
plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx
🔀 Multi-repo context codeready-toolchain/registration-service
Linked Repositories Findings
Based on my exploration of the linked repositories, I found relevant context about the backend API response format that the PR changes interact with.
codeready-toolchain/registration-service
Backend Error Response Structure:
The registration-service returns error responses with a standardized JSON structure defined in ./pkg/errors/error.go [::codeready-toolchain/registration-service::]:
type Error struct {
Status string `json:"status"`
Code int `json:"code"`
Message string `json:"message"`
Details string `json:"details"`
}Success Response Structure:
The GetSignup handler in ./pkg/controller/signup.go returns a signup.Signup JSON object when successful (200 OK). When the UserSignup resource is not found, it returns a 404 without JSON body [::codeready-toolchain/registration-service::].
Error Scenarios the Backend Handles:
- 404 Not Found: When UserSignup resource doesn't exist (no JSON body returned)
- 403 Forbidden: For banned users, rejected access, and CRT admin users with specific error messages [::codeready-toolchain/registration-service::]
- 409 Conflict: For duplicate signup scenarios
- 500 Internal Server Error: For other failures
Key Alignment Finding:
The backend's error response includes a message field containing error text, which the PR's UserSignupError.fromResponse() method correctly parses to extract specific error conditions (e.g., "invalid activation code", "suspended", "denied"). The Error struct in the backend matches the expectations in the PR's error handling logic.
No breaking changes detected: The other repositories (api, toolchain-common, host-operator, toolchain-e2e) do not contain TypeScript/JavaScript files and do not directly consume the RegistrationBackendClient or SandboxContext APIs that were modified. The changes are localized to the sandbox plugin.
Before these changes users were not given any feedback if anything went wrong with the user signup. For example, if the user happened to be banned, or there was some permission error, the Developer Sandbox UI would not properly work but also would not give the users any clues of why. These changes use the existing "alert" system from the backstage to show informative and user friendly messages when something goes wrong. Assisted-by: Cursor with Opus 4.6 Jira-ticket: SANDBOX-1893
Jira-ticket: SANDBOX-1893
Jira-ticket: SANDBOX-1893
Assisted-by: Cursor with Opus 4.6 Jira-ticket: SANDBOX-1893
Technically the declared and initialized variables in a switch statement can be seen by the other branches, so in order to be safe we are enclosing the constant declaration in its own scope. Assisted-by: Cursor with Opus 4.6 Jira-ticket: SANDBOX-1893
Assisted-by: Cursor with Opus 4.6 Jira-ticket: SANDBOX-1893
MikelAlejoBR
force-pushed
the
SANDBOX-1893-feedback-usersignup-errors
branch
from
28ed521 to
6460078
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx (1)
20-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse MSW for API mocking here instead of
createMockResponse.Line 20 introduces a shared response-mocking helper, and Lines 107-118 rely on it for backend failure paths. For
__tests__files, this should be modeled via MSW handlers to match repository testing standards and keep mock behavior consistent with runtime fetch flows.As per coding guidelines: "
**/__tests__/**/*.{ts,tsx}: Use Jest viabackstage-cli,@testing-library/react, and MSW for API mocking in unit tests".Also applies to: 107-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx` around lines 20 - 21, Replace the `createMockResponse` import and its usage throughout the RegistrationBackendClient test file with MSW handlers to align with repository testing standards. Remove the import statement referencing createMockResponse from the test-utils and instead set up MSW handlers for mocking API responses, particularly in the backend failure path test cases (around lines 107-118) to mock fetch flows consistently with the repository's testing conventions.Source: Coding guidelines
plugins/sandbox/src/api/errors/__tests__/UserSignupError.test.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace custom
Responsemocks with MSW handlers in this test suite.Line 1 and the mocked-response cases (for example Lines 17-182) currently use a local helper for API mocking. That conflicts with the test guideline requiring MSW-based API mocks in
__tests__files; please migrate these cases to MSW handlers to keep test behavior aligned with project standards.As per coding guidelines: "
**/__tests__/**/*.{ts,tsx}: Use Jest viabackstage-cli,@testing-library/react, and MSW for API mocking in unit tests".Also applies to: 17-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/sandbox/src/api/errors/__tests__/UserSignupError.test.ts` at line 1, The test file UserSignupError.test.ts uses a custom createMockResponse helper from test-utils instead of MSW handlers for API mocking, which violates the project guideline requiring MSW-based API mocks in __tests__ files. Remove the import statement for createMockResponse and replace all custom response mocking cases (including the mocked-response setup in the 17-182 range) with MSW server handlers and handler definitions that mock the same API endpoints and responses. Ensure the MSW handlers are configured in a beforeAll hook and cleaned up in an afterEach hook to maintain test isolation while aligning with project standards.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@plugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx`:
- Around line 20-21: Replace the `createMockResponse` import and its usage
throughout the RegistrationBackendClient test file with MSW handlers to align
with repository testing standards. Remove the import statement referencing
createMockResponse from the test-utils and instead set up MSW handlers for
mocking API responses, particularly in the backend failure path test cases
(around lines 107-118) to mock fetch flows consistently with the repository's
testing conventions.
In `@plugins/sandbox/src/api/errors/__tests__/UserSignupError.test.ts`:
- Line 1: The test file UserSignupError.test.ts uses a custom createMockResponse
helper from test-utils instead of MSW handlers for API mocking, which violates
the project guideline requiring MSW-based API mocks in __tests__ files. Remove
the import statement for createMockResponse and replace all custom response
mocking cases (including the mocked-response setup in the 17-182 range) with MSW
server handlers and handler definitions that mock the same API endpoints and
responses. Ensure the MSW handlers are configured in a beforeAll hook and
cleaned up in an afterEach hook to maintain test isolation while aligning with
project standards.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7614353d-cdfc-44ed-9ed1-59049ce2c88e
📒 Files selected for processing (9)
plugins/sandbox/src/api/RegistrationBackendClient.tsxplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsxplugins/sandbox/src/api/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/errors/UserSignupError.tsplugins/sandbox/src/api/errors/__tests__/UserSignupError.test.tsplugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsxplugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsxplugins/sandbox/src/hooks/useSandboxContext.tsxplugins/sandbox/src/test-utils/mockResponse.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/registration-service(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/sandbox/src/api/errors/UserSignupError.ts
- plugins/sandbox/src/components/SandboxCatalog/SandboxCatalogBanner.tsx
- plugins/sandbox/src/hooks/useSandboxContext.tsx
- plugins/sandbox/src/api/RegistrationBackendClient.tsx
- plugins/sandbox/src/test-utils/mockResponse.ts
- plugins/sandbox/src/api/tests/UserSignupError.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ci
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (3)
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/__tests__/**/*.{ts,tsx}: Use Jest viabackstage-cli,@testing-library/react, and MSW for API mocking in unit tests
Place test files in__tests__/directories next to the code they test
Files:
plugins/sandbox/src/api/errors/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsxplugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx
plugins/sandbox/src/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Authenticate backend service clients via
SecureFetchClientwhich wraps OAuth2 tokens from Keycloak
Files:
plugins/sandbox/src/api/errors/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsx
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
plugins/sandbox/src/api/errors/__tests__/UserSignupError.test.tsplugins/sandbox/src/api/__tests__/RegistrationBackendClient.test.tsxplugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx
🔀 Multi-repo context codeready-toolchain/registration-service
Linked Repositories Findings
Based on my exploration of the linked repositories, I found relevant context about the backend API response format that the PR changes interact with.
codeready-toolchain/registration-service
Backend Error Response Structure:
The registration-service returns error responses with a standardized JSON structure defined in ./pkg/errors/error.go [::codeready-toolchain/registration-service::]:
type Error struct {
Status string `json:"status"`
Code int `json:"code"`
Message string `json:"message"`
Details string `json:"details"`
}Success Response Structure:
The GetSignup handler in ./pkg/controller/signup.go returns a signup.Signup JSON object when successful (200 OK). When the UserSignup resource is not found, it returns a 404 without JSON body [::codeready-toolchain/registration-service::].
Error Scenarios the Backend Handles:
- 404 Not Found: When UserSignup resource doesn't exist (no JSON body returned)
- 403 Forbidden: For banned users, rejected access, and CRT admin users with specific error messages [::codeready-toolchain/registration-service::]
- 409 Conflict: For duplicate signup scenarios
- 500 Internal Server Error: For other failures
Key Alignment Finding:
The backend's error response includes a message field containing error text, which the PR's UserSignupError.fromResponse() method correctly parses to extract specific error conditions (e.g., "invalid activation code", "suspended", "denied"). The Error struct in the backend matches the expectations in the PR's error handling logic.
No breaking changes detected: The other repositories (api, toolchain-common, host-operator, toolchain-e2e) do not contain TypeScript/JavaScript files and do not directly consume the RegistrationBackendClient or SandboxContext APIs that were modified. The changes are localized to the sandbox plugin.
🔇 Additional comments (1)
plugins/sandbox/src/components/SandboxCatalog/__tests__/SandboxCatalogBanner.test.tsx (1)
38-50: LGTM!Also applies to: 66-66, 78-78, 416-431
| case message.includes('has been suspended'): | ||
| return new UserSignupError( | ||
| 'Access to the Developer Sandbox has been suspended due to suspicious activity or detected abuse', | ||
| ); | ||
| case message.includes('has been denied'): | ||
| return new UserSignupError( | ||
| 'Access to the Developer Sandbox has been denied', | ||
| ); | ||
| case message.includes('failed to create usersignup for'): | ||
| return new UserSignupError('A CRT admin is not allowed to sign up'); | ||
| case message.includes( | ||
| 'there is already an active UserSignup with such a username', | ||
| ): |
There was a problem hiding this comment.
only out of curiosity - couldn't we simply:
- Adjust the messages that are returned from reg-service
- And show the message that is included in the error
instead of matching based on the message content?
There was a problem hiding this comment.
I thought about it, but the problem I see is that the registration service might return some non-user friendly error messages containing information that the user doesn't really care about. So if we take just the message or details and post it as an alert in the UI, it might contain stuff like "error creating UserSignup resource", or "error retrieving K8s object Y", or whatever.
I personally don't like much either having to parse the error messages, but I think it provides the most user friendly error messages without exposing internals that they do not care or know about.
The other option I thought about is modifying the error messages to include some codes that the UI can interpret, or even better, refactor the errors we return from the back end so that they're always user friendly, which I guess it should be. However I didn't think that the effort would be worth it?
If you think otherwise, though, I'd be very much willing to refactor it
Jira-ticket: SANDBOX-1893
The "404" errors when checking for user signups are a valid state that trigger the creation a new user signup, and therefore the behavior needed to be kept. I removed the code by mistake when refactoring it. Assisted-by: Cursor with Opus 4.6 Jira-ticket: SANDBOX-1893
MikelAlejoBR
force-pushed
the
SANDBOX-1893-feedback-usersignup-errors
branch
from
f21face to
6468371
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MatousJobanek, MikelAlejoBR, rajivnathan 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 |
MikelAlejoBR
merged commit 4815902
into
codeready-toolchain:master
3 participants
Before these changes users were not given any feedback if anything went wrong with the user signup. For example, if the user happened to be banned, or there was some permission error, the Developer Sandbox UI would not properly work but also would not give the users any clues of why.
These changes use the existing "alert" system from the backstage to show informative and user friendly messages when something goes wrong.
Assisted-by: Cursor with Opus 4.6
Jira-ticket: SANDBOX-1893
Summary by CodeRabbit