diff --git a/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeForegroundService.kt b/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeForegroundService.kt index a85ccd2214..686fce04dd 100644 --- a/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeForegroundService.kt +++ b/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeForegroundService.kt @@ -58,9 +58,9 @@ class FocusModeForegroundService : Service() { // recents). Mirrors TrackingForegroundService's static-state pattern so // a focus session can be recovered into the Angular store (#7855). // `remainingMs` and `lastUpdateTimestamp` cannot use `private set` - // because the tick Runnable (a nested anonymous object) mutates them; - // the other three are written only from instance methods, so they keep - // `private set`. + // because the completion Runnable (a nested anonymous object) mutates + // them; the other three are written only from instance methods, so they + // keep `private set`. @Volatile var durationMs: Long = 0 private set @@ -81,8 +81,10 @@ class FocusModeForegroundService : Service() { /** * Live remaining time (countdown) or elapsed time (Flowtime, where - * durationMs is 0 and remainingMs accumulates). Accounts for the time - * since the last 1-second tick so a cold-start read stays accurate. + * durationMs is 0 and remainingMs accumulates). The snapshot fields only + * move on start/update/completion, so this derives the live value from + * the wall clock — it is THE time source for the notification, the + * completion scheduling, and the JS readback (#8243). * * Named `liveRemainingMs` rather than `getRemainingMs` to avoid a JVM * signature clash with the `remainingMs` property's generated getter. @@ -105,32 +107,32 @@ class FocusModeForegroundService : Service() { private var hasNotifiedCompletion: Boolean = false private val handler = Handler(Looper.getMainLooper()) - private val updateRunnable = object : Runnable { + + // Fires once at the expected countdown end instead of ticking every second — + // the notification chronometer renders the live timer without app work (#8243). + // Handler delays run on uptime, which stalls in deep sleep, so the runnable + // can only fire at wall-clock >= the requested delay (late completion in + // Doze, same as the old 1s loop). The re-arm branch below therefore only + // triggers when the wall clock moved BACKWARD (manual change/NTP) — re-arming + // keeps completion consistent with the wall-clock-based chronometer. + private val completionRunnable = object : Runnable { override fun run() { - if (isRunning && !isPaused) { - // Update remaining time (countdown mode) - val now = System.currentTimeMillis() - val elapsed = now - lastUpdateTimestamp - lastUpdateTimestamp = now - - if (durationMs > 0) { - // Countdown mode: decrease remaining time - val previousRemaining = remainingMs - remainingMs = (remainingMs - elapsed).coerceAtLeast(0) - - // Check for timer completion (only in countdown mode, not Flowtime) - if (remainingMs == 0L && previousRemaining > 0L && !hasNotifiedCompletion) { - onTimerComplete() - return // Stop the runnable, timer is done - } - } else { - // Flowtime mode: increase elapsed time (remainingMs is actually elapsed) - remainingMs += elapsed - } - - updateNotification() - handler.postDelayed(this, 1000) + if (!isRunning || isPaused || durationMs <= 0 || hasNotifiedCompletion) return + val remaining = liveRemainingMs() + if (remaining > 0) { + handler.postDelayed(this, remaining) + return } + remainingMs = 0 + lastUpdateTimestamp = System.currentTimeMillis() + onTimerComplete() + } + } + + private fun scheduleCompletionCheck() { + handler.removeCallbacks(completionRunnable) + if (isRunning && !isPaused && durationMs > 0) { + handler.postDelayed(completionRunnable, liveRemainingMs()) } } @@ -159,6 +161,10 @@ class FocusModeForegroundService : Service() { title = intent.getStringExtra(EXTRA_TITLE) ?: "Focus" taskTitle = intent.getStringExtra(EXTRA_TASK_TITLE) durationMs = intent.getLongExtra(EXTRA_DURATION_MS, 0L) + // Anchor before remainingMs: a torn liveRemainingMs() read from + // the JS bridge thread then returns a slightly stale value + // instead of subtracting the whole since-last-anchor gap. + lastUpdateTimestamp = System.currentTimeMillis() remainingMs = intent.getLongExtra(EXTRA_REMAINING_MS, 0L) isBreak = intent.getBooleanExtra(EXTRA_IS_BREAK, false) isPaused = intent.getBooleanExtra(EXTRA_IS_PAUSED, false) @@ -176,22 +182,18 @@ class FocusModeForegroundService : Service() { stopForegroundAndSelf() return START_NOT_STICKY } - val wasPaused = isPaused title = intent.getStringExtra(EXTRA_TITLE) ?: title - remainingMs = intent.getLongExtra(EXTRA_REMAINING_MS, remainingMs) + // Defensive fallback only — the sole caller (JavaScriptInterface. + // updateFocusModeService) always sends the extra. Anchor before + // remainingMs (see ACTION_START) to bias torn reads safe. + val newRemainingMs = intent.getLongExtra(EXTRA_REMAINING_MS, liveRemainingMs()) + lastUpdateTimestamp = System.currentTimeMillis() + remainingMs = newRemainingMs isPaused = intent.getBooleanExtra(EXTRA_IS_PAUSED, isPaused) isBreak = intent.getBooleanExtra(EXTRA_IS_BREAK, isBreak) taskTitle = intent.getStringExtra(EXTRA_TASK_TITLE) ?: taskTitle - lastUpdateTimestamp = System.currentTimeMillis() - - // Restart update runnable if resuming from paused state - if (wasPaused && !isPaused) { - handler.removeCallbacks(updateRunnable) - handler.post(updateRunnable) - } else if (!wasPaused && isPaused) { - handler.removeCallbacks(updateRunnable) - } + scheduleCompletionCheck() updateNotification() } @@ -220,9 +222,10 @@ class FocusModeForegroundService : Service() { this, title, taskTitle, - remainingMs, - isPaused, - isBreak + remainingMs = liveRemainingMs(), + isCountdown = durationMs > 0, + isPaused = isPaused, + isBreak = isBreak ) } else { // A content title is required on some OEM skins (notably Samsung @@ -262,11 +265,7 @@ class FocusModeForegroundService : Service() { return false } - // Start update loop if not paused - handler.removeCallbacks(updateRunnable) - if (!isPaused) { - handler.post(updateRunnable) - } + scheduleCompletionCheck() return true } @@ -277,7 +276,7 @@ class FocusModeForegroundService : Service() { private fun stopAfterForegroundFailure(startId: Int) { isRunning = false - handler.removeCallbacks(updateRunnable) + handler.removeCallbacks(completionRunnable) title = "" taskTitle = null durationMs = 0 @@ -301,7 +300,7 @@ class FocusModeForegroundService : Service() { Log.d(TAG, "Stopping focus mode") isRunning = false - handler.removeCallbacks(updateRunnable) + handler.removeCallbacks(completionRunnable) // Clear the mirrored state so a stale session can't be recovered after // it has legitimately ended (#7855). @@ -323,9 +322,10 @@ class FocusModeForegroundService : Service() { this, title, taskTitle, - remainingMs, - isPaused, - isBreak + remainingMs = liveRemainingMs(), + isCountdown = durationMs > 0, + isPaused = isPaused, + isBreak = isBreak ) NotificationManagerCompat.from(this).notify( FocusModeNotificationHelper.NOTIFICATION_ID, @@ -376,7 +376,7 @@ class FocusModeForegroundService : Service() { // before onStartCommand cleared it, drop the stale flag so the next cold // stop uses stopService() rather than needlessly re-spawning the service. clearStartPending() - handler.removeCallbacks(updateRunnable) + handler.removeCallbacks(completionRunnable) } // Do not override onTaskRemoved — foreground service must survive app swipe (#7818). diff --git a/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeNotificationHelper.kt b/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeNotificationHelper.kt index d0f8af1f4a..17c281e217 100644 --- a/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeNotificationHelper.kt +++ b/android/app/src/main/java/com/superproductivity/superproductivity/service/FocusModeNotificationHelper.kt @@ -42,6 +42,7 @@ object FocusModeNotificationHelper { title: String, taskTitle: String?, remainingMs: Long, + isCountdown: Boolean, isPaused: Boolean, isBreak: Boolean ): Notification { @@ -58,7 +59,6 @@ object FocusModeNotificationHelper { val builder = NotificationCompat.Builder(context, CHANNEL_ID) .setSmallIcon(R.drawable.ic_stat_sp) .setContentTitle(buildTitle(title, taskTitle)) - .setContentText(formatDuration(remainingMs)) .setContentIntent(contentPendingIntent) .setOngoing(true) .setOnlyAlertOnce(true) @@ -66,6 +66,25 @@ object FocusModeNotificationHelper { .setCategory(NotificationCompat.CATEGORY_PROGRESS) .setPriority(NotificationCompat.PRIORITY_LOW) + // SystemUI's chronometer renders the ticking timer so the service never + // re-posts the notification each second — the old 1 Hz notify() loop was + // a steady battery drain (#8243). Paused sessions show static text. + if (isPaused) { + builder.setContentText(formatDuration(remainingMs)) + } else if (isCountdown) { + builder + .setWhen(System.currentTimeMillis() + remainingMs) + .setShowWhen(true) + .setUsesChronometer(true) + .setChronometerCountDown(true) + } else { + // Flowtime: remainingMs holds the elapsed time, counting up + builder + .setWhen(System.currentTimeMillis() - remainingMs) + .setShowWhen(true) + .setUsesChronometer(true) + } + // Add Pause/Resume action if (isPaused) { val resumeIntent = Intent(context, CapacitorMainActivity::class.java).apply { diff --git a/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingForegroundService.kt b/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingForegroundService.kt index 3bbac5edd6..94ba7d8b8c 100644 --- a/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingForegroundService.kt +++ b/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingForegroundService.kt @@ -2,9 +2,7 @@ package com.superproductivity.superproductivity.service import android.app.Service import android.content.Intent -import android.os.Handler import android.os.IBinder -import android.os.Looper import android.util.Log import androidx.core.app.NotificationManagerCompat @@ -71,16 +69,6 @@ class TrackingForegroundService : Service() { private var taskTitle: String = "" - private val handler = Handler(Looper.getMainLooper()) - private val updateRunnable = object : Runnable { - override fun run() { - if (isTracking) { - updateNotification() - handler.postDelayed(this, 1000) - } - } - } - override fun onCreate() { super.onCreate() Log.d(TAG, "Service created") @@ -187,7 +175,6 @@ class TrackingForegroundService : Service() { private fun stopAfterForegroundFailure(startId: Int) { isTracking = false - handler.removeCallbacks(updateRunnable) currentTaskId = null startTimestamp = 0 accumulatedMs = 0 @@ -208,21 +195,20 @@ class TrackingForegroundService : Service() { currentTaskId = taskId taskTitle = title - accumulatedMs = timeSpentMs + // Anchor first, accumulated second: a torn getElapsedMs() read from the + // JS bridge thread then under-reports (caught by the negative-duration + // keep-app-value path) instead of double-counting the since-last-anchor + // gap — which can be hours now that nothing re-anchors every second. startTimestamp = System.currentTimeMillis() + accumulatedMs = timeSpentMs isTracking = true // The foreground-service start token was already satisfied at the top // of onStartCommand(). Replace the placeholder notification without // risking a second startForeground() failure resetting tracking state. - if (!updateNotification()) { - return false - } - - // Start update loop - handler.removeCallbacks(updateRunnable) - handler.post(updateRunnable) - return true + // The chronometer in the notification ticks on its own — no update + // loop needed (#8243). + return updateNotification() } private fun updateTimeSpent(timeSpentMs: Long) { @@ -232,9 +218,10 @@ class TrackingForegroundService : Service() { } Log.d(TAG, "Updating time spent: timeSpentMs=$timeSpentMs (was accumulated=$accumulatedMs)") - // Reset the timer with the new accumulated value - accumulatedMs = timeSpentMs + // Reset the timer with the new accumulated value. Anchor first (see + // startTracking) so a torn bridge-thread read errs toward under-reporting. startTimestamp = System.currentTimeMillis() + accumulatedMs = timeSpentMs // Update notification immediately updateNotification() @@ -244,7 +231,6 @@ class TrackingForegroundService : Service() { Log.d(TAG, "Stopping tracking, elapsed=${getElapsedMs()}ms") isTracking = false - handler.removeCallbacks(updateRunnable) // Reset state currentTaskId = null @@ -285,7 +271,6 @@ class TrackingForegroundService : Service() { // before onStartCommand cleared it, drop the stale flag so the next cold // stop uses stopService() rather than needlessly re-spawning the service. clearStartPending() - handler.removeCallbacks(updateRunnable) } // Do not override onTaskRemoved — foreground service must survive app swipe (#7818). diff --git a/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingNotificationHelper.kt b/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingNotificationHelper.kt index bee0c8979d..ac2707902e 100644 --- a/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingNotificationHelper.kt +++ b/android/app/src/main/java/com/superproductivity/superproductivity/service/TrackingNotificationHelper.kt @@ -67,11 +67,17 @@ object TrackingNotificationHelper { PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE ) + // SystemUI renders the ticking elapsed time via the chronometer, so the + // service never has to re-post the notification just to advance the + // timer — a 1 Hz notify() loop kept the process busy around the clock + // and showed up as battery drain (#8243). return NotificationCompat.Builder(context, CHANNEL_ID) .setSmallIcon(R.drawable.ic_stat_sp) .setContentTitle(taskTitle) - .setContentText(formatDuration(elapsedMs)) .setContentIntent(contentPendingIntent) + .setWhen(System.currentTimeMillis() - elapsedMs) + .setShowWhen(true) + .setUsesChronometer(true) .setOngoing(true) .setOnlyAlertOnce(true) .setSilent(true) @@ -81,17 +87,4 @@ object TrackingNotificationHelper { .setPriority(NotificationCompat.PRIORITY_LOW) .build() } - - fun formatDuration(ms: Long): String { - val totalSeconds = ms / 1000 - val hours = totalSeconds / 3600 - val minutes = (totalSeconds % 3600) / 60 - val seconds = totalSeconds % 60 - - return when { - hours > 0 -> String.format("%dh %dm %ds", hours, minutes, seconds) - minutes > 0 -> String.format("%dm %ds", minutes, seconds) - else -> String.format("%ds", seconds) - } - } } diff --git a/docs/wiki/3.03-Keyboard-Shortcuts.md b/docs/wiki/3.03-Keyboard-Shortcuts.md index 2fc32a591b..e45c597947 100755 --- a/docs/wiki/3.03-Keyboard-Shortcuts.md +++ b/docs/wiki/3.03-Keyboard-Shortcuts.md @@ -36,6 +36,8 @@ These work everywhere in the app. - `Ctrl`+`0` (zero): Zoom default (Desktop/Firefox/Chrome) - `Ctrl`+`S`: Trigger sync (if configured) +The search bar searches open tasks by default. Enable **Include completed and archived tasks** in the search view to include older completed results. + ### Add Task Bar (while the add bar input is focused) - `Ctrl+1`: Toggle add to top / bottom of list @@ -119,6 +121,8 @@ Shortcuts are stored in global config (`GlobalConfigState.keyboard`) and persist **Task-level shortcuts:** Actions with a task focus (e.g. mark done, add subtask, schedule, delete, move) apply to the currently selected task. Selection is by keyboard (tab/arrows) or mouse. The [[3.02-Settings-and-Preferences]] keyboard help text describes these as applying to "the currently selected task." +Specifically for the **Next Month** shortcut, the task is scheduled for the **1st day of the next month**. This avoids issues with different month lengths (e.g., scheduling for the 31st when the next month only has 30 days). + **Schedule week view:** `Ctrl`+MouseWheel changes the vertical scale of the week schedule grid. The value is stored locally for future schedule visits. **Focus Mode overlay:** `Escape` closes the overlay. If a focus session or break is running, it continues in the header focus button. @@ -128,7 +132,7 @@ Shortcuts are stored in global config (`GlobalConfigState.keyboard`) and persist ## Conflict and Precedence - **Input focus:** When focus is in an input element or a blocking overlay is open, shortcut handling is skipped so typing is not interrupted. -- **Contextual Precedence:** Task-level shortcuts take precedence over global shortcuts when a task is currently focused. For example, pressing `Shift+S` opens the deadline dialog if a task is focused, but navigates to the Schedule view if no task is focused. Similarly, `Shift+T` moves the selected task to Today, but navigates to the Today view otherwise. +- **Contextual Precedence:** Task-level shortcuts take precedence over global shortcuts when a task is currently focused. For example, pressing `Shift+S` opens the deadline dialog if a task is focused, but navigates to the Schedule view if no task is focused. Similarly, `Shift+T` schedules the selected task for today, but navigates to the Today view otherwise. - **Conflicts:** The configuration UI warns if two actions are assigned the same key combination. The user can press **Escape**, **Backspace**, or **Delete** in the shortcut field to clear an existing assignment. ## Storage and Definition diff --git a/docs/wiki/3.07-Issue-Integration-Comparison.md b/docs/wiki/3.07-Issue-Integration-Comparison.md index 4d4d32e7c9..0737c19df4 100644 --- a/docs/wiki/3.07-Issue-Integration-Comparison.md +++ b/docs/wiki/3.07-Issue-Integration-Comparison.md @@ -111,7 +111,7 @@ Every issue provider supports the same basic operations: checking that the integ - **Issue import:** Issues from projects. - **Status transitions / Worklog / Comments / Subtasks / Attachments:** Not supported. -- **Filtering:** Project-based; scope support. +- **Filtering:** Project-based; scope support; task search supports non-Latin subject matches. - **Auto-import:** Last 100 issues for the project. ### 9. Trello diff --git a/electron/main-window.ts b/electron/main-window.ts index a65483929e..5bfcb5cedd 100644 --- a/electron/main-window.ts +++ b/electron/main-window.ts @@ -29,9 +29,16 @@ import { getIsMinimizeToTray, getIsQuiting, setIsQuiting } from './shared-state' import { loadSimpleStoreAll } from './simple-store'; import { SimpleStoreKey } from './shared-with-frontend/simple-store.const'; import { markGpuStartupSuccess } from './gpu-startup-guard'; +import { isAppOriginUrl } from './navigation-guard'; let mainWin: BrowserWindow; +// The URL passed to `mainWin.loadURL()` — the single source of truth for +// "what is the app's own origin?". Read by the will-navigate / will-redirect +// guards in `initWinEventListeners`. Set in `createWindow`, before listeners +// are wired, so the guard never sees `undefined` at runtime. +let appLoadedUrl: string | undefined; + // Compact WCO band on Win/Linux. Native button width is OS-controlled // (~138px total); only height is configurable. Lower values may be // clamped to the OS minimum (~24–28px on Win11) — Electron silently @@ -296,6 +303,11 @@ export const createWindow = async ({ ? 'http://localhost:4200' : `file://${normalize(join(__dirname, '../.tmp/angular-dist/browser/index.html'))}`; + // Capture the loaded URL so the navigation guard (initWinEventListeners → + // will-navigate) can compare against the actual app origin, not a derived + // guess. Any URL change here automatically tightens the guard. + appLoadedUrl = url; + mainWin.loadURL(url).then(() => { // Set window title for dev mode if (IS_DEV) { @@ -446,31 +458,53 @@ function initWinEventListeners(app: Electron.App): void { }); }; - // open new window links in browser + // Compare the navigation target against the URL the app actually loaded + // (captured at loadURL time in createWindow). Anything else is treated as + // external and routed through the scheme-guarded `openUrlInBrowser`. + // + // The main window has Node integration via the preload bridge (`window.ea`). + // Allowing in-window navigation to ANY other origin — including + // http://127.0.0.1: — would expose that bridge to whatever page + // happens to be served there (a malicious local web server, a sibling + // electron app, etc.). The previous host-only check accepted those. + // + // Hash-only changes do NOT fire will-navigate, so this never fires for + // the app's own hash routes (HashLocationStrategy in src/main.ts). + const guardNavigation = ( + ev: { preventDefault: () => void }, + url: string, + eventLabel: string, + ): void => { + if (appLoadedUrl && isAppOriginUrl(url, appLoadedUrl)) return; + ev.preventDefault(); + log(`Blocked in-window navigation (${eventLabel})`); + openUrlInBrowser(url); + }; + mainWin.webContents.on('will-navigate', (ev, url) => { - // Navigate in-window only for the app's own dev-server origin - // (http://localhost:4200). Prod serves from file:// (start URL above) and - // routes via hash (withHashLocation, src/main.ts), so will-navigate never - // fires for the app's own routes there — only for real external links, - // which we hand to the (scheme-guarded) browser. Compare the host exactly: - // a substring match let hosts like `https://localhost.evil.com` navigate - // the main window directly. - let isInternalNavigation = false; - try { - const { hostname } = new URL(url); - isInternalNavigation = hostname === 'localhost' || hostname === '127.0.0.1'; - } catch { - isInternalNavigation = false; - } - if (!isInternalNavigation) { - ev.preventDefault(); - openUrlInBrowser(url); - } + guardNavigation(ev, url, 'will-navigate'); + }); + // Defense in depth: a same-origin navigation could redirect to a different + // origin server-side. Re-run the same check on the redirect target so a + // ‘302 → http://127.0.0.1:1337’ cannot land the bridge on an attacker page. + mainWin.webContents.on('will-redirect', (ev, url) => { + guardNavigation(ev, url, 'will-redirect'); }); mainWin.webContents.setWindowOpenHandler((details) => { openUrlInBrowser(details.url); return { action: 'deny' }; }); + // Defense in depth: setWindowOpenHandler already denies, so this should + // never fire. If a future code path ever enables window creation, destroy + // the spawned window rather than letting it inherit the preload bridge. + mainWin.webContents.on('did-create-window', (childWin) => { + error('did-create-window fired despite deny handler — destroying child'); + try { + childWin.destroy(); + } catch (e) { + error('Failed to destroy unexpected child window:', e); + } + }); // TODO refactor quitting mess appCloseHandler(app); diff --git a/electron/navigation-guard.test.cjs b/electron/navigation-guard.test.cjs new file mode 100644 index 0000000000..5dba5ae694 --- /dev/null +++ b/electron/navigation-guard.test.cjs @@ -0,0 +1,143 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +require('ts-node/register/transpile-only'); + +// Match the layout used by the other *.test.cjs files: resolve the .ts source +// via a computed path so tools/verify-electron-requires.js doesn't flag a +// literal relative require of a file excluded from app.asar. +const { isAppOriginUrl } = require(path.resolve(__dirname, 'navigation-guard.ts')); + +const DEV_APP_URL = 'http://localhost:4200'; +const PROD_APP_URL = 'file:///Applications/SP.app/Contents/Resources/index.html'; + +test('dev: exact origin matches', () => { + assert.equal(isAppOriginUrl('http://localhost:4200/', DEV_APP_URL), true); + assert.equal(isAppOriginUrl('http://localhost:4200/#/foo', DEV_APP_URL), true); + assert.equal(isAppOriginUrl('http://localhost:4200/some-route', DEV_APP_URL), true); +}); + +test('dev: different port on localhost is rejected', () => { + // The whole point of the fix: 127.0.0.1:1337 or localhost:9999 must NOT + // inherit the preload bridge just because the hostname looks local. + assert.equal(isAppOriginUrl('http://localhost:1337/', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('http://localhost:9999/', DEV_APP_URL), false); +}); + +test('dev: 127.0.0.1 is rejected (different host than localhost)', () => { + // The old guard accepted any "localhost OR 127.0.0.1" — anything serving + // on a loopback address could load in the privileged window. After the + // fix, only the exact origin the app loaded is allowed. + assert.equal(isAppOriginUrl('http://127.0.0.1:4200/', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('http://127.0.0.1:1337/', DEV_APP_URL), false); +}); + +test('dev: localhost.evil.com substring attack is rejected', () => { + // Regression for the earlier substring-match bug (fixed in 4bf699735). + assert.equal(isAppOriginUrl('http://localhost.evil.com/', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('http://evil.localhost:4200/', DEV_APP_URL), false); +}); + +test('dev: userinfo @ trick is rejected (host is the part after @)', () => { + // WHATWG URL parses `http://localhost:4200@evil.com/` with host=evil.com, + // username=localhost. A naive substring/startsWith check would be fooled; + // the host-equality check correctly rejects it. + assert.equal(isAppOriginUrl('http://localhost:4200@evil.com/', DEV_APP_URL), false); + assert.equal( + isAppOriginUrl('http://localhost@evil.com:4200/', DEV_APP_URL), + false, + 'evil.com:4200 with localhost as username is still not our origin', + ); +}); + +test('dev: https variant is rejected (protocol must match)', () => { + assert.equal(isAppOriginUrl('https://localhost:4200/', DEV_APP_URL), false); +}); + +test('dev: external https URL is rejected', () => { + assert.equal(isAppOriginUrl('https://example.com/', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('https://jira.example.com/browse/X-1', DEV_APP_URL), false); +}); + +test('prod: same file:// path matches', () => { + assert.equal(isAppOriginUrl(PROD_APP_URL, PROD_APP_URL), true); +}); + +test('prod: rejects ALL http(s) localhost URLs (no dev-only opt-in)', () => { + // Production never legitimately navigates to http://localhost:*. The fix + // is what makes this hold: no host-based allowlist, only "matches the + // app's actual loaded URL". + assert.equal(isAppOriginUrl('http://localhost:4200/', PROD_APP_URL), false); + assert.equal(isAppOriginUrl('http://localhost:1337/', PROD_APP_URL), false); + assert.equal(isAppOriginUrl('http://127.0.0.1:1337/', PROD_APP_URL), false); +}); + +test('prod: rejects different file:// targets', () => { + assert.equal( + isAppOriginUrl('file:///etc/passwd', PROD_APP_URL), + false, + 'arbitrary file path', + ); + assert.equal( + isAppOriginUrl( + 'file:///Applications/SP.app/Contents/Resources/other.html', + PROD_APP_URL, + ), + false, + 'sibling html under the app bundle', + ); +}); + +test('prod: rejects UNC / remote-host file:// even when pathname matches', () => { + // Regression: `file://` URLs carry a host. A pathname-only check matched + // `file://192.168.1.100/Applications/SP.app/Contents/Resources/index.html` + // against the local `file:///…/index.html` because both pathnames are + // `/Applications/SP.app/Contents/Resources/index.html` — letting an + // attacker-controlled UNC host load in the privileged window. + assert.equal( + isAppOriginUrl( + 'file://192.168.1.100/Applications/SP.app/Contents/Resources/index.html', + PROD_APP_URL, + ), + false, + 'UNC host with matching pathname must be rejected', + ); + assert.equal( + isAppOriginUrl( + 'file://attacker.example.com/Applications/SP.app/Contents/Resources/index.html', + PROD_APP_URL, + ), + false, + 'remote-host file:// with matching pathname must be rejected', + ); + // Even an empty-but-malformed authority pattern should not bypass it. + assert.equal( + isAppOriginUrl('file://localhost/etc/passwd', PROD_APP_URL), + false, + 'localhost-host file:// pointing elsewhere is rejected', + ); +}); + +test('non-http(s)/file schemes are rejected', () => { + // data:/blob:/javascript:/etc must never land in the privileged window. + assert.equal(isAppOriginUrl('data:text/html,

x

', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('javascript:alert(1)', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('blob:http://localhost:4200/abcd', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('about:blank', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('ftp://localhost:4200/x', DEV_APP_URL), false); +}); + +test('malformed URLs are rejected', () => { + assert.equal(isAppOriginUrl('not a url', DEV_APP_URL), false); + assert.equal(isAppOriginUrl('', DEV_APP_URL), false); +}); + +test('custom dev URL: exact origin still required', () => { + // Some setups override the dev URL (electron CLI). The guard always reads + // back the actual loaded URL, so a custom dev origin works without code + // changes — but still rejects look-alikes. + const customUrl = 'http://localhost:9999'; + assert.equal(isAppOriginUrl('http://localhost:9999/route', customUrl), true); + assert.equal(isAppOriginUrl('http://localhost:4200/', customUrl), false); +}); diff --git a/electron/navigation-guard.ts b/electron/navigation-guard.ts new file mode 100644 index 0000000000..cb751c1ca4 --- /dev/null +++ b/electron/navigation-guard.ts @@ -0,0 +1,46 @@ +/** + * Returns true if `targetUrl` is the same origin as the app's loaded URL. + * + * Security boundary: the main window has Node integration / preload bridge. + * A navigation to ANY other origin in-window would expose `window.ea` to + * untrusted content (e.g. http://127.0.0.1: hosting a malicious page). + * `will-navigate` MUST reject anything this returns false for and hand it + * to the (scheme-guarded) external-open path instead. + * + * Comparison rules: + * - Protocols must match exactly. + * - http/https: exact host (incl. port) match. Subdomains differ → different + * origin. This is what blocks `localhost.evil.com` and `http://127.0.0.1:1`. + * - file: BOTH host AND pathname must match the app's loaded html file. + * `file://` URLs may carry a host (UNC paths / remote shares), and Chromium + * resolves the pathname relative to that host — so a target like + * `file://192.168.1.100/Applications/SP.app/Contents/Resources/index.html` + * has the same `.pathname` as the local app start URL. A pathname-only + * check would let an attacker-controlled UNC host load in the privileged + * window. The app's loaded file:// URL is always local (empty host), so we + * require `target.host === ''` explicitly rather than relying on the + * host-equality compare to coincidentally match. + * Hash-only changes do not fire `will-navigate`, so a same-document hash + * route never reaches this check. + * - Anything else (data:, blob:, javascript:, ftp:, …): rejected. + */ +export const isAppOriginUrl = (targetUrl: string, appUrl: string): boolean => { + let target: URL; + let expected: URL; + try { + target = new URL(targetUrl); + expected = new URL(appUrl); + } catch { + return false; + } + if (target.protocol !== expected.protocol) return false; + if (target.protocol === 'http:' || target.protocol === 'https:') { + return target.host === expected.host; + } + if (target.protocol === 'file:') { + return ( + target.host === '' && expected.host === '' && target.pathname === expected.pathname + ); + } + return false; +}; diff --git a/eslint.config.js b/eslint.config.js index 6d6ccacc61..4344757658 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -224,6 +224,21 @@ module.exports = tseslint.config( 'local-rules/no-multi-entity-effect': 'warn', }, }, + // App code must route logging through Log/SyncLog/OpLog/... helpers. + // Direct console.* calls bypass the exportable log history users attach + // to bug reports. The Log implementation itself, tests, and benchmarks + // (which intentionally dump timing numbers to stdout) are exempt. + { + files: ['src/app/**/*.ts'], + ignores: [ + 'src/app/**/*.spec.ts', + 'src/app/**/*.benchmark.ts', + 'src/app/core/log.ts', + ], + rules: { + 'no-console': 'error', + }, + }, // HTML files { files: ['**/*.html'], diff --git a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts index 767f147ee5..0f888a2bca 100644 --- a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts +++ b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts @@ -37,6 +37,7 @@ import { Project } from '../../../features/project/project.model'; import { isSingleEmoji } from '../../../util/extract-first-emoji'; import { expandCollapseAni } from '../../../ui/tree-dnd/tree.animations'; import { Router } from '@angular/router'; +import { Log } from '../../../core/log'; const EXPAND_ANIMATION_RESET_DELAY_MS = 250; @@ -179,10 +180,7 @@ export class NavListTreeComponent implements OnDestroy { node: TreeNode, ): void { // TODO: Implement folder context menu - console.log( - 'Folder context menu for:', - node.data?.k === MenuTreeKind.FOLDER ? node.data.name : 'unknown', - ); + Log.log('Folder context menu for:', { id: node.id, kind: node.data?.k }); } private _toggleFolder(treeNodeId: string): void { diff --git a/src/app/core-ui/work-context-menu/work-context-menu.component.ts b/src/app/core-ui/work-context-menu/work-context-menu.component.ts index acb252e281..653ddc576d 100644 --- a/src/app/core-ui/work-context-menu/work-context-menu.component.ts +++ b/src/app/core-ui/work-context-menu/work-context-menu.component.ts @@ -46,6 +46,7 @@ import { firstValueFrom, Observable, of } from 'rxjs'; import { AsyncPipe } from '@angular/common'; import { DateService } from '../../core/date/date.service'; import { openWorkContextSettingsDialog } from '../../features/work-context/dialog-work-context-settings/open-work-context-settings-dialog'; +import { Log } from '../../core/log'; @Component({ selector: 'work-context-menu', @@ -241,7 +242,7 @@ export class WorkContextMenuComponent implements OnInit { try { return await this._projectService.getCompletionInfo(this.contextId); } catch (err) { - console.error(err); + Log.err(err); this._snackService.open({ type: 'ERROR', msg: T.F.PROJECT.COMPLETE.ERROR }); return null; } @@ -301,7 +302,7 @@ export class WorkContextMenuComponent implements OnInit { msg: T.GLOBAL_SNACK.DUPLICATE_PROJECT_ERROR, type: 'ERROR', }); - console.error(err); + Log.err(err); } } @@ -447,7 +448,7 @@ export class WorkContextMenuComponent implements OnInit { msg: T.GLOBAL_SNACK.OPEN_SETTINGS_ERROR, type: 'ERROR', }); - console.error(err); + Log.err(err); } } diff --git a/src/app/core/clipboard-image/clipboard-image.service.ts b/src/app/core/clipboard-image/clipboard-image.service.ts index 73207bb250..f193a15a19 100644 --- a/src/app/core/clipboard-image/clipboard-image.service.ts +++ b/src/app/core/clipboard-image/clipboard-image.service.ts @@ -5,6 +5,7 @@ import { T } from '../../t.const'; import { GlobalConfigService } from '../../features/config/global-config.service'; import { getDefaultClipboardImagesPath } from '../../util/get-default-clipboard-images-path'; import { MIME_TYPE_EXTENSIONS } from '../../../../electron/shared-with-frontend/mime-type-mapping.const'; +import { Log } from '../log'; const DB_NAME = 'sp-clipboard-images'; const DB_VERSION = 1; @@ -117,7 +118,7 @@ export class ClipboardImageService { return { success: true, imageUrl, markdownText }; } } catch (error) { - console.error('[CLIPBOARD] Error getting file path:', error); + Log.err('[CLIPBOARD] Error getting file path:', error); } } } @@ -164,8 +165,8 @@ export class ClipboardImageService { return { success: true, imageUrl, markdownText }; } catch (error) { - console.error('[CLIPBOARD] Error saving clipboard image:', error); - console.error( + Log.err('[CLIPBOARD] Error saving clipboard image:', error); + Log.err( '[CLIPBOARD] Error stack:', error instanceof Error ? error.stack : 'no stack', ); @@ -234,7 +235,7 @@ export class ClipboardImageService { return { success: true, imageUrl, markdownText }; } catch (error) { - console.error('Error getting image from file paths:', error); + Log.err('Error getting image from file paths:', error); return null; // Fall back to regular clipboard handling } } @@ -316,7 +317,7 @@ export class ClipboardImageService { this._blobUrlCache.set(imageId, blobUrl); return blobUrl; } catch (error) { - console.error('Error resolving indexeddb URL for clipboard image:', error); + Log.err('Error resolving indexeddb URL for clipboard image:', error); return null; } } diff --git a/src/app/core/clipboard-image/resolve-clipboard-images.directive.ts b/src/app/core/clipboard-image/resolve-clipboard-images.directive.ts index ea675631c1..9770380b04 100644 --- a/src/app/core/clipboard-image/resolve-clipboard-images.directive.ts +++ b/src/app/core/clipboard-image/resolve-clipboard-images.directive.ts @@ -9,6 +9,7 @@ import { SimpleChanges, } from '@angular/core'; import { ClipboardImageService } from './clipboard-image.service'; +import { Log } from '../log'; /** * Directive that resolves indexeddb:// URLs in img elements within the host element. @@ -98,7 +99,7 @@ export class ResolveClipboardImagesDirective implements OnInit, OnDestroy, OnCha img.classList.add('indexeddb-image-error'); } } catch (error) { - console.error('Error resolving clipboard image:', error); + Log.err('Error resolving clipboard image:', error); img.alt = 'Error loading image'; img.classList.add('indexeddb-image-error'); } diff --git a/src/app/core/clipboard-image/resolve-clipboard-url.pipe.ts b/src/app/core/clipboard-image/resolve-clipboard-url.pipe.ts index 10c7c018f3..e41f1c5dff 100644 --- a/src/app/core/clipboard-image/resolve-clipboard-url.pipe.ts +++ b/src/app/core/clipboard-image/resolve-clipboard-url.pipe.ts @@ -2,6 +2,7 @@ import { Pipe, PipeTransform, inject } from '@angular/core'; import { ClipboardImageService } from './clipboard-image.service'; import { Observable, from, of, defer } from 'rxjs'; import { map, shareReplay, catchError } from 'rxjs/operators'; +import { Log } from '../log'; @Pipe({ name: 'resolveClipboardUrl', @@ -38,7 +39,7 @@ export class ResolveClipboardUrlPipe implements PipeTransform { return url; }), catchError((error) => { - console.error('Error resolving clipboard URL:', error); + Log.err('Error resolving clipboard URL:', error); return of(url); }), shareReplay({ bufferSize: 1, refCount: false }), diff --git a/src/app/core/error-handler/global-error-handler.util.ts b/src/app/core/error-handler/global-error-handler.util.ts index 557a7ac2bd..778d873f1e 100644 --- a/src/app/core/error-handler/global-error-handler.util.ts +++ b/src/app/core/error-handler/global-error-handler.util.ts @@ -88,7 +88,7 @@ export const logAdvancedStacktrace = ( // NOTE: there is an issue with this sometimes -> https://github.com/stacktracejs/stacktrace.js/issues/202 }) - .catch(console.error); + .catch((err) => Log.err(err)); const _cleanHtml = (str: string): string => { const div = document.createElement('div'); diff --git a/src/app/core/share/share-file.util.ts b/src/app/core/share/share-file.util.ts index 9e8b2d87bc..61bc6e0de0 100644 --- a/src/app/core/share/share-file.util.ts +++ b/src/app/core/share/share-file.util.ts @@ -1,6 +1,7 @@ import { Directory, Filesystem } from '@capacitor/filesystem'; import { IS_NATIVE_PLATFORM } from '../../util/is-native-platform'; import { ShareResult } from './share.model'; +import { Log } from '../log'; /** * Sanitize filename for safe file system usage. @@ -43,7 +44,7 @@ export const downloadBlob = (blob: Blob, filename: string): boolean => { anchor.click(); return true; } catch (error) { - console.warn('Browser download failed:', error); + Log.warn('Browser download failed:', error); return false; } finally { document.body.removeChild(anchor); @@ -65,7 +66,7 @@ export const cleanupCacheFile = async (relativePath: string): Promise => { directory: Directory.Cache, }); } catch (cleanupError) { - console.warn('Failed to cleanup shared file:', cleanupError); + Log.warn('Failed to cleanup shared file:', cleanupError); } }; @@ -120,7 +121,7 @@ export const openDownloadResult = async (result: ShareResult): Promise => (window as any).ea.openPath(path); return; } catch (error) { - console.warn('Failed to open path via Electron bridge:', error); + Log.warn('Failed to open path via Electron bridge:', error); } } @@ -131,7 +132,7 @@ export const openDownloadResult = async (result: ShareResult): Promise => window.open(candidate, '_blank', 'noopener'); return; } catch (error) { - console.warn('Failed to open download in new window:', error); + Log.warn('Failed to open download in new window:', error); } } }; diff --git a/src/app/core/share/share.service.ts b/src/app/core/share/share.service.ts index 77947ab05c..2113eb2f30 100644 --- a/src/app/core/share/share.service.ts +++ b/src/app/core/share/share.service.ts @@ -17,6 +17,7 @@ import * as ShareTextUtil from './share-text.util'; import * as ShareUrlBuilder from './share-url-builder.util'; import * as ShareFileUtil from './share-file.util'; import * as SharePlatformUtil from './share-platform.util'; +import { Log } from '../log'; export type ShareOutcome = 'shared' | 'cancelled' | 'unavailable' | 'failed'; export type { ShareSupport } from './share-platform.util'; @@ -252,7 +253,7 @@ export class ShareService { error: 'Share cancelled', }; } - console.warn('Capacitor share failed:', error); + Log.warn('Capacitor share failed:', error); } } @@ -273,7 +274,7 @@ export class ShareService { if (error instanceof Error && error.name === 'AbortError') { return { success: false, error: 'Share cancelled' }; } - console.warn('Web Share API failed:', error); + Log.warn('Web Share API failed:', error); } } @@ -524,7 +525,7 @@ export class ShareService { ? `file://${resolvedUri}` : `file:///${resolvedUri}`; - console.debug('[ShareService] shareCanvasViaNative', { + Log.debug('[ShareService] shareCanvasViaNative', { resolvedUri, fileUrl, relativePath, @@ -535,9 +536,9 @@ export class ShareService { path: relativePath, directory: Directory.Cache, }); - console.debug('[ShareService] shareCanvasViaNative stat', stat); + Log.debug('[ShareService] shareCanvasViaNative stat', stat); } catch (statError) { - console.warn('[ShareService] stat failed for shared image', statError); + Log.warn('[ShareService] stat failed for shared image', statError); } const canShare = (await sharePlugin.canShare?.())?.value ?? true; @@ -570,7 +571,7 @@ export class ShareService { error: 'Share cancelled', }; } - console.warn('Native image share failed:', error, { + Log.warn('Native image share failed:', error, { fileUrl: fileUrl ?? 'n/a', resolvedUri: resolvedUri ?? 'n/a', relativePath, @@ -633,7 +634,7 @@ export class ShareService { error: 'Share cancelled', }; } - console.warn('Web Share API failed:', error); + Log.warn('Web Share API failed:', error); } return { @@ -683,7 +684,7 @@ export class ShareService { uri, }; } catch (error) { - console.warn('Native download failed, falling back to browser download:', error); + Log.warn('Native download failed, falling back to browser download:', error); } } @@ -703,7 +704,7 @@ export class ShareService { target: 'download', }; } catch (error) { - console.warn('Opening image in new tab failed:', error); + Log.warn('Opening image in new tab failed:', error); } } diff --git a/src/app/core/unsplash/unsplash.service.ts b/src/app/core/unsplash/unsplash.service.ts index 28e6671f46..e18b0a650f 100644 --- a/src/app/core/unsplash/unsplash.service.ts +++ b/src/app/core/unsplash/unsplash.service.ts @@ -3,6 +3,7 @@ import { HttpClient } from '@angular/common/http'; import { Observable, of } from 'rxjs'; import { catchError } from 'rxjs/operators'; import { getEnvOptional } from '../../util/env'; +import { Log } from '../log'; export interface UnsplashPhoto { id: string; @@ -56,7 +57,7 @@ export class UnsplashService { } if (!this.ACCESS_KEY) { - console.warn( + Log.warn( 'No Unsplash Access Key configured. Register at https://unsplash.com/developers?utm_source=super-productivity&utm_medium=referral&utm_campaign=api-credit', ); return of({ results: [], total: 0, total_pages: 0 }); @@ -81,7 +82,7 @@ export class UnsplashService { }) .pipe( catchError((error) => { - console.error('Unsplash API error:', error); + Log.err('Unsplash API error:', error); return of({ results: [], total: 0, total_pages: 0 }); }), ); @@ -122,12 +123,12 @@ export class UnsplashService { */ trackPhotoDownload(photo: UnsplashPhoto): Observable { if (!photo.links?.download_location) { - console.warn('No download_location available for photo', photo.id); + Log.warn('No download_location available for photo', photo.id); return of(null); } if (!this.ACCESS_KEY) { - console.warn('No Unsplash Access Key configured'); + Log.warn('No Unsplash Access Key configured'); return of(null); } @@ -138,7 +139,7 @@ export class UnsplashService { // Call the download endpoint to track usage return this._http.get(photo.links.download_location, { headers }).pipe( catchError((error) => { - console.error('Failed to track photo download:', error); + Log.err('Failed to track photo download:', error); // Don't fail the selection if tracking fails return of(null); }), diff --git a/src/app/features/android/store/android-focus-mode.effects.ts b/src/app/features/android/store/android-focus-mode.effects.ts index a08366d733..9ca23b92a9 100644 --- a/src/app/features/android/store/android-focus-mode.effects.ts +++ b/src/app/features/android/store/android-focus-mode.effects.ts @@ -36,10 +36,16 @@ export const createFocusResumeTick$ = (onResume$: Observable): Observable< /** * Whether the focus-mode notification needs a fresh push to the native service. - * Elapsed-only changes are throttled to 5s (the native handler already ticks - * every second), but pause/purpose changes — and the large elapsed jump a resume - * `tick()` produces (#7856) — must propagate immediately so the notification - * reconciles with the corrected in-app countdown. + * The notification's chronometer ticks natively, so steady-state elapsed + * changes need no push at all — and since prev/curr are consecutive per-tick + * emissions (~1s apart), the 5s gate suppresses them entirely (#8243). Do not + * weaken it: every push re-runs startForeground + a notification rebuild. + * Pause/purpose changes — and the large elapsed jump a resume `tick()` + * produces (#7856) — must propagate immediately so the notification + * reconciles with the corrected in-app countdown. (The 5000 here and + * TIME_SPENT_JUMP_THRESHOLD_MS in android-foreground-tracking.effects.ts + * encode the same "larger than any tick" idea but differ in semantics — + * abs() vs decrease-always-passes — so they are deliberately not shared.) */ export const hasFocusNotificationStateChanged = ( prevTimer: TimerState | undefined, diff --git a/src/app/features/android/store/android-foreground-tracking.effects.spec.ts b/src/app/features/android/store/android-foreground-tracking.effects.spec.ts index b36fc945f9..514058f5e1 100644 --- a/src/app/features/android/store/android-foreground-tracking.effects.spec.ts +++ b/src/app/features/android/store/android-foreground-tracking.effects.spec.ts @@ -4,7 +4,11 @@ import { BehaviorSubject } from 'rxjs'; import { TaskService } from '../../tasks/task.service'; import { DateService } from '../../../core/date/date.service'; import { Task } from '../../tasks/task.model'; -import { parseNativeTrackingData } from './android-foreground-tracking.effects'; +import { + isTimeSpentJumpForNotification, + parseNativeTrackingData, + TIME_SPENT_JUMP_THRESHOLD_MS, +} from './android-foreground-tracking.effects'; // We need to test the effect logic by reimplementing it in tests since // the actual effects are conditionally created based on IS_ANDROID_WEB_VIEW @@ -47,7 +51,7 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => prevState.taskId === currState.taskId && currState.taskId !== null && !currState.isFocusModeActive && - prevState.timeSpent !== currState.timeSpent; + isTimeSpentJumpForNotification(prevState.timeSpent, currState.timeSpent); expect(shouldUpdate).toBeTrue(); @@ -67,7 +71,7 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => prevState.taskId === currState.taskId && currState.taskId !== null && !currState.isFocusModeActive && - prevState.timeSpent !== currState.timeSpent; + isTimeSpentJumpForNotification(prevState.timeSpent, currState.timeSpent); expect(shouldUpdate).toBeFalse(); })); @@ -80,7 +84,7 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => prevState.taskId === currState.taskId && currState.taskId !== null && !currState.isFocusModeActive && - prevState.timeSpent !== currState.timeSpent; + isTimeSpentJumpForNotification(prevState.timeSpent, currState.timeSpent); expect(shouldUpdate).toBeFalse(); })); @@ -93,7 +97,7 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => prevState.taskId === currState.taskId && currState.taskId !== null && !currState.isFocusModeActive && - prevState.timeSpent !== currState.timeSpent; + isTimeSpentJumpForNotification(prevState.timeSpent, currState.timeSpent); expect(shouldUpdate).toBeFalse(); })); @@ -106,7 +110,7 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => prevState.taskId === currState.taskId && currState.taskId !== null && !currState.isFocusModeActive && - prevState.timeSpent !== currState.timeSpent; + isTimeSpentJumpForNotification(prevState.timeSpent, currState.timeSpent); expect(shouldUpdate).toBeFalse(); })); @@ -119,7 +123,7 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => prevState.taskId === currState.taskId && currState.taskId !== null && !currState.isFocusModeActive && - prevState.timeSpent !== currState.timeSpent; + isTimeSpentJumpForNotification(prevState.timeSpent, currState.timeSpent); expect(shouldUpdate).toBeTrue(); @@ -131,6 +135,26 @@ describe('AndroidForegroundTrackingEffects - syncTimeSpentChanges logic', () => })); }); + describe('isTimeSpentJumpForNotification', () => { + it('should suppress increments at or below the threshold', () => { + expect(isTimeSpentJumpForNotification(0, TIME_SPENT_JUMP_THRESHOLD_MS)).toBeFalse(); + expect(isTimeSpentJumpForNotification(60000, 61000)).toBeFalse(); + expect(isTimeSpentJumpForNotification(60000, 60000)).toBeFalse(); + }); + + it('should pass increments above the threshold (manual edit, sync merge)', () => { + expect( + isTimeSpentJumpForNotification(0, TIME_SPENT_JUMP_THRESHOLD_MS + 1), + ).toBeTrue(); + expect(isTimeSpentJumpForNotification(60000, 600000)).toBeTrue(); + }); + + it('should pass any decrease (manual correction, idle time removed)', () => { + expect(isTimeSpentJumpForNotification(60000, 59999)).toBeTrue(); + expect(isTimeSpentJumpForNotification(60000, 0)).toBeTrue(); + }); + }); + describe('distinctUntilChanged behavior', () => { it('should detect changes when only timeSpent differs', () => { const stateA = { taskId: 'task-1', timeSpent: 60000, isFocusModeActive: false }; diff --git a/src/app/features/android/store/android-foreground-tracking.effects.ts b/src/app/features/android/store/android-foreground-tracking.effects.ts index 4b36512b61..bcfedfa037 100644 --- a/src/app/features/android/store/android-foreground-tracking.effects.ts +++ b/src/app/features/android/store/android-foreground-tracking.effects.ts @@ -89,6 +89,30 @@ export const parseNativeTrackingData = ( return { taskId, elapsedMs }; }; +/** + * Whether a timeSpent change must be pushed to the native tracking notification. + * While tracking, timeSpent grows by ~one tick (1s) at a time and the + * notification's chronometer advances on its own — pushing every tick would + * re-post the notification once per second (battery drain, #8243). Only jumps + * (manual edits, remote sync merges, idle/recovery corrections) need to + * propagate; those show up as a decrease or a step larger than any tick. + * Accepted gap: a manual edit that ADDS <= 5s is indistinguishable from a tick + * and stays suppressed — the notification (display-only; persistence happens + * elsewhere) re-anchors on the next jump or task switch. The threshold is 5x + * TRACKING_INTERVAL as headroom for janky/coalesced ticks; a sibling 5000 in + * android-focus-mode.effects.ts has different semantics (abs), don't unify. + * + * Exported so unit tests can exercise it without instantiating the effect + * (which is gated behind IS_ANDROID_WEB_VIEW). + */ +export const TIME_SPENT_JUMP_THRESHOLD_MS = 5000; +export const isTimeSpentJumpForNotification = ( + prevTimeSpent: number, + currTimeSpent: number, +): boolean => + currTimeSpent < prevTimeSpent || + currTimeSpent - prevTimeSpent > TIME_SPENT_JUMP_THRESHOLD_MS; + @Injectable() export class AndroidForegroundTrackingEffects { private _store = inject(Store); @@ -335,12 +359,13 @@ export class AndroidForegroundTrackingEffects { // 1. Same task (not switching tasks - that's handled by syncTrackingToService$) // 2. Task exists // 3. Focus mode is not active (notification is hidden during focus mode) - // 4. timeSpent actually changed + // 4. timeSpent jumped (per-tick increments are rendered by the + // notification chronometer natively — no push needed, #8243) return ( prev.taskId === curr.taskId && curr.taskId !== null && !curr.isFocusModeActive && - prev.timeSpent !== curr.timeSpent + isTimeSpentJumpForNotification(prev.timeSpent, curr.timeSpent) ); }), tap(([, curr]) => { diff --git a/src/app/features/calendar-integration/calendar-event-actions.service.spec.ts b/src/app/features/calendar-integration/calendar-event-actions.service.spec.ts new file mode 100644 index 0000000000..d9b58e3733 --- /dev/null +++ b/src/app/features/calendar-integration/calendar-event-actions.service.spec.ts @@ -0,0 +1,216 @@ +import { TestBed } from '@angular/core/testing'; +import { of } from 'rxjs'; +import { Store } from '@ngrx/store'; +import { MatDialog } from '@angular/material/dialog'; +import { CalendarEventActionsService } from './calendar-event-actions.service'; +import { IssueService } from '../issue/issue.service'; +import { PluginIssueProviderRegistryService } from '../../plugins/issue-provider/plugin-issue-provider-registry.service'; +import { IssueSyncAdapterResolverService } from '../issue/two-way-sync/issue-sync-adapter-resolver.service'; +import { CalendarIntegrationService } from './calendar-integration.service'; +import { HiddenCalendarEventsService } from './hidden-calendar-events.service'; +import { SnackService } from '../../core/snack/snack.service'; +import { IssueSyncAdapter } from '../issue/two-way-sync/issue-sync-adapter.interface'; +import { ScheduleFromCalendarEvent } from '../schedule/schedule.model'; +import { IssueProviderPluginType } from '../issue/issue.model'; +import { Log } from '../../core/log'; +import { T } from '../../t.const'; + +describe('CalendarEventActionsService', () => { + let service: CalendarEventActionsService; + let store: jasmine.SpyObj; + let matDialog: jasmine.SpyObj; + let pluginRegistry: jasmine.SpyObj; + let adapterResolver: jasmine.SpyObj; + let calendarIntegrationService: jasmine.SpyObj; + let snackService: jasmine.SpyObj; + let adapter: jasmine.SpyObj>; + + const providerCfg: IssueProviderPluginType = { + id: 'provider-1', + isEnabled: true, + issueProviderKey: 'plugin:google-calendar-provider', + pluginId: 'google-calendar-provider', + pluginConfig: { readCalendarIds: ['primary'] }, + }; + + const createCalendarEvent = ( + overrides: Partial = {}, + ): ScheduleFromCalendarEvent => ({ + id: 'cal-1:event-1', + calProviderId: 'provider-1', + issueProviderKey: 'plugin:google-calendar-provider', + title: 'Meeting', + start: new Date('2026-03-20T10:00:00').getTime(), + duration: 30 * 60 * 1000, + isAllDay: false, + icon: 'event', + ...overrides, + }); + + beforeEach(() => { + store = jasmine.createSpyObj('Store', ['select']); + store.select.and.returnValue(of(providerCfg)); + matDialog = jasmine.createSpyObj('MatDialog', ['open']); + pluginRegistry = jasmine.createSpyObj( + 'PluginIssueProviderRegistryService', + ['getProvider'], + ); + pluginRegistry.getProvider.and.returnValue({ + definition: { updateIssue: jasmine.createSpy('updateIssue') }, + } as any); + adapter = jasmine.createSpyObj>('IssueSyncAdapter', [ + 'getFieldMappings', + 'getSyncConfig', + 'fetchIssue', + 'pushChanges', + 'extractSyncValues', + ]); + adapter.pushChanges.and.resolveTo(); + adapterResolver = jasmine.createSpyObj( + 'IssueSyncAdapterResolverService', + ['getAdapter'], + ); + adapterResolver.getAdapter.and.returnValue(adapter); + calendarIntegrationService = jasmine.createSpyObj( + 'CalendarIntegrationService', + ['triggerRefresh'], + ); + snackService = jasmine.createSpyObj('SnackService', ['open']); + + TestBed.configureTestingModule({ + providers: [ + CalendarEventActionsService, + { provide: Store, useValue: store }, + { provide: IssueService, useValue: {} }, + { provide: MatDialog, useValue: matDialog }, + { provide: PluginIssueProviderRegistryService, useValue: pluginRegistry }, + { provide: IssueSyncAdapterResolverService, useValue: adapterResolver }, + { provide: CalendarIntegrationService, useValue: calendarIntegrationService }, + { provide: HiddenCalendarEventsService, useValue: {} }, + { provide: SnackService, useValue: snackService }, + ], + }); + + service = TestBed.inject(CalendarEventActionsService); + }); + + it('returns false without loading provider config when no adapter exists', async () => { + adapterResolver.getAdapter.and.returnValue(undefined); + + const wasMoved = await service.moveToStartTime( + createCalendarEvent(), + new Date('2026-03-22T14:30:00').getTime(), + ); + + expect(wasMoved).toBeFalse(); + expect(store.select).not.toHaveBeenCalled(); + expect(snackService.open).not.toHaveBeenCalled(); + }); + + it('logs sanitized errors and reports failed writes', async () => { + const error = { + name: 'HttpErrorResponse', + message: + 'Http failure response for https://www.googleapis.com/calendar/v3/calendars/noam@example.com/events/1: 403 Forbidden', + status: 403, + statusText: 'Forbidden', + url: 'https://www.googleapis.com/calendar/v3/calendars/noam@example.com/events/1', + }; + adapter.pushChanges.and.rejectWith(error); + const logSpy = spyOn(Log, 'err'); + + const wasMoved = await service.moveToStartTime( + createCalendarEvent(), + new Date('2026-03-22T14:30:00').getTime(), + ); + + expect(wasMoved).toBeFalse(); + expect(logSpy).toHaveBeenCalledWith('Failed to move calendar event', { + name: 'HttpErrorResponse', + status: 403, + statusText: 'Forbidden', + }); + expect(JSON.stringify(logSpy.calls.mostRecent().args)).not.toContain( + 'noam@example.com', + ); + expect(snackService.open).toHaveBeenCalledWith( + jasmine.objectContaining({ + type: 'ERROR', + translateParams: { errTxt: '403 Forbidden' }, + }), + ); + }); + + it('offers undo that restores the original timed event values', async () => { + await service.moveToStartTime( + createCalendarEvent(), + new Date('2026-03-22T14:30:00').getTime(), + ); + + const successSnack = snackService.open.calls.mostRecent().args[0] as { + actionStr?: string; + actionFn?: () => Promise; + }; + expect(successSnack.actionStr).toBe(T.G.UNDO); + + await successSnack.actionFn!(); + + expect(adapter.pushChanges.calls.mostRecent().args).toEqual([ + 'cal-1:event-1', + { + ['start_dateTime']: new Date('2026-03-20T10:00:00').toISOString(), + duration_ms: 30 * 60 * 1000, + }, + providerCfg, + ]); + }); + + it('uses the same write path for dialog reschedule', async () => { + matDialog.open.and.returnValue({ + afterClosed: () => of({ date: '2026-03-23', time: '09:15' }), + } as any); + + await service.reschedule(createCalendarEvent()); + + expect(adapter.pushChanges).toHaveBeenCalledWith( + 'cal-1:event-1', + jasmine.objectContaining({ + duration_ms: 30 * 60 * 1000, + }), + providerCfg, + ); + expect(calendarIntegrationService.triggerRefresh).toHaveBeenCalled(); + expect(snackService.open).toHaveBeenCalledWith( + jasmine.objectContaining({ actionStr: T.G.UNDO }), + ); + }); + + it('reschedules all-day events with a date-only change from dialog Date values', async () => { + matDialog.open.and.returnValue({ + afterClosed: () => of({ date: new Date(2026, 2, 23), time: null }), + } as any); + + await service.reschedule( + createCalendarEvent({ + isAllDay: true, + start: new Date(2026, 2, 20).getTime(), + duration: 24 * 60 * 60 * 1000, + }), + ); + + expect(adapter.pushChanges).toHaveBeenCalledWith( + 'cal-1:event-1', + { start_date: '2026-03-23' }, + providerCfg, + ); + }); + + it('does not open the reschedule dialog when the provider cannot update events', async () => { + pluginRegistry.getProvider.and.returnValue({ definition: {} } as any); + + await service.reschedule(createCalendarEvent()); + + expect(matDialog.open).not.toHaveBeenCalled(); + expect(adapter.pushChanges).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/features/calendar-integration/calendar-event-actions.service.ts b/src/app/features/calendar-integration/calendar-event-actions.service.ts index 62fedd74bb..0bdb09bacd 100644 --- a/src/app/features/calendar-integration/calendar-event-actions.service.ts +++ b/src/app/features/calendar-integration/calendar-event-actions.service.ts @@ -9,7 +9,7 @@ import { isPluginIssueProvider, } from '../issue/issue.model'; import { selectIssueProviderById } from '../issue/store/issue-provider.selectors'; -import { IssueSyncAdapterRegistryService } from '../issue/two-way-sync/issue-sync-adapter-registry.service'; +import { IssueSyncAdapterResolverService } from '../issue/two-way-sync/issue-sync-adapter-resolver.service'; import { PluginIssueProviderRegistryService } from '../../plugins/issue-provider/plugin-issue-provider-registry.service'; import { CalendarIntegrationService } from './calendar-integration.service'; import { HiddenCalendarEventsService } from './hidden-calendar-events.service'; @@ -21,7 +21,13 @@ import { IS_ELECTRON } from '../../app.constants'; import { DialogScheduleTaskComponent } from '../planner/dialog-schedule-task/dialog-schedule-task.component'; import { DialogConfirmComponent } from '../../ui/dialog-confirm/dialog-confirm.component'; import { getDateTimeFromClockString } from '../../util/get-date-time-from-clock-string'; -import { getErrorTxt } from '../../util/get-error-text'; +import { getDbDateStr } from '../../util/get-db-date-str'; + +interface CalendarEventPushOptions { + logMessage: string; + undoChanges?: Record; + successMsg?: string; +} @Injectable({ providedIn: 'root', @@ -31,7 +37,7 @@ export class CalendarEventActionsService { private _issueService = inject(IssueService); private _matDialog = inject(MatDialog); private _pluginRegistry = inject(PluginIssueProviderRegistryService); - private _syncAdapterRegistry = inject(IssueSyncAdapterRegistryService); + private _syncAdapterResolver = inject(IssueSyncAdapterResolverService); private _calendarIntegrationService = inject(CalendarIntegrationService); private _hiddenEventsService = inject(HiddenCalendarEventsService); private _snackService = inject(SnackService); @@ -40,6 +46,14 @@ export class CalendarEventActionsService { return isPluginIssueProvider(calEv.issueProviderKey as IssueProviderKey); } + canMoveEvent(calEv: ScheduleFromCalendarEvent): boolean { + if (calEv.isReferenceCalendar || !this.isPluginEvent(calEv)) { + return false; + } + const provider = this._pluginRegistry.getProvider(calEv.issueProviderKey); + return !!provider?.definition.updateIssue; + } + hasEventUrl(calEv: ScheduleFromCalendarEvent): boolean { return this.isPluginEvent(calEv) || !!calEv.url; } @@ -98,7 +112,7 @@ export class CalendarEventActionsService { if (!isConfirm) { return; } - const adapter = this._syncAdapterRegistry.get( + const adapter = this._syncAdapterResolver.getAdapter( calEv.issueProviderKey as IssueProviderKey, ); if (!adapter?.deleteIssue) { @@ -113,25 +127,24 @@ export class CalendarEventActionsService { msg: T.F.CALENDARS.S.EVENT_DELETED, }); } catch (e) { - Log.err('Failed to delete calendar event', e); + Log.err('Failed to delete calendar event', this._sanitizeLogError(e)); this._snackService.open({ type: 'ERROR', msg: T.F.CALENDARS.S.CAL_PROVIDER_ERROR, - translateParams: { errTxt: getErrorTxt(e) }, + translateParams: { errTxt: this._getSafeErrorTxt(e) }, }); } } async reschedule(calEv: ScheduleFromCalendarEvent): Promise { - if (!this.isPluginEvent(calEv)) { + if (!this.canMoveEvent(calEv)) { return; } const eventDate = new Date(calEv.start); - const pad = (n: number): string => String(n).padStart(2, '0'); - const targetDay = `${eventDate.getFullYear()}-${pad(eventDate.getMonth() + 1)}-${pad(eventDate.getDate())}`; + const targetDay = getDbDateStr(eventDate); const targetTime = calEv.isAllDay ? undefined - : `${pad(eventDate.getHours())}:${pad(eventDate.getMinutes())}`; + : `${String(eventDate.getHours()).padStart(2, '0')}:${String(eventDate.getMinutes()).padStart(2, '0')}`; const dialogRef = this._matDialog.open(DialogScheduleTaskComponent, { restoreFocus: true, @@ -158,38 +171,118 @@ export class CalendarEventActionsService { duration_ms: durationMs, }; } else { - // Parse as local midnight to avoid UTC date shift in western timezones - const d = new Date(result.date + 'T00:00:00'); changes = { - start_date: `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}`, + start_date: getDbDateStr(result.date), }; } /* eslint-enable @typescript-eslint/naming-convention */ - const adapter = this._syncAdapterRegistry.get( + await this._pushChanges(calEv, changes, { + logMessage: 'Failed to reschedule calendar event', + undoChanges: this._getCurrentScheduleChanges(calEv), + }); + } + + async moveToStartTime( + calEv: ScheduleFromCalendarEvent, + newStartMs: number, + ): Promise { + if (!this.canMoveEvent(calEv)) { + return false; + } + + /* eslint-disable @typescript-eslint/naming-convention */ + const changes = { + start_dateTime: new Date(newStartMs).toISOString(), + duration_ms: calEv.duration, + }; + /* eslint-enable @typescript-eslint/naming-convention */ + + return this._pushChanges(calEv, changes, { + logMessage: 'Failed to move calendar event', + undoChanges: this._getCurrentScheduleChanges(calEv), + successMsg: T.F.CALENDARS.S.EVENT_MOVED, + }); + } + + private async _pushChanges( + calEv: ScheduleFromCalendarEvent, + changes: Record, + options: CalendarEventPushOptions, + ): Promise { + const adapter = this._syncAdapterResolver.getAdapter( calEv.issueProviderKey as IssueProviderKey, ); if (!adapter) { - return; + return false; } + try { const cfg = await this._getPluginConfig(calEv); await adapter.pushChanges(calEv.id, changes, cfg); this._calendarIntegrationService.triggerRefresh(); this._snackService.open({ type: 'SUCCESS', - msg: T.F.CALENDARS.S.EVENT_RESCHEDULED, + msg: options.successMsg ?? T.F.CALENDARS.S.EVENT_RESCHEDULED, + ...(options.undoChanges + ? { + actionStr: T.G.UNDO, + actionFn: () => + this._pushChanges(calEv, options.undoChanges!, { + logMessage: 'Failed to undo calendar event reschedule', + }), + } + : {}), }); + return true; } catch (e) { - Log.err('Failed to reschedule calendar event', e); + Log.err(options.logMessage, this._sanitizeLogError(e)); this._snackService.open({ type: 'ERROR', msg: T.F.CALENDARS.S.CAL_PROVIDER_ERROR, - translateParams: { errTxt: getErrorTxt(e) }, + translateParams: { errTxt: this._getSafeErrorTxt(e) }, }); + return false; } } + private _getCurrentScheduleChanges( + calEv: ScheduleFromCalendarEvent, + ): Record { + /* eslint-disable @typescript-eslint/naming-convention */ + return calEv.isAllDay + ? { start_date: getDbDateStr(calEv.start) } + : { + start_dateTime: new Date(calEv.start).toISOString(), + duration_ms: calEv.duration, + }; + /* eslint-enable @typescript-eslint/naming-convention */ + } + + private _sanitizeLogError(e: unknown): Record { + const err = e as { + name?: unknown; + status?: unknown; + statusText?: unknown; + }; + return { + ...(typeof err?.name === 'string' ? { name: err.name } : {}), + ...(typeof err?.status === 'number' ? { status: err.status } : {}), + ...(typeof err?.statusText === 'string' ? { statusText: err.statusText } : {}), + }; + } + + private _getSafeErrorTxt(e: unknown): string { + const err = this._sanitizeLogError(e); + if (typeof err.status === 'number' && typeof err.statusText === 'string') { + return `${err.status} ${err.statusText}`; + } + if (typeof err.status === 'number') { + return String(err.status); + } + return typeof err.name === 'string' ? err.name : T.F.CALENDARS.S.CAL_PROVIDER_ERROR; + } + private async _getPluginConfig( calEv: ScheduleFromCalendarEvent, ): Promise { diff --git a/src/app/features/config/clipboard-images-cfg/clipboard-images-cfg.component.ts b/src/app/features/config/clipboard-images-cfg/clipboard-images-cfg.component.ts index 84bc4adffa..70e995e867 100644 --- a/src/app/features/config/clipboard-images-cfg/clipboard-images-cfg.component.ts +++ b/src/app/features/config/clipboard-images-cfg/clipboard-images-cfg.component.ts @@ -24,6 +24,7 @@ import { MatInput } from '@angular/material/input'; import { FormsModule } from '@angular/forms'; import { DialogClipboardImagesManagerComponent } from './dialog-clipboard-images-manager/dialog-clipboard-images-manager.component'; import { getDefaultClipboardImagesPath } from '../../../util/get-default-clipboard-images-path'; +import { Log } from '../../../core/log'; @Component({ selector: 'clipboard-images-cfg', @@ -80,7 +81,7 @@ export class ClipboardImagesCfgComponent implements OnInit { this.defaultImagePath = await getDefaultClipboardImagesPath(); this._cd.markForCheck(); } catch (error) { - console.error('Error loading default clipboard image path:', error); + Log.err('Error loading default clipboard image path:', error); } } diff --git a/src/app/features/config/clipboard-images-cfg/dialog-clipboard-images-manager/dialog-clipboard-images-manager.component.ts b/src/app/features/config/clipboard-images-cfg/dialog-clipboard-images-manager/dialog-clipboard-images-manager.component.ts index da2e1b109e..6c675774a2 100644 --- a/src/app/features/config/clipboard-images-cfg/dialog-clipboard-images-manager/dialog-clipboard-images-manager.component.ts +++ b/src/app/features/config/clipboard-images-cfg/dialog-clipboard-images-manager/dialog-clipboard-images-manager.component.ts @@ -22,6 +22,7 @@ import { MatTooltip } from '@angular/material/tooltip'; import { MatFormField, MatLabel } from '@angular/material/form-field'; import { MatSelect } from '@angular/material/select'; import { MatOption } from '@angular/material/core'; +import { Log } from '../../../../core/log'; type SortOrder = 'newest' | 'oldest' | 'largest' | 'smallest'; @@ -100,7 +101,7 @@ export class DialogClipboardImagesManagerComponent implements OnInit { } this.imageUrls.set(urlMap); } catch (error) { - console.error('Error loading clipboard images:', error); + Log.err('Error loading clipboard images:', error); this._snackService.open({ type: 'ERROR', msg: this._translateService.instant(T.GCF.CLIPBOARD_IMAGES.ERROR_LOADING), @@ -124,7 +125,7 @@ export class DialogClipboardImagesManagerComponent implements OnInit { throw new Error('Delete operation returned false'); } } catch (error) { - console.error('Error deleting clipboard image:', error); + Log.err('Error deleting clipboard image:', error); this._snackService.open({ type: 'ERROR', msg: this._translateService.instant(T.GCF.CLIPBOARD_IMAGES.ERROR_DELETING), @@ -159,7 +160,7 @@ export class DialogClipboardImagesManagerComponent implements OnInit { }); } } catch (error) { - console.error('Error deleting all clipboard images:', error); + Log.err('Error deleting all clipboard images:', error); await this.loadImages(); this._snackService.open({ type: 'ERROR', diff --git a/src/app/features/config/default-global-config.const.ts b/src/app/features/config/default-global-config.const.ts index f26e2356e2..7faf15a3e2 100644 --- a/src/app/features/config/default-global-config.const.ts +++ b/src/app/features/config/default-global-config.const.ts @@ -160,6 +160,10 @@ export const DEFAULT_GLOBAL_CONFIG: GlobalConfigState = { taskOpenNotesFullscreen: null, taskOpenEstimationDialog: 'T', taskSchedule: 'S', + taskScheduleToday: 'Shift+T', + taskScheduleTomorrow: null, + taskScheduleNextWeek: null, + taskScheduleNextMonth: null, taskScheduleDeadline: 'Shift+S', taskUnschedule: 'U', taskToggleDone: 'D', @@ -175,7 +179,6 @@ export const DEFAULT_GLOBAL_CONFIG: GlobalConfigState = { moveTaskToTop: 'Ctrl+Alt+ArrowUp', moveTaskToBottom: 'Ctrl+Alt+ArrowDown', moveToBacklog: 'Shift+B', - moveToTodaysTasks: 'Shift+T', expandSubTasks: null, collapseSubTasks: null, togglePlay: 'Y', diff --git a/src/app/features/config/form-cfgs/keyboard-form.const.ts b/src/app/features/config/form-cfgs/keyboard-form.const.ts index dcd9906e24..67d5dd3159 100644 --- a/src/app/features/config/form-cfgs/keyboard-form.const.ts +++ b/src/app/features/config/form-cfgs/keyboard-form.const.ts @@ -97,6 +97,10 @@ export const KEYBOARD_SETTINGS_FORM_CFG: ConfigFormSection = { kbField('taskOpenNotesPanel', T.GCF.KEYBOARD.TASK_OPEN_NOTES_PANEL), kbField('taskOpenEstimationDialog', T.GCF.KEYBOARD.TASK_OPEN_ESTIMATION_DIALOG), kbField('taskSchedule', T.GCF.KEYBOARD.TASK_SCHEDULE), + kbField('taskScheduleToday', T.GCF.KEYBOARD.TASK_SCHEDULE_TODAY), + kbField('taskScheduleTomorrow', T.GCF.KEYBOARD.TASK_SCHEDULE_TOMORROW), + kbField('taskScheduleNextWeek', T.GCF.KEYBOARD.TASK_SCHEDULE_NEXT_WEEK), + kbField('taskScheduleNextMonth', T.GCF.KEYBOARD.TASK_SCHEDULE_NEXT_MONTH), kbField('taskScheduleDeadline', T.GCF.KEYBOARD.TASK_SCHEDULE_DEADLINE), kbField('taskUnschedule', T.GCF.KEYBOARD.TASK_UNSCHEDULE), kbField('taskToggleDone', T.GCF.KEYBOARD.TASK_TOGGLE_DONE), @@ -113,7 +117,6 @@ export const KEYBOARD_SETTINGS_FORM_CFG: ConfigFormSection = { kbField('moveTaskToTop', T.GCF.KEYBOARD.MOVE_TASK_TO_TOP), kbField('moveTaskToBottom', T.GCF.KEYBOARD.MOVE_TASK_TO_BOTTOM), kbField('moveToBacklog', T.GCF.KEYBOARD.MOVE_TO_BACKLOG), - kbField('moveToTodaysTasks', T.GCF.KEYBOARD.MOVE_TO_REGULARS_TASKS), kbField('expandSubTasks', T.GCF.KEYBOARD.EXPAND_SUB_TASKS), kbField('collapseSubTasks', T.GCF.KEYBOARD.COLLAPSE_SUB_TASKS), kbField('togglePlay', T.GCF.KEYBOARD.TOGGLE_PLAY), diff --git a/src/app/features/config/icon-input/icon-input.component.ts b/src/app/features/config/icon-input/icon-input.component.ts index 44dafcf943..0e6832abfc 100644 --- a/src/app/features/config/icon-input/icon-input.component.ts +++ b/src/app/features/config/icon-input/icon-input.component.ts @@ -20,6 +20,7 @@ import { containsEmoji, extractFirstEmoji } from '../../../util/extract-first-em import { isSingleEmoji } from '../../../util/extract-first-emoji'; import { startWith } from 'rxjs/operators'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { Log } from '../../../core/log'; @Component({ selector: 'icon-input', @@ -80,7 +81,7 @@ export class IconInputComponent extends FieldType implements this.filteredIcons.set(icons.slice(0, 50)); } } catch (error) { - console.error('Failed to load material icons:', error); + Log.err('Failed to load material icons:', error); this.filteredIcons.set([]); } } @@ -123,7 +124,7 @@ export class IconInputComponent extends FieldType implements this.isEmoji.set(false); } } catch (error) { - console.error('Failed to filter icons:', error); + Log.err('Failed to filter icons:', error); this.filteredIcons.set([]); } } diff --git a/src/app/features/config/keyboard-config.model.ts b/src/app/features/config/keyboard-config.model.ts index 6a1d02c464..db5f3492a9 100644 --- a/src/app/features/config/keyboard-config.model.ts +++ b/src/app/features/config/keyboard-config.model.ts @@ -37,6 +37,10 @@ export type KeyboardConfig = Readonly<{ taskOpenContextMenu?: string | null; taskDelete?: string | null; taskSchedule?: string | null; + taskScheduleToday?: string | null; + taskScheduleTomorrow?: string | null; + taskScheduleNextWeek?: string | null; + taskScheduleNextMonth?: string | null; taskScheduleDeadline?: string | null; taskUnschedule?: string | null; selectPreviousTask?: string | null; @@ -46,7 +50,6 @@ export type KeyboardConfig = Readonly<{ moveTaskToTop?: string | null; moveTaskToBottom?: string | null; moveToBacklog?: string | null; - moveToTodaysTasks?: string | null; expandSubTasks?: string | null; collapseSubTasks?: string | null; togglePlay?: string | null; diff --git a/src/app/features/config/local-only-sync-settings.util.spec.ts b/src/app/features/config/local-only-sync-settings.util.spec.ts new file mode 100644 index 0000000000..55a2da0f92 --- /dev/null +++ b/src/app/features/config/local-only-sync-settings.util.spec.ts @@ -0,0 +1,96 @@ +import { + applyLocalOnlySyncSettingsToAppData, + LOCAL_ONLY_SYNC_DEVICE_KEYS, + LOCAL_ONLY_SYNC_KEYS, + LOCAL_ONLY_SYNC_SCHEDULE_KEYS, + stripLocalOnlySyncScheduleSettings, + stripLocalOnlySyncSettingsFromAppData, +} from './local-only-sync-settings.util'; +import { SyncProviderId } from '../../op-log/sync-providers/provider.const'; + +describe('local-only sync settings utils', () => { + it('schedule and device key sets are disjoint and exhaust LOCAL_ONLY_SYNC_KEYS', () => { + const union = new Set([ + ...LOCAL_ONLY_SYNC_SCHEDULE_KEYS, + ...LOCAL_ONLY_SYNC_DEVICE_KEYS, + ]); + expect(union.size).toBe( + LOCAL_ONLY_SYNC_SCHEDULE_KEYS.length + LOCAL_ONLY_SYNC_DEVICE_KEYS.length, + ); + expect(union.size).toBe(LOCAL_ONLY_SYNC_KEYS.length); + }); + + it('should strip sync schedule settings from a sync config object', () => { + const result = stripLocalOnlySyncScheduleSettings({ + syncInterval: 300000, + isManualSyncOnly: true, + isCompressionEnabled: true, + }) as Record; + + expect(result).toEqual({ + isCompressionEnabled: true, + }); + }); + + it('should strip local-only sync settings from app data', () => { + const result = stripLocalOnlySyncSettingsFromAppData({ + globalConfig: { + sync: { + syncProvider: SyncProviderId.WebDAV, + syncInterval: 300000, + isManualSyncOnly: true, + isCompressionEnabled: true, + }, + misc: { isDisableAnimations: true }, + }, + task: { ids: [] }, + }) as Record; + + const globalConfig = result['globalConfig'] as Record; + const sync = globalConfig['sync'] as Record; + + expect(sync['syncProvider']).toBeNull(); + expect(sync['syncInterval']).toBeUndefined(); + expect(sync['isManualSyncOnly']).toBeUndefined(); + expect(sync['isCompressionEnabled']).toBe(true); + expect(globalConfig['misc']).toEqual({ isDisableAnimations: true }); + expect(result['task']).toEqual({ ids: [] }); + }); + + it('should leave data without globalConfig.sync unchanged by reference', () => { + const data = { task: { ids: [] } }; + + expect(stripLocalOnlySyncSettingsFromAppData(data)).toBe(data); + }); + + it('should apply local-only sync settings to app data', () => { + const result = applyLocalOnlySyncSettingsToAppData( + { + globalConfig: { + sync: { + syncProvider: SyncProviderId.Dropbox, + syncInterval: 600000, + isManualSyncOnly: false, + isCompressionEnabled: true, + }, + }, + }, + { + isEnabled: true, + isEncryptionEnabled: false, + syncProvider: SyncProviderId.WebDAV, + syncInterval: 300000, + isManualSyncOnly: true, + }, + ) as Record; + + const globalConfig = result['globalConfig'] as Record; + const sync = globalConfig['sync'] as Record; + + expect(sync['isEnabled']).toBe(true); + expect(sync['syncProvider']).toBe(SyncProviderId.WebDAV); + expect(sync['syncInterval']).toBe(300000); + expect(sync['isManualSyncOnly']).toBe(true); + expect(sync['isCompressionEnabled']).toBe(true); + }); +}); diff --git a/src/app/features/config/local-only-sync-settings.util.ts b/src/app/features/config/local-only-sync-settings.util.ts new file mode 100644 index 0000000000..80ba5aac99 --- /dev/null +++ b/src/app/features/config/local-only-sync-settings.util.ts @@ -0,0 +1,93 @@ +import { SyncConfig } from './global-config.model'; + +/** + * Local-only sync settings — two tiers, both per-device: + * + * - SCHEDULE keys (`syncInterval`, `isManualSyncOnly`): must NEVER leave the + * device. Stripped (`Omit`-style) at every upload boundary. + * - DEVICE-IDENTITY keys (`syncProvider`, `isEnabled`, `isEncryptionEnabled`): + * exist on every device but the local value must win on hydration. On upload + * `syncProvider` is nulled (so a remote import never picks a provider for + * you); on hydration the local values are re-applied via + * {@link applyLocalOnlySyncSettingsToAppData}. + * + * The key arrays below are the single source of truth — both the + * `LocalOnlySyncSettings` type and the reducer-side preservation helper + * (`withLocalOnlySyncSettings` in `global-config.reducer.ts`) derive from them. + * Add a new local-only key here and the type + reducer + strip helpers pick it + * up automatically. + */ +export const LOCAL_ONLY_SYNC_SCHEDULE_KEYS = [ + 'syncInterval', + 'isManualSyncOnly', +] as const satisfies readonly (keyof SyncConfig)[]; + +export const LOCAL_ONLY_SYNC_DEVICE_KEYS = [ + 'syncProvider', + 'isEnabled', + 'isEncryptionEnabled', +] as const satisfies readonly (keyof SyncConfig)[]; + +export const LOCAL_ONLY_SYNC_KEYS = [ + ...LOCAL_ONLY_SYNC_SCHEDULE_KEYS, + ...LOCAL_ONLY_SYNC_DEVICE_KEYS, +] as const; + +export type LocalOnlySyncSettings = Pick< + SyncConfig, + (typeof LOCAL_ONLY_SYNC_KEYS)[number] +>; + +export const stripLocalOnlySyncScheduleSettings = >( + syncConfig: T, +): Omit => { + const stripped = { ...syncConfig }; + for (const key of LOCAL_ONLY_SYNC_SCHEDULE_KEYS) { + delete stripped[key]; + } + return stripped; +}; + +const _updateSyncConfigInAppData = ( + data: T, + updateSyncConfig: (syncConfig: Record) => Record, +): T => { + if (!data || typeof data !== 'object') { + return data; + } + + const typedData = data as Record; + if (!typedData['globalConfig'] || typeof typedData['globalConfig'] !== 'object') { + return data; + } + + const globalConfig = typedData['globalConfig'] as Record; + if (!globalConfig['sync'] || typeof globalConfig['sync'] !== 'object') { + return data; + } + + return { + ...typedData, + globalConfig: { + ...globalConfig, + sync: updateSyncConfig(globalConfig['sync'] as Record), + }, + } as T; +}; + +export const applyLocalOnlySyncSettingsToAppData = ( + data: T, + localOnlySettings: LocalOnlySyncSettings, +): T => { + return _updateSyncConfigInAppData(data, (syncConfig) => ({ + ...syncConfig, + ...localOnlySettings, + })); +}; + +export const stripLocalOnlySyncSettingsFromAppData = (data: unknown): unknown => { + return _updateSyncConfigInAppData(data, (syncConfig) => ({ + ...stripLocalOnlySyncScheduleSettings(syncConfig), + syncProvider: null, + })); +}; diff --git a/src/app/features/config/store/global-config.effects.ts b/src/app/features/config/store/global-config.effects.ts index 5a3d2c54fe..0a226ac0ff 100644 --- a/src/app/features/config/store/global-config.effects.ts +++ b/src/app/features/config/store/global-config.effects.ts @@ -29,6 +29,7 @@ import { AppStateActions } from '../../../root-store/app-state/app-state.actions import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions'; import { selectAllTasks } from '../../tasks/store/task.selectors'; import { normalizeStartOfNextDayConfig } from '../normalize-start-of-next-day-config'; +import { Log } from '../../../core/log'; @Injectable() export class GlobalConfigEffects { @@ -243,7 +244,7 @@ export class GlobalConfigEffects { setTimeout(() => window.location.reload(), 1000); }) .catch((err) => { - console.error('Failed to migrate user profiles:', err); + Log.err('Failed to migrate user profiles:', err); this._snackService.open({ type: 'ERROR', msg: 'Failed to enable user profiles. Please try again.', diff --git a/src/app/features/config/store/global-config.reducer.spec.ts b/src/app/features/config/store/global-config.reducer.spec.ts index edda76ef16..c7b736fb4d 100644 --- a/src/app/features/config/store/global-config.reducer.spec.ts +++ b/src/app/features/config/store/global-config.reducer.spec.ts @@ -17,11 +17,13 @@ import { selectIsFocusModeEnabled, selectTimelineWorkStartEndHours, } from './global-config.reducer'; +import { updateGlobalConfigSection } from './global-config.actions'; import { loadAllData } from '../../../root-store/meta/load-all-data.action'; import { GlobalConfigState } from '../global-config.model'; import { SyncProviderId } from '../../../op-log/sync-providers/provider.const'; import { AppDataComplete } from '../../../op-log/model/model-config'; import { DEFAULT_GLOBAL_CONFIG } from '../default-global-config.const'; +import { LOCAL_ONLY_SYNC_KEYS } from '../local-only-sync-settings.util'; import { INBOX_PROJECT } from '../../project/project.const'; describe('GlobalConfigReducer', () => { @@ -234,6 +236,98 @@ describe('GlobalConfigReducer', () => { expect(result.keyboard.addNewNote).toBe('N'); expect(result.keyboard.taskOpenNotesPanel).toBe('Alt+Shift+N'); }); + + describe('moveToTodaysTasks migration', () => { + it('should migrate moveToTodaysTasks to taskScheduleToday', () => { + const legacyConfig = { + ...initialGlobalConfigState, + keyboard: { + ...initialGlobalConfigState.keyboard, + moveToTodaysTasks: 'Shift+T', + taskScheduleToday: null, + }, + }; + + const result = globalConfigReducer( + initialGlobalConfigState, + loadAllData({ + appDataComplete: { + globalConfig: legacyConfig, + } as unknown as AppDataComplete, + }), + ); + + expect(result.keyboard.taskScheduleToday).toBe('Shift+T'); + expect((result.keyboard as any).moveToTodaysTasks).toBeUndefined(); + }); + + it('should NOT re-migrate if taskScheduleToday is already null (manually disabled)', () => { + const legacyConfigWithBoth = { + ...initialGlobalConfigState, + keyboard: { + ...initialGlobalConfigState.keyboard, + moveToTodaysTasks: 'Shift+T', + taskScheduleToday: null, + }, + }; + + // First migration + const result1 = globalConfigReducer( + initialGlobalConfigState, + loadAllData({ + appDataComplete: { + globalConfig: legacyConfigWithBoth, + } as unknown as AppDataComplete, + }), + ); + expect(result1.keyboard.taskScheduleToday).toBe('Shift+T'); + expect((result1.keyboard as any).moveToTodaysTasks).toBeUndefined(); + + // User disables it + const configWithDisabled = { + ...result1, + keyboard: { + ...result1.keyboard, + taskScheduleToday: null, + }, + }; + + // Second load (e.g. restart) + const result2 = globalConfigReducer( + initialGlobalConfigState, + loadAllData({ + appDataComplete: { + globalConfig: configWithDisabled, + } as unknown as AppDataComplete, + }), + ); + + expect(result2.keyboard.taskScheduleToday).toBeNull(); + }); + + it('should strip moveToTodaysTasks even if no migration is needed', () => { + const legacyConfig = { + ...initialGlobalConfigState, + keyboard: { + ...initialGlobalConfigState.keyboard, + moveToTodaysTasks: 'Shift+T', + taskScheduleToday: 'Ctrl+T', + }, + }; + + const result = globalConfigReducer( + initialGlobalConfigState, + loadAllData({ + appDataComplete: { + globalConfig: legacyConfig, + } as unknown as AppDataComplete, + }), + ); + + expect(result.keyboard.taskScheduleToday).toBe('Ctrl+T'); + expect((result.keyboard as any).moveToTodaysTasks).toBeUndefined(); + }); + }); }); it('should use syncProvider from snapshot when oldState has null (initial load)', () => { @@ -444,13 +538,14 @@ describe('GlobalConfigReducer', () => { expect(result.misc.startOfNextDayTime).toBe('00:00'); }); - it('should update other sync config properties while preserving syncProvider', () => { + it('should update shared sync config properties while preserving local-only ones', () => { const oldState: GlobalConfigState = { ...initialGlobalConfigState, sync: { ...initialGlobalConfigState.sync, syncProvider: SyncProviderId.SuperSync, syncInterval: 300000, + isManualSyncOnly: true, }, }; @@ -460,6 +555,7 @@ describe('GlobalConfigReducer', () => { ...initialGlobalConfigState.sync, syncProvider: null, syncInterval: 600000, + isManualSyncOnly: false, isCompressionEnabled: true, }, }; @@ -471,10 +567,11 @@ describe('GlobalConfigReducer', () => { }), ); - // syncProvider preserved + // Local-only settings preserved expect(result.sync.syncProvider).toBe(SyncProviderId.SuperSync); - // Other sync settings updated - expect(result.sync.syncInterval).toBe(600000); + expect(result.sync.syncInterval).toBe(300000); + expect(result.sync.isManualSyncOnly).toBe(true); + // Shared sync settings updated expect(result.sync.isCompressionEnabled).toBe(true); }); @@ -593,6 +690,62 @@ describe('GlobalConfigReducer', () => { }); }); + describe('local-only sync schedule settings preservation', () => { + it('should use sync schedule settings from snapshot on initial load', () => { + const snapshotConfig: GlobalConfigState = { + ...initialGlobalConfigState, + sync: { + ...initialGlobalConfigState.sync, + syncProvider: SyncProviderId.WebDAV, + syncInterval: 600000, + isManualSyncOnly: true, + }, + }; + + const result = globalConfigReducer( + initialGlobalConfigState, + loadAllData({ + appDataComplete: { globalConfig: snapshotConfig } as AppDataComplete, + }), + ); + + expect(result.sync.syncInterval).toBe(600000); + expect(result.sync.isManualSyncOnly).toBe(true); + }); + + it('should preserve local sync schedule settings during sync hydration', () => { + const oldState: GlobalConfigState = { + ...initialGlobalConfigState, + sync: { + ...initialGlobalConfigState.sync, + syncProvider: SyncProviderId.WebDAV, + syncInterval: 300000, + isManualSyncOnly: true, + }, + }; + + const syncedConfig: GlobalConfigState = { + ...initialGlobalConfigState, + sync: { + ...initialGlobalConfigState.sync, + syncProvider: null, + syncInterval: 600000, + isManualSyncOnly: false, + }, + }; + + const result = globalConfigReducer( + oldState, + loadAllData({ + appDataComplete: { globalConfig: syncedConfig } as AppDataComplete, + }), + ); + + expect(result.sync.syncInterval).toBe(300000); + expect(result.sync.isManualSyncOnly).toBe(true); + }); + }); + describe('focusMode migration: isSyncSessionWithTracking → autoStartFocusOnPlay', () => { // Real persisted JSON never carries `autoStartFocusOnPlay` (it didn't // exist pre-rework). Constructing the fixture as an Object.assign so the @@ -710,6 +863,185 @@ describe('GlobalConfigReducer', () => { }); }); + describe('updateGlobalConfigSection action', () => { + it('should update sync schedule settings for local actions', () => { + const result = globalConfigReducer( + initialGlobalConfigState, + updateGlobalConfigSection({ + sectionKey: 'sync', + sectionCfg: { + syncInterval: 600000, + isManualSyncOnly: true, + }, + }), + ); + + expect(result.sync.syncInterval).toBe(600000); + expect(result.sync.isManualSyncOnly).toBe(true); + }); + + it('should preserve local-only sync settings for remote sync section updates', () => { + const oldState: GlobalConfigState = { + ...initialGlobalConfigState, + sync: { + ...initialGlobalConfigState.sync, + isEnabled: true, + syncProvider: SyncProviderId.WebDAV, + isEncryptionEnabled: true, + syncInterval: 300000, + isManualSyncOnly: true, + isCompressionEnabled: false, + }, + }; + const remoteAction = updateGlobalConfigSection({ + sectionKey: 'sync', + sectionCfg: { + isEnabled: false, + syncProvider: SyncProviderId.LocalFile, + isEncryptionEnabled: false, + syncInterval: 600000, + isManualSyncOnly: false, + isCompressionEnabled: true, + }, + }); + const remoteReplayAction = { + ...remoteAction, + meta: { + ...remoteAction.meta, + isRemote: true, + isApplyingFromOtherClient: true, + }, + }; + + const result = globalConfigReducer(oldState, remoteReplayAction); + + expect(result.sync.isEnabled).toBe(true); + expect(result.sync.syncProvider).toBe(SyncProviderId.WebDAV); + expect(result.sync.isEncryptionEnabled).toBe(true); + expect(result.sync.syncInterval).toBe(300000); + expect(result.sync.isManualSyncOnly).toBe(true); + expect(result.sync.isCompressionEnabled).toBe(true); + }); + + // Round-trip pin (issue #8233): iterates LOCAL_ONLY_SYNC_KEYS so adding a + // new local-only key grows coverage here automatically. + it('preserves every LOCAL_ONLY_SYNC_KEYS value on remote section updates (round-trip)', () => { + const localSync = { + ...initialGlobalConfigState.sync, + isEnabled: true, + isEncryptionEnabled: true, + syncProvider: SyncProviderId.WebDAV, + syncInterval: 300000, + isManualSyncOnly: true, + }; + const remoteSync = { + isEnabled: false, + isEncryptionEnabled: false, + syncProvider: SyncProviderId.Dropbox, + syncInterval: 60000, + isManualSyncOnly: false, + }; + const oldState: GlobalConfigState = { + ...initialGlobalConfigState, + sync: localSync, + }; + const remoteAction = updateGlobalConfigSection({ + sectionKey: 'sync', + sectionCfg: remoteSync, + }); + const remoteReplayAction = { + ...remoteAction, + meta: { + ...remoteAction.meta, + isRemote: true, + isApplyingFromOtherClient: true, + }, + }; + + const result = globalConfigReducer(oldState, remoteReplayAction); + + for (const key of LOCAL_ONLY_SYNC_KEYS) { + expect(result.sync[key]) + .withContext(`sync.${key} must survive remote section update`) + .toBe(localSync[key]); + } + }); + + // Regression (scheduled e2e #8077): replaying the device's OWN sync-setup op + // during hydration is stamped isRemote (to prevent re-logging) but is NOT a + // foreign update. If the crash snapshot predates the setup op, local + // state.sync.syncProvider is still null at replay time. Keying the local-only + // preservation off isRemote (the #8077 bug) overwrote the op's real provider + // with null and silently disabled sync. The bulk meta-reducer sets + // isApplyingFromOtherClient ONLY for ops authored by a DIFFERENT client, so + // own-op replay (isRemote without that flag) must apply the op faithfully. + it('applies own-op replay faithfully when isRemote is set without isApplyingFromOtherClient', () => { + const oldState: GlobalConfigState = { + ...initialGlobalConfigState, + sync: { + ...initialGlobalConfigState.sync, + // Mid-hydration: snapshot predates the setup op → provider not set yet. + syncProvider: null, + isEnabled: false, + isEncryptionEnabled: false, + }, + }; + const ownSetupAction = updateGlobalConfigSection({ + sectionKey: 'sync', + sectionCfg: { + isEnabled: true, + syncProvider: SyncProviderId.WebDAV, + isEncryptionEnabled: true, + syncInterval: 300000, + isManualSyncOnly: true, + }, + }); + const ownReplayAction = { + ...ownSetupAction, + meta: { ...ownSetupAction.meta, isRemote: true }, + }; + + const result = globalConfigReducer(oldState, ownReplayAction); + + // The op's own values win — sync is NOT silently disabled. + expect(result.sync.syncProvider).toBe(SyncProviderId.WebDAV); + expect(result.sync.isEnabled).toBe(true); + expect(result.sync.isEncryptionEnabled).toBe(true); + expect(result.sync.syncInterval).toBe(300000); + expect(result.sync.isManualSyncOnly).toBe(true); + }); + + it('should update shared sync settings for remote sync section updates', () => { + const remoteAction = updateGlobalConfigSection({ + sectionKey: 'sync', + sectionCfg: { + isCompressionEnabled: true, + }, + }); + const remoteReplayAction = { + ...remoteAction, + meta: { + ...remoteAction.meta, + isRemote: true, + isApplyingFromOtherClient: true, + }, + }; + + const result = globalConfigReducer( + { + ...initialGlobalConfigState, + sync: { + ...initialGlobalConfigState.sync, + syncProvider: SyncProviderId.WebDAV, + }, + }, + remoteReplayAction, + ); + + expect(result.sync.isCompressionEnabled).toBe(true); + }); + }); + describe('default misc config (#7891)', () => { it('should NOT persist a default isUseCustomWindowTitleBar', () => { // Guard: a concrete default here would be pushed to Electron on every launch diff --git a/src/app/features/config/store/global-config.reducer.ts b/src/app/features/config/store/global-config.reducer.ts index c69ded339b..5270e8e23b 100644 --- a/src/app/features/config/store/global-config.reducer.ts +++ b/src/app/features/config/store/global-config.reducer.ts @@ -11,12 +11,14 @@ import { FocusModeConfig, GlobalConfigState, MiscConfig, + SyncConfig, } from '../global-config.model'; import type { KeyboardConfig } from '../keyboard-config.model'; import { DEFAULT_GLOBAL_CONFIG } from '../default-global-config.const'; import { loadAllData } from '../../../root-store/meta/load-all-data.action'; import { getHoursFromClockString } from '../../../util/get-hours-from-clock-string'; import { normalizeStartOfNextDayConfig } from '../normalize-start-of-next-day-config'; +import { LOCAL_ONLY_SYNC_KEYS } from '../local-only-sync-settings.util'; /** * Migrate the legacy `isSyncSessionWithTracking` flag (removed in the focus-mode @@ -106,14 +108,24 @@ export const initialGlobalConfigState: GlobalConfigState = { }; const migrateKeyboardConfig = (cfg: KeyboardConfig | undefined): KeyboardConfig => { - const keyboard: KeyboardConfig = { + const { moveToTodaysTasks, ...rest } = + (cfg as (KeyboardConfig & { moveToTodaysTasks?: string | null }) | undefined) ?? {}; + + let keyboard: KeyboardConfig = { ...DEFAULT_GLOBAL_CONFIG.keyboard, - ...cfg, + ...rest, }; + if (moveToTodaysTasks != null && rest.taskScheduleToday == null) { + keyboard = { + ...keyboard, + taskScheduleToday: moveToTodaysTasks, + }; + } + if ( - cfg?.addNewNote === 'N' && - (cfg.taskOpenNotesPanel === undefined || cfg.taskOpenNotesPanel === null) + rest.addNewNote === 'N' && + (rest.taskOpenNotesPanel === undefined || rest.taskOpenNotesPanel === null) ) { return { ...keyboard, @@ -125,6 +137,20 @@ const migrateKeyboardConfig = (cfg: KeyboardConfig | undefined): KeyboardConfig return keyboard; }; +// Overwrite every local-only key on the incoming config with the local value. +// Keys are sourced from LOCAL_ONLY_SYNC_KEYS so adding a new local-only key in +// local-only-sync-settings.util.ts automatically preserves it here too. +const withLocalOnlySyncSettings = ( + incomingSyncConfig: SyncConfig, + localSyncConfig: SyncConfig, +): SyncConfig => { + const merged = { ...incomingSyncConfig } as Record; + for (const key of LOCAL_ONLY_SYNC_KEYS) { + merged[key] = localSyncConfig[key]; + } + return merged as SyncConfig; +}; + export const globalConfigReducer = createReducer( initialGlobalConfigState, @@ -133,29 +159,26 @@ export const globalConfigReducer = createReducer( return oldState; } - const incomingSyncConfig = appDataComplete.globalConfig.sync; + const incomingSyncConfig = { + ...DEFAULT_GLOBAL_CONFIG.sync, + ...appDataComplete.globalConfig.sync, + }; // Preserve local-only sync settings if they're already set. // These settings should remain local to each client: // - syncProvider: Each client can use different providers (Dropbox, WebDAV, etc.) // - isEnabled: Each client independently controls whether sync is enabled // - isEncryptionEnabled: Encryption state must not be overwritten by imports + // - syncInterval: Each client chooses its own automatic sync frequency + // - isManualSyncOnly: Each client chooses automatic vs manual sync // // If oldState.sync.syncProvider is null, we're on first load (using initialGlobalConfigState) // and should use the incoming values (from snapshot). Otherwise, preserve local values. const hasLocalSettings = oldState.sync.syncProvider !== null; - const syncProvider = hasLocalSettings - ? oldState.sync.syncProvider - : incomingSyncConfig.syncProvider; - - const isEnabled = hasLocalSettings - ? oldState.sync.isEnabled - : incomingSyncConfig.isEnabled; - - const isEncryptionEnabled = hasLocalSettings - ? oldState.sync.isEncryptionEnabled - : incomingSyncConfig.isEncryptionEnabled; + const syncConfig = hasLocalSettings + ? withLocalOnlySyncSettings(incomingSyncConfig, oldState.sync) + : incomingSyncConfig; const incomingGlobalConfig = { ...DEFAULT_GLOBAL_CONFIG, @@ -200,27 +223,38 @@ export const globalConfigReducer = createReducer( ...migrateFocusModeConfig(appDataComplete.globalConfig.focusMode), }, keyboard: migrateKeyboardConfig(appDataComplete.globalConfig.keyboard), - sync: { - ...incomingSyncConfig, - syncProvider, - isEnabled, - isEncryptionEnabled, - }, + sync: syncConfig, }; }), - on(updateGlobalConfigSection, (state, { sectionKey, sectionCfg }) => { + on(updateGlobalConfigSection, (state, action) => { + const { sectionKey, sectionCfg } = action; const normalizedSectionCfg = sectionKey === 'misc' ? normalizeStartOfNextDayConfig(sectionCfg as Partial) : sectionCfg; + const updatedSection = { + ...state[sectionKey], + ...normalizedSectionCfg, + }; + + // Preserve this device's local-only sync settings ONLY when the update came + // from ANOTHER client. `isRemote` is also true while replaying the device's + // OWN ops during hydration — keying off it there would overwrite the op's + // real (own) sync settings with whatever local state happens to be mid-replay + // (e.g. a null syncProvider when the crash snapshot predates the setup op), + // silently disabling sync. See bulkOperationsMetaReducer. + const isFromOtherClient = + (action as { meta?: { isApplyingFromOtherClient?: boolean } }).meta + ?.isApplyingFromOtherClient === true; + const nextSection = + sectionKey === 'sync' && isFromOtherClient + ? withLocalOnlySyncSettings(updatedSection as SyncConfig, state.sync) + : updatedSection; return { ...state, - [sectionKey]: { - ...state[sectionKey], - ...normalizedSectionCfg, - }, + [sectionKey]: nextSection, }; }), ); diff --git a/src/app/features/issue-panel/issue-provider-setup-overview/issue-provider-setup-overview.component.ts b/src/app/features/issue-panel/issue-provider-setup-overview/issue-provider-setup-overview.component.ts index d9210065d5..0f04172c72 100644 --- a/src/app/features/issue-panel/issue-provider-setup-overview/issue-provider-setup-overview.component.ts +++ b/src/app/features/issue-panel/issue-provider-setup-overview/issue-provider-setup-overview.component.ts @@ -10,6 +10,7 @@ import { CalendarContextInfoTarget } from '../../issue/providers/calendar/calend import { selectEnabledIssueProviders } from '../../issue/store/issue-provider.selectors'; import { PluginIssueProviderRegistryService } from '../../../plugins/issue-provider/plugin-issue-provider-registry.service'; import { PluginService } from '../../../plugins/plugin.service'; +import { IssueLog } from '../../../core/log'; @Component({ selector: 'issue-provider-setup-overview', @@ -69,7 +70,7 @@ export class IssueProviderSetupOverviewComponent { this.pluginProviders = allProviders.filter((p) => !p.useAgendaView); this.pluginCalendarProviders = allProviders.filter((p) => p.useAgendaView); if (!isValidIssueProviderKey(issueProviderKey)) { - console.error(`Invalid issue provider key from plugin: "${issueProviderKey}"`); + IssueLog.err(`Invalid issue provider key from plugin: "${issueProviderKey}"`); return; } this.openSetupDialog(issueProviderKey); diff --git a/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts b/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts index 8010ed3b3d..06ca8b7bf4 100644 --- a/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts +++ b/src/app/features/issue/dialog-edit-issue-provider/dialog-edit-issue-provider.component.ts @@ -206,7 +206,7 @@ export class DialogEditIssueProviderComponent { constructor() { this._initOAuthAndOptions().catch((err) => { - console.error( + IssueLog.err( '[DialogEditIssueProvider] OAuth init failed', getSafeErrorLogMeta(err), ); @@ -453,7 +453,7 @@ export class DialogEditIssueProviderComponent { } } catch (e) { anyFailed = true; - console.error('[DialogEditIssueProvider] loadOptions failed', { + IssueLog.err('[DialogEditIssueProvider] loadOptions failed', { issueProviderKey: this.issueProviderKey, hasFieldKey: !!field.key, ...getSafeErrorLogMeta(e), diff --git a/src/app/features/issue/providers/redmine/redmine-api.service.spec.ts b/src/app/features/issue/providers/redmine/redmine-api.service.spec.ts index 54fe783bbf..af745bfe6d 100644 --- a/src/app/features/issue/providers/redmine/redmine-api.service.spec.ts +++ b/src/app/features/issue/providers/redmine/redmine-api.service.spec.ts @@ -1,17 +1,19 @@ -import { TestBed } from '@angular/core/testing'; import { HttpClientTestingModule, HttpTestingController, + TestRequest, } from '@angular/common/http/testing'; import { HttpRequest } from '@angular/common/http'; +import { TestBed } from '@angular/core/testing'; +import { SnackService } from '../../../../core/snack/snack.service'; +import { SearchResultItem } from '../../issue.model'; import { RedmineApiService } from './redmine-api.service'; import { RedmineCfg } from './redmine.model'; -import { SearchResultItem } from '../../issue.model'; -import { SnackService } from '../../../../core/snack/snack.service'; describe('RedmineApiService', () => { let service: RedmineApiService; let httpMock: HttpTestingController; + let snackService: jasmine.SpyObj; const mockCfg: RedmineCfg = { isEnabled: true, @@ -34,14 +36,15 @@ describe('RedmineApiService', () => { limit: 100, }; - // by-id lookup is scoped to the configured project (not the global /issues/{id}.json) + const searchUrl = (query: string): string => + `${mockCfg.host}/projects/${mockCfg.projectId}/search.json?limit=100&q=${query}&issues=1&open_issues=1`; + const byIdInProjectMatcher = (issueId: number) => (req: HttpRequest): boolean => req.method === 'GET' && req.url === `${mockCfg.host}/projects/${mockCfg.projectId}/issues.json` && req.params.get('issue_id') === String(issueId) && - // status_id=* so closed issues are found too req.params.get('status_id') === '*'; beforeEach(() => { @@ -57,6 +60,7 @@ describe('RedmineApiService', () => { }); service = TestBed.inject(RedmineApiService); httpMock = TestBed.inject(HttpTestingController); + snackService = TestBed.inject(SnackService) as jasmine.SpyObj; }); afterEach(() => { @@ -68,10 +72,7 @@ describe('RedmineApiService', () => { }); describe('searchIssuesInProject$', () => { - const searchUrl = (query: string): string => - `${mockCfg.host}/projects/${mockCfg.projectId}/search.json?limit=100&q=${query}&issues=1&open_issues=1`; - - it('should only send a text search request for non-numeric queries', () => { + it('should only send a text search request for non-numeric Latin queries', () => { let result: SearchResultItem[] | undefined; service.searchIssuesInProject$('some text', mockCfg).subscribe((r) => (result = r)); @@ -132,7 +133,6 @@ describe('RedmineApiService', () => { service.searchIssuesInProject$('99999', mockCfg).subscribe((r) => (result = r)); const byIdReq = httpMock.expectOne(byIdInProjectMatcher(99999)); - // Redmine returns an empty list (not a 404) for an id outside the project byIdReq.flush({ issues: [], total_count: 0, offset: 0, limit: 1 }); const searchReq = httpMock.expectOne(searchUrl('99999')); @@ -146,18 +146,90 @@ describe('RedmineApiService', () => { let result: SearchResultItem[] | undefined; service.searchIssuesInProject$('424242', mockCfg).subscribe((r) => (result = r)); - // The lookup is scoped to the configured project; an id belonging to another - // project the API key can see is therefore returned as an empty list by Redmine. const byIdReq = httpMock.expectOne(byIdInProjectMatcher(424242)); byIdReq.flush({ issues: [], total_count: 0, offset: 0, limit: 1 }); const searchReq = httpMock.expectOne(searchUrl('424242')); searchReq.flush({ ...mockSearchResponse, results: [] }); - // no global /issues/{id}.json request is ever made httpMock.expectNone(`${mockCfg.host}/issues/424242.json`); expect(result?.length).toBe(0); }); + + it('adds a subject filter fallback for non-Latin queries without full text results', () => { + service.searchIssuesInProject$('修正', mockCfg).subscribe((issues) => { + expect(issues.length).toBe(1); + expect(issues[0].title).toBe('#23 修正ログイン'); + expect(issues[0].issueData.id).toBe(23); + }); + + const searchReq = httpMock.expectOne(searchUrl('%E4%BF%AE%E6%AD%A3')); + searchReq.flush({ + results: [], + total_count: 0, + offset: 0, + limit: 100, + }); + + const fallbackReq = expectSubjectFallbackRequest(httpMock, mockCfg, '修正'); + expect(fallbackReq.request.method).toBe('GET'); + fallbackReq.flush({ + issues: [ + { + id: 23, + subject: '修正ログイン', + title: '修正ログイン', + updated_on: '2026-01-01T00:00:00Z', + url: 'https://redmine.example.com/issues/23', + }, + ], + total_count: 1, + offset: 0, + limit: 100, + }); + }); + + it('does not request the subject fallback when full text search already returns results', () => { + service.searchIssuesInProject$('修正', mockCfg).subscribe((issues) => { + expect(issues.map((issue) => issue.issueData.id)).toEqual([23]); + }); + + const searchReq = httpMock.expectOne(searchUrl('%E4%BF%AE%E6%AD%A3')); + searchReq.flush({ + results: [ + { + id: 23, + title: '#23 修正ログイン', + url: 'https://redmine.example.com/issues/23', + }, + ], + total_count: 1, + offset: 0, + limit: 100, + }); + + httpMock.expectNone((req: HttpRequest) => { + return req.url === `${mockCfg.host}/projects/${mockCfg.projectId}/issues.json`; + }); + }); + + it('keeps full text results when subject fallback fails', () => { + service.searchIssuesInProject$('修正', mockCfg).subscribe((issues) => { + expect(issues).toEqual([]); + }); + + const searchReq = httpMock.expectOne(searchUrl('%E4%BF%AE%E6%AD%A3')); + searchReq.flush({ + results: [], + total_count: 0, + offset: 0, + limit: 100, + }); + + const fallbackReq = expectSubjectFallbackRequest(httpMock, mockCfg, '修正'); + fallbackReq.flush('Forbidden', { status: 403, statusText: 'Forbidden' }); + expect(snackService.open).not.toHaveBeenCalled(); + }); }); describe('global mode (no projectId)', () => { @@ -249,5 +321,57 @@ describe('RedmineApiService', () => { expect(result?.length).toBe(1); }); + + it('should run the non-Latin subject fallback against instance-wide /issues.json', () => { + service.searchIssuesInProject$('修正', globalCfg).subscribe((issues) => { + expect(issues.length).toBe(1); + expect(issues[0].title).toBe('#23 修正ログイン'); + }); + + const searchReq = httpMock.expectOne(globalSearchUrl('%E4%BF%AE%E6%AD%A3')); + searchReq.flush({ + results: [], + total_count: 0, + offset: 0, + limit: 100, + }); + + const fallbackReq = expectSubjectFallbackRequest(httpMock, globalCfg, '修正'); + expect(fallbackReq.request.method).toBe('GET'); + fallbackReq.flush({ + issues: [ + { + id: 23, + subject: '修正ログイン', + title: '修正ログイン', + updated_on: '2026-01-01T00:00:00Z', + url: 'https://redmine.example.com/issues/23', + }, + ], + total_count: 1, + offset: 0, + limit: 100, + }); + }); }); }); + +const expectSubjectFallbackRequest = ( + httpMock: HttpTestingController, + cfg: RedmineCfg, + query: string, +): TestRequest => + httpMock.expectOne((req: HttpRequest) => { + const params = req.params; + + return ( + req.url === + `${cfg.host}${cfg.projectId ? `/projects/${cfg.projectId}` : ''}/issues.json` && + params.get('limit') === '100' && + params.get('status_id') === 'open' && + params.get('set_filter') === '1' && + params.get('f[]') === 'subject' && + params.get('op[subject]') === '~' && + params.get('v[subject][]') === query + ); + }); diff --git a/src/app/features/issue/providers/redmine/redmine-api.service.ts b/src/app/features/issue/providers/redmine/redmine-api.service.ts index ffa5145b1f..91a093fcd5 100644 --- a/src/app/features/issue/providers/redmine/redmine-api.service.ts +++ b/src/app/features/issue/providers/redmine/redmine-api.service.ts @@ -2,7 +2,7 @@ import { Injectable, inject } from '@angular/core'; import { SnackService } from '../../../../core/snack/snack.service'; import { HttpClient, HttpHeaders, HttpParams, HttpRequest } from '@angular/common/http'; import { RedmineCfg } from './redmine.model'; -import { catchError, filter, map } from 'rxjs/operators'; +import { catchError, filter, map, switchMap } from 'rxjs/operators'; import { forkJoin, Observable, of, throwError } from 'rxjs'; import { throwHandledError } from '../../../../util/throw-handled-error'; import { T } from '../../../../t.const'; @@ -12,6 +12,7 @@ import { RedmineActivityResult, RedmineIssue, RedmineIssueResult, + RedmineIssueStatusOptions, RedmineSearchResult, RedmineSearchResultItem, RedmineTimeEntriesResult, @@ -36,48 +37,32 @@ export class RedmineApiService { private _http = inject(HttpClient); searchIssuesInProject$(query: string, cfg: RedmineCfg): Observable { - const textSearch$: Observable = this._sendRequest$( - { - url: `${cfg.host}${this._projectScopePath(cfg)}/search.json`, - params: ParamsBuilder.create() - .withLimit(100) - .withQuery(query) - .onlyIssues(true) - .openIssues(true) - .build(), - }, - cfg, - ).pipe( - map((res: RedmineSearchResult) => { - return res - ? res.results.map((item: RedmineSearchResultItem) => - mapRedmineSearchResultItemToSearchResult(item), - ) - : []; - }), - ); - - // Redmine's search API only does full text search and does not match issue ids, - // so for numeric queries (e.g. "1234" or "#1234") we additionally try to fetch - // the issue by its id and merge it into the results. + const searchResults$ = this._searchTextIssuesInProject$(query, cfg); const idMatch = query.trim().match(ISSUE_ID_QUERY_RGX); - if (!idMatch) { - return textSearch$; + + if (!idMatch && !queryHasNonAscii(query)) { + return searchResults$; } - const issueId = Number(idMatch[1]); - return forkJoin([ - this._getIssueByIdInProject$(issueId, cfg).pipe(catchError(() => of(null))), - textSearch$, - ]).pipe( - map(([issueById, textResults]) => - issueById - ? [ - mapRedmineIssueToSearchResult(issueById), - ...textResults.filter((result) => result.issueData.id !== issueId), - ] - : textResults, - ), + const issueById$: Observable = idMatch + ? this._getIssueByIdInProject$(Number(idMatch[1]), cfg).pipe( + catchError(() => of(null)), + ) + : of(null); + + return forkJoin([issueById$, searchResults$]).pipe( + switchMap(([issueById, searchResults]) => { + const resultsWithId = issueById + ? mergeSearchResults([mapRedmineIssueToSearchResult(issueById)], searchResults) + : searchResults; + + return queryHasNonAscii(query) && resultsWithId.length === 0 + ? this._searchIssuesBySubjectInProject$(query, cfg).pipe( + map((subjectResults) => mergeSearchResults(resultsWithId, subjectResults)), + catchError(() => of(resultsWithId)), + ) + : of(resultsWithId); + }), ); } @@ -99,7 +84,7 @@ export class RedmineApiService { url: `${cfg.host}${this._projectScopePath(cfg)}/issues.json`, params: ParamsBuilder.create() .withParam('issue_id', String(issueId)) - .withState('*') + .withState(RedmineIssueStatusOptions.all) .withLimit(1) .build(), }, @@ -189,6 +174,60 @@ export class RedmineApiService { ); } + private _searchTextIssuesInProject$( + query: string, + cfg: RedmineCfg, + ): Observable { + return this._sendRequest$( + { + url: `${cfg.host}${this._projectScopePath(cfg)}/search.json`, + params: ParamsBuilder.create() + .withLimit(100) + .withQuery(query) + .onlyIssues(true) + .openIssues(true) + .build(), + }, + cfg, + ).pipe( + map((res: RedmineSearchResult) => { + return res + ? res.results.map((item: RedmineSearchResultItem) => + mapRedmineSearchResultItemToSearchResult(item), + ) + : []; + }), + ); + } + + private _searchIssuesBySubjectInProject$( + query: string, + cfg: RedmineCfg, + ): Observable { + const trimmedQuery = query.trim(); + + if (!trimmedQuery) { + return of([]); + } + + return this._sendRequest$( + { + url: `${cfg.host}${this._projectScopePath(cfg)}/issues.json`, + params: ParamsBuilder.create() + .withLimit(100) + .withState(RedmineIssueStatusOptions.open) + .withSubjectContains(trimmedQuery) + .build(), + }, + cfg, + { isSkipErrorHandling: true }, + ).pipe( + map((res: RedmineIssueResult) => + (res?.issues ?? []).map((issue) => mapRedmineIssueToSearchResult(issue)), + ), + ); + } + // Returns the project URL segment to prefix scoped endpoints with: `/projects/` when a // project is configured, or '' when it is not. An empty segment makes requests hit Redmine's // instance-wide endpoints (`/search.json`, `/issues.json`), so a single connection can search @@ -266,6 +305,37 @@ export class RedmineApiService { } } +const queryHasNonAscii = (query: string): boolean => /[^\u0000-\u007F]/.test(query); + +const getIssueId = (item: SearchResultItem): number | string | undefined => { + return item.issueData?.id; +}; + +const mergeSearchResults = ( + ...resultGroups: SearchResultItem[][] +): SearchResultItem[] => { + const seenIssueIds = new Set(); + const mergedResults: SearchResultItem[] = []; + + for (const resultGroup of resultGroups) { + for (const item of resultGroup) { + const issueId = getIssueId(item); + + if (issueId !== undefined && issueId !== null) { + if (seenIssueIds.has(issueId)) { + continue; + } + + seenIssueIds.add(issueId); + } + + mergedResults.push(item); + } + } + + return mergedResults; +}; + class ParamsBuilder { params: any = {}; @@ -303,6 +373,14 @@ class ParamsBuilder { return this; } + withSubjectContains(query: string): ParamsBuilder { + this.params['set_filter'] = '1'; + this.params['f[]'] = 'subject'; + this.params['op[subject]'] = '~'; + this.params['v[subject][]'] = query; + return this; + } + onlyIssues(isOnlyIssues: boolean): ParamsBuilder { this.params['issues'] = isOnlyIssues ? '1' : '0'; return this; diff --git a/src/app/features/issue/providers/trello/trello-api.service.ts b/src/app/features/issue/providers/trello/trello-api.service.ts index ffd8cc130d..954abe364c 100644 --- a/src/app/features/issue/providers/trello/trello-api.service.ts +++ b/src/app/features/issue/providers/trello/trello-api.service.ts @@ -18,6 +18,7 @@ import { ISSUE_PROVIDER_HUMANIZED, TRELLO_TYPE } from '../../issue.const'; import { T } from '../../../../t.const'; import { HANDLED_ERROR_PROP_STR } from '../../../../app.constants'; import { getErrorTxt } from '../../../../util/get-error-text'; +import { IssueLog } from '../../../../core/log'; const BASE_URL = 'https://api.trello.com/1'; const DEFAULT_CARD_FIELDS = [ @@ -195,7 +196,8 @@ export class TrelloApiService { }), catchError((error) => { const errorMsg = `Trello failed to resolve username "${username}": ${getErrorTxt(error)}`; - console.error(errorMsg); + // errorMsg embeds the username (user content) — keep it out of the exportable log + IssueLog.err('Trello failed to resolve username:', getErrorTxt(error)); this._snackService.open({ type: 'ERROR', msg: errorMsg, diff --git a/src/app/features/issue/two-way-sync/issue-sync-adapter-resolver.service.spec.ts b/src/app/features/issue/two-way-sync/issue-sync-adapter-resolver.service.spec.ts new file mode 100644 index 0000000000..286f11340c --- /dev/null +++ b/src/app/features/issue/two-way-sync/issue-sync-adapter-resolver.service.spec.ts @@ -0,0 +1,72 @@ +import { TestBed } from '@angular/core/testing'; +import { IssueSyncAdapterResolverService } from './issue-sync-adapter-resolver.service'; +import { IssueSyncAdapterRegistryService } from './issue-sync-adapter-registry.service'; +import { IssueSyncAdapter } from './issue-sync-adapter.interface'; +import { PluginIssueProviderRegistryService } from '../../../plugins/issue-provider/plugin-issue-provider-registry.service'; +import { PluginHttpService } from '../../../plugins/issue-provider/plugin-http.service'; +import { TagService } from '../../tag/tag.service'; + +describe('IssueSyncAdapterResolverService', () => { + let service: IssueSyncAdapterResolverService; + let adapterRegistry: IssueSyncAdapterRegistryService; + let pluginRegistry: jasmine.SpyObj; + + const existingAdapter = {} as IssueSyncAdapter; + + beforeEach(() => { + pluginRegistry = jasmine.createSpyObj( + 'PluginIssueProviderRegistryService', + ['getProvider'], + ); + + TestBed.configureTestingModule({ + providers: [ + IssueSyncAdapterResolverService, + IssueSyncAdapterRegistryService, + { provide: PluginIssueProviderRegistryService, useValue: pluginRegistry }, + { + provide: PluginHttpService, + useValue: jasmine.createSpyObj('PluginHttpService', [ + 'createHttpHelper', + ]), + }, + { provide: TagService, useValue: { tags: () => [] } }, + ], + }); + + service = TestBed.inject(IssueSyncAdapterResolverService); + adapterRegistry = TestBed.inject(IssueSyncAdapterRegistryService); + }); + + it('returns an existing registered adapter without consulting plugin providers', () => { + adapterRegistry.register('plugin:test', existingAdapter); + + expect(service.getAdapter('plugin:test')).toBe(existingAdapter); + expect(pluginRegistry.getProvider).not.toHaveBeenCalled(); + }); + + it('lazily creates and registers plugin adapters for writable providers', () => { + pluginRegistry.getProvider.and.returnValue({ + allowPrivateNetwork: true, + definition: { + getHeaders: () => ({}), + updateIssue: jasmine.createSpy('updateIssue'), + }, + } as any); + + const adapter = service.getAdapter('plugin:test'); + + expect(adapter).toBeTruthy(); + expect(adapterRegistry.get('plugin:test')).toBe(adapter); + expect(adapter?.pushChanges).toEqual(jasmine.any(Function)); + }); + + it('does not create adapters for plugins without issue side effects', () => { + pluginRegistry.getProvider.and.returnValue({ + definition: { getHeaders: () => ({}) }, + } as any); + + expect(service.getAdapter('plugin:test')).toBeUndefined(); + expect(adapterRegistry.get('plugin:test')).toBeUndefined(); + }); +}); diff --git a/src/app/features/issue/two-way-sync/issue-sync-adapter-resolver.service.ts b/src/app/features/issue/two-way-sync/issue-sync-adapter-resolver.service.ts new file mode 100644 index 0000000000..dda54af370 --- /dev/null +++ b/src/app/features/issue/two-way-sync/issue-sync-adapter-resolver.service.ts @@ -0,0 +1,48 @@ +import { Injectable, inject } from '@angular/core'; +import { PluginIssueProviderRegistryService } from '../../../plugins/issue-provider/plugin-issue-provider-registry.service'; +import { PluginHttpService } from '../../../plugins/issue-provider/plugin-http.service'; +import { TagService } from '../../tag/tag.service'; +import { createPluginSyncAdapter } from '../../../plugins/issue-provider/plugin-sync-adapter.service'; +import { IssueSyncAdapter } from './issue-sync-adapter.interface'; +import { IssueSyncAdapterRegistryService } from './issue-sync-adapter-registry.service'; + +@Injectable({ + providedIn: 'root', +}) +export class IssueSyncAdapterResolverService { + private readonly _adapterRegistry = inject(IssueSyncAdapterRegistryService); + private readonly _pluginRegistry = inject(PluginIssueProviderRegistryService); + private readonly _pluginHttp = inject(PluginHttpService); + private readonly _tagService = inject(TagService); + + getAdapter(issueType: string): IssueSyncAdapter | undefined { + const existing = this._adapterRegistry.get(issueType); + if (existing) { + return existing; + } + + const provider = this._pluginRegistry.getProvider(issueType); + const definition = provider?.definition; + if ( + !provider || + !definition || + // `updateIssue` alone is intentional: calendar moves can push direct + // event changes even when a provider has no task field mappings. + (!definition.createIssue && !definition.deleteIssue && !definition.updateIssue) + ) { + return undefined; + } + + const adapter = createPluginSyncAdapter( + definition, + (getHeadersFn) => + this._pluginHttp.createHttpHelper(getHeadersFn, { + allowPrivateNetwork: provider.allowPrivateNetwork, + }), + this._tagService, + ); + + this._adapterRegistry.register(issueType, adapter); + return adapter; + } +} diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts index 6e4a4dbd01..a434829cd1 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.spec.ts @@ -6,6 +6,7 @@ import { IssueTwoWaySyncEffects } from './issue-two-way-sync.effects'; import { TaskService } from '../../tasks/task.service'; import { IssueProviderService } from '../issue-provider.service'; import { IssueSyncAdapterRegistryService } from './issue-sync-adapter-registry.service'; +import { IssueSyncAdapterResolverService } from './issue-sync-adapter-resolver.service'; import { CaldavSyncAdapterService } from '../providers/caldav/caldav-sync-adapter.service'; import { SnackService } from '../../../core/snack/snack.service'; import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions'; @@ -136,6 +137,13 @@ describe('IssueTwoWaySyncEffects', () => { { provide: IssueProviderService, useValue: issueProviderServiceSpy }, { provide: CaldavSyncAdapterService, useValue: caldavSpy }, { provide: SnackService, useValue: snackServiceSpy }, + { + provide: IssueSyncAdapterResolverService, + useFactory: (registry: IssueSyncAdapterRegistryService) => ({ + getAdapter: (issueType: string) => registry.get(issueType), + }), + deps: [IssueSyncAdapterRegistryService], + }, ], }); diff --git a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts index fd2a08a732..59d41da688 100644 --- a/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts +++ b/src/app/features/issue/two-way-sync/issue-two-way-sync.effects.ts @@ -26,12 +26,10 @@ import { DeletedTagTitlesSidecarService } from './deleted-tag-titles-sidecar.ser import { selectEnabledIssueProviders } from '../store/issue-provider.selectors'; import { getErrorTxt } from '../../../util/get-error-text'; import { T } from '../../../t.const'; -import { PluginIssueProviderRegistryService } from '../../../plugins/issue-provider/plugin-issue-provider-registry.service'; -import { PluginHttpService } from '../../../plugins/issue-provider/plugin-http.service'; -import { TagService } from '../../tag/tag.service'; -import { createPluginSyncAdapter } from '../../../plugins/issue-provider/plugin-sync-adapter.service'; import { PlannerActions } from '../../planner/store/planner.actions'; import { deleteTag, deleteTags } from '../../tag/store/tag.actions'; +import { IssueSyncAdapterResolverService } from './issue-sync-adapter-resolver.service'; +import { PluginIssueProviderRegistryService } from '../../../plugins/issue-provider/plugin-issue-provider-registry.service'; const SYNCABLE_TASK_FIELDS: ReadonlySet = new Set([ 'isDone', @@ -124,9 +122,8 @@ export class IssueTwoWaySyncEffects { private readonly _snackService = inject(SnackService); private readonly _deletedTaskIssueSidecar = inject(DeletedTaskIssueSidecarService); private readonly _deletedTagTitlesSidecar = inject(DeletedTagTitlesSidecarService); + private readonly _adapterResolver = inject(IssueSyncAdapterResolverService); private readonly _pluginRegistry = inject(PluginIssueProviderRegistryService); - private readonly _pluginHttp = inject(PluginHttpService); - private readonly _tagService = inject(TagService); private _syncOriginatedTaskIds = new Set(); private static readonly _MAX_SYNC_ORIGINATED_IDS = 1000; @@ -464,32 +461,7 @@ export class IssueTwoWaySyncEffects { } private _getAdapter(issueType: string): IssueSyncAdapter | undefined { - const existing = this._adapterRegistry.get(issueType); - if (existing) return existing; - - const provider = this._pluginRegistry.getProvider(issueType); - const definition = provider?.definition; - if ( - !provider || - !definition || - (!definition.createIssue && - !definition.deleteIssue && - !(definition.fieldMappings?.length && definition.updateIssue)) - ) { - return undefined; - } - - const adapter = createPluginSyncAdapter( - definition, - (getHeadersFn) => - this._pluginHttp.createHttpHelper(getHeadersFn, { - allowPrivateNetwork: provider.allowPrivateNetwork, - }), - this._tagService, - ); - - this._adapterRegistry.register(issueType, adapter); - return adapter; + return this._adapterResolver.getAdapter(issueType); } private _deleteRemoteIssue$(info: DeletedTaskIssueInfo | Task): Observable { diff --git a/src/app/features/metric/activity-heatmap/activity-heatmap.component.ts b/src/app/features/metric/activity-heatmap/activity-heatmap.component.ts index d423ae7231..4caa1a8a0e 100644 --- a/src/app/features/metric/activity-heatmap/activity-heatmap.component.ts +++ b/src/app/features/metric/activity-heatmap/activity-heatmap.component.ts @@ -27,6 +27,7 @@ import { } from '../../../ui/heatmap/heatmap.component'; import { DateAdapter } from '@angular/material/core'; import { Worklog } from '../../worklog/worklog.model'; +import { Log } from '../../../core/log'; interface YearlyActivityData { dayMap: Map; @@ -368,7 +369,7 @@ export class ActivityHeatmapComponent { }); } } else if (result.error && result.error !== 'Share cancelled') { - console.error('Share failed:', result.error); + Log.err('Share failed:', result.error); this._snackService.open({ type: 'ERROR', msg: 'Failed to share heatmap', @@ -377,7 +378,7 @@ export class ActivityHeatmapComponent { } catch (error: any) { const isAbort = error?.name === 'AbortError' || error?.error === 'Share cancelled'; if (!isAbort) { - console.error('Share failed:', error); + Log.err('Share failed:', error); this._snackService.open({ type: 'ERROR', msg: 'Failed to share heatmap', diff --git a/src/app/features/metric/lazy-chart/lazy-chart.component.ts b/src/app/features/metric/lazy-chart/lazy-chart.component.ts index 7f900d552f..82c97a8507 100644 --- a/src/app/features/metric/lazy-chart/lazy-chart.component.ts +++ b/src/app/features/metric/lazy-chart/lazy-chart.component.ts @@ -31,6 +31,7 @@ import { ChartLazyLoaderService } from '../chart-lazy-loader.service'; import { ShareService } from '../../../core/share/share.service'; import { SnackService } from '../../../core/snack/snack.service'; import { T } from '../../../t.const'; +import { Log } from '../../../core/log'; interface ChartClickEvent { active: ActiveElement[]; @@ -226,10 +227,10 @@ export class LazyChartComponent implements OnDestroy { } if (!result.success && result.error && result.error !== 'Share cancelled') { - console.error('Share failed:', result.error); + Log.err('Share failed:', result.error); } } catch (error) { - console.error('Share failed:', error); + Log.err('Share failed:', error); } finally { this.isSharing.set(false); } @@ -247,14 +248,14 @@ export class LazyChartComponent implements OnDestroy { await this.chartLoaderService.loadChartModule(); this.isModuleLoaded.set(true); } catch (error) { - console.error('Failed to load chart module:', error); + Log.err('Failed to load chart module:', error); } } private initChart(canvas: ElementRef): void { const chartModule = this.chartLoaderService.getChartModule(); if (!chartModule) { - console.error('Chart module not loaded'); + Log.err('Chart module not loaded'); return; } @@ -269,7 +270,7 @@ export class LazyChartComponent implements OnDestroy { this.chartInstance?.destroy(); this.chartInstance = new Chart(canvas.nativeElement, chartConfig); } catch (error) { - console.error('Failed to initialize chart:', error); + Log.err('Failed to initialize chart:', error); } } diff --git a/src/app/features/planner/store/planner.bug-8220.spec.ts b/src/app/features/planner/store/planner.bug-8220.spec.ts new file mode 100644 index 0000000000..ee41c8f09c --- /dev/null +++ b/src/app/features/planner/store/planner.bug-8220.spec.ts @@ -0,0 +1,177 @@ +import * as fromSelectors from './planner.selectors'; +import { PlannerState } from './planner.reducer'; +import { Task } from '../../tasks/task.model'; +import { getDbDateStr } from '../../../util/get-db-date-str'; +import { + DEFAULT_TASK_REPEAT_CFG, + TaskRepeatCfg, +} from '../../task-repeat-cfg/task-repeat-cfg.model'; + +// Regression for #8220: a recurring task that already has an instance in a day +// (created, and possibly marked done) was counted twice in the Planner: once as +// the real task instance and again as a repeat projection. The "Today" view +// (work-context.util) only ever counted the real instance, so the two views +// disagreed on remaining time. The Planner must defer to the real instance and +// drop the now-redundant projection. +describe('Planner Selectors - #8220 recurring done task double-count', () => { + const today = getDbDateStr(); + const ONE_HOUR = 60 * 60 * 1000; + + const createMockTask = (overrides: Partial & { id: string }): Task => { + const { id, ...rest } = overrides; + return { + id, + title: `Task ${id}`, + created: Date.now(), + isDone: false, + subTaskIds: [], + tagIds: [], + projectId: 'project1', + timeSpentOnDay: {}, + timeEstimate: 0, + timeSpent: 0, + attachments: [], + ...rest, + }; + }; + + const createTasksMapFromTasksArray = (tasks: Task[]): Map => + new Map(tasks.map((t) => [t.id, t])); + + const emptyPlannerState: PlannerState = { + days: {}, + addPlannedTasksDialogLastShown: undefined, + }; + + const defaultScheduleConfig = { + isWorkStartEndEnabled: false, + workStart: '09:00', + workEnd: '17:00', + isLunchBreakEnabled: false, + lunchBreakStart: '12:00', + lunchBreakEnd: '13:00', + }; + + // A daily cfg that still projects for today (startDate in the past, + // lastTaskCreationDay not yet advanced to today) and has no startTime, so it + // contributes its defaultEstimate via noStartTimeRepeatProjections. + const dailyRepeatCfg = (id: string, defaultEstimate: number): TaskRepeatCfg => ({ + ...DEFAULT_TASK_REPEAT_CFG, + id, + repeatCycle: 'DAILY', + startDate: '2020-01-01', + lastTaskCreationDay: '2020-01-01', + startTime: undefined, + defaultEstimate, + }); + + it('counts a done recurring instance once (0 remaining), not also as a projection', () => { + const cfg = dailyRepeatCfg('R1', ONE_HOUR); + const task = createMockTask({ + id: 't1', + repeatCfgId: 'R1', + isDone: true, + timeEstimate: ONE_HOUR, + timeSpent: 0, + }); + + // Pass t1 as a today-list task id (the reporter sees this in the Today column). + const selector = fromSelectors.selectPlannerDays( + [today], + [cfg], + ['t1'], + [], + [], + today, + ); + const result = selector.projector( + createTasksMapFromTasksArray([task]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(0); + expect(result[0].itemsTotal).toBe(1); + }); + + it('counts an undone recurring instance once, not doubled', () => { + const cfg = dailyRepeatCfg('R1', ONE_HOUR); + const task = createMockTask({ + id: 't1', + repeatCfgId: 'R1', + isDone: false, + timeEstimate: ONE_HOUR, + timeSpent: 0, + }); + + const selector = fromSelectors.selectPlannerDays( + [today], + [cfg], + ['t1'], + [], + [], + today, + ); + const result = selector.projector( + createTasksMapFromTasksArray([task]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(ONE_HOUR); + expect(result[0].itemsTotal).toBe(1); + }); + + it('dedupes a timed (startTime) projection too, keeping it out of scheduledIItems', () => { + // A cfg WITH a startTime flows through repeatProjectionsForDay (the timed + // list) rather than noStartTimeRepeatProjections, so this exercises the other + // filtered array and the scheduledIItems dedup. + const cfg: TaskRepeatCfg = { + ...dailyRepeatCfg('R1', ONE_HOUR), + startTime: '09:00', + }; + const task = createMockTask({ + id: 't1', + repeatCfgId: 'R1', + isDone: true, + timeEstimate: ONE_HOUR, + timeSpent: 0, + }); + + const selector = fromSelectors.selectPlannerDays( + [today], + [cfg], + ['t1'], + [], + [], + today, + ); + const result = selector.projector( + createTasksMapFromTasksArray([task]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(0); + expect(result[0].itemsTotal).toBe(1); + expect(result[0].scheduledIItems.length).toBe(0); + }); + + it('still projects a recurring cfg that has no task instance in the day', () => { + const cfg = dailyRepeatCfg('R2', ONE_HOUR); + + const selector = fromSelectors.selectPlannerDays([today], [cfg], [], [], [], today); + const result = selector.projector( + createTasksMapFromTasksArray([]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(ONE_HOUR); + expect(result[0].itemsTotal).toBe(1); + }); +}); diff --git a/src/app/features/planner/store/planner.bug-8232.spec.ts b/src/app/features/planner/store/planner.bug-8232.spec.ts new file mode 100644 index 0000000000..40614110dd --- /dev/null +++ b/src/app/features/planner/store/planner.bug-8232.spec.ts @@ -0,0 +1,191 @@ +import * as fromSelectors from './planner.selectors'; +import { PlannerState } from './planner.reducer'; +import { Task, TaskWithDueTime } from '../../tasks/task.model'; +import { getDbDateStr } from '../../../util/get-db-date-str'; +import { dateStrToUtcDate } from '../../../util/date-str-to-utc-date'; +import { getDateTimeFromClockString } from '../../../util/get-date-time-from-clock-string'; +import { + DEFAULT_TASK_REPEAT_CFG, + TaskRepeatCfg, +} from '../../task-repeat-cfg/task-repeat-cfg.model'; + +// Regression for #8232: the dedup added in #8220/#8229 keyed only off +// `normalTasks`, but a recurring task with `startTime` is created with +// `dueWithTime` and therefore lives in `allPlannedTasks` -> `scheduledTaskItems`, +// never in `normalTasks` on the Today column. The projection was not dropped, +// and `getScheduledTaskItems` ignored `isDone`, so a done timed recurring +// instance contributed roughly 2x its estimate (full from scheduledTaskItems + +// full from the projection). Today only ever counts the real (done-aware) task, +// so the two views disagreed again — the original #8220 symptom for the timed +// subset. +describe('Planner Selectors - #8232 timed recurring done task double-count', () => { + const today = getDbDateStr(); + const ONE_HOUR = 60 * 60 * 1000; + + const createMockTask = (overrides: Partial & { id: string }): Task => { + const { id, ...rest } = overrides; + return { + id, + title: `Task ${id}`, + created: Date.now(), + isDone: false, + subTaskIds: [], + tagIds: [], + projectId: 'project1', + timeSpentOnDay: {}, + timeEstimate: 0, + timeSpent: 0, + attachments: [], + ...rest, + }; + }; + + const createTasksMapFromTasksArray = (tasks: Task[]): Map => + new Map(tasks.map((t) => [t.id, t])); + + const emptyPlannerState: PlannerState = { + days: {}, + addPlannedTasksDialogLastShown: undefined, + }; + + const defaultScheduleConfig = { + isWorkStartEndEnabled: false, + workStart: '09:00', + workEnd: '17:00', + isLunchBreakEnabled: false, + lunchBreakStart: '12:00', + lunchBreakEnd: '13:00', + }; + + // A daily cfg with a fixed startTime — projects via `repeatProjectionsForDay` + // (the timed list) and, when an instance exists, that instance has + // `dueWithTime` set so it flows through `allPlannedTasks` / + // `scheduledTaskItems`. + const dailyTimedRepeatCfg = ( + id: string, + defaultEstimate: number, + startTime = '09:00', + ): TaskRepeatCfg => ({ + ...DEFAULT_TASK_REPEAT_CFG, + id, + repeatCycle: 'DAILY', + startDate: '2020-01-01', + lastTaskCreationDay: '2020-01-01', + startTime, + defaultEstimate, + }); + + const todayAt = (clock: string): number => + getDateTimeFromClockString(clock, dateStrToUtcDate(today).getTime()); + + it('done timed recurring instance contributes 0 remaining, not estimate x 2', () => { + const cfg = dailyTimedRepeatCfg('R1', ONE_HOUR); + const task = createMockTask({ + id: 't1', + repeatCfgId: 'R1', + isDone: true, + timeEstimate: ONE_HOUR, + timeSpent: 0, + dueWithTime: todayAt('09:00'), + }) as TaskWithDueTime; + + // The reporter's setup: instance is in today's list AND in allPlannedTasks + // (because dueWithTime is set on the recurring instance). + const selector = fromSelectors.selectPlannerDays( + [today], + [cfg], + ['t1'], + [], + [task], + today, + ); + const result = selector.projector( + createTasksMapFromTasksArray([task]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(0); + // One real (done) instance, projection dropped. + expect(result[0].itemsTotal).toBe(1); + // Scheduled list still shows the instance (visible in the day), but as a + // zero-length item — no double-booked projection alongside it. + expect(result[0].scheduledIItems.length).toBe(1); + }); + + it('undone timed recurring instance contributes one estimate, not two', () => { + const cfg = dailyTimedRepeatCfg('R1', ONE_HOUR); + const task = createMockTask({ + id: 't1', + repeatCfgId: 'R1', + isDone: false, + timeEstimate: ONE_HOUR, + timeSpent: 0, + dueWithTime: todayAt('09:00'), + }) as TaskWithDueTime; + + const selector = fromSelectors.selectPlannerDays( + [today], + [cfg], + ['t1'], + [], + [task], + today, + ); + const result = selector.projector( + createTasksMapFromTasksArray([task]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(ONE_HOUR); + expect(result[0].itemsTotal).toBe(1); + expect(result[0].scheduledIItems.length).toBe(1); + }); + + it('done timed task with no recurring cfg also contributes 0 (getScheduledTaskItems is done-aware)', () => { + // Independent of the dedup fix: a one-off done timed task used to contribute + // its full estimate via scheduledTaskItems. + const task = createMockTask({ + id: 't1', + isDone: true, + timeEstimate: ONE_HOUR, + timeSpent: 0, + dueWithTime: todayAt('10:00'), + }) as TaskWithDueTime; + + const selector = fromSelectors.selectPlannerDays( + [today], + [], + ['t1'], + [], + [task], + today, + ); + const result = selector.projector( + createTasksMapFromTasksArray([task]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(0); + }); + + it('still projects a timed recurring cfg that has no instance in the day', () => { + const cfg = dailyTimedRepeatCfg('R2', ONE_HOUR); + + const selector = fromSelectors.selectPlannerDays([today], [cfg], [], [], [], today); + const result = selector.projector( + createTasksMapFromTasksArray([]), + emptyPlannerState, + defaultScheduleConfig, + 0, + ); + + expect(result[0].timeEstimate).toBe(ONE_HOUR); + expect(result[0].itemsTotal).toBe(1); + }); +}); diff --git a/src/app/features/planner/store/planner.selectors.ts b/src/app/features/planner/store/planner.selectors.ts index 6eaf0eb688..e1351346ae 100644 --- a/src/app/features/planner/store/planner.selectors.ts +++ b/src/app/features/planner/store/planner.selectors.ts @@ -186,14 +186,39 @@ const getPlannerDay = ( // Filter out tasks with dueDay in future if it is Today's column .filter((t) => !isTodayI || !t.dueDay || t.dueDay <= todayStr); - const { repeatProjectionsForDay, noStartTimeRepeatProjections } = - getAllRepeatableTasksForDay(taskRepeatCfgs, currentDayTimestamp); + const { + repeatProjectionsForDay: allRepeatProjectionsForDay, + noStartTimeRepeatProjections: allNoStartTimeRepeatProjections, + } = getAllRepeatableTasksForDay(taskRepeatCfgs, currentDayTimestamp); const scheduledTaskItems = getScheduledTaskItems( allPlannedTasks, dayDate, startOfNextDayDiffMs, ); + + // A recurring task can already have a real instance in this day, either in + // normalTasks (untimed instance, or any non-Today column) or in + // scheduledTaskItems (timed `dueWithTime` instance on Today — excluded from + // normalTasks because allPlannedTasks owns it). Drop the projection for any + // config that already has an instance here so the same recurring task is not + // counted twice — once as the real (done-aware) task and once as a full- + // estimate projection. The "Today" view only ever counts the real task, so + // without this the two views disagree on remaining time for done recurring + // tasks. See #8220, #8232. + const coveredRepeatCfgIds = new Set(); + for (const t of normalTasks) { + if (t.repeatCfgId) coveredRepeatCfgIds.add(t.repeatCfgId); + } + for (const si of scheduledTaskItems) { + if (si.task.repeatCfgId) coveredRepeatCfgIds.add(si.task.repeatCfgId); + } + const repeatProjectionsForDay = allRepeatProjectionsForDay.filter( + (rp) => !coveredRepeatCfgIds.has(rp.repeatCfg.id), + ); + const noStartTimeRepeatProjections = allNoStartTimeRepeatProjections.filter( + (rp) => !coveredRepeatCfgIds.has(rp.repeatCfg.id), + ); const { timedEvents, allDayEvents } = getIcalEventsForDay( calendarEvents, dayDate, @@ -326,7 +351,11 @@ const getScheduledTaskItems = ( ) .map((task) => { const start = task.dueWithTime; - const end = start + Math.max(task.timeEstimate - task.timeSpent, 0); + // Mirror normalTasks: a done task contributes 0 remaining time. Without + // this, a done timed task still adds its full estimate to the day total + // (and a done timed recurring task double-counted with its projection + // before the dedup above). See #8232. + const end = start + (task.isDone ? 0 : getTimeLeftForTask(task)); return { id: task.id, type: ScheduleItemType.Task, diff --git a/src/app/features/right-panel/right-panel.component.ts b/src/app/features/right-panel/right-panel.component.ts index 976918bd83..e4dc55bf64 100644 --- a/src/app/features/right-panel/right-panel.component.ts +++ b/src/app/features/right-panel/right-panel.component.ts @@ -23,6 +23,7 @@ import { of, Subscription, timer } from 'rxjs'; import { SwipeDirective } from '../../ui/swipe-gesture/swipe.directive'; import { CssString, StyleObject, StyleObjectToString } from '../../util/styles'; import { LS } from '../../core/persistence/storage-keys.const'; +import { Log } from '../../core/log'; import { readNumberLSBounded } from '../../util/ls-util'; import { MatIconModule } from '@angular/material/icon'; import { MatRippleModule } from '@angular/material/core'; @@ -558,7 +559,7 @@ export class RightPanelComponent implements AfterViewInit, OnDestroy { try { localStorage.setItem(LS.RIGHT_PANEL_WIDTH, this.currentWidth().toString()); } catch (error) { - console.warn('Failed to save right panel width to localStorage:', error); + Log.warn('Failed to save right panel width to localStorage:', error); } } @@ -577,7 +578,7 @@ export class RightPanelComponent implements AfterViewInit, OnDestroy { // Additional validation if (!Number.isFinite(width) || width < RIGHT_PANEL_CONFIG.MIN_WIDTH) { - console.warn('Invalid right panel width detected, using default'); + Log.warn('Invalid right panel width detected, using default'); this.currentWidth.set(RIGHT_PANEL_CONFIG.DEFAULT_WIDTH); return; } @@ -586,7 +587,7 @@ export class RightPanelComponent implements AfterViewInit, OnDestroy { const validatedWidth = clampWidth(width, RIGHT_PANEL_CONFIG.MAX_WIDTH); this.currentWidth.set(validatedWidth); } catch (error) { - console.warn('Failed to initialize right panel width:', error); + Log.warn('Failed to initialize right panel width:', error); this.currentWidth.set(RIGHT_PANEL_CONFIG.DEFAULT_WIDTH); } } diff --git a/src/app/features/schedule/schedule-event/schedule-event.component.html b/src/app/features/schedule/schedule-event/schedule-event.component.html index 69673b8bb5..747f8400dd 100644 --- a/src/app/features/schedule/schedule-event/schedule-event.component.html +++ b/src/app/features/schedule/schedule-event/schedule-event.component.html @@ -113,7 +113,7 @@ {{ T.F.CALENDARS.CONTEXT_MENU.OPEN_IN_CALENDAR | translate }} } - @if (isCalendarEventFromPlugin()) { + @if (canRescheduleCalendarEvent()) { } diff --git a/src/app/features/tasks/task-context-menu/task-context-menu-inner/task-context-menu-inner.component.ts b/src/app/features/tasks/task-context-menu/task-context-menu-inner/task-context-menu-inner.component.ts index 45dda57742..58261ec09f 100644 --- a/src/app/features/tasks/task-context-menu/task-context-menu-inner/task-context-menu-inner.component.ts +++ b/src/app/features/tasks/task-context-menu/task-context-menu-inner/task-context-menu-inner.component.ts @@ -64,6 +64,7 @@ import { getDbDateStr } from '../../../../util/get-db-date-str'; import { PlannerActions } from '../../../planner/store/planner.actions'; import { addSubTask } from '../../../tasks/store/task.actions'; import { combineDateAndTime } from '../../../../util/combine-date-and-time'; +import { getNextWeekDayOffset } from '../../../../util/get-next-week-day-offset'; import { DateAdapter } from '@angular/material/core'; import { ICAL_TYPE } from '../../../issue/issue.const'; import { IssueIconPipe } from '../../../issue/issue-icon/issue-icon.pipe'; @@ -723,11 +724,7 @@ export class TaskContextMenuInnerComponent implements AfterViewInit, OnDestroy { break; case 3: const nextFirstDayOfWeek = tDate; - const dayOffset = - (this._dateAdapter.getFirstDayOfWeek() - - this._dateAdapter.getDayOfWeek(nextFirstDayOfWeek) + - 7) % - 7 || 7; + const dayOffset = getNextWeekDayOffset(this._dateAdapter, nextFirstDayOfWeek); nextFirstDayOfWeek.setDate(nextFirstDayOfWeek.getDate() + dayOffset); this._schedule(nextFirstDayOfWeek); break; diff --git a/src/app/features/tasks/task-shortcut.service.spec.ts b/src/app/features/tasks/task-shortcut.service.spec.ts index 7e55efc5b7..db39a8c00d 100644 --- a/src/app/features/tasks/task-shortcut.service.spec.ts +++ b/src/app/features/tasks/task-shortcut.service.spec.ts @@ -37,7 +37,7 @@ describe('TaskShortcutService', () => { taskEditTags: 'G', taskOpenContextMenu: null, moveToBacklog: 'B', - moveToTodaysTasks: 'F', + taskScheduleToday: 'F', selectPreviousTask: 'K', selectNextTask: 'J', collapseSubTasks: 'H', diff --git a/src/app/features/tasks/task-shortcut.service.ts b/src/app/features/tasks/task-shortcut.service.ts index 9509f8f501..11f7e6d65a 100644 --- a/src/app/features/tasks/task-shortcut.service.ts +++ b/src/app/features/tasks/task-shortcut.service.ts @@ -169,6 +169,21 @@ export class TaskShortcutService { ev.preventDefault(); return true; } + if (checkKeyCombo(ev, keys.taskScheduleTomorrow)) { + this._handleTaskShortcut(focusedTaskId, 'scheduleTaskTomorrow'); + ev.preventDefault(); + return true; + } + if (checkKeyCombo(ev, keys.taskScheduleNextWeek)) { + this._handleTaskShortcut(focusedTaskId, 'scheduleTaskNextWeek'); + ev.preventDefault(); + return true; + } + if (checkKeyCombo(ev, keys.taskScheduleNextMonth)) { + this._handleTaskShortcut(focusedTaskId, 'scheduleTaskNextMonth'); + ev.preventDefault(); + return true; + } if (checkKeyCombo(ev, keys.taskScheduleDeadline)) { this._handleTaskShortcut(focusedTaskId, 'openDeadlineDialog'); ev.preventDefault(); @@ -229,7 +244,7 @@ export class TaskShortcutService { return true; } - if (checkKeyCombo(ev, keys.moveToTodaysTasks)) { + if (checkKeyCombo(ev, keys.taskScheduleToday)) { this._handleTaskShortcut(focusedTaskId, 'moveToTodayWithFocus'); ev.preventDefault(); ev.stopPropagation(); diff --git a/src/app/features/tasks/task/task-hover-controls/task-hover-controls.component.html b/src/app/features/tasks/task/task-hover-controls/task-hover-controls.component.html index 62e9799e09..4093eb239a 100644 --- a/src/app/features/tasks/task/task-hover-controls/task-hover-controls.component.html +++ b/src/app/features/tasks/task/task-hover-controls/task-hover-controls.component.html @@ -29,7 +29,7 @@