-
Notifications
You must be signed in to change notification settings - Fork 6
Implement Invalidate search type for Potential Match rules (CO-2690) #70
base: develop
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -972,7 +972,59 @@ public function searchReferenceId(string $sor, string $sorid, AttributeManager $ | |
| // We use a psuedo-rule name | ||
| $potentialMatches->add($downgraded['attrs'], $downgraded['rule']); | ||
| } | ||
|
|
||
|
|
||
| // Check for any potential matches which should be dropped due to Invalidate mode | ||
| if ($potentialMatches->count() > 0) { | ||
| $results = $potentialMatches->getRawResults(); | ||
|
|
||
| // Check if any Attribute Rules in "Invalidate" mode will demote this result. | ||
| foreach($results as $rowId => $attrs) { | ||
| // Find the rule that generated this match. | ||
| $rulename = $potentialMatches->getRuleForResult($rowId); | ||
|
|
||
| foreach($this->mgConfig->potential_rules as $rule) { | ||
| if($rule->name == $rulename) { | ||
| // This is the correct rule, check the Rule Attributes. Note there could | ||
| // be more than one Rule Attribute with Search Type "Invalidate". | ||
|
|
||
| foreach($rule->rule_attributes as $ruleAttribute) { | ||
| if($ruleAttribute->search_type == SearchTypeEnum::Invalidate) { | ||
| // The name of the attribute for this Rule Attribute, eg "dob". | ||
| // This is the column name, not the API name. | ||
| $ruleAttributeName = $ruleAttribute->attribute->name; | ||
|
|
||
| // The value in the search request | ||
| $searchValue = $attributes->getValueByAttribute($ruleAttribute->attribute); | ||
| // The value in the matched row | ||
| $rowValue = $results[$rowId][$ruleAttributeName]; | ||
|
|
||
| // We only perform the validation check if both values are not empty. | ||
| if(!empty($searchValue) | ||
| && !empty($rowValue) | ||
| && ($searchValue !== $rowValue)) { | ||
| // The returned value does not match, invalidate the request and drop to no match | ||
| // by removing the result from the potential match results | ||
| Log::write('debug', $sor . "/" . $sorid . " Invalidate mode for $ruleAttributeName ($rulename) removing result ". $rowId . "\t" . $results[$rowId]['sorid']); | ||
|
|
||
| // remove the result from potentialMatches ResultManager object | ||
| $found = $potentialMatches->remove($rowId); | ||
| if (!$found) { | ||
| Log::write('warning', $sor . "/" . $sorid . " Invalidate mode - error occurred removing result from potential match result set ". $rowId . "\t" . $results[$rowId]['sorid']); | ||
| } | ||
|
Comment on lines
+1010
to
+1013
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. Under what circumstances is this error likely to happen and be actionable by an administrator? 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. This feels like one of those "this line should never be printed" messages. This would only occur if we tried to remove() a potential match from the result set because it passes the invalidate test, but remove() can't find the rowId in the result set. It seems unlikely to occur. I could make this scenario fatal and throw a RuntimeException in case it does happen. |
||
|
|
||
| // No need to continue with any loop | ||
| break 2; | ||
|
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 2 correct here? (It's 3 in the canonical loop.) 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 this should be "break 2", not "break 3" because we need to continue processing any other potential matches to determine if they also should be invalidated. Once we have successfully dropped a potential match to no match, there's no need to continue with the loop for rule attributes or for rules. But we need to continue with the loop that iterates through all of the $results. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| // We found the rule we're looking for, no need to continue with the loop | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // The calling code generally checks to see if any rules successfully ran, | ||
| // since if there were no valid attributes or rules we treat that as an error. | ||
| // If no potential rules ran, we return the canonicalMatches result instead. | ||
|
|
||
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 loop is fairly duplicative of the canonical check, and perhaps should be refactored into a separate call.