fix: harden paused-entry lifecycle and recovery flow

aSoftwareByDesignRepository · aSoftwareByDesignRepository · commit 5de8ad4c27af · 2026-04-21T12:37:47.000+02:00
Repair legacy paused records, allow safe edit/delete access for paused entries, and resume same-day paused sessions without creating duplicates while preserving break accounting.
diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -54,7 +54,7 @@ gehosteten Nextcloud.
 - PHP 8.1–8.4
 	]]></description>
 
-	<version>1.2.0</version>
+	<version>1.2.1</version>
 	<licence>AGPL-3.0-or-later</licence>
 	<author mail="info@software-by-design.de" homepage="https://software-by-design.de">Alexander Mäule</author>
 
diff --git a/lib/Controller/TimeEntryController.php b/lib/Controller/TimeEntryController.php
@@ -452,27 +452,17 @@ public function edit(int $id): TemplateResponse
 				return $this->configureCSP($response);
 			}
 
-			// Check if entry can be edited
-			// Allow editing if:
-			// 1. It's a manual entry (user created it themselves) - but not if already approved
-			// 2. It has pending approval status (correction request)
-			// 3. It's an automatic entry that is completed (not yet approved) - allow direct editing for convenience
-			// 4. The entry date is within the last 2 weeks (14 days) - for data integrity and compliance
-			// Do NOT allow editing if entry is already approved (approvedBy is set) or older than 2 weeks
-			$isApproved = $entry->getApprovedBy() !== null;
-			$entryDate = $entry->getStartTime();
-			$editCutoff = new \DateTime();
-			$editCutoff->modify('-' . Constants::EDIT_WINDOW_DAYS . ' days');
-			$editCutoff->setTime(0, 0, 0);
-			$isWithinEditWindow = $entryDate && $entryDate >= $editCutoff;
-
-			$canEdit = !$isApproved && $isWithinEditWindow && (
-				$entry->getIsManualEntry()
-				|| $entry->getStatus() === TimeEntry::STATUS_PENDING_APPROVAL
-				|| ($entry->getStatus() === TimeEntry::STATUS_COMPLETED && !$entry->getIsManualEntry())
-			);
+			// Check if entry can be edited (logic lives in TimeEntry::canEdit to stay DRY)
+			$canEdit = $entry->canEdit(Constants::EDIT_WINDOW_DAYS);
 
 			if (!$canEdit) {
+				$isApproved        = $entry->getApprovedBy() !== null;
+				$entryDate         = $entry->getStartTime();
+				$editCutoff        = new \DateTime();
+				$editCutoff->modify('-' . Constants::EDIT_WINDOW_DAYS . ' days');
+				$editCutoff->setTime(0, 0, 0);
+				$isWithinEditWindow = $entryDate && $entryDate >= $editCutoff;
+
 				$errorMessage = $isApproved
 					? $this->l10n->t('Cannot edit this time entry. Please use "Request Correction" for approved entries.')
 					: (!$isWithinEditWindow
@@ -781,27 +771,17 @@ public function update(int $id, ?string $date = null, ?float $hours = null, ?str
 				return $mc0;
 			}
 
-			// Check if entry can be edited
-			// Allow editing if:
-			// 1. It's a manual entry (user created it themselves) - but not if already approved
-			// 2. It has pending approval status (correction request)
-			// 3. It's an automatic entry that is completed (not yet approved) - allow direct editing for convenience
-			// 4. The entry date is within the last 2 weeks (14 days) - for data integrity and compliance
-			// Do NOT allow editing if entry is already approved (approvedBy is set) or older than 2 weeks
-			$isApproved = $entry->getApprovedBy() !== null;
-			$entryDate = $entry->getStartTime();
-			$editCutoff = new \DateTime();
-			$editCutoff->modify('-' . Constants::EDIT_WINDOW_DAYS . ' days');
-			$editCutoff->setTime(0, 0, 0);
-			$isWithinEditWindow = $entryDate && $entryDate >= $editCutoff;
-
-			$canEdit = !$isApproved && $isWithinEditWindow && (
-				$entry->getIsManualEntry()
-				|| $entry->getStatus() === TimeEntry::STATUS_PENDING_APPROVAL
-				|| ($entry->getStatus() === TimeEntry::STATUS_COMPLETED && !$entry->getIsManualEntry())
-			);
+			// Check if entry can be edited (logic lives in TimeEntry::canEdit to stay DRY)
+			$canEdit = $entry->canEdit(Constants::EDIT_WINDOW_DAYS);
 
 			if (!$canEdit) {
+				$isApproved        = $entry->getApprovedBy() !== null;
+				$entryDate         = $entry->getStartTime();
+				$editCutoff        = new \DateTime();
+				$editCutoff->modify('-' . Constants::EDIT_WINDOW_DAYS . ' days');
+				$editCutoff->setTime(0, 0, 0);
+				$isWithinEditWindow = $entryDate && $entryDate >= $editCutoff;
+
 				$errorMessage = $isApproved
 					? $this->l10n->t('Cannot edit this time entry. Please use "Request Correction" for approved entries.')
 					: (!$isWithinEditWindow
@@ -1047,6 +1027,16 @@ public function update(int $id, ?string $date = null, ?float $hours = null, ?str
 				$entry->setProjectCheckProjectId($project_check_project_id);
 			}
 
+			// Finalise paused entries: when the user supplies an end_time, the entry
+			// is no longer orphaned – promote it to completed so all downstream logic
+			// (compliance checks, reports, approval flow) treats it correctly.
+			if ($entry->getStatus() === TimeEntry::STATUS_PAUSED && $entry->getEndTime() !== null) {
+				$entry->setStatus(TimeEntry::STATUS_COMPLETED);
+				if (!$entry->getEndedReason()) {
+					$entry->setEndedReason(TimeEntry::ENDED_REASON_MANUAL_CLOCK_OUT);
+				}
+			}
+
 			// Check rest period compliance before saving (ArbZG §5)
 			if ($entry->getStartTime()) {
 				$restPeriodCheck = $this->complianceService->checkRestPeriodForStartTime($userId, $entry->getStartTime(), $id);
@@ -1224,8 +1214,8 @@ public function getDeletionImpact(int $id): JSONResponse
 				], Http::STATUS_FORBIDDEN);
 			}
 
-			// Check if entry can be deleted
-			$canDelete = $entry->getIsManualEntry();
+			// Check if entry can be deleted (manual entries + orphaned paused entries)
+			$canDelete = $entry->canDelete();
 			$impact = [
 				'canDelete' => $canDelete,
 				'isManualEntry' => $entry->getIsManualEntry(),
@@ -1577,8 +1567,8 @@ public function delete(int $id): JSONResponse
 				return $mcDel;
 			}
 
-			// Check if entry can be deleted (only manual entries)
-			if (!$entry->getIsManualEntry()) {
+			// Check if entry can be deleted (manual entries + orphaned paused entries)
+			if (!$entry->canDelete()) {
 				return new JSONResponse([
 					'success' => false,
 					'error' => $this->l10n->t('Cannot delete automatic time entries')
diff --git a/lib/Db/TimeEntry.php b/lib/Db/TimeEntry.php
@@ -453,6 +453,53 @@ public function isValid(): bool
 		return empty($this->validate());
 	}
 
+	/**
+	 * Whether the entry's owner may edit it.
+	 *
+	 * Centralises the business rule so controllers and templates stay in sync:
+	 *  – not already approved
+	 *  – start time within the edit window (default 14 days)
+	 *  – status is one that allows editing: manual, pending_approval,
+	 *    completed-automatic, or paused (orphaned/unfinished automatic entry)
+	 *
+	 * @param int $editWindowDays Number of past days within which editing is permitted.
+	 */
+	public function canEdit(int $editWindowDays = 14): bool
+	{
+		if ($this->approvedBy !== null) {
+			return false;
+		}
+
+		$entryDate = $this->startTime;
+		if (!$entryDate instanceof \DateTime) {
+			return false;
+		}
+
+		$cutoff = new \DateTime();
+		$cutoff->modify('-' . $editWindowDays . ' days');
+		$cutoff->setTime(0, 0, 0);
+		if ($entryDate < $cutoff) {
+			return false;
+		}
+
+		return $this->isManualEntry
+			|| $this->status === self::STATUS_PENDING_APPROVAL
+			|| ($this->status === self::STATUS_COMPLETED && !$this->isManualEntry)
+			|| $this->status === self::STATUS_PAUSED;
+	}
+
+	/**
+	 * Whether the entry's owner may delete it.
+	 *
+	 * Manual entries can always be deleted. Paused (orphaned automatic) entries
+	 * are also deletable because they were never properly completed and have no
+	 * approved payroll impact.
+	 */
+	public function canDelete(): bool
+	{
+		return $this->isManualEntry || $this->status === self::STATUS_PAUSED;
+	}
+
 	/**
 	 * Get a summary array for API responses
 	 *
diff --git a/lib/Migration/Version1020Date20260421000000.php b/lib/Migration/Version1020Date20260421000000.php
@@ -0,0 +1,91 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Repair all remaining orphaned paused time entries.
+ *
+ * Migration 1015 already closed paused entries with end_time IS NULL via a one-time
+ * flag-guarded pass. This migration handles every remaining case unconditionally:
+ *
+ *  1. paused + end_time NOT NULL   → status = completed   (should never occur in normal use)
+ *  2. paused + end_time IS NULL    → end_time = updated_at (or start_time as fallback), status = completed
+ *
+ * This migration is idempotent: it has no effect when no paused rows remain.
+ *
+ * @copyright Copyright (c) 2026
+ * @license AGPL-3.0-or-later
+ */
+
+namespace OCA\ArbeitszeitCheck\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version1020Date20260421000000 extends SimpleMigrationStep
+{
+	public function __construct(
+		private IDBConnection $db,
+	) {
+	}
+
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void
+	{
+		$output->info('Repairing orphaned paused time entries…');
+
+		try {
+			// 1. paused entries that already have an end_time: just fix the status.
+			$qb1 = $this->db->getQueryBuilder();
+			$fixed1 = (int)$qb1
+				->update('at_entries')
+				->set('status', $qb1->createNamedParameter('completed', IQueryBuilder::PARAM_STR))
+				->where($qb1->expr()->eq('status', $qb1->createNamedParameter('paused', IQueryBuilder::PARAM_STR)))
+				->andWhere($qb1->expr()->isNotNull('end_time'))
+				->executeStatement();
+
+			// 2. paused entries without end_time but with updated_at → end_time = updated_at.
+			//    createFunction() is used to reference the column value directly because
+			//    QueryBuilder does not support SET col = other_col via named parameters.
+			$qb2 = $this->db->getQueryBuilder();
+			$fixed2 = (int)$qb2
+				->update('at_entries')
+				->set('end_time', $qb2->createFunction('updated_at'))
+				->set('status', $qb2->createNamedParameter('completed', IQueryBuilder::PARAM_STR))
+				->where($qb2->expr()->eq('status', $qb2->createNamedParameter('paused', IQueryBuilder::PARAM_STR)))
+				->andWhere($qb2->expr()->isNull('end_time'))
+				->andWhere($qb2->expr()->isNotNull('updated_at'))
+				->executeStatement();
+
+			// 3. Edge case: paused entry with neither end_time nor updated_at.
+			//    Use start_time so the entry has at least a zero-duration completed record.
+			$qb3 = $this->db->getQueryBuilder();
+			$fixed3 = (int)$qb3
+				->update('at_entries')
+				->set('end_time', $qb3->createFunction('start_time'))
+				->set('status', $qb3->createNamedParameter('completed', IQueryBuilder::PARAM_STR))
+				->where($qb3->expr()->eq('status', $qb3->createNamedParameter('paused', IQueryBuilder::PARAM_STR)))
+				->andWhere($qb3->expr()->isNull('end_time'))
+				->andWhere($qb3->expr()->isNull('updated_at'))
+				->executeStatement();
+
+			$total = $fixed1 + $fixed2 + $fixed3;
+			$output->info(sprintf('Done: %d paused entr%s repaired.', $total, $total === 1 ? 'y' : 'ies'));
+		} catch (\Throwable $e) {
+			// Non-fatal: log and continue. A subsequent upgrade will re-run this check.
+			$output->warning('Could not repair paused entries: ' . $e->getMessage());
+			\OCP\Log\logger('arbeitszeitcheck')->error(
+				'Version1020 migration failed: ' . $e->getMessage(),
+				['exception' => $e]
+			);
+		}
+	}
+
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper
+	{
+		return null;
+	}
+}
diff --git a/lib/Service/TimeTrackingService.php b/lib/Service/TimeTrackingService.php
@@ -101,6 +101,14 @@ public function clockIn(string $userId, ?string $projectCheckProjectId = null, ?
 			throw new \Exception($this->l10n->t('User is currently on break. End break first.'));
 		}
 
+		// Check for a paused (orphaned) entry from today and resume it instead of
+		// creating a duplicate. The pause gap is archived as a break interval so that
+		// working-time calculations remain accurate.
+		$pausedTodayEntry = $this->timeEntryMapper->findPausedOrUnfinishedTodayByUser($userId);
+		if ($pausedTodayEntry !== null && $pausedTodayEntry->getStatus() === TimeEntry::STATUS_PAUSED) {
+			return $this->resumePausedEntry($userId, $pausedTodayEntry, $projectCheckProjectId, $description);
+		}
+
 		// Validate project ID length (DB column limit; ProjectCheck may not enforce)
 		if ($projectCheckProjectId !== null && mb_strlen($projectCheckProjectId) > TimeEntry::PROJECT_CHECK_PROJECT_ID_MAX_LENGTH) {
 			throw new \Exception($this->l10n->t('Project ID must not exceed %d characters', [TimeEntry::PROJECT_CHECK_PROJECT_ID_MAX_LENGTH]));
@@ -220,6 +228,71 @@ public function clockOut(
 		return $updatedEntry;
 	}
 
+	/**
+	 * Resume a same-day paused entry instead of creating a new one.
+	 *
+	 * The gap between the moment the entry was paused (updated_at) and now is
+	 * archived as a break interval so that working-time calculations stay correct.
+	 * Project and description are updated when the caller supplies new values.
+	 *
+	 * @throws \Exception When the month is already closed or the daily maximum is reached.
+	 */
+	private function resumePausedEntry(
+		string $userId,
+		TimeEntry $pausedEntry,
+		?string $projectCheckProjectId = null,
+		?string $description = null
+	): TimeEntry {
+		$this->monthClosureGuard->assertTimeEntryMutable($pausedEntry);
+
+		// Respect the daily maximum – the paused entry's own hours already count.
+		$today = new \DateTime();
+		$today->setTime(0, 0, 0);
+		$tomorrow = clone $today;
+		$tomorrow->modify('+1 day');
+		$todayHours   = $this->timeEntryMapper->getTotalHoursByUserAndDateRange($userId, $today, $tomorrow);
+		$maxDailyHours = $this->getMaxDailyHours();
+		if ($todayHours >= $maxDailyHours) {
+			throw new \Exception($this->l10n->t(
+				'Cannot clock in: Maximum daily working hours (%1$dh) already reached. You have already worked %2$.1f hours today (ArbZG §3).',
+				[(int)$maxDailyHours, $todayHours]
+			));
+		}
+
+		$now      = new \DateTime();
+		$pausedAt = $pausedEntry->getUpdatedAt() ?? $now;
+
+		// Archive the gap since the entry was paused as a break so it is excluded
+		// from net working time. Only do so when the gap is positive.
+		if ($pausedAt < $now) {
+			$this->archiveBreakToJson($pausedEntry, $pausedAt, $now);
+		}
+
+		// Update optional fields if the caller supplied new values.
+		if ($projectCheckProjectId !== null) {
+			$pausedEntry->setProjectCheckProjectId($projectCheckProjectId);
+		}
+		if ($description !== null) {
+			$pausedEntry->setDescription($description);
+		}
+
+		$pausedEntry->setStatus(TimeEntry::STATUS_ACTIVE);
+		$pausedEntry->setUpdatedAt($now);
+
+		$resumed = $this->timeEntryMapper->update($pausedEntry);
+
+		$this->auditLogMapper->logAction(
+			$userId,
+			'clock_in',
+			'time_entry',
+			$resumed->getId(),
+			null,
+			$this->safeGetSummary($resumed, $userId)
+		);
+
+		return $resumed;
+	}
+
 	/**
 	 * Start break for a user
 	 *
@@ -345,8 +418,33 @@ public function getStatus(string $userId): array
 			$currentEntry = $activeEntry ?: $breakEntry;
 			$currentEntry = $this->enforceBreakAutoFallbackForUser($userId, $currentEntry);
 
-			// If no active entry we are fully clocked out.
+			// If no active/break entry, check for a paused entry from today before
+			// declaring the user fully clocked out.
 			if ($currentEntry === null) {
+				$pausedEntry = $this->timeEntryMapper->findPausedOrUnfinishedTodayByUser($userId);
+				if ($pausedEntry !== null && $pausedEntry->getStatus() === TimeEntry::STATUS_PAUSED) {
+					$sessionStart = $pausedEntry->getStartTime();
+					$pausedAt     = $pausedEntry->getUpdatedAt() ?? new \DateTime();
+					$sessionDuration = $sessionStart
+						? max(0, $pausedAt->getTimestamp() - $sessionStart->getTimestamp() - (int)($pausedEntry->getBreakDurationHours() * 3600))
+						: 0;
+					try {
+						$pausedSummary = $pausedEntry->getSummary();
+					} catch (\Throwable $e) {
+						\OCP\Log\logger('arbeitszeitcheck')->error(
+							'Error getting summary for paused entry in getStatus: ' . $e->getMessage(),
+							['exception' => $e]
+						);
+						$pausedSummary = ['id' => $pausedEntry->getId(), 'userId' => $userId, 'status' => TimeEntry::STATUS_PAUSED];
+					}
+					return $this->appendAutoClockoutNotice([
+						'status' => TimeEntry::STATUS_PAUSED,
+						'current_entry' => $pausedSummary,
+						'working_today_hours' => $this->getTodayHours($userId),
+						'current_session_duration' => $sessionDuration,
+					], $userId);
+				}
+
 				return $this->appendAutoClockoutNotice([
 					'status' => 'clocked_out',
 					'current_entry' => null,
diff --git a/templates/dashboard.php b/templates/dashboard.php
diff --git a/templates/time-entries.php b/templates/time-entries.php
