Fix: Remove incorrect validation message for GraphQL Endpoint input #1223
Conversation
- Modified ProvideGraphQL.jsx to skip strict URL validation for GraphQL Endpoint - Allows domain names without protocol (e.g., countries.trevorblades.com) - Backend API validation will handle endpoint validation - Fixes issue where users saw 'URL should not be empty' for valid domain inputs Fixes #68
ranuka-laksika
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
WalkthroughThe change modifies the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User Input
participant ValidateURL as validateURL()
participant Validation as APIValidation
participant Debounce as debouncedValidateURLOrEndpoint
rect rgb(200, 240, 220)
Note over ValidateURL: New: ENDPOINT Type Path
User->>ValidateURL: Input value, inputType="ENDPOINT"
alt value is empty
ValidateURL->>ValidateURL: Set error: "GraphQL Endpoint<br/>should not be empty"
ValidateURL->>User: onValidate(false)
else value is not empty
ValidateURL->>Debounce: Trigger debounced validation
Debounce->>User: Proceed with validation
end
end
rect rgb(220, 235, 245)
Note over ValidateURL: Existing: Other Input Types
User->>ValidateURL: Input value, inputType≠"ENDPOINT"
ValidateURL->>Validation: APIValidation.url.required().validate(value)
alt validation succeeds
Validation->>Debounce: Trigger debounced validation
else validation fails
Validation->>ValidateURL: Return error
ValidateURL->>User: Update validity, onValidate(false)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/GraphQL/Steps/ProvideGraphQL.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/GraphQL/Steps/ProvideGraphQL.jsx (2)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/GraphQL/ApiCreateGraphQL.jsx (1)
apiInputs(117-124)portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/OpenAPI/Steps/ProvideOpenAPI.jsx (2)
apiInputs(82-82)isValid(85-85)
🔇 Additional comments (1)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/GraphQL/Steps/ProvideGraphQL.jsx (1)
407-443: Backend validation behavior must be verified with the API service team.The frontend offloads GraphQL endpoint validation to a separate backend service (via
validateGraphQL()API call withuseIntrospection: true). This backend code is not present in the repository. Before merging, confirm with the backend API team that the validation service correctly handles:
- Domain-only endpoints (e.g.,
countries.trevorblades.com)- Endpoints with custom ports (e.g.,
example.com:8080)- Endpoints with/without protocol prefixes
- Appropriate error messages for invalid formats
Check the backend API specification or service documentation to verify these scenarios are supported.
| function validateURL(value) { | ||
| const state = APIValidation.url.required().validate(value).error; | ||
| if (state === null) { | ||
| setIsValidating(true); | ||
| debouncedValidateURLOrEndpoint(apiInputs.inputValue); | ||
| // For GraphQL Endpoint, skip strict URL validation and allow domain names without protocol | ||
| if (inputType === ProvideGraphQL.INPUT_TYPES.ENDPOINT) { | ||
| if (value && value.trim() !== '') { | ||
| setIsValidating(true); | ||
| debouncedValidateURLOrEndpoint(apiInputs.inputValue); | ||
| } else { | ||
| setValidity({ ...isValid, url: { message: 'GraphQL Endpoint should not be empty' } }); | ||
| onValidate(false); | ||
| } | ||
| } else { | ||
| setValidity({ ...isValid, url: state }); | ||
| onValidate(false); | ||
| const state = APIValidation.url.required().validate(value).error; | ||
| if (state === null) { | ||
| setIsValidating(true); | ||
| debouncedValidateURLOrEndpoint(apiInputs.inputValue); | ||
| } else { | ||
| setValidity({ ...isValid, url: state }); | ||
| onValidate(false); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix parameter usage: validate the passed value, not stale state.
The conditional validation logic correctly addresses the reported issue by allowing domain names for GraphQL Endpoints. However, there's a significant concern with parameter usage:
Issue: Lines 201 and 210 pass apiInputs.inputValue to debouncedValidateURLOrEndpoint, but the function receives value as a parameter from the blur event (lines 390, 428). Since onChange updates apiInputs.inputValue asynchronously and onBlur fires immediately after, there's a potential race condition where the state hasn't been updated yet, causing validation of a stale value rather than what the user just entered.
Solution: Use the value parameter consistently throughout the function.
Apply this diff:
function validateURL(value) {
// For GraphQL Endpoint, skip strict URL validation and allow domain names without protocol
if (inputType === ProvideGraphQL.INPUT_TYPES.ENDPOINT) {
if (value && value.trim() !== '') {
setIsValidating(true);
- debouncedValidateURLOrEndpoint(apiInputs.inputValue);
+ debouncedValidateURLOrEndpoint(value);
} else {
setValidity({ ...isValid, url: { message: 'GraphQL Endpoint should not be empty' } });
onValidate(false);
}
} else {
const state = APIValidation.url.required().validate(value).error;
if (state === null) {
setIsValidating(true);
- debouncedValidateURLOrEndpoint(apiInputs.inputValue);
+ debouncedValidateURLOrEndpoint(value);
} else {
setValidity({ ...isValid, url: state });
onValidate(false);
}
}
}Minor note: Storing endpoint validation state in isValid.url (line 203) works functionally but is semantically confusing. Consider using a more generic key like isValid.input or isValid.value in a future refactor.
📝 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.
| function validateURL(value) { | |
| const state = APIValidation.url.required().validate(value).error; | |
| if (state === null) { | |
| setIsValidating(true); | |
| debouncedValidateURLOrEndpoint(apiInputs.inputValue); | |
| // For GraphQL Endpoint, skip strict URL validation and allow domain names without protocol | |
| if (inputType === ProvideGraphQL.INPUT_TYPES.ENDPOINT) { | |
| if (value && value.trim() !== '') { | |
| setIsValidating(true); | |
| debouncedValidateURLOrEndpoint(apiInputs.inputValue); | |
| } else { | |
| setValidity({ ...isValid, url: { message: 'GraphQL Endpoint should not be empty' } }); | |
| onValidate(false); | |
| } | |
| } else { | |
| setValidity({ ...isValid, url: state }); | |
| onValidate(false); | |
| const state = APIValidation.url.required().validate(value).error; | |
| if (state === null) { | |
| setIsValidating(true); | |
| debouncedValidateURLOrEndpoint(apiInputs.inputValue); | |
| } else { | |
| setValidity({ ...isValid, url: state }); | |
| onValidate(false); | |
| } | |
| } | |
| } | |
| function validateURL(value) { | |
| // For GraphQL Endpoint, skip strict URL validation and allow domain names without protocol | |
| if (inputType === ProvideGraphQL.INPUT_TYPES.ENDPOINT) { | |
| if (value && value.trim() !== '') { | |
| setIsValidating(true); | |
| debouncedValidateURLOrEndpoint(value); | |
| } else { | |
| setValidity({ ...isValid, url: { message: 'GraphQL Endpoint should not be empty' } }); | |
| onValidate(false); | |
| } | |
| } else { | |
| const state = APIValidation.url.required().validate(value).error; | |
| if (state === null) { | |
| setIsValidating(true); | |
| debouncedValidateURLOrEndpoint(value); | |
| } else { | |
| setValidity({ ...isValid, url: state }); | |
| onValidate(false); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/GraphQL/Steps/ProvideGraphQL.jsx
around lines 196 to 216, the function validateURL uses apiInputs.inputValue when
calling debouncedValidateURLOrEndpoint, which can validate a stale value due to
async state updates; change those calls to pass the local parameter value
instead (i.e., call debouncedValidateURLOrEndpoint(value)) so the blur handler
validates the exact input passed in; keep the existing setIsValidating,
setValidity and onValidate behavior unchanged.
1 participant
Fixes wso2/api-manager#3724 (comment)
Issue URL:wso2/api-manager#3724 (comment)
Problem
Publisher portal shows an incorrect validation message "URL should not be empty" when creating a GraphQL API with an Endpoint, even when the user has entered a valid domain name (e.g., "countries.trevorblades.com").
Root Cause
The
validateURLfunction inProvideGraphQL.jsxusesAPIValidation.url.required().validate()which requires a full URL with http:// or https:// scheme. When users enter just a domain name, this strict URL validation fails and displays the misleading "URL should not be empty" message.Solution
Modified the validation logic in
ProvideGraphQL.jsx(line 196-216) to:Changes Made
portals/publisher/src/main/webapp/source/src/app/components/Apis/Create/GraphQL/Steps/ProvideGraphQL.jsxBuild Information
target/publisher.warArtifacts Replaced
publisherfolder inwso2am-4.6.0/repository/deployment/server/webapps/publisher.warto create updated folder structureTesting
No testing required for frontend changes (as per workflow guidelines)
Modified wso2am Pack Download
The complete modified
wso2am-4.6.0pack is available as a GitHub Actions artifact.🔗 Download from GitHub Actions
Artifact Details:
wso2am-4.6.0-issue-68.zipContents: Complete wso2am pack with all updated artifacts ready to use.
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]
Summary by CodeRabbit