LEAF 4455 metadata and userMetadata reading #2615
Conversation
This reverts commit f2cec1b. Keep lookup so that display will reflect current account status
…re info for disabled accts
userID for site search initiator
aerinkayne
requested review from
mgaoVA,
pete-nerantzinis,
Pelentan,
shaneodd,
jampaul3 and
nk2136
LEAF_Request_Portal/sources/Form.php
Outdated
| //backwards compat: userMetadata properties are empty for accounts that were inactive when prior md values were updated. | ||
| //userMetadata alternatives here prevent the initiator field from displaying 'null, null' if metadata is empty. | ||
| //Handled here due to high customization of view_reports, view_search and other reports | ||
| $resSQL = 'SELECT *, |
There was a problem hiding this comment.
This concerns me. Not because of the select *, but because of the amount of processing it has to do on each line. How much is this going to slow down db responses, especially during the middle of the day? It might be better to do the pull for * first, process it on the machine designed for these operations, and then reach back out for the data it needs in addition.
There was a problem hiding this comment.
It looks like it might have added 10-25% to a 'select * ' query. Going to rework this to keep the initiatorNames "join" logic, but only use it in a backwards compatible way to add the additional SQL if it's needed. This will keep queries where initiator information is not requested the same speed, but still use json instead of the join when it is (where it should be considerably faster)
| $name = isset($user[0]) ? "{$user[0]['Fname']} {$user[0]['Lname']}" : $res[0]['userID']; | ||
| $userMetadata = json_decode($res[0]['userMetadata'], true); | ||
| $name = isset($userMetadata) && trim("{$userMetadata['firstName']} {$userMetadata['lastName']}") !== "" ? | ||
| "{$userMetadata['firstName']} {$userMetadata['lastName']}" : $res[0]['userID']; |
There was a problem hiding this comment.
{$userMetadata['firstName']} {$userMetadata['lastName']} gets used a lot in this scope. Both as a variable and as a determinant. I'm thinking the first time it comes up, two variables should be created: A string with the value and a binary for if it exists.
There was a problem hiding this comment.
I simplified the check for an empty value to just the lastName. PHP does not seem to care, but I'd still rather check if it's set before assessing the property.
There was a problem hiding this comment.
That doesn't resolve the issue I brought up. The efficiency wasn't the concern, it was the repeated code.
| $res[$i]['description'] = $res[$i]['stepTitle'] . ' (Requestor followup)'; | ||
| $res[$i]['approverName'] = '(Requestor followup)'; | ||
| $approverMetadata = json_decode($res[$i]['userMetadata'], true); | ||
| $nameInfo = isset($approverMetadata) && trim($approverMetadata['firstName'] . " " . $approverMetadata['lastName'] ) !== '' ? |
Pelentan
added
the
With QA
Uses data saved in the new metadata and userMetadata columns to replace display related orgchart lookups.
Lookups are still used for requests pending direct action and for emailing.
server side:
Replaces lookupLogin calls with available metadata.
Replaces an orgchart join when getting initiator information with Report Builder queries
front:
data.metadata
Form print view, Report Builder. Display of user info associated with orgchart employee format data entries.
For orgchart_employee format entries with metadata, the entered user name will continue to display even if the associated account is disabled. Accounts disabled prior to metadata entry will continue to display disabled account information.
action_history.userMetadata
Request history modal, Report Builder (Approval History, Resolved By, Cancellation Date). Display of user names associated with workflow and admin actions.
notes.userMetadata
Notes panel. Request history modal (notes). Display of user names.
records.userMetadata
Form print view. Some workflow form fields (text only). Report Builder Initiator queries.
The previous join used for initiator information looked up disabled accounts. Records with empty records.userMetadata will display userID, (inactive account)
Testing / Impact
These updates should not impact access but do have high visibility.
Confirm no unusual function or display changes.
See Jira LEAF-4455 for more details about the above listed areas.
Associated Test DB branch
department-of-veterans-affairs/LEAF-Automated-Tests#21
This brings the dev boilerplate up to date with a version containing all metadata columns and adds appropriate metadata values to initially inserted records