Since we have #1525204: Make it possible to alter the request to GSA in 7.x-1.x now, it's possible for a module to alter the GSA request and switch the access attribute of the search from public to secure. On secure searches, the GSA omits a resultcount ($payload->RES->M) - forcing this module to display no results at all, even when there are results provided.
The current response parser searches for $payload->RES->M, and skips resultset if empty. We need to switch this behaviour to fall back to $payload->RES (the actual results) if $payload->RES->M is empty and calculate the possible results ourself.

CommentFileSizeAuthor
#3 google_appliance-total_results_on_secure_search-1962422-3.patch1.16 KBbkonetzny
#1 google_appliance-total_results_on_secure_search-1962422-1.patch1.4 KBbkonetzny

Comments

bkonetzny’s picture

bkonetzny commented
Status: Active » Needs review
StatusFileSize
new google_appliance-total_results_on_secure_search-1962422-1.patch1.4 KB

Attached patch adds a fallback calculation to the resultset, if counter was omitted due to secure search limitations.

mpgeek’s picture

mpgeek commented
Status: Needs review » Needs work

NIce catch @bkonetzny; just coding standards on this one:

+++ b/google_appliance.moduleundefined
@@ -552,6 +552,23 @@ function google_appliance_parse_device_response_xml($gsa_xml) {
+      if ($results['total_results'] == 0 && isset($payload->RES)) {
+      	// set total count to at least the amout of results we got
+      	$results['total_results'] = (integer) $payload->RES['EN'];
+
+      	// get paging parameters and calculate possible max results
+      	$param_start = $payload->xpath('/GSP/PARAM[@name="start"]');
+      	$param_num = $payload->xpath('/GSP/PARAM[@name="num"]');
+      	$request_max_total = (integer) $param_start[0]['value'] + (integer) $param_num[0]['value'];
+
+      	// assume more results, if the current max result matches to possible total result
+      	if($results['total_results'] == $request_max_total){
+      	  // increasing this to exceed the current page total will add another page to the pager
+      	  $results['total_results']++;
+      	}

Use 2 spaces instead of 4 spaces or tab. More info: http://drupal.org/coding-standards.

bkonetzny’s picture

bkonetzny commented
StatusFileSize
new google_appliance-total_results_on_secure_search-1962422-3.patch1.16 KB

Fixed coding issues.

bkonetzny’s picture

bkonetzny commented
Status: Needs work » Needs review

Forgot to switch issue status after attaching new patch. :)

mpgeek’s picture

mpgeek commented
Status: Needs review » Reviewed & tested by the community

@bkonetzny, looks good, works as designed. Commited to dev:

Sidenote: there are some comment formatting issues in the patch, but the all of the module files have the same issues. I guess we need a separate issue filed for a full code review in one patch.

bkonetzny’s picture

bkonetzny commented

@mpgeek, already thought that too. almost every comment is not written as stated in the coding guidelines. Should we open an issue where this stuff gets cleaned up?

mpgeek’s picture

mpgeek commented

@bkonetzny, started that issue here: http://drupal.org/node/1980910

iamEAP’s picture

iamEAP commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

FYI, this is available as of Google Appliance 7.x-1.12.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.