Fix successive leave adjustments replacing each other instead of adding up

SPECIFICATION.md

@@ -771,6 +771,11 @@ the SPA). All list endpoints paginate and accept filters.
 - `GET  /api/employees/{uid}/balance` — manager (reports) / HR only.
 - `GET  /api/entitlements` / `PUT /api/entitlements/{id}` — HR manage.
 - `POST /api/entitlements/bulk` — HR bulk set.
+- `PUT  /api/entitlements/{id}` — HR manage. **`adjustmentDelta` adds to** the stored
+  manual adjustment; `manualAdjustment` **sets** it outright. Corrections are made as
+  deltas ("+2 for the wedding", later "−2, booked in error") and must cancel to
+  nothing — treating the second as an absolute set is what made 25 → +2 → 27 → −2
+  land on 23 instead of back on 25. Sending both is refused rather than guessed at.
 - `GET  /api/entitlements/{id}/history` — HR only: who changed which figure on an
   entitlement, from what to what, and the note they gave. One row per figure per
   save, so a single adjustment reads on its own rather than having to be diffed

lib/Controller/EntitlementController.php

@@ -39,13 +39,14 @@ public function index(string $employeeUid, ?int $year = null): DataResponse {
 
 	#[NoAdminRequired]
 	#[UserRateLimit(limit: 30, period: 60)]
-	public function create(string $employeeUid, int $year, int $typeId, ?float $baseDays = null, ?float $carryOverDays = null, ?float $manualAdjustment = null, ?string $adjustmentNote = null): DataResponse {
-		return $this->handle(function () use ($employeeUid, $year, $typeId, $baseDays, $carryOverDays, $manualAdjustment, $adjustmentNote) {
+	public function create(string $employeeUid, int $year, int $typeId, ?float $baseDays = null, ?float $carryOverDays = null, ?float $manualAdjustment = null, ?float $adjustmentDelta = null, ?string $adjustmentNote = null): DataResponse {
+		return $this->handle(function () use ($employeeUid, $year, $typeId, $baseDays, $carryOverDays, $manualAdjustment, $adjustmentDelta, $adjustmentNote) {
 			$this->permission->assertHr((string)$this->userId);
 			$data = array_filter([
 				'baseDays' => $baseDays,
 				'carryOverDays' => $carryOverDays,
 				'manualAdjustment' => $manualAdjustment,
+				'adjustmentDelta' => $adjustmentDelta,
 				'adjustmentNote' => $adjustmentNote,
 			], static fn ($v) => $v !== null);
 			return $this->service->setForEmployee((string)$this->userId, $employeeUid, $year, $typeId, $data)->jsonSerialize();
@@ -54,13 +55,14 @@ public function create(string $employeeUid, int $year, int $typeId, ?float $base
 
 	#[NoAdminRequired]
 	#[UserRateLimit(limit: 30, period: 60)]
-	public function update(int $id, ?float $baseDays = null, ?float $carryOverDays = null, ?float $manualAdjustment = null, ?string $adjustmentNote = null): DataResponse {
-		return $this->handle(function () use ($id, $baseDays, $carryOverDays, $manualAdjustment, $adjustmentNote) {
+	public function update(int $id, ?float $baseDays = null, ?float $carryOverDays = null, ?float $manualAdjustment = null, ?float $adjustmentDelta = null, ?string $adjustmentNote = null): DataResponse {
+		return $this->handle(function () use ($id, $baseDays, $carryOverDays, $manualAdjustment, $adjustmentDelta, $adjustmentNote) {
 			$this->permission->assertHr((string)$this->userId);
 			$data = array_filter([
 				'baseDays' => $baseDays,
 				'carryOverDays' => $carryOverDays,
 				'manualAdjustment' => $manualAdjustment,
+				'adjustmentDelta' => $adjustmentDelta,
 				'adjustmentNote' => $adjustmentNote,
 			], static fn ($v) => $v !== null);
 			return $this->service->update((string)$this->userId, $id, $data)->jsonSerialize();

lib/Service/EntitlementService.php

@@ -45,14 +45,27 @@ public function listForEmployee(string $employeeUid, ?int $year = null): array {
 	/**
 	 * Update an entitlement row (HR). Manual adjustments require a note (§6.1).
 	 *
-	 * @param array{baseDays?:float,carryOverDays?:float,manualAdjustment?:float,adjustmentNote?:string} $data
+	 * Two ways to move the manual adjustment, and the difference matters:
+	 *
+	 *  - `adjustmentDelta` **adds to** what is already there. This is how corrections
+	 *    are actually made — "+2 for the wedding", later "−2, booked in error" — and
+	 *    the two must cancel to nothing.
+	 *  - `manualAdjustment` **sets** the running total outright, for the rare case of
+	 *    overwriting it wholesale.
+	 *
+	 * Sending both is refused rather than guessed at.
+	 *
+	 * @param array{baseDays?:float,carryOverDays?:float,manualAdjustment?:float,adjustmentDelta?:float,adjustmentNote?:string} $data
 	 */
 	public function update(string $actorUid, int $id, array $data): Entitlement {
 		try {
 			$ent = $this->entitlementMapper->find($id);
 		} catch (DoesNotExistException) {
 			throw new NotFoundException('Entitlement not found');
 		}
+		if (array_key_exists('adjustmentDelta', $data) && array_key_exists('manualAdjustment', $data)) {
+			throw new ValidationException('Send either an adjustment to apply or an absolute adjustment, not both.');
+		}
 		// Read before any setter runs: these are what the history reports moving from.
 		$before = [
 			EntitlementEvent::FIELD_BASE_DAYS => $ent->getBaseDays(),
@@ -67,7 +80,21 @@ public function update(string $actorUid, int $id, array $data): Entitlement {
 		if (array_key_exists('carryOverDays', $data)) {
 			$ent->setCarryOverDays((float)$data['carryOverDays']);
 		}
-		if (array_key_exists('manualAdjustment', $data)) {
+		if (array_key_exists('adjustmentDelta', $data)) {
+			// A correction on top of whatever corrections came before, so "+2" and a
+			// later "−2" cancel and the allowance returns to where it started.
+			// Assigning here instead — which is what the absolute branch below does —
+			// made the second correction *replace* the first: 25 → +2 → 27, then a
+			// −2 correction landed on 23 rather than back on 25.
+			$delta = (float)$data['adjustmentDelta'];
+			if (abs($delta) > 0.001) {
+				if ($note === '') {
+					throw new ValidationException('A note is required when adjusting an entitlement.');
+				}
+				$ent->setManualAdjustment($ent->getManualAdjustment() + $delta);
+				$ent->setAdjustmentNote($data['adjustmentNote'] ?? $ent->getAdjustmentNote());
+			}
+		} elseif (array_key_exists('manualAdjustment', $data)) {
 			$adjustment = (float)$data['manualAdjustment'];
 			if ($adjustment !== $ent->getManualAdjustment() && $note === '') {
 				throw new ValidationException('A note is required when adjusting an entitlement.');

src/views/hr/HrBalances.vue

@@ -136,8 +136,17 @@
 				<h3>{{ editing.displayName }} · {{ editing.typeLabel }} · {{ year }}</h3>
 				<label>{{ t('absence', 'Base days') }}</label>
 				<NcTextField v-model="form.baseDays" type="number" />
-				<label>{{ t('absence', 'Manual adjustment (+/−)') }}</label>
-				<NcTextField v-model="form.manualAdjustment" type="number" />
+				<label>{{ t('absence', 'Adjust by (+/−)') }}</label>
+				<NcTextField
+					v-model="form.adjustmentDelta"
+					type="number"
+					:placeholder="t('absence', 'e.g. 2 to add two days, -2 to take them back')" />
+				<!-- The running total, so it is clear the field above adds to this rather
+				     than replacing it — entering −2 to undo an earlier +2 used to set the
+				     total to −2 and take two days off the allowance instead. -->
+				<p class="edit__hint">
+					{{ t('absence', 'Adjustments so far: {total} · leaving the field empty changes nothing', { total: signed(currentAdjustment) }) }}
+				</p>
 				<label>{{ t('absence', 'Adjustment note') }}</label>
 				<NcTextField v-model="form.adjustmentNote" :placeholder="t('absence', 'Why is this being adjusted?')" />
 				<div class="edit__actions">
@@ -216,7 +225,8 @@ export default {
 			years: [y - 1, y, y + 1],
 			editing: null,
 			saving: false,
-			form: { baseDays: 0, manualAdjustment: 0, adjustmentNote: '' },
+			form: { baseDays: 0, adjustmentDelta: '', adjustmentNote: '' },
+			currentAdjustment: 0,
 			history: [],
 			historyLoading: false,
 		}
@@ -359,14 +369,17 @@ export default {
 			try {
 				const list = await api.listEntitlements(row.employeeUid, this.year)
 				const ent = list.find((e) => e.typeId === row.typeId)
+				this.currentAdjustment = ent ? ent.manualAdjustment : (row.manualAdjustment || 0)
 				this.form = {
 					baseDays: ent ? ent.baseDays : row.baseDays,
-					manualAdjustment: ent ? ent.manualAdjustment : 0,
+					// Always empty: this field is the correction to apply, not the total.
+					adjustmentDelta: '',
 					adjustmentNote: '',
 					entitlementId: ent ? ent.id : row.entitlementId,
 				}
 			} catch {
-				this.form = { baseDays: row.baseDays, manualAdjustment: 0, adjustmentNote: '', entitlementId: row.entitlementId }
+				this.currentAdjustment = row.manualAdjustment || 0
+				this.form = { baseDays: row.baseDays, adjustmentDelta: '', adjustmentNote: '', entitlementId: row.entitlementId }
 			}
 			await this.loadHistory()
 		},
@@ -392,6 +405,17 @@ export default {
 			}
 		},
 
+		/**
+		 * A signed day count, so "+2" and "−2" read as corrections rather than totals.
+		 *
+		 * @param {number} value the accumulated adjustment
+		 * @return {string}
+		 */
+		signed(value) {
+			const n = Number(value) || 0
+			return (n > 0 ? '+' : n < 0 ? '−' : '') + this.fmt(Math.abs(n))
+		},
+
 		fieldLabel(field) {
 			return {
 				base_days: t('absence', 'Base days'),
@@ -407,11 +431,16 @@ export default {
 		async save() {
 			this.saving = true
 			try {
+				const delta = Number(this.form.adjustmentDelta)
 				const data = {
 					baseDays: Number(this.form.baseDays),
-					manualAdjustment: Number(this.form.manualAdjustment),
 					adjustmentNote: this.form.adjustmentNote,
 				}
+				// Omitted entirely when blank, so saving a base-days change on its own
+				// never touches the accumulated adjustment.
+				if (this.form.adjustmentDelta !== '' && !Number.isNaN(delta)) {
+					data.adjustmentDelta = delta
+				}
 				if (this.form.entitlementId) {
 					await api.updateEntitlement(this.form.entitlementId, data)
 				} else {
@@ -475,6 +504,12 @@ td.low {
 		gap: 8px;
 		margin-top: 8px;
 	}
+
+	&__hint {
+		margin: -4px 0 0;
+		font-size: 0.85rem;
+		color: var(--color-text-maxcontrast);
+	}
 }
 
 .log {

tests/Unit/Service/EntitlementServiceTest.php

@@ -167,6 +167,49 @@ public function testAdjustingRecordsTheChangeWithItsNote(): void {
 		self::assertSame('bob', $recorded[0]->getEmployeeUid());
 	}
 
+	/**
+	 * The reported bug, in full. 25 days, "+2 wedding", then "−2 booked in error"
+	 * has to land back on 25 — it landed on 23, because the second correction
+	 * *replaced* the first instead of adding to it.
+	 */
+	public function testTwoOppositeCorrectionsCancelOut(): void {
+		$ent = $this->priorEntitlement(25.0);
+		$ent->setManualAdjustment(0.0);
+		$this->entitlementMapper->method('find')->with(1)->willReturn($ent);
+		$this->entitlementMapper->method('update')->willReturnArgument(0);
+
+		$after = $this->service->update('hr', 1, ['adjustmentDelta' => 2.0, 'adjustmentNote' => 'Wedding']);
+		self::assertSame(2.0, $after->getManualAdjustment());
+		self::assertSame(27.0, $after->getEntitlement());
+
+		$after = $this->service->update('hr', 1, ['adjustmentDelta' => -2.0, 'adjustmentNote' => 'Booked in error']);
+		self::assertSame(0.0, $after->getManualAdjustment(), 'the two corrections must cancel');
+		self::assertSame(25.0, $after->getEntitlement(), 'the allowance returns to where it started');
+	}
+
+	public function testAbsoluteAdjustmentStillSetsTheTotalOutright(): void {
+		$ent = $this->priorEntitlement(25.0);
+		$ent->setManualAdjustment(2.0);
+		$this->entitlementMapper->method('find')->with(1)->willReturn($ent);
+		$this->entitlementMapper->method('update')->willReturnArgument(0);
+
+		// The other half of the contract: manualAdjustment overwrites rather than adds.
+		$after = $this->service->update('hr', 1, ['manualAdjustment' => 5.0, 'adjustmentNote' => 'Recount']);
+		self::assertSame(5.0, $after->getManualAdjustment());
+	}
+
+	public function testSendingBothAdjustmentFormsIsRefused(): void {
+		$ent = $this->priorEntitlement(25.0);
+		$this->entitlementMapper->method('find')->with(1)->willReturn($ent);
+
+		$this->expectException(ValidationException::class);
+		$this->service->update('hr', 1, [
+			'adjustmentDelta' => 2.0,
+			'manualAdjustment' => 5.0,
+			'adjustmentNote' => 'Ambiguous',
+		]);
+	}
+
 	public function testSavingWithoutChangingAnythingRecordsNothing(): void {
 		$ent = $this->priorEntitlement(28.0);
 		$this->entitlementMapper->method('find')->with(1)->willReturn($ent);