Skip to content

Add automatic column synchronization for Excel-to-table syncs #198

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.

Sign up for GitHub

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

Open
envyrovasileski wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add automatic column synchronization for Excel-to-table syncs #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
303bdc4
Automatic column addition and deletion
envyrovasileski Oct 21, 2025
646c61c
Refine polling times
envyrovasileski Oct 22, 2025
97f47c6
Bump integration version
envyrovasileski Oct 22, 2025
c6f9a1e
fix schema handling and improve validation
envyrovasileski Oct 30, 2025
7622e4d
Directly use client table methods over PAT + TablesAPI
Napeel Oct 31, 2025
1024d1c
run prettier
Napeel Nov 3, 2025
aee0435
Improve code organization and error handling
envyrovasileski Nov 6, 2025
8fa2736
Improve code readability and API consistency
envyrovasileski Nov 16, 2025
83f5475
refactor(sharepoint-excel): separate Excel parsing from Botpress tabl…
envyrovasileski Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions integrations/sharepoint-excel/integration.definition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { z, IntegrationDefinition } from '@botpress/sdk'

export default new IntegrationDefinition({
name: 'plus/sharepoint-excel',
version: '2.2.1',
version: '2.3.1',
title: 'SharePoint Excel',
description: 'Sync one or many SharePoint document libraries with one or more Botpress knowledge bases.',
readme: 'hub.md',
Expand All @@ -19,7 +19,6 @@ export default new IntegrationDefinition({
privateKey: z.string().min(1).describe('PEM‑formatted certificate private key'),
primaryDomain: z.string().min(1).describe('SharePoint primary domain (e.g. contoso)'),
siteName: z.string().min(1).describe('SharePoint site name'),
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

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

issue: The personalAccessToken configuration field was removed from the integration definition (line 22 removed), but the old code relied on it for authentication to the Tables API. The new code uses the client parameter instead, which is correct. However, users who have this field configured will experience a breaking change. Consider:

  1. Adding a migration note in the changelog
  2. Documenting that users should remove this configuration field
  3. Potentially keeping it as optional/deprecated with a warning if still present
Suggested change
siteName: z.string().min(1).describe('SharePoint site name'),
siteName: z.string().min(1).describe('SharePoint site name'),
// Deprecated: This field is no longer used for authentication. Please remove it from your configuration.
personalAccessToken: z
.string()
.optional()
.describe(
'[DEPRECATED] Personal Access Token for legacy authentication. No longer required; will be removed in a future version.'
),

Copilot uses AI. Check for mistakes.
personalAccessToken: z.string().min(1).describe('Botpress Personal Access Token (PAT) for Tables API access'),
}),
},

Expand All @@ -41,7 +40,7 @@ export default new IntegrationDefinition({
sheetTableMapping: z
.string()
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

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

issue: The description for sheetTableMapping states "Table names must end with 'Table'" but there's no validation in the code to enforce this requirement. If this is truly a requirement, add validation that throws an error when table names don't end with 'Table'. If it's not actually required, remove it from the description to avoid confusion.

Suggested change
.string()
.string()
.refine((val) => {
// Accept either comma-separated or JSON mapping
let mapping: Record<string, string> = {};
try {
if (val.trim().startsWith('{')) {
mapping = JSON.parse(val);
} else {
val.split(',').forEach(pair => {
const [sheet, table] = pair.split(':').map(s => s.trim());
if (sheet && table) mapping[sheet] = table;
});
}
} catch {
// If parsing fails, let Zod handle invalid format
return false;
}
// Validate all table names
return Object.values(mapping).every(tableName => typeof tableName === 'string' && tableName.endsWith('Table'));
}, {
message: "All table names must end with 'Table'."
})

Copilot uses AI. Check for mistakes.
.describe(
'Map sheets to tables. Format: \'Sheet1:table1,Sheet2:table2\' or JSON: \'{"Sheet1":"table1","Sheet2":"table2"}\''
'Map sheets to tables. Format: \'Sheet1:dataTable,Sheet2:itemsTable\' or JSON: \'{"Sheet1":"dataTable","Sheet2":"itemsTable"}\'. Table names must end with \'Table\'.'
),
}),
},
Expand All @@ -55,7 +54,15 @@ export default new IntegrationDefinition({
rowCount: z.number(),
})
)
.optional(),
.describe('Successfully processed sheets'),
failedSheets: z
.array(
z.object({
sheetName: z.string(),
error: z.string(),
})
)
.describe('Sheets that failed to process'),
}),
},
},
Expand Down
12 changes: 6 additions & 6 deletions integrations/sharepoint-excel/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@
"private": true,
"dependencies": {
"@azure/msal-node": "^2.15.0",
"@botpress/client": "^1.13.0",
"@botpress/client": "1.22.0",
"@botpress/sdk": "4.15.2",
"axios": "^1.8.1",
"@botpress/sdk": "0.11.8",
"xlsx": "^0.18.5"
},
"devDependencies": {
"@botpress/cli": "^0.11.5",
"@types/node": "^18.11.17",
"@botpress/cli": "^4.17.2",
"@types/node": "^20.19.0",
"nodemon": "^3.1.7",
"ts-node": "^10.9.1",
"typescript": "^4.9.4"
"ts-node": "^10.9.2",
"typescript": "^5.8.2"
Comment on lines +14 to +24
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

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

issue: Major version bumps in dependencies without checking for breaking changes. The package.json shows:

  • @botpress/sdk from 0.11.8 to 4.15.2 (major version jump)
  • @botpress/client from ^1.13.0 to 1.22.0
  • @botpress/cli from ^0.11.5 to ^4.17.2
  • typescript from ^4.9.4 to ^5.8.2

These are significant version jumps that likely include breaking changes. Ensure:

  1. All breaking changes have been reviewed and addressed
  2. The code has been tested against these new versions
  3. The version pinning strategy is intentional (note the removal of ^ from @botpress/client)

Copilot uses AI. Check for mistakes.
}
}
Loading
Loading