Skip to content

Commit

Permalink
Merge pull request #2222 from department-of-veterans-affairs/optimize…
Browse files Browse the repository at this point in the history 
…-group-designated

LEAF-4123: Optimize "group designated" hotspot
  • Loading branch information
@Pelentan
Pelentan authored Nov 14, 2023
2 parents 38cc8dd + f20a49f commit 22db4bd
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 32 deletions.
45 changes: 36 additions & 9 deletions LEAF_Request_Portal/sources/Form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1972,7 +1972,7 @@ public function hasDependencyAccess($dependencyID, $details)

/**
* batchUpdateDependencyAccess amends $accessList for specific dependencyIDs to optimize
* performance related to dynamic assignments, such as "person designated by requestor"
* performance related to dynamic assignments, such as "person/group designated by requestor"
*
* @param array $accessList Map of recordID->int of the current user's access. 1 = has access
* @param array $records List of records to process
Expand All @@ -1981,19 +1981,26 @@ public function hasDependencyAccess($dependencyID, $details)
private function batchUpdateDependencyAccess(array $accessList, array $records): array
{
// get sanitized lists for DB query
$indicatorIDs = [];
$recordIDs = [];
$indicatorIDs_pd = [];
$recordIDs_pd = [];
$indicatorIDs_gd = [];
$recordIDs_gd = [];
foreach($records as $dep) {
if($accessList[$dep['recordID']] == 0 && $dep['dependencyID'] == -1) {
$indicatorIDs[] = (int)$dep['indicatorID_for_assigned_empUID'];
$recordIDs[] = (int)$dep['recordID'];
$indicatorIDs_pd[] = (int)$dep['indicatorID_for_assigned_empUID'];
$recordIDs_pd[] = (int)$dep['recordID'];
}

if($accessList[$dep['recordID']] == 0 && $dep['dependencyID'] == -3) {
$indicatorIDs_gd[] = (int)$dep['indicatorID_for_assigned_groupID'];
$recordIDs_gd[] = (int)$dep['recordID'];
}
}

// get the list of records related to dependencyID -1 (person designated by requestor)
if(count($recordIDs) > 0) {
$indicators = implode(',', $indicatorIDs);
$records = implode(',', $recordIDs);
if(count($recordIDs_pd) > 0) {
$indicators = implode(',', array_unique($indicatorIDs_pd));
$records = implode(',', $recordIDs_pd);
$query = "SELECT recordID, `data` FROM `data`
WHERE indicatorID IN ({$indicators})
AND recordID IN ({$records})
Expand All @@ -2012,6 +2019,24 @@ private function batchUpdateDependencyAccess(array $accessList, array $records):
}
}

// get the list of records related to dependencyID -3 (group designated by requestor)
if(count($recordIDs_gd) > 0) {
$indicators = implode(',', array_unique($indicatorIDs_gd));
$records = implode(',', $recordIDs_gd);
$query = "SELECT recordID, `data` FROM `data`
WHERE indicatorID IN ({$indicators})
AND recordID IN ({$records})
AND series=1";
$res = $this->db->prepared_query($query, []);

foreach($res as $record) {
// check if the current users is a member of the designated group
if($this->login->checkGroup($record['data'])) {
$accessList[$record['recordID']] = 1;
}
}
}

return $accessList;
}

Expand Down Expand Up @@ -2197,7 +2222,9 @@ public function checkReadAccess($records)
$temp[$dep['recordID']] = 0;

// Use optimized path for certain dependencyIDs. See batchUpdateDependencyAccess.
if($dep['dependencyID'] != -1) {
if($dep['dependencyID'] != -1 // person designated by requestor
&& $dep['dependencyID'] != -3) // group designated by requestor
{
$temp[$dep['recordID']] = $this->hasDependencyAccess($dep['dependencyID'], $dep) ? 1 : 0;
}

Expand Down
131 changes: 108 additions & 23 deletions LEAF_Request_Portal/sources/FormWorkflow.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ private function getActionableGroupName(int|string|null $groupID): string|null {
}

/**
* includePersonDesignatedApproverData efficiently merges approver data to $srcRecords, for a
* includePersonDesignatedData efficiently merges approver data to $srcRecords, for a
* given list of $pdRecordList and $pdIndicators.
*
* When $skipNames is false, $srcRecords is expected to contain dependencyIDs
Expand Down Expand Up @@ -222,6 +222,97 @@ private function includePersonDesignatedData(array $srcRecords, array $pdRecords
return $srcRecords;
}

/**
* includeGroupDesignatedData efficiently merges approver data to $srcRecords, for a
* given list of $gdRecordList and $gdIndicators.
*
* When $skipNames is false, $srcRecords is expected to contain dependencyIDs
*
* WARNING: This function should only be used to support getRecordsDependencyData() and getActionable().
* Usage in other areas must be carefully reviewed as this retrieves data without
* checking for valid access. The $gdIndicators variable must only contain indicator IDs
* that are related to a "group designated" field AND workflow that utilizes the "group
* designated" feature.
* @param array $srcRecords list of records
* @param array $gdRecordsMap map of record IDs => indicatorID that utilize "group designated"
* @param array $gdIndicator list of indicator IDs mapped to "group designated" fields
* @param bool $skipNames set true to exclude employee lookups
* @return array Amended records
*/
private function includeGroupDesignatedData(array $srcRecords, array $gdRecordsMap, array $gdIndicators, bool $skipNames = false): array
{
// sanitize for use in query
$gdIndicators = array_map(function($x) {
return (int)$x;
}, $gdIndicators);
$gdIndicators = implode(',', $gdIndicators);

$gdRecordIDs = array_keys($gdRecordsMap);
$gdRecordIDs = array_map(function($x) {
return (int)$x;
}, $gdRecordIDs);

$gdRecordIDs = implode(',', $gdRecordIDs);

$query = "SELECT recordID, `data`, `name`, indicatorID FROM `data`
LEFT JOIN indicators USING (indicatorID)
WHERE indicatorID IN ({$gdIndicators})
AND recordID IN ({$gdRecordIDs})
AND series=1
AND `disabled`=0";
$res = $this->db->prepared_query($query, []);

$groupIDs = [];
// create map of recordIDs with "group designated"
$dRecords = [];
foreach($res as $record) {
if(isset($gdRecordsMap[$record['recordID']][$record['indicatorID']])) {
$dRecords[$record['recordID']]['data'] = $record['data'];
$dRecords[$record['recordID']]['name'] = $record['name'];
$groupIDs[$record['data']] = 1;
}
}

$groupNames = [];
if(!$skipNames) {
$groupIDs = array_keys($groupIDs);
$groupIDs = array_map(function($x) {
return (int)$x;
}, $groupIDs);

$groupIDs = implode(',', $groupIDs);
$res = $this->db->prepared_query("SELECT groupID, name FROM `groups`
WHERE groupID IN ({$groupIDs})", []);
foreach($res as $group) {
$groupNames[$group['groupID']] = $group['name'];
}
}

// loop through all srcRecords
foreach($srcRecords as $i => $v) {
// amend actionable status
if(isset($dRecords[$v['recordID']])) {
if($srcRecords[$i]['isActionable'] == 0) {
$srcRecords[$i]['isActionable'] = $this->login->checkGroup($dRecords[$v['recordID']]['data']);
}

if($skipNames) {
continue;
}

// Only amend group name for group designated records
if($v['dependencyID'] == -3) {
$groupName = isset($groupNames[$group['groupID']]) ? $groupNames[$group['groupID']] : 'Warning: Group has not been imported into the User Access Group';
$srcRecords[$i]['description'] = $srcRecords[$i]['stepTitle'] . ' (' . $groupName . ')';
$srcRecords[$i]['approverName'] = $groupName;
$srcRecords[$i]['approverUID'] = 'groupID:' . $dRecords[$v['recordID']]['data'];
}
}
}

return $srcRecords;
}

/**
* Adds an "isActionable" parameter for each record within $records
* @param object $form instance of Form
Expand Down Expand Up @@ -260,6 +351,8 @@ public function getActionable(object $form, array $records): array {

$personDesignatedRecords = []; // map of records using "person designated" => associated indicatorID
$personDesignatedIndicators = []; // map of indicators using "person designated"
$groupDesignatedRecords = []; // map of records using "group designated" => associated indicatorID
$groupDesignatedIndicators = []; // map of indicators using "group designated"
foreach ($res as $depRecord) {
$depRecordID = $depRecord['recordID'];
if(!isset($records[$depRecordID]['isActionable'])) {
Expand Down Expand Up @@ -299,11 +392,8 @@ public function getActionable(object $form, array $records): array {
break;

case -3: // dependencyID -3 is for a group designated by the requestor
$resGroupID = $form->getIndicator($depRecord['indicatorID_for_assigned_groupID'], 1, $depRecord['recordID'], null, true);
$groupID = $resGroupID[$depRecord['indicatorID_for_assigned_groupID']]['value'];

// make sure the right person has access
$records[$depRecordID]['isActionable'] = $this->login->checkGroup($groupID);
$groupDesignatedRecords[$depRecord['recordID']][$depRecord['indicatorID_for_assigned_groupID']] = 1;
$groupDesignatedIndicators[$depRecord['indicatorID_for_assigned_groupID']] = 1;
break;

default:
Expand All @@ -324,6 +414,9 @@ public function getActionable(object $form, array $records): array {
if(count($personDesignatedRecords) > 0) {
$records = $this->includePersonDesignatedData($records, $personDesignatedRecords, array_keys($personDesignatedIndicators), true);
}
if(count($groupDesignatedRecords) > 0) {
$records = $this->includeGroupDesignatedData($records, $groupDesignatedRecords, array_keys($groupDesignatedIndicators), true);
}

return $records;
}
Expand Down Expand Up @@ -377,6 +470,8 @@ public function getRecordsDependencyData(object $form, array $records, bool $sel

$personDesignatedRecords = []; // map of records using "person designated"
$personDesignatedIndicators = []; // map of indicators using "person designated"
$groupDesignatedRecords = []; // map of records using "group designated"
$groupDesignatedIndicators = []; // map of indicators using "group designated"
foreach ($res as $i => $record) {
// override access if user is in the admin group
$res[$i]['isActionable'] = $this->login->checkGroup(1); // initialize isActionable
Expand Down Expand Up @@ -407,13 +502,8 @@ public function getRecordsDependencyData(object $form, array $records, bool $sel
}
break;
case -3: // dependencyID -3 is for a group designated by the requestor
$resGroupID = $form->getIndicator($res[$i]['indicatorID_for_assigned_groupID'], 1, $res[$i]['recordID'], null, true);
$groupID = $resGroupID[$res[$i]['indicatorID_for_assigned_groupID']]['value'];

// get actual group name
$res[$i]['description'] = $res[$i]['stepTitle'] . ' (' . $this->getActionableGroupName($groupID) . ')';
$res[$i]['approverName'] = $this->getActionableGroupName($groupID);
$res[$i]['approverUID'] = 'groupID:'. $groupID;
$groupDesignatedRecords[$res[$i]['recordID']][$res[$i]['indicatorID_for_assigned_groupID']] = 1;
$groupDesignatedIndicators[$res[$i]['indicatorID_for_assigned_groupID']] = 1;
break;
default:
break;
Expand Down Expand Up @@ -462,16 +552,8 @@ public function getRecordsDependencyData(object $form, array $records, bool $sel
break;

case -3: // dependencyID -3 is for a group designated by the requestor
$resGroupID = $form->getIndicator($res[$i]['indicatorID_for_assigned_groupID'], 1, $res[$i]['recordID'], null, true);
$groupID = $resGroupID[$res[$i]['indicatorID_for_assigned_groupID']]['value'];

// make sure the right person has access
$res[$i]['isActionable'] = $this->login->checkGroup($groupID);

// get actual group name
$res[$i]['description'] = $res[$i]['stepTitle'] . ' (' . $this->getActionableGroupName($groupID) . ')';
$res[$i]['approverName'] = $this->getActionableGroupName($groupID);
$res[$i]['approverUID'] = 'groupID:' . $groupID;
$groupDesignatedRecords[$res[$i]['recordID']][$res[$i]['indicatorID_for_assigned_groupID']] = 1;
$groupDesignatedIndicators[$res[$i]['indicatorID_for_assigned_groupID']] = 1;
break;

default:
Expand All @@ -492,6 +574,9 @@ public function getRecordsDependencyData(object $form, array $records, bool $sel
if(count($personDesignatedRecords) > 0) {
$res = $this->includePersonDesignatedData($res, $personDesignatedRecords, array_keys($personDesignatedIndicators));
}
if(count($groupDesignatedRecords) > 0) {
$res = $this->includeGroupDesignatedData($res, $groupDesignatedRecords, array_keys($groupDesignatedIndicators));
}

return $res;
}
Expand Down

0 comments on commit 22db4bd

Please sign in to comment.