-
Notifications
You must be signed in to change notification settings - Fork 916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Worksapce]Add blank check for workspace name #7512
[Worksapce]Add blank check for workspace name #7512
Conversation
Signed-off-by: Hailong Cui <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7512 +/- ##
==========================================
- Coverage 63.64% 63.64% -0.01%
==========================================
Files 3629 3629
Lines 79522 79525 +3
Branches 12604 12605 +1
==========================================
+ Hits 50611 50612 +1
- Misses 25842 25844 +2
Partials 3069 3069
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
const workspaceNameSchema = schema.string({ | ||
validate(value) { | ||
if (!value || value.trim().length === 0) { | ||
return "can't be empty or blank."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if the error message is clear enough for customers to understand what happens here, and from the description of the input, it seems space are valid input though the name can not be pure spaces.
And should we wrap it with i18n?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
field name will prepend to actual message, the whole error message will be
{
"statusCode": 400,
"error": "Bad Request",
"message": "[request body.attributes.name]: can't be empty or blank."
}
And should we wrap it with i18n?
as the default validation message of schema.string()haven't been wrapped by i18n. i think we can keep it same
* add workspace blank check Signed-off-by: Hailong Cui <[email protected]> * Changeset file for PR #7512 created/updated * add integ test Signed-off-by: Hailong Cui <[email protected]> * Changeset file for PR #7512 created/updated --------- Signed-off-by: Hailong Cui <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit b5f4942) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* add workspace blank check * Changeset file for PR #7512 created/updated * add integ test * Changeset file for PR #7512 created/updated --------- (cherry picked from commit b5f4942) Signed-off-by: Hailong Cui <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
Add blank or empty check for workspace name
Issues Resolved
#7280
Screenshot
when input a blank workspace name, validation will fail
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration