Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Application/User/Services/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,20 @@ public function updatePass(int $userId, string $pass): void
* @throws ConstraintException
* @throws QueryException
* @throws JsonException
* @throws ServiceException
*/
public function updatePreferencesById(int $userId, UserPreferences $userPreferences): int
{
return $this->userRepository->updatePreferencesById($userId, $userPreferences);
$affected = $this->userRepository->updatePreferencesById($userId, $userPreferences);

// An update whose WHERE matched nothing has saved nothing. Reporting success for it told
// the user their preferences had been stored — and the caller then refreshed the session
// with them, so the page even looked right until the next sign-in.
if ($affected === 0) {
throw ServiceException::error(__u('Error while updating the preferences'));
}

return $affected;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,49 @@ public function saveIsReportedAsFailedAndTheSessionIsLeftUntouchedWhenTheUpdateT
);
}

/**
* And the same when the update raises nothing at all but simply matches no row — the user's
* own row deleted from another session, say. It saved nothing, so it is reported as a failure
* and the session is left alone, rather than the page showing settings that exist nowhere.
*
* @throws ContainerExceptionInterface
* @throws Exception
* @throws NotFoundExceptionInterface
*/
#[Test]
public function saveIsReportedAsFailedWhenTheUpdateMatchesNoRow()
{
$this->databaseQueryResolver = function (QueryData $queryData): QueryResult {
$statement = $queryData->getQuery()->getStatement();

if (str_starts_with($statement, 'UPDATE') && str_contains($statement, 'preferences')) {
return new QueryResult([], 0, 0);
}

return new QueryResult([], 1, 100);
};

$container = $this->buildContainer(
IntegrationTestCase::buildRequest(
'post',
'index.php',
['r' => 'userSettingsGeneral/save'],
self::fullPreferencesFields()
)
);

IntegrationTestCase::runApp($container);

$this->expectOutputString(
'{"status":"ERROR","description":"Error while updating the preferences","data":null}'
);

self::assertNull(
$this->refreshedSessionUserData,
'the session must not be refreshed with preferences that were never actually written'
);
}

/**
* Registers a resolver that captures the bind values of the UPDATE issued against the
* preferences column into {@see $capturedUpdateBindValues}. Every other query on the
Expand Down
24 changes: 24 additions & 0 deletions tests/Unit/Application/User/Services/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,30 @@ public function testUpdatePreferencesById()
$this->assertEquals(10, $out);
}

/**
* A user whose row is no longer there has not had their preferences saved, and saying so beats
* reporting success — the caller refreshes the session with what it thinks was stored, so
* without this the page looks right until the next sign-in throws it away.
*
* @throws Exception
* @throws ServiceException
*/
public function testUpdatePreferencesByIdOfAUserThatIsNotThereIsReported()
{
$userPreferences = UserDataGenerator::factory()->buildUserPreferencesData();

$this->userRepository
->expects($this->once())
->method('updatePreferencesById')
->with(100, $userPreferences)
->willReturn(0);

$this->expectException(ServiceException::class);
$this->expectExceptionMessage('Error while updating the preferences');

$this->user->updatePreferencesById(100, $userPreferences);
}

public function testGetAll()
{
$user = UserDataGenerator::factory()->buildUserData();
Expand Down