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.

Sign up for GitHub

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

LEAF 4455 metadata and userMetadata reading #2615

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

LEAF 4455 metadata and userMetadata reading #2615

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d3a1a1f
LEAF 4455 data.metadata read for forms and report builder cells
aerinkayne Oct 18, 2024
1c63e27
Merge branch 'master' into enhance/LEAF-4455/metadata_read_all
aerinkayne Oct 23, 2024
37614a9
LEAF 4455 action_history table read metadata
aerinkayne Oct 30, 2024
d1f3ee0
LEAF 4455 get updates from master, resolve submodule conflicts
aerinkayne Nov 27, 2024
f015626
LEAF 4455 records and notes userMetadata read
aerinkayne Dec 2, 2024
e1ea6a5
LEAF 4455 display val logic consistency, read approverName, fix appro…
aerinkayne Dec 2, 2024
b197710
LEAF 4455 read metadata for resolvedBy, remove VAMC init for that lookup
aerinkayne Dec 2, 2024
f2cec1b
LEAF 4455 metadata read, dep -2 requestor followup steps
aerinkayne Dec 2, 2024
1911d7a
Revert "LEAF 4455 metadata read, dep -2 requestor followup steps"
aerinkayne Dec 4, 2024
39b09e3
LEAF 4455 keep lookup if pending action, use metadata if avail for mo…
aerinkayne Dec 4, 2024
a60a75a
LEAF 4455 update q to use record usermetadata instead of join
aerinkayne Dec 5, 2024
f9b7b15
LEAF 4455 use metadata info for comment when setting initiator
aerinkayne Dec 5, 2024
56140ce
LEAF 4455 Report Builder, userID instead of null for inactive accounts
aerinkayne Dec 5, 2024
4955503
LEAF 4455 add actionType to filterData for cancelled by display,
aerinkayne Dec 5, 2024
4b0b014
LEAF 4455 revert view reports change except for join call, handle dis…
aerinkayne Dec 6, 2024
f3cfc8a
LEAF 4455 remove unneeded sql trim for comparison
aerinkayne Dec 6, 2024
a5ac942
LEAF 4455 revert -2 text update since this info is already elsewhere
aerinkayne Dec 9, 2024
a6ea261
LEAF 4455 keep initiatorName query distinct from general records query
aerinkayne Dec 9, 2024
f8c11b7
LEAF 4455 inactive user verbiage, simplify empty metadata check
aerinkayne Dec 10, 2024
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
124 changes: 61 additions & 63 deletions LEAF_Request_Portal/sources/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,15 +573,16 @@ public function getIndicator($indicatorID, $series, $recordID = null, $parseTemp
$form[$idx]['value'] = $this->fileToArray($data[0]['data']);
$form[$idx]['raw'] = $data[0]['data'];
}
// special handling for org chart data types
// special handling for org chart data types (request header questions, edited report builder cells)
else if ($data[0]['format'] == 'orgchart_employee'
&& !empty($data[0]['data']))
{
$empRes = $this->employee->lookupEmpUID($data[0]['data']);
if (!empty($empRes)) {
$form[$idx]['displayedValue'] = "{$empRes[0]['firstName']} {$empRes[0]['lastName']}";
} else {
$form[$idx]['displayedValue'] = '';
$form[$idx]['displayedValue'] = '';
if (isset($data[0]['metadata'])) {
$orgchartInfo = json_decode($data[0]['metadata'], true);
if(trim("{$orgchartInfo['firstName']} {$orgchartInfo['lastName']}") !== "") {
$form[$idx]['displayedValue'] = "{$orgchartInfo['firstName']} {$orgchartInfo['lastName']}";
}
}
}
else if ($data[0]['format'] == 'orgchart_position'
Expand Down Expand Up @@ -1008,9 +1009,9 @@ public function getRecordInfo($recordID)
);
}

$dir = new VAMC_Directory;
$user = $dir->lookupLogin($res[0]['userID']);
$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'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

{$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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That doesn't resolve the issue I brought up. The efficiency wasn't the concern, it was the repeated code.


$data = array('name' => $name,
'service' => $res[0]['service'],
Expand Down Expand Up @@ -2605,16 +2606,14 @@ public function getCustomData(array $recordID_list, string|null $indicatorID_lis
}
}
break;
case 'orgchart_employee':
$empRes = $this->employee->lookupEmpUID($item['data']);
if (isset($empRes[0]))
{
$item['data'] = "{$empRes[0]['firstName']} {$empRes[0]['lastName']}";
$item['dataOrgchart'] = $empRes[0];
}
else
{
$item['data'] = '';
case 'orgchart_employee': //report builder cells
$item['data'] = '';
if (isset($item['metadata'])) {
Pelentan marked this conversation as resolved.
Show resolved Hide resolved
$orgchartInfo = json_decode($item['metadata'], true);
if(trim("{$orgchartInfo['firstName']} {$orgchartInfo['lastName']}") !== "") {
$item['data'] = "{$orgchartInfo['firstName']} {$orgchartInfo['lastName']}";
$item['dataOrgchart'] = $orgchartInfo;
}
}
break;
case 'orgchart_position':
Expand Down Expand Up @@ -2732,28 +2731,27 @@ public function getActionComments(int $recordID): array
} else {
$vars = array(':recordID' => $recordID);

$sql = 'SELECT actionTextPasttense, comment, time, userID
$sql = 'SELECT actionTextPasttense, comment, time, userID, userMetadata
FROM action_history
LEFT JOIN dependencies USING (dependencyID)
LEFT JOIN actions USING (actionType)
WHERE recordID = :recordID
AND comment != ""
UNION
SELECT "Note Added", note, timestamp, userID
SELECT "Note Added", note, timestamp, userID, userMetadata
FROM notes
WHERE recordID = :recordID
AND deleted IS NULL
ORDER BY time DESC';

$res = $this->db->prepared_query($sql, $vars);

$dir = new VAMC_Directory;

$total = count($res);

for ($i = 0; $i < $total; $i++) {
$user = $dir->lookupLogin($res[$i]['userID']);
$name = isset($user[0]) ? "{$user[0]['Fname']} {$user[0]['Lname']}" : $res[$i]['userID'];
$userMetadata = json_decode($res[$i]['userMetadata'], true);
$name = isset($userMetadata) && trim("{$userMetadata['firstName']} {$userMetadata['lastName']}") !== "" ?
"{$userMetadata['firstName']} {$userMetadata['lastName']}" : $res[$i]['userID'];

$res[$i]['name'] = $name;
}

Expand Down Expand Up @@ -2905,11 +2903,9 @@ public function setInitiator($recordID, $userID)
userID=:userID, userMetadata=:userMetadata
WHERE recordID=:recordID', $vars);

// write log entry
$dir = new VAMC_Directory;

$user = $dir->lookupLogin($userID);
$name = isset($user[0]) ? "{$user[0]['Fname']} {$user[0]['Lname']}" : $userID;
$newInitiatorInfo = json_decode($newInitiatorMetadata, true);
$name = "{$newInitiatorInfo['firstName']} {$newInitiatorInfo['lastName']}";

$actionUserID = $this->login->getUserID();
$actionUserMetadata = $this->employee->getInfoForUserMetadata($actionUserID, false);
Expand Down Expand Up @@ -3580,7 +3576,6 @@ public function query(string $inQuery): mixed
$joinActionHistory = false;
$joinRecordResolutionData = false;
$joinRecordResolutionBy = false;
$joinInitiatorNames = false;
$joinUnfilledDependencies = false;
if (isset($query['joins']))
{
Expand Down Expand Up @@ -3627,10 +3622,6 @@ public function query(string $inQuery): mixed
case 'recordResolutionBy':
$joinRecordResolutionBy = true;

break;
case 'initiatorName':
$joinInitiatorNames = true;

break;
case 'destructionDate':
$joinRecordResolutionData = true;
Expand Down Expand Up @@ -3712,14 +3703,23 @@ public function query(string $inQuery): mixed
WHERE format = 'orgchart_employee') rj_OCEmployeeData ON (lj_data.indicatorID = rj_OCEmployeeData.indicatorID) ";
}

if ($joinInitiatorNames)
{
$joins .= "LEFT JOIN (SELECT userName, lastName, firstName FROM {$this->oc_dbName}.employee) lj_OCinitiatorNames ON records.userID = lj_OCinitiatorNames.userName ";
}
//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 *,
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

IF(
JSON_EXTRACT(`userMetadata`, "$.userName") != "",
TRIM(BOTH \'\"\' FROM JSON_EXTRACT(`userMetadata`, "$.firstName")), "(inactive account)"
) AS `firstName`,
IF(
JSON_EXTRACT(`userMetadata`, "$.userName") != "",
TRIM(BOTH \'\"\' FROM JSON_EXTRACT(`userMetadata`, "$.lastName")), `userID`
) AS `lastName`
FROM `records` ' . $joins . ' WHERE ' . $conditions . $sort . $limit;

if(isset($_GET['debugQuery'])) {
if($this->login->checkGroup(1)) {
$debugQuery = str_replace(["\r", "\n","\t", "%0d","%0a","%09","%20", ";"], ' ', 'SELECT * FROM records ' . $joins . 'WHERE ' . $conditions . $sort . $limit);
$debugQuery = str_replace(["\r", "\n","\t", "%0d","%0a","%09","%20", ";"], ' ', $resSQL);
$debugVars = [];
foreach($vars as $key => $value) {
if(strpos($key, ':data') !== false
Expand All @@ -3733,17 +3733,14 @@ public function query(string $inQuery): mixed

header('X-LEAF-Query: '. str_replace(array_keys($debugVars), $debugVars, $debugQuery));

return $res = $this->db->prepared_query('EXPLAIN SELECT * FROM records
' . $joins . '
WHERE ' . $conditions . $sort . $limit, $vars);
return $res = $this->db->prepared_query('EXPLAIN ' . $resSQL, $vars);
}
else {
return XSSHelpers::scrubObjectOrArray(json_decode(html_entity_decode(html_entity_decode($_GET['q'])), true));
}
}
$res = $this->db->prepared_query('SELECT * FROM records
' . $joins . '
WHERE ' . $conditions . $sort . $limit, $vars);

$res = $this->db->prepared_query($resSQL, $vars);

$data = array();
$recordIDs = '';
Expand Down Expand Up @@ -3819,17 +3816,15 @@ public function query(string $inQuery): mixed

if ($joinActionHistory)
{
$dir = new VAMC_Directory;

$actionHistorySQL =
'SELECT recordID, stepID, userID, time, description,
'SELECT recordID, stepID, userID, userMetadata, time, description,
actionTextPasttense, actionType, comment
FROM action_history
LEFT JOIN dependencies USING (dependencyID)
LEFT JOIN actions USING (actionType)
WHERE recordID IN (' . $recordIDs . ')
UNION
SELECT recordID, "-5", userID, timestamp, "Note Added",
SELECT recordID, "-5", userID, userMetadata, timestamp, "Note Added",
"Note Added", "LEAF_note", note
FROM notes
WHERE recordID IN (' . $recordIDs . ')
Expand All @@ -3839,8 +3834,10 @@ public function query(string $inQuery): mixed
$res2 = $this->db->prepared_query($actionHistorySQL, array());
foreach ($res2 as $item)
{
$user = $dir->lookupLogin($item['userID'], true);
$name = isset($user[0]) ? "{$user[0]['Fname']} {$user[0]['Lname']}" : $res[0]['userID'];
$userMetadata = json_decode($item['userMetadata'], true);
$name = isset($userMetadata) && trim("{$userMetadata['firstName']} {$userMetadata['lastName']}") !== "" ?
"{$userMetadata['firstName']} {$userMetadata['lastName']}" : $item['userID'];

$item['approverName'] = $name;

$data[$item['recordID']]['action_history'][] = $item;
Expand Down Expand Up @@ -3878,9 +3875,7 @@ public function query(string $inQuery): mixed
}

if ($joinRecordResolutionBy === true) {
$dir = new VAMC_Directory;

$recordResolutionBySQL = "SELECT recordID, action_history.userID as resolvedBy, action_history.stepID, action_history.actionType
$recordResolutionBySQL = "SELECT recordID, action_history.userID as resolvedBy, action_history.userMetadata, action_history.stepID, action_history.actionType
FROM action_history
LEFT JOIN records USING (recordID)
INNER JOIN workflow_routes USING (stepID)
Expand All @@ -3895,8 +3890,9 @@ public function query(string $inQuery): mixed
$res2 = $this->db->prepared_query($recordResolutionBySQL, array());

foreach ($res2 as $item) {
$user = $dir->lookupLogin($item['resolvedBy'], true);
$nameResolved = isset($user[0]) ? "{$user[0]['Lname']}, {$user[0]['Fname']} " : $item['resolvedBy'];
$userMetadata = json_decode($item['userMetadata'], true);
$nameResolved = isset($userMetadata) && trim("{$userMetadata['firstName']} {$userMetadata['lastName']}") !== "" ?
"{$userMetadata['firstName']} {$userMetadata['lastName']} " : $item['resolvedBy'];
$data[$item['recordID']]['recordResolutionBy']['resolvedBy'] = $nameResolved;
}
}
Expand Down Expand Up @@ -4308,14 +4304,15 @@ private function buildFormTree($id, $series = null, $recordID = null, $parseTemp
{
$var = array(':series' => (int)$series,
':recordID' => (int)$recordID, );
$res2 = $this->db->prepared_query('SELECT data, timestamp, indicatorID, groupID, userID FROM data
$res2 = $this->db->prepared_query('SELECT data, metadata, timestamp, indicatorID, groupID, userID FROM data
LEFT JOIN indicator_mask USING (indicatorID)
WHERE indicatorID IN (' . $indicatorList . ') AND series=:series AND recordID=:recordID', $var);

foreach ($res2 as $resIn)
{
$idx = $resIn['indicatorID'];
$data[$idx]['data'] = isset($resIn['data']) ? $resIn['data'] : '';
$data[$idx]['metadata'] = isset($resIn['metadata']) ? $resIn['metadata'] : null;
$data[$idx]['timestamp'] = isset($resIn['timestamp']) ? $resIn['timestamp'] : 0;
$data[$idx]['groupID'] = isset($resIn['groupID']) ? $resIn['groupID'] : null;
$data[$idx]['userID'] = isset($resIn['userID']) ? $resIn['userID'] : '';
Expand Down Expand Up @@ -4382,14 +4379,15 @@ private function buildFormTree($id, $series = null, $recordID = null, $parseTemp
$child[$idx]['value'] = $this->fileToArray($data[$idx]['data']);
}

// special handling for org chart data types
// special handling for org chart data types (request subquestions / child)
if ($field['format'] == 'orgchart_employee')
{
$empRes = $this->employee->lookupEmpUID($data[$idx]['data']);
$child[$idx]['displayedValue'] = '';
if (isset($empRes[0]))
{
$child[$idx]['displayedValue'] = ($child[$idx]['isMasked']) ? '[protected data]' : "{$empRes[0]['firstName']} {$empRes[0]['lastName']}";
if (isset($data[$idx]['metadata'])) {
$orgchartInfo = json_decode($data[$idx]['metadata'], true);
if(trim("{$orgchartInfo['firstName']} {$orgchartInfo['lastName']}") !== "") {
$child[$idx]['displayedValue'] = "{$orgchartInfo['firstName']} {$orgchartInfo['lastName']}";
}
}
}
if ($field['format'] == 'orgchart_position')
Expand Down
Loading
Loading