Skip to content

Conversation

@jankuca
Copy link
Member

@jankuca jankuca commented Nov 3, 2025

Creation:

Screenshot 2025-11-03 at 14 56 40 Screenshot 2025-11-03 at 14 56 55 Screenshot 2025-11-03 at 14 56 59

Deletion:

Screenshot 2025-11-03 at 14 57 06 Screenshot 2025-11-03 at 14 57 09

Summary by CodeRabbit

  • New Features

    • Separate Reset (clear credentials) and Delete (remove entry) flows with confirmations.
    • Integration Type selector to add integrations by category and auto-generate default names.
    • Webview now includes project name in integration updates.
  • UI/Styling

    • Refined integration panel layout, type icons, clearer action buttons, and responsive styles.
  • Localization

    • New integration-related labels and a default-name generator; several type labels updated (e.g., Google BigQuery, Microsoft SQL Server).
  • Tests

    • Updated text/fixture expectations to match new labels.
  • Documentation

    • Updated integration display names in reference doc.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

📝 Walkthrough

Walkthrough

Adds integration management features and UI refinements: new localization keys and default-name helper; centralized integration metadata (labels, icons) and getDefaultIntegrationName; webview message flows for save, reset (clear credentials but keep entry), and delete (remove entry); IntegrationTypeSelector for creating integrations; IntegrationList/Item/Panel updated to surface reset vs delete and start creation by type; project integrations syncing after changes; CSS and asset adjustments. Several component props and a webview UpdateMessage gained new fields; many integration forms now use the centralized default-name helper.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Panel as IntegrationPanel (UI)
    participant Webview
    participant Store as Project Storage

    rect rgb(240,248,255)
    Note over User,Panel: Reset flow (preserve entry)
    User->>Panel: Click Reset
    Panel->>Panel: Confirm prompt
    User->>Panel: Confirm
    Panel->>Webview: postMessage "reset" (integrationId)
    Webview->>Store: clear integration.config (keep entry)
    Webview-->>Panel: postMessage success/error
    Panel->>Panel: refresh UI / list
    end

    rect rgb(255,240,245)
    Note over User,Panel: Delete flow (remove entry)
    User->>Panel: Click Delete
    Panel->>Panel: Confirm prompt
    User->>Panel: Confirm
    Panel->>Webview: postMessage "delete" (integrationId)
    Webview->>Store: remove integration entry and config
    Webview-->>Panel: postMessage success/error
    Panel->>Panel: refresh UI / list
    end

    rect rgb(240,255,240)
    Note over User,Panel: Create flow (type selector)
    User->>Panel: Click Add New Integration
    Panel->>Panel: Show IntegrationTypeSelector
    User->>Panel: Select type
    Panel->>Panel: init form with getDefaultIntegrationName(type)
    User->>Panel: Save
    Panel->>Webview: postMessage "save" (new config)
    Webview->>Store: create integration entry
    Webview-->>Panel: postMessage success
    Panel->>Panel: refresh UI / list
    end
Loading

Possibly related PRs

Suggested reviewers

  • Artmann
  • saltenasl
  • andyjakubowski

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: Creating+deleting of integrations, fix stylesheet load' clearly and concisely summarizes the main changes. It identifies the primary features (integration creation/deletion) and the secondary fix (stylesheet loading), which align with the extensive changeset across integration components, utility functions, and styling files.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6dcb177 and 54ebcb9.

📒 Files selected for processing (10)
  • INTEGRATIONS_CREDENTIALS.md (2 hunks)
  • src/notebooks/deepnote/integrations/integrationWebview.ts (8 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (3 hunks)
  • src/platform/common/utils/localize.ts (2 hunks)
  • src/webviews/webview-side/integrations/IntegrationItem.tsx (3 hunks)
  • src/webviews/webview-side/integrations/IntegrationPanel.tsx (9 hunks)
  • src/webviews/webview-side/integrations/integrationUtils.ts (1 hunks)
  • src/webviews/webview-side/integrations/integrations.css (5 hunks)
  • src/webviews/webview-side/integrations/types.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{test,spec}.ts

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined then property to avoid hanging tests

Files:

  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
src/platform/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging

Files:

  • src/platform/common/utils/localize.ts
🧬 Code graph analysis (6)
src/notebooks/deepnote/integrations/integrationWebview.ts (2)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationStatus (7-7)
src/platform/logging/index.ts (1)
  • logger (35-48)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeLabels (26-44)
src/webviews/webview-side/integrations/types.ts (1)
  • ConfigurableDatabaseIntegrationType (3-3)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (4)
src/webviews/webview-side/integrations/types.ts (1)
  • ConfigurableDatabaseIntegrationType (3-3)
src/platform/common/uuid.ts (1)
  • generateUuid (10-61)
src/webviews/webview-side/integrations/IntegrationList.tsx (1)
  • IntegrationList (13-35)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)
  • IntegrationTypeSelector (109-155)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/IntegrationItem.tsx (2)
src/webviews/webview-side/integrations/types.ts (2)
  • IntegrationWithStatus (9-15)
  • ConfigurableDatabaseIntegrationType (3-3)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeIcons (47-65)
src/platform/common/utils/localize.ts (1)
src/platform/notebooks/deepnote/integrationStorage.ts (1)
  • save (79-97)
🔇 Additional comments (35)
src/webviews/webview-side/integrations/integrations.css (6)

3-4: LGTM: Centered layout.

Max-width with auto margins provides clean centering.


17-22: LGTM: Project name styling.

Descriptive color and smaller font differentiate project context from title.


68-81: LGTM: Icon container.

Fixed 32×32 sizing with flexbox centering and object-fit contain handles varied icon dimensions.


93-110: LGTM: Meta layout update.

Flex layout with separator cleanly presents type and status inline.


161-175: LGTM: Icon button styling.

Transparent background with hover state and 16px codicon sizing follows VSCode conventions.


325-404: Past review addressed: Responsive grid implemented.

Line 359 now uses repeat(auto-fit, minmax(180px, 1fr)) as suggested. Integration type selector styling is comprehensive.

INTEGRATIONS_CREDENTIALS.md (2)

43-43: LGTM: Simplified branding.

"Google AlloyDB" aligns with labels in integrationUtils.ts.


54-54: LGTM: Consistent branding.

"Google Spanner" matches integration type labels elsewhere.

src/webviews/webview-side/integrations/types.ts (1)

26-26: LGTM: Optional project name.

Backward-compatible addition for displaying project context.

src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (1)

999-1026: LGTM: Test updated for Google BigQuery branding.

Expectations align with updated labels in integrationUtils.ts.

src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2)

39-57: LGTM: Localized integration labels.

All 17 types mapped with l10n.t() for proper localization. Duplication with webview-side is necessary per architecture constraints.

Based on learnings


377-377: LGTM: Safe label resolution.

Fallback to integrationType prevents runtime errors for unknown types.

src/webviews/webview-side/integrations/IntegrationItem.tsx (5)

9-9: LGTM: Extended interface.

onReset callback added to support reset flow.


16-16: LGTM: Updated branding.

Google and Microsoft prefixes align with integrationUtils.ts labels.

Also applies to: 20-20, 44-44, 46-46


76-80: LGTM: Conditional icon rendering.

Safe null check before rendering icon.


83-87: LGTM: Enhanced meta display.

Type label and status with separator improves information hierarchy.


98-108: LGTM: Delete button with accessibility.

Title and aria-label attributes ensure screen reader support.

src/webviews/webview-side/integrations/integrationUtils.ts (4)

5-23: LGTM: SVG imports.

Require syntax necessary in webview context. eslint-disable is appropriate.


25-44: LGTM: Integration type labels.

Hardcoded English strings are acceptable for webview-side mapping. Localization handled separately where needed.

Based on learnings


46-65: LGTM: Icon mapping.

Complete coverage of all 17 configurable types.


67-75: LGTM: Default name generation.

Localized template string with type label interpolation provides consistent naming.

src/webviews/webview-side/integrations/IntegrationPanel.tsx (6)

3-3: Past review addressed: Using generateUuid utility.

Defensive UUID generation with fallback for environments lacking crypto.randomUUID().


109-142: Past review addressed: Reset sends distinct message type.

handleConfirmReset sends 'reset' message (line 127), not 'delete'. Timer cleanup is consistent.


196-205: LGTM: New integration creation flow.

UUID generation with generateUuid and state initialization for form display.


214-221: LGTM: Integration list with reset action.

onReset prop wired correctly to handleReset. IntegrationTypeSelector renders below list.


234-264: LGTM: Reset confirmation overlay.

Clear messaging with localized strings. Timer-based confirmation flow.


266-296: LGTM: Delete confirmation overlay.

Separate from reset with appropriate warning about permanence.

src/notebooks/deepnote/integrations/integrationWebview.ts (7)

1-33: DI setup follows guidelines.

Inversify decorators correctly applied: @injectable() on class, @inject() on constructor parameters.


412-417: LGTM.

Optional chaining safely handles missing project.


445-454: Clarify reset vs. delete semantics.

Both resetConfiguration (line 528) and deleteConfiguration (line 565) call integrationStorage.delete(integrationId). The only difference is local state: reset keeps the entry with null config; delete removes it entirely. Since both remove credentials from storage, the "reset" name may be misleading.


495-503: Verify duplicate integration ID handling.

When creating new integrations, only the local Map is checked (line 488). If integrationId exists in storage from a previous session, it's silently overwritten. Confirm this is intentional.


593-632: LGTM.

Appropriate error handling and logging for missing types and projects.


673-673: CSP appropriately tightened.

data: directive retained as needed for bundled images; https: removed per previous discussion.

Based on learnings


6-6: Logger usage follows guidelines.

Centralized logger used throughout; no console.log detected.

As per coding guidelines

src/platform/common/utils/localize.ts (1)

825-864: LGTM.

Localization additions for integration CRUD operations and clarified type labels with vendor prefixes.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73%. Comparing base (5400045) to head (54ebcb9).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/notebooks/deepnote/sqlCellStatusBarProvider.ts 50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #163   +/-   ##
=====================================
  Coverage     73%     73%           
=====================================
  Files        552     552           
  Lines      44488   44480    -8     
  Branches    5314    5314           
=====================================
+ Hits       32708   32716    +8     
+ Misses     10017   10001   -16     
  Partials    1763    1763           
Files with missing lines Coverage Δ
src/messageTypes.ts 100% <ø> (ø)
...oks/deepnote/sqlCellStatusBarProvider.unit.test.ts 99% <100%> (ø)
src/platform/common/utils/localize.ts 91% <100%> (+<1%) ⬆️
src/notebooks/deepnote/sqlCellStatusBarProvider.ts 96% <50%> (+8%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/notebooks/deepnote/integrations/integrationWebview.ts (1)

513-543: Misleading method name and missing true delete.

deleteConfiguration clears the config but retains the integration in the map (line 518-522), which is reset semantics. Given the frontend sends both 'reset' and 'delete' messages (once the IntegrationPanel bug is fixed), implement two handlers: one for reset (current behavior) and one for true deletion (remove from this.integrations map).

Suggested approach:

private async resetConfiguration(integrationId: string): Promise<void> {
    // Current deleteConfiguration logic (clear config, keep entry)
}

private async deleteConfiguration(integrationId: string): Promise<void> {
    await this.integrationStorage.delete(integrationId);
    this.integrations.delete(integrationId); // Actually remove
    await this.updateProjectIntegrationsList();
    // ... send success message
}

Update handleMessage to route both message types.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5400045 and 3bbc0bf.

⛔ Files ignored due to path filters (17)
  • src/webviews/webview-side/integrations/icons/alloydb.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/athena.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/bigquery.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/clickhouse.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/databricks.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/dremio.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/mariadb.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/materialize.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/mindsdb.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/mongodb.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/mysql.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/postgresql.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/redshift.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/snowflake.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/spanner.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/sql-server.svg is excluded by !**/*.svg
  • src/webviews/webview-side/integrations/icons/trino.svg is excluded by !**/*.svg
📒 Files selected for processing (30)
  • src/messageTypes.ts (2 hunks)
  • src/notebooks/deepnote/integrations/integrationWebview.ts (4 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2 hunks)
  • src/platform/common/utils/localize.ts (1 hunks)
  • src/webviews/webview-side/integrations/AlloyDBForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/AthenaForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/BigQueryForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/ClickHouseForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/ConfigurationForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/DatabricksForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/DremioForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/IntegrationItem.tsx (3 hunks)
  • src/webviews/webview-side/integrations/IntegrationList.tsx (2 hunks)
  • src/webviews/webview-side/integrations/IntegrationPanel.tsx (8 hunks)
  • src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1 hunks)
  • src/webviews/webview-side/integrations/MariaDBForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/MaterializeForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/MindsDBForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/MongoDBForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/MySQLForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/PostgresForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/RedshiftForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/SQLServerForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/SnowflakeForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/SpannerForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/TrinoForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/images.d.ts (1 hunks)
  • src/webviews/webview-side/integrations/index.tsx (1 hunks)
  • src/webviews/webview-side/integrations/integrationUtils.ts (1 hunks)
  • src/webviews/webview-side/integrations/integrations.css (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/platform/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging

Files:

  • src/platform/common/utils/localize.ts
🧬 Code graph analysis (24)
src/webviews/webview-side/integrations/BigQueryForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/SQLServerForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeLabels (5-23)
src/webviews/webview-side/integrations/types.ts (1)
  • ConfigurableDatabaseIntegrationType (3-3)
src/webviews/webview-side/integrations/MySQLForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/AlloyDBForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/ClickHouseForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/SpannerForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/MindsDBForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/MariaDBForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/MaterializeForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/notebooks/deepnote/integrations/integrationWebview.ts (1)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationStatus (7-7)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/PostgresForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/SnowflakeForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/MongoDBForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/DatabricksForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/TrinoForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/DremioForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/RedshiftForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/AthenaForm.tsx (1)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • getDefaultIntegrationName (30-33)
src/webviews/webview-side/integrations/IntegrationItem.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/ConfigurationForm.tsx (2)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeLabels (5-23)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (3)
src/webviews/webview-side/integrations/types.ts (1)
  • ConfigurableDatabaseIntegrationType (3-3)
src/webviews/webview-side/integrations/IntegrationList.tsx (1)
  • IntegrationList (13-35)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)
  • IntegrationTypeSelector (124-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build & Test
🔇 Additional comments (41)
src/webviews/webview-side/integrations/images.d.ts (1)

1-8: LGTM!

Standard TypeScript module declarations for asset imports. Enables importing image files in the integration UI components.

src/platform/common/utils/localize.ts (1)

825-843: LGTM!

New localization strings for delete/create integration flows follow existing patterns. The defaultName function properly formats integration names with type labels.

src/webviews/webview-side/integrations/RedshiftForm.tsx (2)

2-6: LGTM!

Properly migrates to centralized default name utility by importing getDefaultIntegrationName and removing unused format.


13-13: LGTM!

Correctly uses getDefaultIntegrationName('redshift') for default naming, aligning with the broader refactoring pattern.

src/webviews/webview-side/integrations/index.tsx (1)

9-10: LGTM!

CSS imports for codicon fonts and integration styles. Load order (after common styles) allows proper specificity for integration-specific overrides.

src/webviews/webview-side/integrations/BigQueryForm.tsx (2)

4-4: LGTM!

Adds centralized default name utility import.


11-11: LGTM!

Correctly uses getDefaultIntegrationName('big-query') for default naming.

src/webviews/webview-side/integrations/SnowflakeForm.tsx (2)

2-4: LGTM!

Properly migrates to centralized default name utility by importing getDefaultIntegrationName and removing unused format.


18-18: LGTM!

Correctly uses getDefaultIntegrationName('snowflake') for default naming.

src/webviews/webview-side/integrations/MariaDBForm.tsx (2)

2-6: LGTM!

Properly migrates to centralized default name utility by importing getDefaultIntegrationName and removing unused format.


14-14: LGTM!

Correctly uses getDefaultIntegrationName('mariadb') for default naming.

src/webviews/webview-side/integrations/AlloyDBForm.tsx (2)

2-6: LGTM!

Properly migrates to centralized default name utility by importing getDefaultIntegrationName and removing unused format.


22-22: LGTM!

Correctly uses getDefaultIntegrationName('alloydb') for default naming.

src/webviews/webview-side/integrations/DremioForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('dremio').

Also applies to: 5-5, 13-13

src/webviews/webview-side/integrations/MaterializeForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('materialize').

Also applies to: 6-6, 22-22

src/webviews/webview-side/integrations/MindsDBForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('mindsdb').

Also applies to: 6-6, 22-22

src/webviews/webview-side/integrations/MySQLForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('mysql').

Also applies to: 6-6, 14-14

src/webviews/webview-side/integrations/ClickHouseForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('clickhouse').

Also applies to: 6-6, 22-22

src/webviews/webview-side/integrations/TrinoForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('trino').

Also applies to: 6-6, 22-22

src/webviews/webview-side/integrations/SQLServerForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('sql-server').

Also applies to: 5-5, 21-21

src/webviews/webview-side/integrations/DatabricksForm.tsx (1)

2-2: LGTM—Centralized default naming.

Refactor correctly replaces inline name formatting with getDefaultIntegrationName('databricks').

Also applies to: 5-5, 13-13

src/webviews/webview-side/integrations/ConfigurationForm.tsx (1)

40-41: LGTM!

Centralized label lookup with sensible fallback. Clean refactoring.

src/webviews/webview-side/integrations/PostgresForm.tsx (1)

2-2: LGTM!

Consistent refactoring to use centralized default name generation.

Also applies to: 6-6, 14-14

src/webviews/webview-side/integrations/AthenaForm.tsx (1)

2-2: LGTM!

Consistent with other form refactorings.

Also applies to: 4-4, 12-12

src/webviews/webview-side/integrations/SpannerForm.tsx (1)

2-2: LGTM!

Consistent refactoring pattern.

Also applies to: 4-4, 12-12

src/webviews/webview-side/integrations/integrations.css (3)

3-4: LGTM!

Constrains and centers panel. Standard approach.


121-137: LGTM!

Icon button styling follows VSCode patterns.


286-351: LGTM!

Well-structured integration type selector with proper grid layout and hover states.

src/notebooks/deepnote/sqlCellStatusBarProvider.ts (1)

377-377: LGTM!

Proper fallback for unknown integration types.

src/webviews/webview-side/integrations/IntegrationList.tsx (1)

9-9: LGTM!

Properly threads onReset callback through to IntegrationItem.

Also applies to: 13-13, 29-29

src/webviews/webview-side/integrations/integrationUtils.ts (1)

30-33: LGTM!

Clean centralized approach for default naming.

src/webviews/webview-side/integrations/IntegrationPanel.tsx (3)

30-35: LGTM!

Proper state and ref setup for reset confirmation flow.


192-201: LGTM!

Correct initialization for new integration creation flow.


229-291: Confirmation dialogs correctly distinguish reset from delete.

The text clearly differentiates the two actions: reset removes credentials (reconfigurable), delete is permanent. Ensure the backend implements both behaviors distinctly.

src/webviews/webview-side/integrations/MongoDBForm.tsx (1)

1-20: LGTM!

Clean adoption of centralized default naming utility.

src/webviews/webview-side/integrations/IntegrationItem.tsx (2)

5-10: LGTM!

Correct addition of onReset prop to support separate reset action.


82-97: LGTM!

Proper UI separation: Reset button for clearing credentials, Delete icon for removal.

src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)

124-148: LGTM!

Clean component structure for integration type selection.

src/notebooks/deepnote/integrations/integrationWebview.ts (3)

126-136: LGTM!

Localization strings properly extended for delete and reset flows.


628-628: LGTM!

CSP updated to allow image sources for integration icons.


474-488: LGTM!

Correctly handles both updating existing integrations and creating new ones.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3bbc0bf and 1d35c99.

📒 Files selected for processing (9)
  • src/notebooks/deepnote/integrations/integrationWebview.ts (7 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2 hunks)
  • src/platform/common/utils/localize.ts (2 hunks)
  • src/webviews/webview-side/integrations/IntegrationItem.tsx (3 hunks)
  • src/webviews/webview-side/integrations/IntegrationPanel.tsx (8 hunks)
  • src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1 hunks)
  • src/webviews/webview-side/integrations/images.d.ts (1 hunks)
  • src/webviews/webview-side/integrations/integrationUtils.ts (1 hunks)
  • src/webviews/webview-side/integrations/integrations.css (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/platform/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging

Files:

  • src/platform/common/utils/localize.ts
🧬 Code graph analysis (6)
src/webviews/webview-side/integrations/IntegrationItem.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (2)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeLabels (5-23)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/notebooks/deepnote/integrations/integrationWebview.ts (2)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationStatus (7-7)
src/platform/logging/index.ts (1)
  • logger (35-48)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeLabels (5-23)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
  • ConfigurableDatabaseIntegrationType (144-144)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (3)
src/webviews/webview-side/integrations/types.ts (1)
  • ConfigurableDatabaseIntegrationType (3-3)
src/webviews/webview-side/integrations/IntegrationList.tsx (1)
  • IntegrationList (13-35)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)
  • IntegrationTypeSelector (125-149)
🔇 Additional comments (11)
src/webviews/webview-side/integrations/images.d.ts (1)

1-2: Verify copyright attribution.

Microsoft copyright appears in a Deepnote repository.

src/platform/common/utils/localize.ts (1)

825-843: New strings wired cleanly. Fresh delete/add/database/default-name keys follow existing conventions and give webview flows the loc hooks they needed.

src/webviews/webview-side/integrations/integrations.css (1)

3-350: Styling update looks solid. Centering, icon-button tweaks, and type-selector grid all respect VS Code theme vars; no issues.

src/notebooks/deepnote/sqlCellStatusBarProvider.ts (1)

39-377: Label map refactor looks good. Localized record keeps status bar and picker aligned; fallback to raw type protects unknowns.

src/webviews/webview-side/integrations/integrationUtils.ts (1)

4-33: Utility helper fits the bill. Shared type labels + default-name builder give forms a single home, and duplication is the necessary cross-context compromise. Based on learnings.

src/webviews/webview-side/integrations/IntegrationItem.tsx (1)

5-96: Action split works. Distinct Reset and Delete handlers, both localized, keep the item UI tidy.

src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)

1-149: LGTM! Label inconsistency resolved.

The component now uses centralized integrationTypeLabels for all integration types, ensuring consistency across the codebase.

src/webviews/webview-side/integrations/IntegrationPanel.tsx (1)

114-128: Reset flow fixed.

Now correctly sends type: 'reset' instead of type: 'delete', properly differentiating the two actions.

src/notebooks/deepnote/integrations/integrationWebview.ts (3)

435-439: Reset message handler added.

The new 'reset' case properly differentiates credential clearing from full deletion, matching the frontend changes.


479-493: Save handles both create and update flows.

The logic correctly distinguishes between updating an existing integration and creating a new one, maintaining proper state in both cases.


516-548: Reset and delete properly separated.

resetConfiguration clears credentials while preserving the entry (sets Disconnected status), and deleteConfiguration removes the integration entirely. Both update project state and provide appropriate user feedback.

Also applies to: 550-578

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/webviews/webview-side/integrations/IntegrationItem.tsx (1)

13-52: Duplication: getIntegrationTypeLabel vs. integrationTypeLabels.

This function duplicates the type-to-label mapping from integrationUtils.ts, but adds getLocString localization. Consider exporting a localized helper from integrationUtils.ts (e.g., getLocalizedIntegrationTypeLabel) to eliminate the duplication while preserving localization.

♻️ Duplicate comments (3)
src/webviews/webview-side/integrations/integrationUtils.ts (1)

25-44: Past review addressed: Duplication is acceptable.

Based on learnings, the duplication from sqlCellStatusBarProvider.ts is necessary due to webview vs. extension context separation.

src/webviews/webview-side/integrations/IntegrationPanel.tsx (2)

3-3: Past review addressed: Using generateUuid utility.

The defensive generateUuid() utility is now imported, resolving the crypto.randomUUID() availability concern.


107-140: Past review addressed: Reset sends 'reset' message.

The reset flow now correctly sends type: 'reset' (line 125), distinguishing it from delete. Timer management follows the established pattern.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between beb83af and 6dcb177.

📒 Files selected for processing (5)
  • src/webviews/webview-side/integrations/IntegrationItem.tsx (3 hunks)
  • src/webviews/webview-side/integrations/IntegrationPanel.tsx (8 hunks)
  • src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1 hunks)
  • src/webviews/webview-side/integrations/integrationUtils.ts (1 hunks)
  • src/webviews/webview-side/integrations/integrations.css (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (2)
src/webviews/webview-side/integrations/integrationUtils.ts (2)
  • integrationTypeLabels (26-44)
  • integrationTypeIcons (47-65)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/webviews/webview-side/integrations/IntegrationItem.tsx (2)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationWithStatus (9-15)
src/webviews/webview-side/integrations/integrationUtils.ts (1)
  • integrationTypeIcons (47-65)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (4)
src/webviews/webview-side/integrations/types.ts (1)
  • ConfigurableDatabaseIntegrationType (3-3)
src/platform/common/uuid.ts (1)
  • generateUuid (10-61)
src/webviews/webview-side/integrations/IntegrationList.tsx (1)
  • IntegrationList (13-35)
src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (1)
  • IntegrationTypeSelector (109-155)
🔇 Additional comments (19)
src/webviews/webview-side/integrations/IntegrationItem.tsx (2)

76-87: LGTM: Icon and metadata layout.

The icon, type label, and status are displayed clearly with proper conditionals and separators.


93-108: LGTM: Reset and Delete actions.

Both buttons are correctly shown for configured integrations, with proper accessibility attributes on the Delete button. The distinction between reset (clear credentials) and delete (remove entry) aligns with the PR objectives.

src/webviews/webview-side/integrations/integrationUtils.ts (2)

46-65: LGTM: Icon mapping is complete.

All 17 integration types have corresponding icons with consistent naming.


72-75: LGTM: Default name generation.

The template replacement logic is correct. Using .replace('{0}', typeLabel) is safe for a single placeholder.

src/webviews/webview-side/integrations/IntegrationTypeSelector.tsx (2)

17-107: LGTM: Complete type coverage.

All 17 integration types from integrationUtils.ts are included. The categorization into warehouses (8) and databases (9) is reasonable.


109-155: LGTM: Selector rendering.

The component is well-structured with proper localization, accessibility (alt text), and semantic button elements. The two-section layout clearly separates warehouses from databases.

src/webviews/webview-side/integrations/IntegrationPanel.tsx (6)

32-32: LGTM: Reset state mirrors delete pattern.

The confirmReset state and timer ref follow the same pattern as the delete flow.

Also applies to: 36-36


46-49: LGTM: Timer cleanup.

Reset timer cleanup follows the same pattern as the delete timer.


187-192: LGTM: Cancel handler clears creation state.

Clearing selectedIntegrationDefaultName and selectedIntegrationType prevents stale state when canceling the creation flow.


194-203: LGTM: Creation flow initialization.

The handler correctly generates a UUID and initializes form state for creating a new integration of the selected type.


211-218: LGTM: List and selector integration.

IntegrationList now receives onReset, and IntegrationTypeSelector is correctly positioned to start the creation flow.


231-293: LGTM: Reset and delete overlays.

The two confirmation overlays clearly distinguish reset (clear credentials, line 247) from delete (permanent removal, lines 279-280). Localization and UI structure are consistent.

src/webviews/webview-side/integrations/integrations.css (7)

3-4: LGTM: Centered layout with max-width.

Constraining the panel to 640px improves readability and centers the content.


61-74: LGTM: Icon container.

The 32×32 icon container with flex-shrink: 0 and centering ensures icons display consistently.


78-78: LGTM: Text truncation support.

min-width: 0 on .integration-info enables proper text truncation when names are long.


86-100: LGTM: Metadata layout.

The flex layout with type label, separator, and status is clear and consistent.


154-168: LGTM: Icon button style.

The transparent icon button with hover feedback aligns with VSCode UI patterns.


318-348: LGTM: Selector section styling.

The top border and spacing clearly separate the selector from the list. Section titles use uppercase and letter-spacing for emphasis.


356-397: LGTM: Card layout and hover effects.

The card layout with 32×32 icons and hover feedback is consistent with the integration item styling.

@jankuca jankuca marked this pull request as ready for review November 3, 2025 14:20
@jankuca jankuca requested a review from a team as a code owner November 3, 2025 14:20
Copy link
Member

@saltenasl saltenasl left a comment

Choose a reason for hiding this comment

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

good job! the delete looks a bit awkward (particularly the bottom part - the vertical gap between border and the buttons, feels like the gap should be padding from all sides instead).

@saltenasl saltenasl merged commit 90bfefd into main Nov 3, 2025
13 checks passed
@saltenasl saltenasl deleted the jk/integration-crud branch November 3, 2025 15:47
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.

3 participants