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

Support notes for table group #601

Merged
merged 9 commits into from
Aug 14, 2024
Merged

Support notes for table group #601

merged 9 commits into from
Aug 14, 2024

Conversation

xuantho573
Copy link
Contributor

@xuantho573 xuantho573 commented Aug 1, 2024

Summary

  • Update @dbml/parse elements to support notes for Table Group
    • Validator for Table Group
    • Validator for Note
    • Interpreter for Table Group
  • Add a filed to Table Group for notes
  • Update TypeScript types related to Table Group notes
  • Update test cases

Issue

(issue link here)

Lasting Changes (Technical)

(please list down: code changes/things that have wide-effect; new libraries/functions added that can be used by others; examples below)

  • (Added class EmailValidator to validate email address' validity)
  • (Added Tenant#is_trial? check)

Checklist

Please check directly on the box once each of these are done

  • Documentation (if necessary)
  • Tests (integration test/unit test)
  • Integration Tests Passed
  • Code Review

- Update setting list validator of TableGroupValidator

- Add setting list interpreter for TableGroupInterpreter

- Update TableGroup interpreter

- Update model_structure

- Update testcases

- Update pegjs
@xuantho573 xuantho573 force-pushed the support-notes-for-table-group branch from 129bf95 to 6f8a13c Compare August 7, 2024 09:56
@xuantho573 xuantho573 marked this pull request as ready for review August 7, 2024 10:04
@xuantho573 xuantho573 requested review from NQPhuc and TeaNguyen August 7, 2024 10:06
@NQPhuc NQPhuc requested a review from Huy-DNA August 7, 2024 10:11
Comment on lines 77 to 94
for (const name in settingMap) {
const attrs = settingMap[name];
switch (name) {
case 'headercolor':
errors.push(...attrs.map((attr) => new CompileError(CompileErrorCode.INVALID_TABLE_SETTING, '\'headercolor\' is not supported', attr)))
break;
case 'note':
if (attrs.length > 1) {
errors.push(...attrs.map((attr) => new CompileError(CompileErrorCode.DUPLICATE_TABLE_SETTING, '\'note\' can only appear once', attr)))
}
attrs.forEach((attr) => {
if (!isExpressionAQuotedString(attr.value)) {
errors.push(new CompileError(CompileErrorCode.INVALID_TABLE_SETTING, '\'note\' must be a string literal', attr.value || attr.name!));
}
});
break;
default:
errors.push(...attrs.map((attr) => new CompileError(CompileErrorCode.INVALID_TABLE_SETTING, `Unknown \'${name}\' setting`, attr)))
Copy link
Contributor

Choose a reason for hiding this comment

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

This only need the default and note case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the headercolor case because Table has it, and we may support it for Table Group in the future

@xuantho573 xuantho573 requested a review from Huy-DNA August 8, 2024 07:24
Copy link
Contributor

@Huy-DNA Huy-DNA left a comment

Choose a reason for hiding this comment

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

seems fine by me

@xuantho573 xuantho573 added the PR: New Feature 🚀 A type of pull request used for changelog categories label Aug 14, 2024
@Huy-DNA Huy-DNA self-requested a review August 14, 2024 07:14
@NQPhuc NQPhuc merged commit 7135e60 into master Aug 14, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: New Feature 🚀 A type of pull request used for changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants