Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
107 changes: 107 additions & 0 deletions PR_DESCRIPTION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
## Description

This PR refactors the applications update system to improve user experience, security, and maintainability. It introduces a redesigned update settings UI, securely handles external links via IPC, and fixes several bugs related to dialog visibility and change detection. Additionally, it addresses unit test failures by updating mocks to match new interfaces.

### Goal

- **Improve UX:** Provide a clear and responsive interface for checking updates and viewing release notes.
- **Enhance Security:** Replace direct `shell.openExternal` calls in the renderer with a secure IPC handler.
- **Fix Bugs:** Resolve issues where the release notes dialog would not appear immediately due to Angular change detection timing.
- **Maintain Code Quality:** Fix linting errors and update unit tests to reflect recent changes in `UpdateService`.

### Key Changes

- **Refactored Update UI:** Redesigned `OpenSettingsUpdatesComponent` and introduced `UpdateDialogComponent` to display release notes with proper formatting.
- **Secure External Links:** Implemented a new `open-external` IPC handler in the main process and exposed it via `preload.ts`, ensuring all links open in the default browser securely.
- **Dialog Visibility Fix:** Implemented `setTimeout` and `NgZone.run` in `showCurrentVersionNotes` to force Angular change detection and ensure the dialog opens immediately.
- **Type Safety:** Added `GitHubRelease` interface and updated `electron.d.ts` and `UpdateService` to use strict typing instead of `any`.
- **Unit Test Fixes:** Updated mocks in `app.spec.ts` and `open-settings-updates.spec.ts` to include the missing `lastChecked` signal and correct `GitHubRelease` structure, resolving generic `TypeError` failures.

## Type of Change

- [x] 🐛 Bug fix (non-breaking change which fixes an issue)
- [x] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] 📝 Documentation update
- [x] 🎨 Style/UI changes
- [x] ♻️ Refactoring (no functional changes)
- [ ] ⚡ Performance improvement
- [x] ✅ Test updates
- [ ] 🔧 Build/configuration changes

## Impact Assessment

### Database Impact

- [x] No database changes
- [ ] New migration(s) included
- [ ] Existing data migration required

### Backup Impact

- [x] No impact on backups
- [ ] Backup format changed
- [ ] Restore compatibility maintained

## Testing

### How Has This Been Tested?

- [x] Unit tests
- [ ] Integration tests
- [x] Manual testing
- [x] Tested with SonarQube analysis

### Test Steps

1. **Check for Updates:** Go to Settings -> Updates and click "Check for updates". Verify the spinner appears and the result (up to date or new version) is displayed.
2. **View Release Notes:** Click "View Release Notes". Verify the dialog opens immediately and the markdown content is rendered correctly.
3. **External Links:** Click on any link within the release notes or the "View Releases on GitHub" button. Verify the link opens in your default system browser, not inside the Electron app.
4. **Auto-check:** Toggle the "Check for updates automatically" switch and verify the preference is saved (check LocalStorage or restart app).

### Test Configuration

- **Node version**: v20.x
- **npm version**: 10.x
- **Platform tested**: Windows 11

## UI Changes

### Before

_Previous update settings screen (simple buttons, no dialog for notes)._

### After

_New `OpenSettingsUpdatesComponent` with "Check for updates" card, "Current Version" display, and a dedicated `UpdateDialogComponent` for viewing formatted release notes._

## Checklist

- [x] My code follows the project's coding standards
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings or errors
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have run `npm run lint` and fixed any issues
- [x] I have run `npm test` and all tests pass
- [x] I have run `npm run test:electron` and all tests pass
- [x] I have run `npm run sonar:check` and the analysis passes
- [ ] Any dependent changes have been merged and published

## Breaking Changes

- [ ] This PR contains breaking changes

## Related Issues

Closes # (Add issue number if applicable)

## Additional Context

The unit test failures were caused by a mismatch between the `UpdateService` implementation (which uses Signals like `lastChecked`) and the mock objects used in tests, which were missing these properties. This PR aligns the mocks with the actual service implementation.

## Reviewer Notes

Please focus on the `preload.ts` changes regarding `openExternal` to ensure no security regressions were introduced, and verify that the `NgZone` fix in `OpenSettingsUpdatesComponent` effectively solves the dialog visibility issue on all platforms.
3 changes: 2 additions & 1 deletion electron/src/main/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { WindowManager } from './window.js';
import { DatabaseManager } from '../services/database/database.js';
import { setupIpcHandlers } from '../services/ipc/index.js';
import { BackupService } from '../services/backup/index.js';
import { UpdateService } from '../services/update/update.service.js';

let windowManager: WindowManager | null = null;
let dbManager: DatabaseManager | null = null;
Expand All @@ -29,7 +30,7 @@ const initializeApp = async (): Promise<void> => {
},
);

setupIpcHandlers(dbManager, backupService);
setupIpcHandlers(dbManager, backupService, new UpdateService());
windowManager = new WindowManager();
await windowManager.createMainWindow();
};
Expand Down
29 changes: 27 additions & 2 deletions electron/src/preload/preload.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { contextBridge, ipcRenderer, shell } from 'electron';
import { contextBridge, ipcRenderer } from 'electron';

/**
* Type definitions for database entities
Expand Down Expand Up @@ -128,8 +128,21 @@ interface BackupResult {
path?: string;
}

interface GitHubRelease {
tag_name: string;
html_url: string;
body: string;
name: string;
published_at: string;
}

try {
const electronAPI = {
// ...
// Release Info
getReleaseByTag: (tag: string): Promise<GitHubRelease> =>
ipcRenderer.invoke('get-release-by-tag', tag),

// Projects
getProjects: (): Promise<Project[]> => ipcRenderer.invoke('get-projects'),
createProject: (name: string, description?: string): Promise<Project> =>
Expand Down Expand Up @@ -395,7 +408,19 @@ try {
getBackupDir: (): Promise<string> => ipcRenderer.invoke('backup-get-dir'),

// System
openExternal: (url: string): Promise<void> => shell.openExternal(url),
openExternal: (url: string): Promise<void> =>
ipcRenderer.invoke('open-external', url),

// Updates
checkForUpdates: (): Promise<{
updateAvailable: boolean;
version: string;
url: string;
releaseNotes?: string;
}> => ipcRenderer.invoke('check-for-updates'),

// App Info
getVersion: (): Promise<string> => ipcRenderer.invoke('get-version'),
};

contextBridge.exposeInMainWorld('electronAPI', electronAPI);
Expand Down
19 changes: 14 additions & 5 deletions electron/src/services/ipc/index.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
import { DatabaseManager } from '../database/database.js';
import { BackupService } from '../backup/index.js';
import { UpdateService } from '../update/update.service.js';
import { setupDatabaseHandlers } from './database-handlers.js';
import { setupThemeHandlers } from './theme-handlers.js';
import { setupLanguageHandlers } from './language-handlers.js';
import { setupBackupHandlers } from './backup-handlers.js';
import { setupUpdateHandlers } from './update-handlers.js';

import { setupSystemHandlers } from './system-handlers.js';

/**
* Sets up all IPC handlers
*/
export const setupIpcHandlers = (
dbManager: DatabaseManager,
backupService?: BackupService,
dbManager: DatabaseManager | null,
backupService: BackupService | null,
updateService: UpdateService,
): void => {
setupDatabaseHandlers(dbManager);
setupThemeHandlers(dbManager);
setupLanguageHandlers(dbManager);
if (dbManager) {
setupDatabaseHandlers(dbManager);
setupThemeHandlers(dbManager);
setupLanguageHandlers(dbManager);
}
if (backupService) {
setupBackupHandlers(backupService);
}
setupUpdateHandlers(updateService);
setupSystemHandlers();
};
50 changes: 50 additions & 0 deletions electron/src/services/ipc/system-handlers.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { describe, it, expect, beforeEach, vi, type Mock } from 'vitest';
import { ipcMain, shell } from 'electron';
import { setupSystemHandlers } from './system-handlers.js';

vi.mock('electron', () => ({
ipcMain: {
handle: vi.fn(),
},
shell: {
openExternal: vi.fn(),
},
}));

describe('System Handlers', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('should setup open-external handler', () => {
setupSystemHandlers();
expect(ipcMain.handle).toHaveBeenCalledWith(
'open-external',
expect.any(Function),
);
});

it('should handle open-external call', async () => {
setupSystemHandlers();
const handler = (ipcMain.handle as Mock).mock.calls.find(
(call) => call[0] === 'open-external',
)?.[1];

const url = 'https://example.com';
await handler(null, url);

expect(shell.openExternal).toHaveBeenCalledWith(url);
});

it('should handle errors in open-external', async () => {
setupSystemHandlers();
const handler = (ipcMain.handle as Mock).mock.calls.find(
(call) => call[0] === 'open-external',
)?.[1];

const error = new Error('Failed to open');
(shell.openExternal as Mock).mockRejectedValue(error);

await expect(handler(null, 'https://example.com')).rejects.toThrow(error);
});
});
16 changes: 16 additions & 0 deletions electron/src/services/ipc/system-handlers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { ipcMain, shell } from 'electron';

/**
* Sets up system-related IPC handlers
*/
export const setupSystemHandlers = (): void => {
// Open external links
ipcMain.handle('open-external', async (_event, url: string) => {
try {
await shell.openExternal(url);
} catch (error) {
console.error('Error opening external URL:', error);
throw error;
}
});
};
Loading