Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add basic validation to database names #23842

Merged
merged 11 commits into from
Jul 14, 2023
Merged

Add basic validation to database names #23842

merged 11 commits into from
Jul 14, 2023

Conversation

barbaravaldez
Copy link
Contributor

@barbaravaldez barbaravaldez commented Jul 12, 2023 •
edited
Loading

Add basic dynamic validation for database names:

  • No empty values
  • No more than 128 characters

For #23823
For #23822

image

@corivera
Copy link
Member

Where did the 20 character limit come from? According to the CREATE DATABASE docs the name limit is 128 characters. https://learn.microsoft.com/en-us/sql/t-sql/statements/create-database-transact-sql?view=sql-server-ver16&tabs=sqlpool#database_name

@corivera
Copy link
Member

Non-alphanumeric values are also allowed, since I was just able to make a database called "@#)(&"

Charles-Gagnon
Charles-Gagnon reviewed Jul 13, 2023
View reviewed changes
Copy link
Contributor

@Charles-Gagnon Charles-Gagnon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also attach screenshots/gif showing the behavior and different validation errors (make sure to include the OK button in them so we can make sure it's being enabled/disabled correctly)

extensions/mssql/src/ui/dialogBase.ts Outdated Show resolved Hide resolved
extensions/mssql/src/objectManagement/ui/databaseDialog.ts Outdated Show resolved Hide resolved
@barbaravaldez
Copy link
Contributor Author

Please also attach screenshots/gif showing the behavior and different validation errors (make sure to include the OK button in them so we can make sure it's being enabled/disabled correctly)

@Charles-Gagnon- I added a screenshot for empty values. If the max length is over the limit, then it won't let the user type anymore.

Charles-Gagnon
Charles-Gagnon reviewed Jul 13, 2023
View reviewed changes
extensions/mssql/src/objectManagement/ui/databaseDialog.ts
this.objectInfo.name = this.nameInput.value;
await this.runValidation(false);
});
}, props);
containers.push(this.createLabelInputContainer(localizedConstants.NameText, this.nameInput));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be marked as required so it displays in the UI (* next to the label)

@barbaravaldez barbaravaldez merged commit 6e84766 into main Jul 14, 2023
7 checks passed
@barbaravaldez barbaravaldez deleted the bavaldez/databaseValidation branch July 14, 2023 17:11
barbaravaldez added a commit that referenced this pull request Jul 14, 2023
@barbaravaldez @corivera
Add basic validation to database names (#23842)
3cd8b6e 
Co-authored-by: Cory Rivera <corivera@microsoft.com>
Co-authored-by: Cory Rivera <corivera@microsoft.com>
kisantia pushed a commit that referenced this pull request Jul 14, 2023
@barbaravaldez @corivera
Add basic validation to database names (#23842) (#23881)
2be85a3 
Co-authored-by: Cory Rivera <corivera@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corivera corivera corivera approved these changes

@Charles-Gagnon Charles-Gagnon Charles-Gagnon approved these changes

@ssreerama ssreerama Awaiting requested review from ssreerama

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@barbaravaldez @corivera @Charles-Gagnon