dev to main (0.8.1)#179
Conversation
C177 to dev (0.8.1): Removed WhatsApp integration
WalkthroughThis change removes all WhatsApp integration from the application. It deletes backend handlers, frontend services, UI components, dialogs, tests, and localization strings related to WhatsApp functionality. Corresponding dependencies are removed, and the application menu is updated. All code, tests, and documentation referencing WhatsApp features are eliminated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App UI
participant Main Process
Note over App UI,Main Process: WhatsApp integration removed<br>— no WhatsApp-related flows —
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (4)
package.json (1)
28-31: Typo in build “files” entry likely excludes assets from packaging.
"src/assetes//*" should be "src/assets//*" (or remove if dist already includes assets)."files": [ "dist/**/*", "src/core/**/*", - "src/assetes/**/*" + "src/assets/**/*" ],src/app/components/files-table/files-table.component.ts (1)
27-27: Initialize allFiles to an empty array to avoid undefined access.
Prevents runtime errors if addFiles/addFile runs before the first allFiles$ emission.- allFiles!: CFile[]; + allFiles: CFile[] = [];src/app/components/control/control.component.ts (2)
1-12: Subscriptions in ngOnInit are not torn down (memory leak).Use takeUntilDestroyed to auto-unsubscribe on component destroy.
Apply this diff:
import {Component, OnInit} from '@angular/core'; +import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; @@ ngOnInit() { - this.filesService.selectedFiles$.subscribe(files => { - this.selectedFiles = files; - }); + this.filesService.selectedFiles$ + .pipe(takeUntilDestroyed()) + .subscribe(files => { + this.selectedFiles = files; + }); - this.filesService.allFiles$.subscribe(files => { - this.allFiles = files; - }); + this.filesService.allFiles$ + .pipe(takeUntilDestroyed()) + .subscribe(files => { + this.allFiles = files; + }); }Also applies to: 36-44
27-28: Initialize arrays to avoid undefined access in template and helpers.Prevents possible runtime errors when calling
.some()before the first emission.- allFiles!: CFile[]; - selectedFiles!: CFile[]; + allFiles: CFile[] = []; + selectedFiles: CFile[] = [];
🧹 Nitpick comments (9)
package.json (1)
117-117: Move jest to devDependencies.
Jest is only needed for tests; keeping it in dependencies bloats production install."dependencies": { @@ - "jest": "^29.7.0", @@ }, "devDependencies": { @@ + "jest": "^29.7.0",Also applies to: 139-139
src/app/app.component.ts (1)
13-13: Avoid redundant imports: prefer NgIf or CommonModule, not both.
If you only need NgIf, drop CommonModule to keep the bundle lean.-import {CommonModule, NgIf} from '@angular/common'; +import {NgIf} from '@angular/common';Also remove CommonModule from the component imports array:
ProgressSpinner, NgIf, - CommonModule,src/app/components/files-table/files-table.component.ts (1)
89-92: Defensive check when reading allFiles.
In rare timing edge cases, use a fallback to ensure .some() is safe.- private addFile(inputFile: InputFile) { - const exists = this.allFiles.some(f => f.path === inputFile.path); + private addFile(inputFile: InputFile) { + const exists = (this.allFiles ?? []).some(f => f.path === inputFile.path);src/app/components/files-table/files-table.component.spec.ts (1)
9-11: Trim unused UI modules from the testbed.
TooltipModule and PopoverModule appear unused after UI changes; removing them speeds up test compilation.-import { TooltipModule } from 'primeng/tooltip'; -import { PopoverModule } from 'primeng/popover'; @@ - TooltipModule, - PopoverModule +Also applies to: 77-79
src/app/components/control/control.component.ts (3)
63-69: Handle unknown type in switch and avoid silent no-ops.Add a default branch to fail fast or log, and avoid adding undefined.
showEncryptDialog(type: string): void { switch (type) { case 'ALL': this.fileEncryptionService.addFilesToPending(this.allFiles); break; case 'SELECTED': this.fileEncryptionService.addFilesToPending(this.selectedFiles); break; + default: + console.warn('showEncryptDialog: unsupported type', type); + return; } this.encryptDialogService.showEncryptDialog(); }
71-77: Mirror the defensive default branch for decrypt path.Keep behavior consistent and explicit on invalid input.
showDecryptDialog(type: string): void { switch (type) { case 'ALL': this.fileEncryptionService.addFilesToPending(this.allFiles); break; case 'SELECTED': this.fileEncryptionService.addFilesToPending(this.selectedFiles); break; + default: + console.warn('showDecryptDialog: unsupported type', type); + return; } this.encryptDialogService.showDecryptDialog(); }
26-26: Avoid any for Electron bridge; add a typed global.Improves maintainability and DX.
Add a global type declaration (outside this file), e.g. in src/types/global.d.ts:
export {}; declare global { interface Window { electron?: { openFileDialog: () => void; // add other exposed APIs as needed }; } }Then in the component:
electron = window.electron;src/app/services/dialog.service.ts (1)
10-11: Mark public observables as readonly.Prevents accidental reassignment and clarifies intent.
- encryptDialogVisible$ = this.encryptDialogVisibleSubject.asObservable(); - decryptDialogVisible$ = this.decryptDialogVisibleSubject.asObservable(); + readonly encryptDialogVisible$ = this.encryptDialogVisibleSubject.asObservable(); + readonly decryptDialogVisible$ = this.decryptDialogVisibleSubject.asObservable();src/__tests__/core/handlers/app/main-instance-handler.spec.js (1)
22-22: Add assertions to verify menu wiring (build and set) in testsYou mock the return value, but there’s no assertion that the menu is actually built and applied. Add a focused test to ensure the wiring doesn’t regress.
Example test to add:
it('should build and set the application menu', () => { const builtMenu = { id: 'test-menu' }; Menu.buildFromTemplate.mockReturnValue(builtMenu); initializeMainWindow(); expect(Menu.buildFromTemplate).toHaveBeenCalledTimes(1); expect(Menu.setApplicationMenu).toHaveBeenCalledWith(builtMenu); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/icons/signal.svgis excluded by!**/*.svgsrc/assets/icons/threema.svgis excluded by!**/*.svgsrc/assets/icons/whatsapp.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
README.md(1 hunks)package.json(1 hunks)src/__tests__/core/handlers/app/main-instance-handler.spec.js(2 hunks)src/__tests__/core/handlers/ipc/whatsapp-handler.spec.js(0 hunks)src/app/app.component.html(0 hunks)src/app/app.component.spec.ts(0 hunks)src/app/app.component.ts(1 hunks)src/app/components/control/control.component.html(0 hunks)src/app/components/control/control.component.spec.ts(0 hunks)src/app/components/control/control.component.ts(1 hunks)src/app/components/dialogs/whats-app-contact-list-dialog/whats-app-contact-list-dialog.component.html(0 hunks)src/app/components/dialogs/whats-app-contact-list-dialog/whats-app-contact-list-dialog.component.spec.ts(0 hunks)src/app/components/dialogs/whats-app-contact-list-dialog/whats-app-contact-list-dialog.component.ts(0 hunks)src/app/components/dialogs/whats-app-file-too-large/whats-app-file-too-large.component.html(0 hunks)src/app/components/dialogs/whats-app-file-too-large/whats-app-file-too-large.component.spec.ts(0 hunks)src/app/components/dialogs/whats-app-file-too-large/whats-app-file-too-large.component.ts(0 hunks)src/app/components/dialogs/whats-app-qrdialog/whats-app-qrdialog.component.html(0 hunks)src/app/components/dialogs/whats-app-qrdialog/whats-app-qrdialog.component.spec.ts(0 hunks)src/app/components/dialogs/whats-app-qrdialog/whats-app-qrdialog.component.ts(0 hunks)src/app/components/files-table/files-table.component.html(0 hunks)src/app/components/files-table/files-table.component.spec.ts(1 hunks)src/app/components/files-table/files-table.component.ts(1 hunks)src/app/models/ccontact.spec.ts(0 hunks)src/app/models/ccontact.ts(0 hunks)src/app/services/dialog.service.spec.ts(0 hunks)src/app/services/dialog.service.ts(1 hunks)src/app/services/send-files.service.spec.ts(0 hunks)src/app/services/send-files.service.ts(0 hunks)src/app/services/whats-app.service.spec.ts(0 hunks)src/app/services/whats-app.service.ts(0 hunks)src/assets/i18n/en.json(0 hunks)src/assets/i18n/ua.json(0 hunks)src/core/electron-main.js(0 hunks)src/core/handlers/app/main-instance-handler.js(2 hunks)src/core/handlers/ipc/whatsapp-handler.js(0 hunks)src/core/preload.js(0 hunks)
💤 Files with no reviewable changes (27)
- src/app/services/send-files.service.spec.ts
- src/app/app.component.html
- src/app/app.component.spec.ts
- src/app/components/control/control.component.html
- src/app/components/dialogs/whats-app-file-too-large/whats-app-file-too-large.component.html
- src/app/models/ccontact.spec.ts
- src/app/services/dialog.service.spec.ts
- src/app/components/dialogs/whats-app-contact-list-dialog/whats-app-contact-list-dialog.component.html
- src/tests/core/handlers/ipc/whatsapp-handler.spec.js
- src/app/components/dialogs/whats-app-file-too-large/whats-app-file-too-large.component.ts
- src/app/components/dialogs/whats-app-file-too-large/whats-app-file-too-large.component.spec.ts
- src/core/preload.js
- src/core/electron-main.js
- src/app/services/whats-app.service.spec.ts
- src/app/components/dialogs/whats-app-qrdialog/whats-app-qrdialog.component.html
- src/app/components/dialogs/whats-app-qrdialog/whats-app-qrdialog.component.spec.ts
- src/app/components/files-table/files-table.component.html
- src/app/services/send-files.service.ts
- src/app/components/dialogs/whats-app-contact-list-dialog/whats-app-contact-list-dialog.component.ts
- src/assets/i18n/ua.json
- src/app/components/control/control.component.spec.ts
- src/app/components/dialogs/whats-app-qrdialog/whats-app-qrdialog.component.ts
- src/assets/i18n/en.json
- src/app/services/whats-app.service.ts
- src/app/components/dialogs/whats-app-contact-list-dialog/whats-app-contact-list-dialog.component.spec.ts
- src/core/handlers/ipc/whatsapp-handler.js
- src/app/models/ccontact.ts
🔇 Additional comments (10)
package.json (1)
6-6: Approve version bump; no leftover WhatsApp deps/refs foundVerified recursively that neither the source files nor package-lock.json reference
whatsapp-web.js,angularx-qrcode, or
- package.json: version set to
"0.8.1"- No residual references in codebase or lockfile
README.md (1)
46-49: LGTM: corrected step numbering.
Start step numbering now consistent after build step.src/app/components/files-table/files-table.component.ts (1)
7-7: LGTM: import updated for template structural directives.
Importing NgIf directly is appropriate with standalone components.src/app/components/files-table/files-table.component.spec.ts (1)
180-180: LGTM: updated expected column count to 8.
Matches removal of the WhatsApp “send” column.src/app/components/control/control.component.ts (1)
8-8: NgIf import is correct for standalone usage.Good addition to support structural directives in the template.
src/app/services/dialog.service.ts (1)
1-2: LGTM on service simplification.Clean removal of WhatsApp state; encrypt/decrypt dialog streams remain intact.
src/__tests__/core/handlers/app/main-instance-handler.spec.js (2)
2-6: Electron Menu mock addition looks goodMock shape matches usage in production. This is sufficient for isolating menu creation in tests.
10-10: Importing Menu alongside BrowserWindow is correctMatches the updated production module usage and enables explicit assertions on menu behavior.
src/core/handlers/app/main-instance-handler.js (2)
1-1: Menu import is correctNeeded for building and setting the application menu; aligns with test updates.
63-63: Verify UX: autoHideMenuBar with an application menuWith autoHideMenuBar: true, the new menu will be hidden by default on Windows/Linux (shown on Alt). Confirm this matches the intended UX now that a standard menu exists.
| const menu = Menu.buildFromTemplate([ | ||
| { | ||
| label: 'File', | ||
| submenu: [ | ||
| { role: 'quit' } | ||
| ] | ||
| }, | ||
| { | ||
| label: 'Edit', | ||
| submenu: [ | ||
| { role: 'undo' }, | ||
| { role: 'redo' }, | ||
| { type: 'separator' }, | ||
| { role: 'cut' }, | ||
| { role: 'copy' }, | ||
| { role: 'paste' }, | ||
| { role: 'selectall' } | ||
| ] | ||
| }, | ||
| { | ||
| label: 'View', | ||
| submenu: [ | ||
| { role: 'reload' }, | ||
| { role: 'forceReload' }, | ||
| { role: 'toggleDevTools' }, | ||
| { type: 'separator' }, | ||
| { role: 'resetZoom' }, | ||
| { role: 'zoomIn' }, | ||
| { role: 'zoomOut' }, | ||
| { type: 'separator' }, | ||
| { role: 'togglefullscreen' } | ||
| ] | ||
| }, | ||
| { | ||
| label: 'Window', | ||
| submenu: [ | ||
| { role: 'minimize' }, | ||
| { role: 'close' } | ||
| ] | ||
| } | ||
| ]); |
There was a problem hiding this comment.
Bug: role 'selectall' should be 'selectAll'
Electron role names are case-sensitive. Using 'selectall' will not invoke the standard role.
Apply this fix:
- { role: 'selectall' }
+ { role: 'selectAll' }Optional improvements:
- Consider platform-specific templates (e.g., include
{ role: 'appMenu' }and{ role: 'windowMenu' }on macOS, and use{ role: isMac ? 'close' : 'quit' }under File). - Extract the template to a helper (e.g., buildAppMenuTemplate()) to keep initializeMainWindow lean.
📝 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 menu = Menu.buildFromTemplate([ | |
| { | |
| label: 'File', | |
| submenu: [ | |
| { role: 'quit' } | |
| ] | |
| }, | |
| { | |
| label: 'Edit', | |
| submenu: [ | |
| { role: 'undo' }, | |
| { role: 'redo' }, | |
| { type: 'separator' }, | |
| { role: 'cut' }, | |
| { role: 'copy' }, | |
| { role: 'paste' }, | |
| { role: 'selectall' } | |
| ] | |
| }, | |
| { | |
| label: 'View', | |
| submenu: [ | |
| { role: 'reload' }, | |
| { role: 'forceReload' }, | |
| { role: 'toggleDevTools' }, | |
| { type: 'separator' }, | |
| { role: 'resetZoom' }, | |
| { role: 'zoomIn' }, | |
| { role: 'zoomOut' }, | |
| { type: 'separator' }, | |
| { role: 'togglefullscreen' } | |
| ] | |
| }, | |
| { | |
| label: 'Window', | |
| submenu: [ | |
| { role: 'minimize' }, | |
| { role: 'close' } | |
| ] | |
| } | |
| ]); | |
| const menu = Menu.buildFromTemplate([ | |
| { | |
| label: 'File', | |
| submenu: [ | |
| { role: 'quit' } | |
| ] | |
| }, | |
| { | |
| label: 'Edit', | |
| submenu: [ | |
| { role: 'undo' }, | |
| { role: 'redo' }, | |
| { type: 'separator' }, | |
| { role: 'cut' }, | |
| { role: 'copy' }, | |
| { role: 'paste' }, | |
| { role: 'selectAll' } | |
| ] | |
| }, | |
| { | |
| label: 'View', | |
| submenu: [ | |
| { role: 'reload' }, | |
| { role: 'forceReload' }, | |
| { role: 'toggleDevTools' }, | |
| { type: 'separator' }, | |
| { role: 'resetZoom' }, | |
| { role: 'zoomIn' }, | |
| { role: 'zoomOut' }, | |
| { type: 'separator' }, | |
| { role: 'togglefullscreen' } | |
| ] | |
| }, | |
| { | |
| label: 'Window', | |
| submenu: [ | |
| { role: 'minimize' }, | |
| { role: 'close' } | |
| ] | |
| } | |
| ]); |
🤖 Prompt for AI Agents
In src/core/handlers/app/main-instance-handler.js between lines 21 and 61, the
menu template uses the role 'selectall' which is incorrect due to case
sensitivity in Electron; it should be 'selectAll'. Update the role to
'selectAll' to ensure the standard role is invoked properly. Optionally,
refactor the menu template into a helper function and consider platform-specific
menu adjustments for better maintainability and user experience.
1 participant
Summary by CodeRabbit
Removed Features
Bug Fixes
Chores