-
Notifications
You must be signed in to change notification settings - Fork 3
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
Changes from 1 commit
9c938f8
dd932af
10a7ab2
b1e972e
355bfcc
4df2543
d1f7a14
683033b
0718072
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,4 +60,48 @@ public function findByCaseNumber(string $caseNumber) | |
->getQuery() | ||
->getResult(); | ||
} | ||
|
||
public function getNewClientsForExistingDeputiesArray(): array | ||
{ | ||
$conn = $this->getEntityManager()->getConnection(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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::bigint = u.deputy_uid | ||
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(); | ||
$this->_em->getFilters()->enable('softdeleteable'); | ||
|
||
return $result; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Case,ClientFirstname,ClientSurname,ClientAddress1,ClientAddress2,ClientAddress3,ClientAddress4,ClientAddress5,ClientPostcode,DeputyUid,DeputyFirstname,DeputySurname,DeputyAddress1,DeputyAddress2,DeputyAddress3,DeputyAddress4,DeputyAddress5,DeputyPostcode,ReportType,MadeDate,OrderType,CoDeputy,Hybrid,CourtOrderUid | ||
61111001,Client 1,Clientsurname,Client Road,,,,,CL1 3NT,700761111001,Lay-OPG102-User-1,User,ABC Road,,,,,AB1 2CD,OPG102,2011-04-14,pfa,no,SINGLE,45123468745 | ||
12345673,Client 2,SecondClient,64 Zoo Lane,Vrombaut,Beebies,London,,CL1 3NT,700761111001,Julie,DUCK,ABC Road,,,,,D3P UTY,OPG102,2011-04-15,pfa,no,SINGLE,34518742346 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't appear in logs unless you use our verboseLogger (private LoggerInterface $verboseLogger) as normal logger is set to warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same thing, I think, apart from this class happens to use a different variable name? I can change the variable names from $logger to $verboseLogger to make it clearer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe Symfony uses the variable name to determine which logger to actually inject into the service. So if this message needs logging, I would suggest changing it to warning.
Changing the logger to the verbose logger will require reviewing the rest of the logging in this file to make sure it is appropriate for logging (i.e. no personally identifiable information)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it uses the variable name and there's a setup for it where we call it verbose logger. If you search for verboseLogger in the repo you should see other setups for it. I think it should be fine to replace everything in the file with it as it's just logging errors elsewhere and I don't think the verbose logger actually logs more stuff, it's just the log level that changes.. It's a bit confusing, we did it this way so we didn't have to review all our notices across the app but in the long run it would probably have been easier to set base logger to notice and review everything... oh well.