fix: v3 meta api corrections #11450
fix: v3 meta api corrections #11450
Conversation
📝 WalkthroughWalkthroughThe changes introduce enhanced support for button-type columns and related webhook properties in API v3 data transformation utilities. Key updates include new mappings for button subtypes, unified naming conventions for relation keys, and additional meta property handling for AI and webhook integrations. Option key renaming and extended transformation logic are also implemented. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_V3_Transformation
participant Database
Client->>API_V3_Transformation: Submit column configuration (with button type)
API_V3_Transformation->>API_V3_Transformation: Detects button type and subtype (formula/webhook/ai)
API_V3_Transformation->>API_V3_Transformation: Maps subtype-specific properties (e.g., webhook_id, prompt)
API_V3_Transformation->>Database: Store transformed column configuration
Database-->>API_V3_Transformation: Acknowledge storage
API_V3_Transformation-->>Client: Return transformed configuration response
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
2d1e9c8 to
4183722
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/nocodb/src/utils/transformProperties.ts (1)
30-36:⚠️ Potential issueLong-text “AI” flag may be silently dropped
metaObj.aiis read and forwarded, but the server-side JSON key was just renamed (see builder mapping fromai -> generate_text_using_ai).
If the API already emits the new key (generate_text_using_ai),metaObj.aiwill beundefined, causing the UI to think the flag is off.- ai: metaObj.ai || false, + ai: (metaObj.ai ?? metaObj.generate_text_using_ai) || false,Consider keeping both for one release cycle, then remove the deprecated one.
🧹 Nitpick comments (1)
packages/nocodb/src/utils/transformProperties.ts (1)
162-170: Risk of overwriting existingpromptin AI button optionsIf the backend already sends
prompt, the spread order overwrites it withformula || '', potentially erasing a user-defined prompt.- newField.options = { - ...newField.options, - prompt: newField.options.formula || '', - }; + newField.options = { + prompt: newField.options.prompt ?? newField.options.formula ?? '', + ...newField.options, + };Placing the spread last gives precedence to the existing value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/nocodb/src/schema/swagger-v3.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
packages/nocodb/src/utils/api-v3-data-transformation.builder.ts(8 hunks)packages/nocodb/src/utils/transformProperties.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: DarkPhoenix2704
PR: nocodb/nocodb#8781
File: packages/nc-gui/components/smartsheet/grid/GroupBy.vue:225-232
Timestamp: 2024-06-18T13:58:19.464Z
Learning: DarkPhoenix2704 prefers to keep code implementations that prioritize readability, even if an alternative might be more concise.
🧬 Code Graph Analysis (1)
packages/nocodb/src/utils/api-v3-data-transformation.builder.ts (1)
packages/nocodb-sdk/src/lib/globals.ts (1)
LongTextAiMetaProp(206-206)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: release-docker / buildx
- GitHub Check: pre-build-for-playwright / playwright
- GitHub Check: unit-tests-pg
- GitHub Check: unit-tests
🔇 Additional comments (2)
packages/nocodb/src/utils/api-v3-data-transformation.builder.ts (2)
303-333: Dead mapping entry — never reached because of theallowedlist
related_table_rollup_field_idis mapped back tofk_rollup_column_id, but the key is not in theallowedarray, so it will be stripped beforeremapColumns()runs.Either add it to
allowedor remove the mapping to avoid confusion:allowed: [ 'formula_raw', …, + 'related_table_rollup_field_id', 'fk_webhook_id', ],Or drop the mapping if truly unnecessary.
500-556: Column-options V3→V2 builder:button_hook_id&webhook_idmapping conflictYou allow both keys but map only to
fk_webhook_id.
If both are present, whichever is last inObject.entriesorder wins, leading to non-deterministic output.Add explicit precedence or mutual-exclusion handling:
transformFn: (data) => { + if ('button_hook_id' in data && 'webhook_id' in data) { + console.warn( + '[builder] both button_hook_id and webhook_id provided; preferring webhook_id', + ); + }Alternatively, drop one key from the
allowedlist.
| case UITypes.Number: | ||
| newField.options = { | ||
| ...newField.options, | ||
| thousands_separator: metaObj.isLocaleString || false, | ||
| locale_string: metaObj.isLocaleString || false, | ||
| }; | ||
| break; |
There was a problem hiding this comment.
🛠 8000 Refactor suggestion
locale_string option is populated from the old meta key
Similar to the previous issue: you now expose locale_string to the UI but still look up isLocaleString in meta. After the V3 builder starts emitting locale_string, this will always be false.
- locale_string: metaObj.isLocaleString || false,
+ locale_string: (metaObj.isLocaleString ?? metaObj.locale_string) || false,Mirror the same change in the Decimal / Roll-up branch below.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case UITypes.Number: | |
| newField.options = { | |
| ...newField.options, | |
| thousands_separator: metaObj.isLocaleString || false, | |
| locale_string: metaObj.isLocaleString || false, | |
| }; | |
| break; | |
| case UITypes.Number: | |
| newField.options = { | |
| ...newField.options, | |
| - locale_string: metaObj.isLocaleString || false, | |
| + locale_string: (metaObj.isLocaleString ?? metaObj.locale_string) || false, | |
| }; | |
| break; |
🤖 Prompt for AI Agents
In packages/nocodb/src/utils/transformProperties.ts around lines 47 to 52, the
code sets the locale_string option using metaObj.isLocaleString, which is
outdated. Update the code to read locale_string directly from
metaObj.locale_string instead. Apply the same update to the Decimal and Roll-up
case branches below to ensure consistency with the new V3 builder output.
| 'fk_value_col_id', | ||
| ], | ||
| mappings: { | ||
| fk_column_id: 'field_id', | ||
| fk_parent_id: 'parent_id', | ||
| // fk_hook_id: 'hook_id', | ||
| // fk_link_col_id: 'link_field_id', | ||
| // fk_related_col_id: 'related_field_id', | ||
| fk_value_col_id: 'value_field_id', | ||
| comparison_op: 'operator', |
There was a problem hiding this comment.
Relation-filter renaming incomplete
fk_related_col_id is now allowed, but the mapper line that should convert it to related_field_id is commented out. As a result, the field name changes between layers, breaking round-trips.
Uncomment the mapping or delete the allowance.
- // fk_related_col_id: 'related_field_id',
+ fk_related_col_id: 'related_field_id',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'fk_value_col_id', | |
| ], | |
| mappings: { | |
| fk_column_id: 'field_id', | |
| fk_parent_id: 'parent_id', | |
| // fk_hook_id: 'hook_id', | |
| // fk_link_col_id: 'link_field_id', | |
| // fk_related_col_id: 'related_field_id', | |
| fk_value_col_id: 'value_field_id', | |
| comparison_op: 'operator', | |
| 'fk_value_col_id', | |
| ], | |
| mappings: { | |
| fk_column_id: 'field_id', | |
| fk_parent_id: 'parent_id', | |
| // fk_hook_id: 'hook_id', | |
| fk_related_col_id: 'related_field_id', | |
| fk_value_col_id: 'value_field_id', | |
| comparison_op: 'operator', |
🤖 Prompt for AI Agents
In packages/nocodb/src/utils/api-v3-data-transformation.builder.ts around lines
720 to 728, the mapping for 'fk_related_col_id' to 'related_field_id' is
commented out, causing inconsistent field name translation and breaking
round-trips. To fix this, uncomment the mapping line for 'fk_related_col_id' so
it properly converts to 'related_field_id', ensuring consistent naming across
layers.
| const { type, ...rest } = data.options as Record<string, any>; | ||
|
|
||
| // Transform button properties based on type | ||
| if (type === 'formula') { | ||
| options = { | ||
| type, | ||
| formula: rest.formula, | ||
| label: rest.label, | ||
| color: rest.color, | ||
| theme: rest.theme, | ||
| icon: rest.icon, | ||
| }; | ||
| } else if (type === 'webhook') { | ||
| options = { | ||
| type, | ||
| webhook_id: rest.button_hook_id, | ||
| label: rest.label, | ||
| color: rest.color, | ||
| theme: rest.theme, | ||
| icon: rest.icon, | ||
| }; | ||
| } else if (type === 'ai') { | ||
| options = { | ||
| type, | ||
| prompt: rest.prompt, | ||
| integration_id: rest.integration_id, | ||
| output_column_ids: rest.output_column_ids, | ||
| label: rest.label, | ||
| color: rest.color, | ||
| theme: rest.theme, | ||
| icon: rest.icon, | ||
| }; | ||
| } else { | ||
| // Fallback to original transformation | ||
| options = { ...rest, button_type: type }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Button-option normalisation misses SDK enum and duplicates logic
-
The code switches on string literals (
'formula','webhook','ai') while the rest of the codebase relies onButtonActionsType.
Mixing both increases the chance of typos. -
The fallback branch adds
button_type, whereas the named branches drop it. Downstream consumers might depend on this field.
Consider unifying on the enum and always retaining the raw type:
- const { type, ...rest } = data.options as Record<string, any>;
+ const { type, ...rest } = data.options as Record<string, any>;
+
+ // Ensure we’re dealing with canonical values
+ const btnType =
+ typeof type === 'string' ? type.toLowerCase() : String(type);
- if (type === 'formula') {
+ if (btnType === ButtonActionsType.Formula) {
options = {
- type,
+ type: btnType,
formula: rest.formula,
...
};
- } else if (type === 'webhook') {
+ } else if (btnType === ButtonActionsType.Webhook) {
options = {
- type,
+ type: btnType,
webhook_id: rest.button_hook_id,
...
};
- } else if (type === 'ai') {
+ } else if (btnType === ButtonActionsType.Ai) {
options = {
- type,
+ type: btnType,
prompt: rest.prompt,
...
};
} else {
- options = { ...rest, button_type: type };
+ options = { ...rest, button_type: btnType };
}This removes string duplication and guarantees consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { type, ...rest } = data.options as Record<string, any>; | |
| // Transform button properties based on type | |
| if (type === 'formula') { | |
| options = { | |
| type, | |
| formula: rest.formula, | |
| label: rest.label, | |
| color: rest.color, | |
| theme: rest.theme, | |
| icon: rest.icon, | |
| }; | |
| } else if (type === 'webhook') { | |
| options = { | |
| type, | |
| webhook_id: rest.button_hook_id, | |
| label: rest.label, | |
| color: rest.color, | |
| theme: rest.theme, | |
| icon: rest.icon, | |
| }; | |
| } else if (type === 'ai') { | |
| options = { | |
| type, | |
| prompt: rest.prompt, | |
| integration_id: rest.integration_id, | |
| output_column_ids: rest.output_column_ids, | |
| label: rest.label, | |
| color: rest.color, | |
| theme: rest.theme, | |
| icon: rest.icon, | |
| }; | |
| } else { | |
| // Fallback to original transformation | |
| options = { ...rest, button_type: type }; | |
| } | |
| const { type, ...rest } = data.options as Record<string, any>; | |
| // Ensure we’re dealing with canonical values | |
| const btnType = | |
| typeof type === 'string' ? type.toLowerCase() : String(type); | |
| // Transform button properties based on type | |
| if (btnType === ButtonActionsType.Formula) { | |
| options = { | |
| type: btnType, | |
| formula: rest.formula, | |
| label: rest.label, | |
| color: rest.color, | |
| theme: rest.theme, | |
| icon: rest.icon, | |
| }; | |
| } else if (btnType === ButtonActionsType.Webhook) { | |
| options = { | |
| type: btnType, | |
| webhook_id: rest.button_hook_id, | |
| label: rest.label, | |
| color: rest.color, | |
| theme: rest.theme, | |
| icon: rest.icon, | |
| }; | |
| } else if (btnType === ButtonActionsType.Ai) { | |
| options = { | |
| type: btnType, | |
| prompt: rest.prompt, | |
| integration_id: rest.integration_id, | |
| 8000 | output_column_ids: rest.output_column_ids, |
| label: rest.label, | |
| color: rest.color, | |
| theme: rest.theme, | |
| icon: rest.icon, | |
| }; | |
| } else { | |
| // Fallback to original transformation | |
| options = { ...rest, button_type: btnType }; | |
| } |
🤖 Prompt for AI Agents
In packages/nocodb/src/utils/api-v3-data-transformation.builder.ts around lines
444 to 479, replace the string literal checks for button types ('formula',
'webhook', 'ai') with the corresponding values from the ButtonActionsType enum
to avoid typos and ensure consistency. Also, modify all branches to retain the
original type field (e.g., as button_type) instead of dropping it in named
branches, so downstream consumers receive a consistent structure. Refactor the
code to unify the transformation logic by always including the enum-based type
and preserving the raw type field in the options object.
Change Summary
Fix: Missing v3 Meta props and corrections
Change type