Nc fix/prompt existing issues #9997
Conversation
dosubot
bot
added
the
size:L
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications across multiple Vue components, primarily enhancing the handling of AI features and simplifying conditional logic related to feature flags. Key changes include the introduction of computed properties to manage the visibility of AI buttons, the refinement of validation logic for button types, and the transition of components to TypeScript. Additionally, the error handling and loading states have been improved to enhance user feedback during interactions. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
o1lab
force-pushed
the
nc-fix/prompt-existing-issues
branch
from
6ed1bae to
e1cac7b
Compare
dosubot
bot
added
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (4)
packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue (1)
Line range hint
61-66: Fix incorrect index usage in fallback logicThere's a bug in the fallback logic where
activeFieldIndex.valueis used even when falling back to the first option. This could cause undefined behavior.Apply this fix:
const handleKeydownEnter = () => { if (filteredOptions.value[activeFieldIndex.value]) { onClick(filteredOptions.value[activeFieldIndex.value].name) } else if (filteredOptions.value[0]) { - onClick(filteredOptions.value[activeFieldIndex.value].name) + onClick(filteredOptions.value[0].name) } }packages/nc-gui/components/smartsheet/column/AiButtonOptions.vue (3)
Line range hint
308-870: Avoid rendering empty<div>inv-elseblock.The
v-elseblock renders an empty<div>, which is unnecessary and can be removed to simplify the template.Apply this diff to remove the unnecessary
v-elseblock:</div> - <div v-else></div>
614-614: Consider using responsive units for width properties.Using fixed pixel values for
min-widthandmax-widthmay affect responsiveness on different screen sizes. Consider using relative units like percentages orremfor better adaptability.Apply this diff to adjust the width properties:
class="!w-auto min-w-[550px] max-w-[550px]" +class="!w-auto min-w-[50%] max-w-[80%]"
701-703: Use predefined text sizes for consistency.Instead of hardcoding
text-[10px]andleading-[14px], consider using predefined classes from your design system to maintain consistency.Apply this diff to use predefined classes:
-<div class="text-[10px] leading-[14px] text-gray-300 uppercase mb-1">Preview checklist</div> +<div class="text-xs leading-tight text-gray-300 uppercase mb-1">Preview checklist</div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
packages/nc-gui/components/ai/Settings.vue(1 hunks)packages/nc-gui/components/dashboard/TreeView/CreateViewBtn.vue(1 hunks)packages/nc-gui/components/smartsheet/column/AiButtonOptions.vue(7 hunks)packages/nc-gui/components/smartsheet/column/ButtonOptions.vue(7 hunks)packages/nc-gui/components/smartsheet/column/EditOrAdd.vue(1 hunks)packages/nc-gui/components/smartsheet/column/LongTextOptions.vue(4 hunks)packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue(1 hunks)packages/nc-gui/components/smartsheet/details/Fields.vue(5 hunks)packages/nc-gui/components/smartsheet/topbar/ViewListDropdown.vue(2 hunks)
🔇 Additional comments (10)
packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue (1)
20-20: LGTM! Simplified filtering logic.
The simplified filtering logic using searchCompare is cleaner and more maintainable. This change reduces complexity while maintaining the same functionality.
packages/nc-gui/components/smartsheet/details/Fields.vue (4)
7-7: Importing isAIPromptCol Function Is Appropriate
The addition of the isAIPromptCol import correctly integrates the function needed for handling AI prompt columns.
719-723: Assignment of AI Button Properties Is Correct
The properties output_column_ids and fk_integration_id are correctly assigned for AI buttons, ensuring proper configuration.
743-752: Initialization of prompt_raw for AI Prompt Columns
The code correctly initializes the prompt_raw property for AI prompt columns, which ensures the prompt content is properly set up.
Line range hint 199-206: Conditional Rendering of AI Features Using Feature Flag
The use of isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) to conditionally display AI-related UI elements ensures that AI features are only accessible when enabled.
packages/nc-gui/components/dashboard/TreeView/CreateViewBtn.vue (1)
Line range hint 199-206: Conditional Rendering of AI Menu Item Using Feature Flag
Wrapping the AI menu item with v-if="isFeatureEnabled(FEATURE_FLAG.AI_FEATURES)" appropriately controls its visibility based on the feature flag, ensuring users only see AI options when the feature is enabled.
packages/nc-gui/components/smartsheet/topbar/ViewListDropdown.vue (1)
24-24: Feature Flag Integration for AI Features Is Appropriate
Using isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) to manage AI feature availability aligns with the application's feature toggling strategy.
packages/nc-gui/components/smartsheet/column/AiButtonOptions.vue (3)
35-35: Integration of fromTableExplorer is appropriate.
Adding fromTableExplorer from the useColumnCreateStoreOrThrow() store allows conditional rendering based on the table explorer context.
249-256: Logic for isAiButtonEnabled is sound.
The computed property isAiButtonEnabled correctly determines the visibility of the AI button based on editing state and feature flags.
346-346: Conditional rendering of submit button is appropriate.
The v-if="!fromTableExplorer" condition ensures that the submit button is only displayed when not in the table explorer context.
| if (column.uidt === UITypes.Button) { | ||
| if (isNew) { | ||
| if (column.type === ButtonActionsType.Url && !column.formula_raw) return false | ||
| if (column.type === ButtonActionsType.Webhook && !column.fk_webhook_id) return false | ||
| } | ||
| if (column.type === ButtonActionsType.Ai) { | ||
| return !( | ||
| !column.fk_integration_id || | ||
| !column.formula_raw?.trim() || | ||
| !column.output_column_ids?.length || | ||
| !column.output_column_ids?.split(',')?.length | ||
| ) | ||
| } |
There was a problem hiding this comment.
Potential Logical Error in AI Button Validation
The validation logic in the isColumnValid function for ButtonActionsType.Ai may incorrectly validate output_column_ids when it's an empty string. Splitting an empty string results in [''], which has a length of 1, causing the validation to pass when it should fail.
Apply this diff to correct the validation logic:
if (column.type === ButtonActionsType.Ai) {
return !(
!column.fk_integration_id ||
!column.formula_raw?.trim() ||
- !column.output_column_ids?.length ||
- !column.output_column_ids?.split(',')?.length
+ !column.output_column_ids ||
+ !column.output_column_ids.split(',').filter(id => id.trim()).length
)
}This change ensures that output_column_ids is not empty and contains at least one valid ID after splitting and trimming.
📝 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.
| if (column.uidt === UITypes.Button) { | |
| if (isNew) { | |
| if (column.type === ButtonActionsType.Url && !column.formula_raw) return false | |
| if (column.type === ButtonActionsType.Webhook && !column.fk_webhook_id) return false | |
| } | |
| if (column.type === ButtonActionsType.Ai) { | |
| return !( | |
| !column.fk_integration_id || | |
| !column.formula_raw?.trim() || | |
| !column.output_column_ids?.length || | |
| !column.output_column_ids?.split(',')?.length | |
| ) | |
| } | |
| if (column.uidt === UITypes.Button) { | |
| if (isNew) { | |
| if (column.type === ButtonActionsType.Url && !column.formula_raw) return false | |
| if (column.type === ButtonActionsType.Webhook && !column.fk_webhook_id) return false | |
| } | |
| if (column.type === ButtonActionsType.Ai) { | |
| return !( | |
| !column.fk_integration_id || | |
| !column.formula_raw?.trim() || | |
| !column.output_column_ids || | |
| !column.output_column_ids.split(',').filter(id => id.trim()).length | |
| ) | |
| } |
| if (!vFkIntegrationId.value && !isEditColumn.value) { | ||
| if (aiIntegrations.value.length > 0 && aiIntegrations.value[0].id) { | ||
| vFkIntegrationId.value = aiIntegrations.value[0].id | ||
| nextTick(() => { | ||
| onIntegrationChange() | ||
| }) | ||
| } | ||
| } else { | ||
| } else if (vFkIntegrationId.value) { |
There was a problem hiding this comment.
Undefined Variable isEditColumn May Cause Runtime Error
The variable isEditColumn used in the condition at line 66 is not defined within this component, which will result in a ReferenceError at runtime.
Ensure that isEditColumn is properly defined or imported. If it's intended to be a prop or injected value, declare it accordingly. For example:
+const { isEditColumn } = useSomeHookOrContext()
onMounted(async () => {
if (!vFkIntegrationId.value && !isEditColumn.value) {
if (aiIntegrations.value.length > 0 && aiIntegrations.value[0].id) {
vFkIntegrationId.value = aiIntegrations.value[0].id
nextTick(() => {
onIntegrationChange()
})
}
} else if (vFkIntegrationId.value) {
lastIntegrationId.value = vFkIntegrationId.value
}
})Committable suggestion skipped: line range outside the PR's diff.
| <template v-if="isFeatureEnabled(FEATURE_FLAG.AI_FEATURES)"> | ||
| <NcDivider /> | ||
| <a-menu-item data-testid="sidebar-view-create-ai" @click="onOpenModal({ type: 'AI' })"> | ||
| <div class="nc-viewlist-submenu-popup-item"> | ||
| <GeneralIcon icon="ncAutoAwesome" class="!w-4 !h-4 text-nc-fill-purple-dark" /> | ||
| <div>{{ $t('labels.aiSuggested') }}</div> | ||
| </div> | ||
| </a-menu-item> | ||
| </template> |
There was a problem hiding this comment.
💡 Codebase verification
AI Menu Item Visibility Should Also Check Integration Availability
The AI menu item should check both the feature flag and AI integration availability to ensure proper visibility. Add a check for aiIntegrationAvailable alongside the existing feature flag check.
<template v-if="isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) && aiIntegrationAvailable">packages/nc-gui/components/smartsheet/topbar/ViewListDropdown.vue: Update the condition to include both checks
This is consistent with how other components handle AI features, as seen in SelectOptions.vue where both conditions are considered for the tooltip message.
🔗 Analysis chain
Verify AI Menu Item Visibility When No AI Integration Is Configured
By relying solely on isFeatureEnabled(FEATURE_FLAG.AI_FEATURES), users may see the AI menu item even if no AI integration is configured. Ensure that the UI behaves correctly when AI integrations are not available.
Run the following script to check if AI integrations are properly accounted for in the UI:
This script verifies if other components consider AI integration availability alongside the feature flag, helping ensure consistent behavior across the UI.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for components that check both feature flag and AI integration availability
# Search for components that use both `isFeatureEnabled` and `aiIntegrationAvailable`
rg 'isFeatureEnabled\(.+AI_FEATURES.+\).*&&.*aiIntegrationAvailable' --type vue
Length of output: 109
Script:
#!/bin/bash
# Let's try a different approach to search for AI-related checks in the codebase
# First, let's find files that contain both isFeatureEnabled and AI_FEATURES
rg -l "isFeatureEnabled.*AI_FEATURES|AI_FEATURES.*isFeatureEnabled"
# Then, let's look for any AI integration availability checks
rg -l "aiIntegrationAvailable"
# Let's also check for any AI-related configuration or integration setup
rg -l "AI.*[Ii]ntegration|[Ii]ntegration.*AI"
# Finally, let's look at the actual implementation of isFeatureEnabled
rg "isFeatureEnabled.*\{" -A 5
Length of output: 3868
Script:
#!/bin/bash
# Let's examine the useNocoAi composable which seems to handle AI integration availability
cat packages/nc-gui/composables/useNocoAi.ts
# Also check the implementation of isFeatureEnabled
rg -l "export.*isFeatureEnabled"
Length of output: 9419
Script:
#!/bin/bash
# Let's search for the feature flag implementation and usage
rg "FEATURE_FLAG.AI_FEATURES" -B 2 -A 2
# Also check where aiIntegrationAvailable is used in conditions
rg "aiIntegrationAvailable.*\?" -B 2 -A 2
Length of output: 9048
| validator: (_: any, value: any) => { | ||
| return new Promise<void>((resolve, reject) => { | ||
| if (isEnabledGenerateText.value && !value) { | ||
| reject(new Error(t('title.aiIntegrationMissing'))) | ||
| } | ||
| resolve() | ||
| }) |
There was a problem hiding this comment.
Fix Promise logic in fk_integration_id validator
In the custom validator for fk_integration_id, the Promise executor calls resolve() after reject(). This means that resolve() will be called even when validation fails, causing the validation to incorrectly pass.
Apply this diff to fix the issue:
return new Promise<void>((resolve, reject) => {
if (isEnabledGenerateText.value && !value) {
reject(new Error(t('title.aiIntegrationMissing')))
+ return
}
resolve()
})📝 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.
| validator: (_: any, value: any) => { | |
| return new Promise<void>((resolve, reject) => { | |
| if (isEnabledGenerateText.value && !value) { | |
| reject(new Error(t('title.aiIntegrationMissing'))) | |
| } | |
| resolve() | |
| }) | |
| validator: (_: any, value: any) => { | |
| return new Promise<void>((resolve, reject) => { | |
| if (isEnabledGenerateText.value && !value) { | |
| reject(new Error(t('title.aiIntegrationMissing'))) | |
| return | |
| } | |
| resolve() | |
| }) |
| v-model:model="vModel.model" | ||
| v-model:randomness="vModel.randomness" | ||
| :workspace-id="activeWorkspaceId" |
There was a problem hiding this comment.
Correct invalid v-model attribute names
The v-model argument names in Vue should not contain hyphens, as they are invalid JavaScript identifiers and will cause syntax errors. Replace hyphens with camelCase in the attribute names.
Apply this diff to correct the attribute names:
<AiSettings
- v-model:fk-integration-id="vModel.fk_integration_id"
+ v-model:fkIntegrationId="vModel.fk_integration_id"
v-model:model="vModel.model"
v-model:randomness="vModel.randomness"
:workspace-id="activeWorkspaceId"
:show-tooltip="false"
:is-edit-column="isEdit"
placement="bottomRight"
>Committable suggestion skipped: line range outside the PR's diff.
| return new Promise<void>((resolve, reject) => { | ||
| if (vModel.value.type === ButtonActionsType.Ai && !value) { | ||
| reject(new Error('At least one output field is required for AI Button')) | ||
| } | ||
| resolve() | ||
| }) |
There was a problem hiding this comment.
Fix Promise logic in output_column_ids validator
In the custom validator for output_column_ids, the Promise executor calls resolve() after reject(). This will incorrectly pass the validation even when it should fail.
Apply this diff to fix the issue:
return new Promise<void>((resolve, reject) => {
if (vModel.value.type === ButtonActionsType.Ai && !value) {
reject(new Error('At least one output field is required for AI Button'))
+ return
}
resolve()
})📝 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.
| return new Promise<void>((resolve, reject) => { | |
| if (vModel.value.type === ButtonActionsType.Ai && !value) { | |
| reject(new Error('At least one output field is required for AI Button')) | |
| } | |
| resolve() | |
| }) | |
| return new Promise<void>((resolve, reject) => { | |
| if (vModel.value.type === ButtonActionsType.Ai && !value) { | |
| reject(new Error('At least one output field is required for AI Button')) | |
| return | |
| } | |
| resolve() | |
| }) |
| if (vModel.value.type === ButtonActionsType.Ai && !value) { | ||
| reject(new Error(t('title.aiIntegrationMissing'))) | ||
| } | ||
| resolve() | ||
| }) | ||
| }, |
There was a problem hiding this comment.
Fix Promise logic in fk_integration_id validator
Similar to the previous validator, the fk_integration_id validator calls resolve() after reject(), causing incorrect validation results.
Apply this diff to correct the issue:
return new Promise<void>((resolve, reject) => {
if (vModel.value.type === ButtonActionsType.Ai && !value) {
reject(new Error(t('title.aiIntegrationMissing')))
+ return
}
resolve()
})📝 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.
| if (vModel.value.type === ButtonActionsType.Ai && !value) { | |
| reject(new Error(t('title.aiIntegrationMissing'))) | |
| } | |
| resolve() | |
| }) | |
| }, | |
| if (vModel.value.type === ButtonActionsType.Ai && !value) { | |
| reject(new Error(t('title.aiIntegrationMissing'))) | |
| return | |
| } | |
| resolve() | |
| }) | |
| }, |
| const showAiFields = [AIPrompt, AIButton].includes(t.name) ? isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) && !isEdit.value : true | ||
| const isAllowToAddInFormView = isForm.value ? !formViewHiddenColTypes.includes(t.name) : true | ||
| return ( | ||
| systemFiledNotEdited && | ||
| geoDataToggle && | ||
| !specificDBType && | ||
| showDeprecatedField && | ||
| isAllowToAddInFormView && | ||
| hideAiPromptOnEdit && | ||
| hideBetaAi | ||
| ) | ||
| return systemFiledNotEdited && geoDataToggle && !specificDBType && showDeprecatedField && isAllowToAddInFormView && showAiFields |
There was a problem hiding this comment.
Correct typo in variable name systemFiledNotEdited
The variable systemFiledNotEdited has a typo in its name. This could lead to undefined variable usage and potential runtime errors. It should be systemFieldNotEdited.
Apply this diff to fix the typo:
- const systemFiledNotEdited = !isSystemField(t) || formState.value.uidt === t.name || !isEdit.value
+ const systemFieldNotEdited = !isSystemField(t) || formState.value.uidt === t.name || !isEdit.value
...
- return systemFiledNotEdited && geoDataToggle && !specificDBType && showDeprecatedField && isAllowToAddInFormView && showAiFields
+ return systemFieldNotEdited && geoDataToggle && !specificDBType && showDeprecatedField && isAllowToAddInFormView && showAiFieldsCommittable suggestion skipped: line range outside the PR's diff.
| const { isFeatureEnabled } = useBetaFeatureToggle() | ||
There was a problem hiding this comment.
Ensure FEATURE_FLAG is imported.
The FEATURE_FLAG.AI_FEATURES is used but not imported or defined in this component. Please import FEATURE_FLAG to avoid reference errors.
Apply this diff to import FEATURE_FLAG:
+import { FEATURE_FLAG } from '~/helpers/featureFlagHelpers'📝 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 { isFeatureEnabled } = useBetaFeatureToggle() | |
| import { FEATURE_FLAG } from '~/helpers/featureFlagHelpers' | |
| const { isFeatureEnabled } = useBetaFeatureToggle() |
o1lab
force-pushed
the
nc-fix/prompt-existing-issues
branch
from
e1cac7b to
66259d3
Compare
dosubot
bot
added
size:L
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
packages/nc-gui/components/smartsheet/column/AiButtonOptions.vue (3)
249-255: Simplify the computed property logicThe computed property can be simplified using a single return statement.
-const isAiButtonEnabled = computed(() => { - if (isEdit.value) { - return true - } - - return isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) -}) +const isAiButtonEnabled = computed(() => + isEdit.value || isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) +)
875-875: Remove unnecessary empty divThe empty div serves no purpose and can be removed to reduce DOM nodes.
- <div v-else></div>
Line range hint
308-315: Add ARIA label for better accessibilityThe AI configuration button should have an ARIA label for better screen reader support.
- <NcButton type="secondary" size="small" theme="ai" @click.stop="isOpenConfigModal = true"> + <NcButton + type="secondary" + size="small" + theme="ai" + aria-label="Configure AI Button" + @click.stop="isOpenConfigModal = true" + >packages/nc-gui/components/smartsheet/column/ButtonOptions.vue (2)
54-60: Simplify the computed property logicThe computed property can be simplified using a single return statement.
const isAiButtonEnabled = computed(() => { - if (isEdit.value) { - return true - } - - return isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) + return isEdit.value || isFeatureEnabled(FEATURE_FLAG.AI_FEATURES) })
Line range hint
231-245: Consider extracting default values as constantsThe default values for AI button styling could be extracted into constants for better maintainability.
+const AI_BUTTON_DEFAULTS = { + theme: 'light', + label: 'Generate data', + color: 'purple', + icon: 'ncAutoAwesome', +} as const if (vModel.value.type === ButtonActionsType.Ai) { - vModel.value.theme = 'light' - vModel.value.label = 'Generate data' - vModel.value.color = 'purple' - vModel.value.icon = 'ncAutoAwesome' + Object.assign(vModel.value, AI_BUTTON_DEFAULTS) vModel.value.output_column_ids = '' }packages/nc-gui/components/smartsheet/details/Fields.vue (1)
421-421: Remove unnecessary empty lineThere's an extraneous empty line at line 421:
Consider removing this line to maintain code cleanliness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
packages/nc-gui/components/ai/Settings.vue(1 hunks)packages/nc-gui/components/dashboard/TreeView/CreateViewBtn.vue(1 hunks)packages/nc-gui/components/smartsheet/column/AiButtonOptions.vue(7 hunks)packages/nc-gui/components/smartsheet/column/ButtonOptions.vue(7 hunks)packages/nc-gui/components/smartsheet/column/EditOrAdd.vue(1 hunks)packages/nc-gui/components/smartsheet/column/LongTextOptions.vue(3 hunks)packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue(1 hunks)packages/nc-gui/components/smartsheet/details/Fields.vue(5 hunks)packages/nc-gui/components/smartsheet/topbar/ViewListDropdown.vue(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/nc-gui/components/dashboard/TreeView/CreateViewBtn.vue
- packages/nc-gui/components/smartsheet/topbar/ViewListDropdown.vue
- packages/nc-gui/components/ai/Settings.vue
- packages/nc-gui/components/smartsheet/column/UITypesOptionsWithSearch.vue
🔇 Additional comments (19)
packages/nc-gui/components/smartsheet/column/EditOrAdd.vue (2)
186-186: Fix typo in variable name systemFiledNotEdited
The variable name contains a typo: systemFiledNotEdited should be systemFieldNotEdited.
186-189: LGTM: AI feature visibility logic is well implemented
The new condition showAiFields correctly handles the visibility of AI-related fields (AIPrompt and AIButton) based on:
- Feature flag status (
AI_FEATURES) - Edit mode status
- Field type
The integration with existing filters is clean and logical.
packages/nc-gui/components/smartsheet/column/AiButtonOptions.vue (2)
40-41: Import FEATURE_FLAG constant
The FEATURE_FLAG.AI_FEATURES is used but not imported.
Line range hint 346-357: LGTM: Submit button implementation
The submit button's visibility is correctly controlled by the fromTableExplorer flag, and it includes proper loading states and labels.
packages/nc-gui/components/smartsheet/column/ButtonOptions.vue (3)
Line range hint 62-80: LGTM! Well-structured button types configuration
The computed property effectively manages the dynamic inclusion of AI button type based on the feature flag.
189-200:
Fix Promise logic in output_column_ids validator
The Promise executor calls resolve() after reject(). This will incorrectly pass the validation even when it should fail.
return new Promise<void>((resolve, reject) => {
if (vModel.value.type === ButtonActionsType.Ai && !value) {
reject(new Error('At least one output field is required for AI Button'))
+ return
}
resolve()
})201-212:
Fix Promise logic in fk_integration_id validator
Similar to the previous validator, the Promise executor calls resolve() after reject().
return new Promise<void>((resolve, reject) => {
if (vModel.value.type === ButtonActionsType.Ai && !value) {
reject(new Error(t('title.aiIntegrationMissing')))
+ return
}
resolve()
})packages/nc-gui/components/smartsheet/details/Fields.vue (4)
720-723: Initialize AI Button properties correctly
In the updateDefaultColumnValues function, when handling a button of type ButtonActionsType.Ai, ensure that output_column_ids and fk_integration_id are properly initialized:
if (column.type === ButtonActionsType.Ai) {
column.output_column_ids = colOptions?.output_column_ids
column.fk_integration_id = colOptions?.fk_integration_id
}Verify that colOptions contains the necessary properties and that they are correctly assigned to column.
743-751: Handle prompt_raw initialization for AI Prompt Columns
In the updateDefaultColumnValues function, for LongText columns identified as AI Prompt columns, prompt_raw is being set:
if (column.uidt === UITypes.LongText && isAIPromptCol(column)) {
if (column?.id) {
const colOptions = column.colOptions as Record<string, any>
column.prompt_raw = colOptions?.prompt_raw
} else {
column.prompt_raw = column.prompt_raw || ''
}
}Ensure that for existing columns (column.id is defined), colOptions?.prompt_raw is available and correctly assigned. For new columns, confirm that prompt_raw is initialized appropriately.
647-660:
Correct validation logic for AI Button Types
In the isColumnValid function, the validation logic for ButtonActionsType.Ai may incorrectly validate output_column_ids when it's an empty string. Splitting an empty string results in [''], which has a length of 1, causing the validation to pass when it should fail.
Apply this diff to correct the validation logic:
if (column.type === ButtonActionsType.Ai) {
return !(
!column.fk_integration_id ||
!column.formula_raw?.trim() ||
- !column.output_column_ids?.length ||
- !column.output_column_ids?.split(',')?.length
+ !column.output_column_ids ||
+ !column.output_column_ids.split(',').filter(id => id.trim()).length
)
}This change ensures that output_column_ids is not empty and contains at least one valid ID after splitting and trimming.
7-7: Ensure isAIPromptCol is correctly imported and utilized
The isAIPromptCol function is imported from 'nocodb-sdk':
isAIPromptCol,Verify that this function is correctly defined in 'nocodb-sdk' and that all usages in this file are appropriate.
Run the following script to confirm the definition of isAIPromptCol in the codebase:
✅ Verification successful
Let me verify the usage of isAIPromptCol in the Fields.vue file to ensure it's being used correctly.
isAIPromptCol is correctly imported and used
The function is properly defined in nocodb-sdk and is being used correctly in Fields.vue to check if a LongText column is an AI prompt column. The implementation and usage align perfectly with its intended purpose.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for the definition of 'isAIPromptCol' in the codebase
# Test: Expect to find at least one definition
ast-grep --pattern 'function isAIPromptCol($_) { $$$ }' --lang ts
Length of output: 546
Script:
#!/bin/bash
# Check how isAIPromptCol is imported and used in Fields.vue
rg "isAIPromptCol" "packages/nc-gui/components/smartsheet/details/Fields.vue" -A 2 -B 2
Length of output: 332
packages/nc-gui/components/smartsheet/column/LongTextOptions.vue (8)
2-2: Imported modules are correctly added
The required modules isAIPromptCol and UITypes are correctly imported from 'nocodb-sdk'.
108-114: isPromptEnabled computed property is properly implemented
The logic within isPromptEnabled accurately determines the availability of AI prompt features based on the edit state and feature flags.
140-143: Additional validations are set correctly
The validators are appropriately added using setAdditionalValidations, ensuring that custom validations are included in the form processing.
195-240: AI prompt UI elements are effectively integrated
The template code additions for AI prompt settings and toggles enhance the user interface and are implemented following best practices.
284-297: Ensure the preview generation button handles disabled state correctly
The NcButton component controlling preview generation should correctly reflect the disabled state based on isPreviewEnabled. This prevents users from initiating preview generation when prerequisites are not met.
335-335: AiIntegrationNotFound component is conditionally rendered appropriately
The AiIntegrationNotFound component is correctly shown when AI integration is unavailable but AI text generation is enabled and the prompt is active.
121-134:
Fix Promise logic in fk_integration_id validator
In the custom validator for fk_integration_id, the Promise executor calls resolve() after reject() without stopping execution. This means that resolve() will be called even when validation fails, causing the validation to incorrectly pass.
Apply this diff to fix the issue:
return new Promise<void>((resolve, reject) => {
if (isEnabledGenerateText.value && !value) {
reject(new Error(t('title.aiIntegrationMissing')))
+ return
}
resolve()
})225-227:
Correct invalid v-model attribute names
The v-model argument names in Vue should not contain hyphens, as they are invalid JavaScript identifiers and will cause syntax errors. Replace hyphens with camelCase in the attribute names.
Apply this diff to correct the attribute names:
<AiSettings
- v-model:fk-integration-id="vModel.fk_integration_id"
+ v-model:fkIntegrationId="vModel.fk_integration_id"
v-model:model="vModel.model"
v-model:randomness="vModel.randomness"
:workspace-id="activeWorkspaceId"
:show-tooltip="false"
:is-edit-column="isEdit"
placement="bottomRight"
>
|
Uffizzi Preview |
Change Summary
Provide summary of changes with issue number if any.
Change type
Test/ Verification
Provide summary of changes.
Additional information / screenshots (optional)
Anything for maintainers to be made aware of