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.

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

fix: default value fixes #1166

Merged
merged 3 commits into from
Aug 16, 2024
Merged

fix: default value fixes #1166

merged 3 commits into from
Aug 16, 2024

Conversation

sareyu
Copy link
Collaborator

@sareyu sareyu commented Aug 15, 2024

image
  • fixed display non-string default values
  • added "-" for empty default value as in yc

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
108 104 0 4 0

Bundle Size: ✅

Current: 78.90 MB | Main: 78.90 MB
Diff: 0.03 KB (-0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

@sareyu sareyu requested a review from Raubzeug August 15, 2024 11:18
@sareyu sareyu marked this pull request as ready for review August 15, 2024 11:23
@@ -81,7 +81,7 @@ function prepareRowTableSchema(data: TTableDescription = {}): SchemaData[] {
type: Type,
notNull: NotNull,
autoIncrement: Boolean(DefaultFromSequence),
defaultValue: DefaultFromLiteral?.value?.text_value,
defaultValue: Object.values(DefaultFromLiteral?.value || {})[0] || '-',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you decide to use such a logic? It seems less obvious than prev version.
Why not just DefaultFromLiteral?.value?.text_value || '-' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because field with value has different naming depends on it's type. i.e. bool_value for bool, text_value for string...

Copy link
Contributor

Choose a reason for hiding this comment

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

Now i see... but lets also fix it in types DefaultFromLiteral.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -102,7 +102,7 @@ const defaultValueColumn: SchemaColumn = {
return i18n('column-title.defaultValue');
},
width: 100,
render: ({row}) => row.defaultValue,
render: ({row}) => String(row.defaultValue),
Copy link
Contributor

Choose a reason for hiding this comment

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

No need in explicit transformation, typescript should catch that defaultValue is always string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it doesn't work for bool such way

@sareyu sareyu merged commit a210ba1 into main Aug 16, 2024
6 checks passed
@sareyu sareyu deleted the sareyu.default-value-fixup branch August 16, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants