NotificationsYou must be signed in to change notification settings
Fork530
Star8.6k

feat (dashboard): support empty string as default text value in database tables#3112

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Draft

dbm03 wants to merge6 commits intomain

base:main

Choose a base branch

fromfeat/add-empty-string-default

Draft

feat (dashboard): support empty string as default text value in database tables#3112

dbm03 wants to merge6 commits intomainfromfeat/add-empty-string-default

Conversation

Copy link

Member

dbm03 commentedJan 1, 2025•
edited by github-actionsbot
Loading

User description

Resolves#2925

PR Type

Enhancement

Description

Add empty string default for text fields
Update UI to support new default value
Modify form handling for empty string defaults
Adjust column classification in record forms

Changes walkthrough 📝

Relevant files

Enhancement

BaseRecordForm.tsx `Update form handling for empty string defaults` dashboard/src/features/orgs/projects/database/dataGrid/components/BaseRecordForm/BaseRecordForm.tsx Updated column classification logic Modified insertable values handling Changed description for optional columns	+11/-3
ColumnEditorRow.tsx `Enhance DefaultValueAutocomplete for empty string` dashboard/src/features/orgs/projects/database/dataGrid/components/BaseTableForm/ColumnEditorRow.tsx Added formatting for empty string default value Updated DefaultValueAutocomplete component	+9/-2
DatabaseRecordInputGroup.tsx `Update DatabaseRecordInputGroup for empty defaults` dashboard/src/features/orgs/projects/database/dataGrid/components/DatabaseRecordInputGroup/DatabaseRecordInputGroup.tsx Updated getPlaceholder function for empty string Modified DatabaseRecordInputGroup component	+7/-0
postgresqlConstants.ts `Add empty string default to PostgreSQL functions` dashboard/src/features/orgs/projects/database/dataGrid/utils/postgresqlConstants/postgresqlConstants.ts Added ''::text to postgresFunctions for text type	+1/-1

Documentation

dry-kids-chew.md `Add changeset for empty string default feature` .changeset/dry-kids-chew.md Added changeset for new feature	+5/-0

💡PR-Agent usage: Comment/help "your question" on any pull request to receive relevant information

dbm03 requested a review fromdbarrosop

January 1, 2025 16:50

dbm03 self-assigned this

Jan 1, 2025

dbm03 requested review fromnunopato andonehassan ascode owners

January 1, 2025 16:50

Copy link

changeset-botbot commentedJan 1, 2025•
edited
Loading

🦋 Changeset detected

Latest commit:f539713

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@nhost/dashboard	Minor

Not sure what this means?Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercelbot commentedJan 1, 2025•
edited
Loading

The latest updates on your projects. Learn more aboutVercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
dashboard-staging	✅ Ready (Inspect)	Visit Preview	Mar 17, 2025 10:55am
example-nextjs-server-components	✅ Ready (Inspect)	Visit Preview	Mar 17, 2025 10:55am
example-react-apollo	✅ Ready (Inspect)	Visit Preview	Mar 17, 2025 10:55am
example-sveltekit	✅ Ready (Inspect)	Visit Preview	Mar 17, 2025 10:55am
example-vue-apollo	✅ Ready (Inspect)	Visit Preview	Mar 17, 2025 10:55am

1 Skipped Deployment

Name	Status	Preview	Updated (UTC)
dashboard	⬜️ Ignored (Inspect)	Visit Preview	Mar 17, 2025 10:55am

vercelbotdeployed toPreview – example-sveltekit

January 1, 2025 16:50

View deployment

nhost-build added the dashboardNhost Dashboard label

Jan 1, 2025

github-actionsbot added the Review effort [1-5]: 3 label

Jan 1, 2025

Copy link

Contributor

github-actionsbot commentedJan 1, 2025•
edited
Loading

PR Reviewer Guide 🔍

(Review updated until commit`4f00d97`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 2925 - Fully compliant Fully compliant requirements: Add option ''::text for Text default values Include ''::text as a default option for Text fields in the dropdown Allow non-nullable Text fields to have empty string as the default value
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Logic Complexity The changes in the condition for determining required columns and handling default values have increased the complexity of the code. This should be carefully reviewed to ensure it correctly handles all edge cases. if(column.isPrimary\|\|(!column.isNullable&&!column.defaultValue&&!column.isDefaultValueCustom&&!column.isIdentity) UI Behavior The new handling of empty string default values in the UI should be thoroughly tested to ensure it correctly displays and processes these values in all scenarios. constformattedInputValue=useMemo(()=>{if(defaultValue?.value===''){return"''::text";}returnisIdentity ?'' :inputValue;},[isIdentity,defaultValue?.value,inputValue]);

vercelbotdeployed toPreview – example-vue-apollo

January 1, 2025 16:51

View deployment

Copy link

Contributor

github-actionsbot commentedJan 1, 2025•
edited
Loading

PR Code Suggestions ✨

Latest suggestions up to4f00d97
Explore these optional code suggestions:

Category	Suggestion	Score
General	Extract complex condition into a separate function to improve code readability and maintainability Consider simplifying the condition for required columns by extracting it into a separate function. This will improve readability and make the code easier to maintain. dashboard/src/features/orgs/projects/database/dataGrid/components/BaseRecordForm/BaseRecordForm.tsx [45-51] -if (- column.isPrimary \|\|- (!column.isNullable &&- !column.defaultValue &&- !column.isDefaultValueCustom &&- !column.isIdentity)-) {+const isRequiredColumn = (column) =>+ column.isPrimary \|\|+ (!column.isNullable && !column.defaultValue && !column.isDefaultValueCustom && !column.isIdentity);+if (isRequiredColumn(column)) {+ Suggestion importance[1-10]: 6 Why: The suggestion improves code readability and maintainability by extracting a complex condition into a separate function. This change makes the code easier to understand and modify in the future.	6
General	Extract complex condition into a separate function to enhance code clarity and testability The condition for checking if a value should be included in insertableValues is complex and may be prone to errors. Consider extracting this logic into a separate function to improve readability and testability. dashboard/src/features/orgs/projects/database/dataGrid/components/BaseRecordForm/BaseRecordForm.tsx [102-107] -if (- !value &&- (gridColumn?.defaultValue \|\|- gridColumn?.defaultValue === '' \|\|- gridColumn?.isIdentity)-) {+const shouldUseDefaultValue = (value, gridColumn) =>+ !value && (gridColumn?.defaultValue !== undefined \|\| gridColumn?.isIdentity);+if (shouldUseDefaultValue(value, gridColumn)) {+ Suggestion importance[1-10]: 6 Why: The suggestion enhances code clarity and testability by extracting a complex condition into a separate function. This improves readability and makes the logic easier to test independently.	6

Previous suggestions

Suggestions up to commit021bc2f

Category	Suggestion	Score
General	Simplify the condition for checking default values and identity columns using modern JavaScript features Use optional chaining and nullish coalescing operators to simplify the condition for checking default values and identity columns. This will make the code more concise and easier to understand. dashboard/src/features/orgs/projects/database/dataGrid/components/BaseRecordForm/BaseRecordForm.tsx [102-107] -if (- !value &&- (gridColumn?.defaultValue \|\|- gridColumn?.defaultValue === '' \|\|- gridColumn?.isIdentity)-) {+if (!value && (gridColumn?.defaultValue !== undefined \|\| gridColumn?.isIdentity)) { Suggestion importance[1-10]: 8 Why: This suggestion effectively simplifies the condition using optional chaining and nullish coalescing, which aligns well with the PR's intent to handle empty string default values. It improves code readability and maintainability.	8
	Simplify the condition for determining required columns to improve readability and reduce potential errors Simplify the condition for determining required columns by using the logical OR operator (\|\|) instead of multiple AND (&&) operators. This will make the code more readable and less prone to errors. dashboard/src/features/orgs/projects/database/dataGrid/components/BaseRecordForm/BaseRecordForm.tsx [45-51] if ( column.isPrimary \|\| (!column.isNullable &&- !column.defaultValue &&- column.defaultValue !== '' &&+ (column.defaultValue === null \|\| column.defaultValue === undefined) && !column.isIdentity) ) { Suggestion importance[1-10]: 7 Why: The suggestion simplifies the complex condition, making it more readable and less error-prone. It correctly addresses the logic for determining required columns, which is a key part of the PR changes.	7
	Improve readability of string formatting using template literals Use a template literal for the formatted input value to improve readability and maintainability. This will make it easier to understand the logic for displaying empty string values. dashboard/src/features/orgs/projects/database/dataGrid/components/BaseTableForm/ColumnEditorRow.tsx [154-159] const formattedInputValue = useMemo(() => { if (defaultValue?.value === '') {- return "''::text";+ return `''::text`; } return isIdentity ? '' : inputValue; }, [isIdentity, defaultValue?.value, inputValue]); Suggestion importance[1-10]: 3 Why: While the suggestion to use template literals is valid, it offers only a minor improvement in readability. The existing code is already functional and clear, so the impact of this change is relatively low.	3