Skip to content

Commit addaa7d

Browse files
committed
feat: expose error message from failed project saving
1 parent a1df9f1 commit addaa7d

File tree

4 files changed

+64
-24
lines changed

4 files changed

+64
-24
lines changed

src/notebooks/deepnote/deepnoteNotebookManager.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { injectable } from 'inversify';
22

3-
import { IDeepnoteNotebookManager } from '../types';
3+
import { IDeepnoteNotebookManager, ProjectIntegration } from '../types';
44
import type { DeepnoteProject } from './deepnoteTypes';
55

66
/**
@@ -78,17 +78,16 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager {
7878
/**
7979
* Updates the integrations list in the project data.
8080
* This modifies the stored project to reflect changes in configured integrations.
81-
* @param projectId Project identifier
82-
* @param integrations Array of integration metadata to store in the project
81+
*
82+
* @param projectId - Project identifier
83+
* @param integrations - Array of integration metadata to store in the project
84+
* @returns `true` if the project was found and updated successfully, `false` if the project does not exist
8385
*/
84-
updateProjectIntegrations(
85-
projectId: string,
86-
integrations: Array<{ id: string; name: string; type: string }>
87-
): void {
86+
updateProjectIntegrations(projectId: string, integrations: ProjectIntegration[]): boolean {
8887
const project = this.originalProjects.get(projectId);
8988

9089
if (!project) {
91-
return;
90+
return false;
9291
}
9392

9493
const updatedProject = JSON.parse(JSON.stringify(project)) as DeepnoteProject;
@@ -99,6 +98,8 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager {
9998
if (currentNotebookId) {
10099
this.storeOriginalProject(projectId, updatedProject, currentNotebookId);
101100
}
101+
102+
return true;
102103
}
103104

104105
/**

src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -184,21 +184,23 @@ suite('DeepnoteNotebookManager', () => {
184184
});
185185

186186
suite('updateProjectIntegrations', () => {
187-
test('should update integrations list for existing project', () => {
187+
test('should update integrations list for existing project and return true', () => {
188188
manager.storeOriginalProject('project-123', mockProject, 'notebook-456');
189189

190190
const integrations = [
191191
{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' },
192192
{ id: 'int-2', name: 'BigQuery', type: 'big-query' }
193193
];
194194

195-
manager.updateProjectIntegrations('project-123', integrations);
195+
const result = manager.updateProjectIntegrations('project-123', integrations);
196+
197+
assert.strictEqual(result, true);
196198

197199
const updatedProject = manager.getOriginalProject('project-123');
198200
assert.deepStrictEqual(updatedProject?.project.integrations, integrations);
199201
});
200202

201-
test('should replace existing integrations list', () => {
203+
test('should replace existing integrations list and return true', () => {
202204
const projectWithIntegrations: DeepnoteProject = {
203205
...mockProject,
204206
project: {
@@ -214,13 +216,15 @@ suite('DeepnoteNotebookManager', () => {
214216
{ id: 'new-int-2', name: 'New Integration 2', type: 'big-query' }
215217
];
216218

217-
manager.updateProjectIntegrations('project-123', newIntegrations);
219+
const result = manager.updateProjectIntegrations('project-123', newIntegrations);
220+
221+
assert.strictEqual(result, true);
218222

219223
const updatedProject = manager.getOriginalProject('project-123');
220224
assert.deepStrictEqual(updatedProject?.project.integrations, newIntegrations);
221225
});
222226

223-
test('should handle empty integrations array', () => {
227+
test('should handle empty integrations array and return true', () => {
224228
const projectWithIntegrations: DeepnoteProject = {
225229
...mockProject,
226230
project: {
@@ -231,26 +235,33 @@ suite('DeepnoteNotebookManager', () => {
231235

232236
manager.storeOriginalProject('project-123', projectWithIntegrations, 'notebook-456');
233237

234-
manager.updateProjectIntegrations('project-123', []);
238+
const result = manager.updateProjectIntegrations('project-123', []);
239+
240+
assert.strictEqual(result, true);
235241

236242
const updatedProject = manager.getOriginalProject('project-123');
237243
assert.deepStrictEqual(updatedProject?.project.integrations, []);
238244
});
239245

240-
test('should do nothing for unknown project', () => {
241-
// Should not throw an error
242-
manager.updateProjectIntegrations('unknown-project', [{ id: 'int-1', name: 'Integration', type: 'pgsql' }]);
246+
test('should return false for unknown project', () => {
247+
const result = manager.updateProjectIntegrations('unknown-project', [
248+
{ id: 'int-1', name: 'Integration', type: 'pgsql' }
249+
]);
250+
251+
assert.strictEqual(result, false);
243252

244253
const project = manager.getOriginalProject('unknown-project');
245254
assert.strictEqual(project, undefined);
246255
});
247256

248-
test('should preserve other project properties', () => {
257+
test('should preserve other project properties and return true', () => {
249258
manager.storeOriginalProject('project-123', mockProject, 'notebook-456');
250259

251260
const integrations = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }];
252261

253-
manager.updateProjectIntegrations('project-123', integrations);
262+
const result = manager.updateProjectIntegrations('project-123', integrations);
263+
264+
assert.strictEqual(result, true);
254265

255266
const updatedProject = manager.getOriginalProject('project-123');
256267
assert.strictEqual(updatedProject?.project.id, mockProject.project.id);

src/notebooks/deepnote/integrations/integrationWebview.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { IExtensionContext } from '../../../platform/common/types';
55
import * as localize from '../../../platform/common/utils/localize';
66
import { logger } from '../../../platform/logging';
77
import { LocalizedMessages, SharedMessages } from '../../../messageTypes';
8-
import { IDeepnoteNotebookManager } from '../../types';
8+
import { IDeepnoteNotebookManager, ProjectIntegration } from '../../types';
99
import { IIntegrationStorage, IIntegrationWebviewProvider } from './types';
1010
import {
1111
INTEGRATION_TYPE_TO_DEEPNOTE,
@@ -311,7 +311,7 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider {
311311
}
312312

313313
// Build the integrations list from current integrations
314-
const projectIntegrations = Array.from(this.integrations.entries())
314+
const projectIntegrations: ProjectIntegration[] = Array.from(this.integrations.entries())
315315
.map(([id, integration]) => {
316316
// Get the integration type from config or integration metadata
317317
const type = integration.config?.type || integration.integrationType;
@@ -333,14 +333,23 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider {
333333
type: deepnoteType
334334
};
335335
})
336-
.filter((integration): integration is { id: string; name: string; type: string } => integration !== null);
336+
.filter((integration): integration is ProjectIntegration => integration !== null);
337337

338338
logger.debug(
339339
`IntegrationWebviewProvider: Updating project ${this.projectId} with ${projectIntegrations.length} integrations`
340340
);
341341

342342
// Update the project in the notebook manager
343-
this.notebookManager.updateProjectIntegrations(this.projectId, projectIntegrations);
343+
const success = this.notebookManager.updateProjectIntegrations(this.projectId, projectIntegrations);
344+
345+
if (!success) {
346+
logger.error(
347+
`IntegrationWebviewProvider: Failed to update integrations for project ${this.projectId} - project not found`
348+
);
349+
void window.showErrorMessage(
350+
l10n.t('Failed to update integrations: project not found. Please reopen the notebook and try again.')
351+
);
352+
}
344353
}
345354

346355
/**

src/notebooks/types.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ export interface INotebookPythonEnvironmentService {
2525
getPythonEnvironment(uri: Uri): EnvironmentPath | undefined;
2626
}
2727

28+
/**
29+
* Represents a Deepnote project integration with basic metadata.
30+
*/
31+
export interface ProjectIntegration {
32+
id: string;
33+
name: string;
34+
type: string;
35+
}
36+
2837
export const IDeepnoteNotebookManager = Symbol('IDeepnoteNotebookManager');
2938
export interface IDeepnoteNotebookManager {
3039
getCurrentNotebookId(projectId: string): string | undefined;
@@ -33,7 +42,17 @@ export interface IDeepnoteNotebookManager {
3342
selectNotebookForProject(projectId: string, notebookId: string): void;
3443
storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void;
3544
updateCurrentNotebookId(projectId: string, notebookId: string): void;
36-
updateProjectIntegrations(projectId: string, integrations: Array<{ id: string; name: string; type: string }>): void;
45+
46+
/**
47+
* Updates the integrations list in the project data.
48+
* This modifies the stored project to reflect changes in configured integrations.
49+
*
50+
* @param projectId - Project identifier
51+
* @param integrations - Array of integration metadata to store in the project
52+
* @returns `true` if the project was found and updated successfully, `false` if the project does not exist
53+
*/
54+
updateProjectIntegrations(projectId: string, integrations: ProjectIntegration[]): boolean;
55+
3756
hasInitNotebookBeenRun(projectId: string): boolean;
3857
markInitNotebookAsRun(projectId: string): void;
3958
}

0 commit comments

Comments
 (0)