Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DDLS-360 Add new clients found in the LayDeputy csv #1771

Merged
merged 9 commits into from
Dec 19, 2024
12 changes: 7 additions & 5 deletions api/app/src/Command/ProcessLayCSVCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class ProcessLayCSVCommand extends Command
public function __construct(
private readonly S3Client $s3,
private readonly ParameterBagInterface $params,
private readonly LoggerInterface $logger,
private readonly LoggerInterface $verboseLogger,
private readonly CSVDeputyshipProcessing $csvProcessing,
private readonly PreRegistrationRepository $preReg,
) {
Expand Down Expand Up @@ -107,7 +107,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
$logMessage = sprintf($logMessage, $layReportFile, $bucket);

$this->logger->error($logMessage);
$this->verboseLogger->error($logMessage);
$this->cliOutput->writeln(sprintf('%s - failure - %s', self::JOB_NAME, $logMessage));

return Command::FAILURE;
Expand All @@ -118,7 +118,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
if (!unlink($fileLocation)) {
$logMessage = sprintf('Unable to delete file %s.', $fileLocation);

$this->logger->error($logMessage);
$this->verboseLogger->error($logMessage);
$this->cliOutput->writeln(
sprintf(
'%s - failure - (partial) %s Output: %s',
Expand Down Expand Up @@ -164,7 +164,7 @@ private function csvToArray(string $fileName): array
} catch (\RuntimeException $e) {
$logMessage = sprintf('Error processing CSV: %s', $e->getMessage());

$this->logger->error($logMessage);
$this->verboseLogger->error($logMessage);
$this->cliOutput->writeln(self::JOB_NAME.' - failure - '.$logMessage);
}

Expand All @@ -179,11 +179,13 @@ private function process(mixed $data): bool
$chunks = array_chunk($data, self::CHUNK_SIZE);

foreach ($chunks as $index => $chunk) {
$this->logger->notice(sprintf('Uploading chunk with Id: %s', $index));
$this->verboseLogger->notice(sprintf('Uploading chunk with Id: %s', $index));

$result = $this->csvProcessing->layProcessing($chunk, $index);
$this->storeOutput($result);
}
$this->verboseLogger->notice('Directly creating any new Lay clients for active deputies');
$this->csvProcessing->layProcessingHandleNewMultiClients();

return true;
}
Expand Down
2 changes: 1 addition & 1 deletion api/app/src/Entity/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ public function setAddress5(?string $address5): Client
return $this;
}

public function getAddress5(): string
public function getAddress5(): ?string
{
return $this->address5;
}
Expand Down
18 changes: 17 additions & 1 deletion api/app/src/Repository/ClientRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,22 @@ public function findByCaseNumber(string $caseNumber): ?Client
->getOneOrNullResult();
}

public function findByCaseNumberIncludingDischarged(string $caseNumber): ?Client
{
$filter = $this->_em->getFilters()->getFilter('softdeleteable');
$filter->disableForEntity(Client::class);

$client = $this
->getEntityManager()
->createQuery('SELECT c FROM App\Entity\Client c WHERE LOWER(c.caseNumber) = LOWER(:caseNumber)')
->setParameter('caseNumber', $caseNumber)
->getOneOrNullResult();

$this->_em->getFilters()->enable('softdeleteable');

return $client;
}

public function findByIdIncludingDischarged(int $id): ?Client
{
/** @var SoftDeleteableFilter $filter */
Expand All @@ -152,7 +168,7 @@ public function findByFiltersWithCounts(
$q,
$offset,
$limit,
$id
$id,
) {
// BASE QUERY BUILDER with filters (for both count and results)
$qb = $this->createQueryBuilder('c');
Expand Down
43 changes: 43 additions & 0 deletions api/app/src/Repository/PreRegistrationRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,47 @@ public function findByCaseNumber(string $caseNumber)
->getQuery()
->getResult();
}

public function getNewClientsForExistingDeputiesArray(): array
{
$conn = $this->getEntityManager()->getConnection();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would need to disable the softdeletable filter here before running the query.
The reason being is that when we probably should include the discharged clients in the check so that if they appear in the CSV for this particular deputy, they aren't recreated again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we're going for strings as that's whats on sirius for deputy uid I'd do the cast the other way round and cast the bigint (that we're ditching eventually) as a string (or varchar or whatever it's called in database language) instead

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it just a case of removing the softdeletable filter Gugs on line 103? I think the SQL should bring back all the discharged ones fine but then the filter gets rid of them right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you have to disable it before the query, run the query (it will now ignore the filter), and then enable it again afterwards so it doesn't affect any other queries afterwards.

Thinking about it, it's not the greatest implementation because if you've got a query that is going to run for a long time then the filter is disabled for a long time too. Which means there's a higher chance some other query in the app is going to be running and that might be expecting the filter to be enabled.


$newMultiClentsQuery = <<<SQL
SELECT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this query locally, and it does return clients that are already created.
However this isn't an immediate issue as the logic on the LayDeputyshipUploader will only create the client if they don't already exist. Just wanted to comment for awareness if there are any issues with this in the future

pr.client_case_number AS "Case",
pr.client_lastname AS "ClientSurname",
pr.deputy_uid AS "DeputyUid",
pr.deputy_lastname AS "DeputySurname",
pr.deputy_address_1 AS "DeputyAddress1",
pr.deputy_address_2 AS "DeputyAddress2",
pr.deputy_address_3 AS "DeputyAddress3",
pr.deputy_address_4 AS "DeputyAddress4",
pr.deputy_address_5 AS "DeputyAddress5",
pr.deputy_postcode AS "DeputyPostcode",
pr.type_of_report AS "ReportType",
pr.order_date AS "MadeDate",
pr.order_type AS "OrderType",
CASE WHEN pr.is_co_deputy THEN 'yes' ELSE 'no' END AS "CoDeputy",
pr.created_at AS "MadeDate",
pr.hybrid AS "Hybrid",
pr.deputy_firstname AS "DeputyFirstname",
pr.client_firstname AS "ClientFirstname",
pr.client_address_1 AS "ClientAddress1",
pr.client_address_2 AS "ClientAddress2",
pr.client_address_3 AS "ClientAddress3",
pr.client_address_4 AS "ClientAddress4",
pr.client_address_5 AS "ClientAddress5",
pr.client_postcode AS "ClientPostcode"
FROM pre_registration pr
LEFT JOIN dd_user u ON pr.deputy_uid = u.deputy_uid::varchar(30)
LEFT JOIN deputy_case dc ON u.id = dc.user_id
LEFT JOIN client c ON dc.client_id = c.id
WHERE c.case_number != pr.client_case_number
SQL;

$stmt = $conn->executeQuery($newMultiClentsQuery);
$result = $stmt->fetchAllAssociative();

return $result;
}
}
19 changes: 19 additions & 0 deletions api/app/src/Repository/UserRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -419,4 +419,23 @@ public function findDeputyUidsForClient(int $clientId)

return $query->getArrayResult();
}

public function findActiveClientsCountForDeputyUid(string $deputyUid): int
{
$query = $this
->getEntityManager()
->createQuery("SELECT COUNT(u) FROM App\Entity\User u INNER JOIN u.clients c where c.archivedAt IS NULL and c.deletedAt IS NULL AND u.deputyUid = :deputyUid")
->setParameter('deputyUid', $deputyUid);

return $query->getSingleScalarResult();
}

public function findPrimaryUserByDeputyUid(string $deputyUid): ?User
{
$query = $this
->getEntityManager()
->createQuery("SELECT u FROM App\Entity\User u WHERE u.deputyUid = :deputyUid AND u.isPrimary = True")
->setParameter('deputyUid', $deputyUid);
return $query->getSingleResult();
}
}
23 changes: 23 additions & 0 deletions api/app/src/v2/Assembler/ClientAssembler.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use App\v2\DTO\DtoPropertySetterTrait;
use App\v2\DTO\OrganisationDto;
use App\v2\DTO\UserDto;
use App\v2\Registration\DTO\LayDeputyshipDto;
use App\v2\Registration\DTO\OrgDeputyshipDto;

class ClientAssembler
Expand Down Expand Up @@ -131,6 +132,28 @@ public function assembleFromOrgDeputyshipDto(OrgDeputyshipDto $dto)

return $client;
}

public function assembleFromLayDeputyshipDto(LayDeputyshipDto $dto)
{
$client = (new Client())
->setCaseNumber($dto->getCaseNumber())
->setFirstname($dto->getClientFirstname() ?: null)
->setLastname($dto->getClientSurname())
->setAddress($dto->getClientAddress1() ?: null)
->setAddress2($dto->getClientAddress2() ?: null)
->setAddress3($dto->getClientAddress3() ?: null)
->setAddress4($dto->getClientAddress4() ?: null)
->setAddress5($dto->getClientAddress5() ?: null)
->setPostcode($dto->getClientPostcode() ?: null)
->setCourtDate($dto->getOrderDate() ?: null);

if (!empty($dto->getClientPostCode())) {
$client->setPostcode($dto->getClientPostCode());
$client->setCountry('GB'); // postcode given means a UK address is given
}

return $client;
}

private function assembleClientDeputies(array $deputies)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ public function layProcessing(array $data, ?int $chunkId)
return $result;
}

public function layProcessingHandleNewMultiClients(): array
{
$result = $this->layUploader->handleNewMultiClients();
return $result;
}

public function orgProcessing(array $data)
{
$rowCount = count($data);
Expand Down
108 changes: 107 additions & 1 deletion api/app/src/v2/Registration/Uploader/LayDeputyshipUploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@

namespace App\v2\Registration\Uploader;

use App\Entity\Client;
use App\Entity\PreRegistration;
use App\Entity\Report\Report;
use App\Entity\User;
use App\Repository\ReportRepository;
use App\v2\Assembler\ClientAssembler;
use App\v2\Registration\Assembler\SiriusToLayDeputyshipDtoAssembler;
use App\v2\Registration\DTO\LayDeputyshipDto;
use App\v2\Registration\DTO\LayDeputyshipDtoCollection;
use App\v2\Registration\SelfRegistration\Factory\PreRegistrationCreationException;
use App\v2\Registration\SelfRegistration\Factory\PreRegistrationFactory;
use Doctrine\Common\Persistence\Mapping\MappingException;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\NonUniqueResultException;
use Doctrine\ORM\NoResultException;
use Doctrine\ORM\OptimisticLockException;
use Doctrine\ORM\ORMException;
use Psr\Log\LoggerInterface;
Expand All @@ -34,7 +39,9 @@ public function __construct(
private EntityManagerInterface $em,
private ReportRepository $reportRepository,
private PreRegistrationFactory $preRegistrationFactory,
private LoggerInterface $logger
private LoggerInterface $logger,
private SiriusToLayDeputyshipDtoAssembler $assembler,
private ClientAssembler $clientAssembler,
) {
}

Expand Down Expand Up @@ -81,6 +88,105 @@ public function upload(LayDeputyshipDtoCollection $collection): array
];
}

public function handleNewMultiClients(): array
{
$preRegistrationNewClients = $this->em->getRepository(PreRegistration::class)->getNewClientsForExistingDeputiesArray();
$clientsAdded = 0;
$errors = [];

foreach ($preRegistrationNewClients as $preReg) {
$layDeputyshipDto = $this->assembler->assembleFromArray($preReg);
try {
$this->em->beginTransaction();
$user = $this->handleNewUser($layDeputyshipDto);
$client = $this->handleNewClient($layDeputyshipDto, $user);
$this->handleNewReport($layDeputyshipDto, $client);
$this->commitTransactionToDatabase();
++$clientsAdded;
} catch (\Throwable $e) {
$message = sprintf('Error when creating additional client for deputyUID %s for case %s: %s',
$layDeputyshipDto->getDeputyUid(),
$layDeputyshipDto->getCaseNumber(),
str_replace(PHP_EOL, '', $e->getMessage())
);
$this->logger->warning($message);
$errors[] = $message;
continue;
}
}

return [
'new-clients-found' => count($preRegistrationNewClients),
'clients-added' => $clientsAdded,
'errors' => $errors,
];
}

private function handleNewUser(LayDeputyshipDto $dto): ?User
{
try {
$primaryDeputyUser = $this->em->getRepository(User::class)->findPrimaryUserByDeputyUid($dto->getDeputyUid());
} catch (NoResultException|NonUniqueResultException $e) {
throw new \RuntimeException(sprintf('The primary user for deputy UID %s was either missing or not unique', $dto->getDeputyUid()));
}

$users = $this->em->getRepository(User::class)->findBy(['deputyUid' => $dto->getDeputyUid()]);
$newSecondaryUser = clone $primaryDeputyUser;
$newSecondaryUser->setEmail('secondary-'.count($users).'-'.$primaryDeputyUser->getEmail());
$newSecondaryUser->setIsPrimary(false);
$this->em->persist($newSecondaryUser);

return $newSecondaryUser;
}

private function handleNewClient(LayDeputyshipDto $dto, User $newUser): ?Client
{
$existingClient = $this->em->getRepository(Client::class)->findByCaseNumberIncludingDischarged($dto->getCaseNumber());

if ($existingClient instanceof Client) {
foreach ($existingClient->getUsers() as $user) {
if ($user->getDeputyUid() == $newUser->getDeputyUid()) {
throw new \RuntimeException(sprintf('a client with case number %s already exists that is associated with a user with deputy UID %s', $existingClient->getCaseNumber(), $newUser->getDeputyUid()));
}
}
} else {
$newClient = $this->clientAssembler->assembleFromLayDeputyshipDto($dto);
$newClient->addUser($newUser);
$this->em->persist($newClient);
$this->added['clients'][] = $dto->getCaseNumber();
}

return $newClient;
}

private function handleNewReport(LayDeputyshipDto $dto, Client $newClient): ?Report
{
$existingReport = $newClient->getCurrentReport();

if ($existingReport instanceof Report) {
throw new \RuntimeException('report already exists');
} else {
$determinedReportType = PreRegistration::getReportTypeByOrderType($dto->getTypeOfReport(), $dto->getOrderType(), PreRegistration::REALM_LAY);

$reportStartDate = clone $dto->getOrderDate();
$reportEndDate = clone $reportStartDate;
$reportEndDate->add(new \DateInterval('P364D'));

$newReport = new Report(
$newClient,
$determinedReportType,
$reportStartDate,
$reportEndDate,
false
);

$newReport->setClient($newClient);
$this->em->persist($newReport);
}

return $newReport;
}

private function throwExceptionIfDataTooLarge(LayDeputyshipDtoCollection $collection): void
{
if ($collection->count() > self::MAX_UPLOAD) {
Expand Down
Loading
Loading