Skip to content

Commit

Permalink
Added note delete to permissions check and fixed client delete/restor…
Browse files Browse the repository at this point in the history
…e [SLE-192]
  • Loading branch information
samuelgfeller committed Jan 24, 2024
1 parent e212112 commit ab64617
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
namespace App\Application\Action\Client\Ajax;

use App\Application\Renderer\JsonEncoder;
use App\Domain\Client\Exception\InvalidClientFilterException;
use App\Domain\Client\Service\ClientUtilFinder;
use Fig\Http\Message\StatusCodeInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;

Expand All @@ -31,19 +29,7 @@ public function __invoke(
ResponseInterface $response,
array $args
): ResponseInterface {
try {
$dropdownOptions = $this->clientUtilFinder->findClientDropdownValues();
} catch (InvalidClientFilterException $invalidClientFilterException) {
return $this->jsonEncoder->encodeAndAddToResponse(
$response,
// Response format tested in PostFilterProvider.php
[
'status' => 'error',
'message' => $invalidClientFilterException->getMessage(),
],
StatusCodeInterface::STATUS_UNPROCESSABLE_ENTITY
);
}
$dropdownOptions = $this->clientUtilFinder->findClientDropdownValues();

return $this->jsonEncoder->encodeAndAddToResponse($response, $dropdownOptions);
}
Expand Down
18 changes: 11 additions & 7 deletions src/Domain/Client/Repository/ClientFinderRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public function findClientsWithResultAggregate(array $whereArray = ['client.dele
'client_status_name' => 'client_status.name',
])// Multiple joins doc: https://book.cakephp.org/4/en/orm/query-builder.html#adding-joins
->join([
// `user` is alias and has to be the same as $this->clientListAggregateSelectFields
'user' => ['table' => 'user', 'type' => 'LEFT', 'conditions' => 'client.user_id = user.id'],
'client_status' => [
'table' => 'client_status',
Expand All @@ -69,17 +68,21 @@ public function findClientsWithResultAggregate(array $whereArray = ['client.dele
* otherwise null.
*
* @param string|int $id
* @param bool $includeDeleted
*
* @return ClientData
*/
public function findClientById(string|int $id): ClientData
public function findClientById(string|int $id, bool $includeDeleted = false): ClientData
{
$query = $this->queryFactory->selectQuery()->select(['*'])->from('client')->where(
['deleted_at IS' => null, 'id' => $id]
['id' => $id]
);
$postRow = $query->execute()->fetch('assoc') ?: [];
if ($includeDeleted === false) {
$query->andWhere(['deleted_at IS' => null]);
}
$clientRow = $query->execute()->fetch('assoc') ?: [];

return new ClientData($postRow);
return new ClientData($clientRow);
}

/**
Expand Down Expand Up @@ -123,7 +126,6 @@ public function findClientAggregateByIdIncludingDeleted(int $id): ClientReadResu
'note_updated_at' => 'note.updated_at',
])
->join([
// `user` is alias and has to be the same as $this->clientListAggregateSelectFields
'user' => ['table' => 'user', 'type' => 'LEFT', 'conditions' => 'client.user_id = user.id'],
'client_status' => [
'table' => 'client_status',
Expand All @@ -133,7 +135,9 @@ public function findClientAggregateByIdIncludingDeleted(int $id): ClientReadResu
'note' => [
'table' => 'note',
'type' => 'LEFT',
'conditions' => 'client.id = note.client_id AND note.deleted_at IS NULL AND note.is_main = 1',
// If there is a deleted main note and a non-deleted one, it creates a conflict,
// but this should not happen as there is no way to delete the main note.
'conditions' => 'client.id = note.client_id AND note.is_main = 1',
],
])
->andWhere(
Expand Down
30 changes: 30 additions & 0 deletions src/Domain/Client/Repository/ClientUpdaterRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,34 @@ public function updateClient(array $data, int $clientId): bool

return $query->execute()->rowCount() > 0;
}

/**
* Restore all notes from the client with the same most recent deleted_at date.
*
* @param int $clientId
*
* @return bool
*/
public function restoreNotesFromClient(int $clientId): bool
{
// Find the most recent deleted_at date for the notes of the given client
$mostRecentDeletedAt = $this->queryFactory->selectQuery()
->select('MAX(deleted_at) as max_deleted_at')
->from('note')
->where(['client_id' => $clientId])
->execute()->fetch('assoc')['max_deleted_at'];

if (!$mostRecentDeletedAt) {
// No notes found for the given client
return false;
}

// Restore all notes with the most recent deleted_at date
$query = $this->queryFactory->updateQuery()
->update('note')
->set(['deleted_at' => null])
->where(['client_id' => $clientId, 'deleted_at' => $mostRecentDeletedAt]);

return $query->execute()->rowCount() > 0;
}
}
11 changes: 6 additions & 5 deletions src/Domain/Client/Service/ClientFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
use App\Domain\Client\Repository\ClientStatus\ClientStatusFinderRepository;
use App\Domain\Client\Service\Authorization\ClientPermissionVerifier;
use App\Domain\Client\Service\Authorization\ClientPrivilegeDeterminer;
use App\Domain\Note\Repository\NoteFinderRepository;
use App\Domain\Note\Service\Authorization\NotePermissionVerifier;
use App\Domain\Note\Service\Authorization\NotePrivilegeDeterminer;
use App\Domain\Note\Service\NoteFinder;
use App\Domain\User\Repository\UserFinderRepository;
use App\Domain\User\Service\UserNameAbbreviator;

Expand All @@ -25,7 +25,7 @@ public function __construct(
private UserFinderRepository $userFinderRepository,
private UserNameAbbreviator $userNameAbbreviator,
private ClientStatusFinderRepository $clientStatusFinderRepository,
private NoteFinder $noteFinder,
private NoteFinderRepository $noteFinderRepository,
private ClientPermissionVerifier $clientPermissionVerifier,
private ClientPrivilegeDeterminer $clientPrivilegeDeterminer,
private NotePrivilegeDeterminer $notePrivilegeDeterminer,
Expand Down Expand Up @@ -91,12 +91,13 @@ private function findClientsWhereWithResultAggregate(array $whereArray = ['clien
* Find one client in the database.
*
* @param int $id
* @param mixed $includeDeleted
*
* @return ClientData
*/
public function findClient(int $id): ClientData
public function findClient(int $id, $includeDeleted = false): ClientData
{
return $this->clientFinderRepository->findClientById($id);
return $this->clientFinderRepository->findClientById($id, $includeDeleted);
}

/**
Expand Down Expand Up @@ -145,7 +146,7 @@ public function findClientReadAggregate(int $clientId): ClientReadResult
false
) ? Privilege::CR->name : Privilege::N->name;

$clientResultAggregate->notesAmount = $this->noteFinder->findClientNotesAmount($clientId);
$clientResultAggregate->notesAmount = $this->noteFinderRepository->findClientNotesAmount($clientId);

return $clientResultAggregate;
}
Expand Down
6 changes: 6 additions & 0 deletions src/Domain/Client/Service/ClientUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,15 @@ public function updateClient(int $clientId, array $clientValues): array
if (isset($updateData['user_id'])) {
$updateData['assigned_at'] = date('Y-m-d H:i:s');
}

// Update client
$updated = $this->clientUpdaterRepository->updateClient($updateData, $clientId);
if ($updated) {
// If client is undeleted, the notes should also be restored
if (array_key_exists('deleted_at', $updateData) && $updateData['deleted_at'] === null) {
$this->clientUpdaterRepository->restoreNotesFromClient($clientId);
}

$this->userActivityLogger->logUserActivity(
UserActivity::UPDATED,
'client',
Expand Down
8 changes: 4 additions & 4 deletions src/Domain/Client/Service/ClientUtilFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ public function __construct(
public function findClientDropdownValues(?int $alreadyAssignedUserId = null): ClientDropdownValuesData
{
$allUsers = $this->userFinderRepository->findAllUsers();
// Filter users that authenticated user is authorized to assign to the client that is being created.
// Filter users, which the authenticated user is authorized to assign to a client.
// Available user roles for dropdown and privilege
$grantedAssignableUsers = [];
foreach ($allUsers as $userData) {
if (// If the user is already assigned to client the value is added so that it's displayed in the dropdown
// for it to be visible in GUI even if select is greyed out
if (// If the user is already assigned to the client, the value is added so that it's displayed in the
// dropdown for it to be visible in the user interface, even if the <select> is greyed out
($alreadyAssignedUserId !== null && $userData->id === $alreadyAssignedUserId)
// Check if authenticated user is allowed to assign the currently iterating user to a client
// Check if the authenticated user is allowed to assign the currently iterating user to a client
|| $this->clientPermissionVerifier->isGrantedToAssignUserToClient($userData->id)
) {
$grantedAssignableUsers[] = $userData;
Expand Down
3 changes: 2 additions & 1 deletion src/Domain/Note/Repository/NoteDeleterRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public function deleteNote(int $id): bool
*/
public function deleteNotesFromClient(int $clientId): bool
{
$query = $this->queryFactory->softDeleteQuery('note')->where(['client_id' => $clientId]);
$query = $this->queryFactory->softDeleteQuery('note')
->where(['client_id' => $clientId, 'deleted_at IS' => null]);

return $query->execute()->rowCount() > 0;
}
Expand Down
7 changes: 6 additions & 1 deletion src/Domain/Note/Repository/NoteFinderRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class NoteFinderRepository
'hidden' => 'note.hidden',
'updated_at' => 'note.updated_at',
'created_at' => 'note.created_at',
'deleted_at' => 'note.deleted_at',
'user_id' => 'note.user_id',
];

Expand Down Expand Up @@ -133,12 +134,16 @@ public function findAllNotesExceptMainWithUserByClientId(int $clientId): array

$query->select(array_merge($this->noteResultFields, ['user_full_name' => $concatName]))
->join(['user' => ['table' => 'user', 'type' => 'LEFT', 'conditions' => 'note.user_id = user.id']])
->join(['client' => ['table' => 'client', 'type' => 'LEFT', 'conditions' => 'note.client_id = client.id']])
->where(
[
// Not unsafe as it's not an expression and thus escaped by querybuilder
'note.client_id' => $clientId,
'note.is_main' => 0,
'note.deleted_at IS' => null,
'OR' => [
'note.deleted_at IS' => null,
'note.deleted_at' => $query->identifier('client.deleted_at'),
],
]
)->orderByDesc('note.created_at');
$resultRows = $query->execute()->fetchAll('assoc') ?: [];
Expand Down
10 changes: 8 additions & 2 deletions src/Domain/Note/Service/Authorization/NotePermissionVerifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ public function __construct(
*
* @param int $isMain
* @param int|null $noteOwnerId optional owner id when main note
* @param int|null $clientOwnerId client owner might become relevant
* @param int|null $clientOwnerId client owner user id might become relevant
* @param int|null $isHidden when note message is hidden
* @param bool $isDeleted note is deleted
* @param bool $log
*
* @return bool
Expand All @@ -32,6 +33,7 @@ public function isGrantedToRead(
?int $noteOwnerId = null,
?int $clientOwnerId = null,
?int $isHidden = null,
bool $isDeleted = false,
bool $log = true
): bool {
if (($loggedInUserId = (int)$this->session->get('user_id')) !== 0) {
Expand All @@ -41,8 +43,12 @@ public function isGrantedToRead(
// Returns array with role name as key and hierarchy as value ['role_name' => hierarchy_int]
// * Lower hierarchy number means higher privileged role
$userRoleHierarchies = $this->userRoleFinderRepository->getUserRolesHierarchies();
// newcomers may see all notes and main notes
// newcomers may see all notes and main notes except deleted ones that only managing advisors can see
if (($authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::NEWCOMER->value])
&& ( // If the note is deleted, authenticated user must be managing advisors or higher
$isDeleted === false
|| $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::MANAGING_ADVISOR->value]
)
&& ( // When hidden is not null or 0, user has to be advisor to read note
in_array($isHidden, [null, 0, false], true)
|| $authenticatedUserRoleHierarchy <= $userRoleHierarchies[UserRole::ADVISOR->value]
Expand Down
18 changes: 15 additions & 3 deletions src/Domain/Note/Service/Authorization/NotePrivilegeDeterminer.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,16 @@ public function getMainNotePrivilege(?int $noteOwnerId, ?int $clientOwnerId): st
* @param int $noteOwnerId
* @param int|null $clientOwnerId
* @param ?int $hidden
* @param bool $noteDeleted
*
* @return string
*/
public function getNotePrivilege(int $noteOwnerId, ?int $clientOwnerId = null, ?int $hidden = null): string
{
public function getNotePrivilege(
int $noteOwnerId,
?int $clientOwnerId = null,
?int $hidden = null,
bool $noteDeleted = false,
): string {
// Check first against the highest privilege, if allowed, directly return otherwise continue down the chain
if ($this->notePermissionVerifier->isGrantedToDelete($noteOwnerId, $clientOwnerId, false)) {
return Privilege::CRUD->name;
Expand All @@ -60,7 +65,14 @@ public function getNotePrivilege(int $noteOwnerId, ?int $clientOwnerId = null, ?
}
// Create must NOT be included here as it's irrelevant on specific notes and has an impact on "READ" privilege as
// read is lower than create in the hierarchy.
if ($this->notePermissionVerifier->isGrantedToRead(0, $noteOwnerId, $clientOwnerId, $hidden, false)) {
if ($this->notePermissionVerifier->isGrantedToRead(
0,
$noteOwnerId,
$clientOwnerId,
$hidden,
$noteDeleted,
false
)) {
return Privilege::R->name;
}

Expand Down
36 changes: 17 additions & 19 deletions src/Domain/Note/Service/NoteFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
namespace App\Domain\Note\Service;

use App\Domain\Authorization\Privilege;
use App\Domain\Client\Repository\ClientFinderRepository;
use App\Domain\Client\Service\Authorization\ClientPermissionVerifier;
use App\Domain\Client\Service\ClientFinder;
use App\Domain\Note\Data\NoteData;
use App\Domain\Note\Data\NoteResultData;
use App\Domain\Note\Repository\NoteFinderRepository;
Expand All @@ -14,7 +15,8 @@
public function __construct(
private NoteFinderRepository $noteFinderRepository,
private NotePrivilegeDeterminer $notePrivilegeDeterminer,
private ClientFinderRepository $clientFinderRepository,
private ClientFinder $clientFinder,
private ClientPermissionVerifier $clientPermissionVerifier,
) {
}

Expand Down Expand Up @@ -70,22 +72,25 @@ private function setNotePrivilegeAndRemoveMessageOfHidden(

// Get client owner id if not given
if ($clientOwnerId === null && $clientId !== null) {
$clientOwnerId = $this->clientFinderRepository->findClientById($clientId)->userId;
$clientOwnerId = $this->clientFinder->findClient($clientId)->userId;
}

foreach ($notes as $noteResultData) {
// Privilege only create possible if user may not see the note but may create one
// Privilege "only create" possible if user may not see the note but may create one
$noteResultData->privilege = $this->notePrivilegeDeterminer->getNotePrivilege(
(int)$noteResultData->userId,
$clientOwnerId,
$noteResultData->hidden,
(bool)$noteResultData->deletedAt
);
// If not allowed to read
if (!str_contains($noteResultData->privilege, 'R')) {
// Change message of note to lorem ipsum
$noteResultData->message = substr($randomText, 0, strlen($noteResultData->message ?? ''));
// Remove line breaks and extra spaces from string
$noteResultData->message = preg_replace('/\s\s+/', ' ', $noteResultData->message);
// If user has no read right, set note to hidden
$noteResultData->hidden = 1;
}
}
}
Expand Down Expand Up @@ -119,9 +124,14 @@ public function findAllNotesFromClientExceptMain(int $clientId): array
// meaning the reference is passed and changes are made on the original reference that can be used further
// https://www.php.net/manual/en/language.oop5.references.php; https://stackoverflow.com/a/65805372/9013718
$this->setNotePrivilegeAndRemoveMessageOfHidden($allNotes, null, $clientId);
// Add client message as last note (it's always the "oldest" as it's the same age as the client entry itself)
$clientData = $this->clientFinderRepository->findClientById($clientId);
if (!empty($clientData->clientMessage)) {

// Add client message as last note (it's always the "oldest" as it's the same age as the client entry itself)
$clientData = $this->clientFinder->findClient($clientId, true);
// The authorization for each note is verified, but the client message is added to the request
// separately
if (!empty($clientData->clientMessage)
&& $this->clientPermissionVerifier->isGrantedToRead($clientData->userId, $clientData->deletedAt)
) {
$clientMessageNote = new NoteResultData();
$clientMessageNote->message = $clientData->clientMessage;
// The "userFullName" has to be the client itself as it's his client_message that is being displayed as note
Expand All @@ -135,16 +145,4 @@ public function findAllNotesFromClientExceptMain(int $clientId): array

return $allNotes;
}

/**
* Return the number of notes attached to a client.
*
* @param int $clientId
*
* @return int
*/
public function findClientNotesAmount(int $clientId): int
{
return $this->noteFinderRepository->findClientNotesAmount($clientId);
}
}
Loading

0 comments on commit ab64617

Please sign in to comment.